aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorKees Cook <[email protected]>2023-11-30 12:41:00 -0800
committerMartin K. Petersen <[email protected]>2023-12-05 21:01:52 -0500
commit0d224b1088afef95f7069b7ca907cb199a5802ff (patch)
tree4e5283f3677fde2b5398d69127b4e47e52c5b048
parent28c58f8a0947f70ddee275b90fe45431146bb2cc (diff)
scsi: zfcp: Replace strlcpy() with strscpy()
strlcpy() reads the entire source buffer first. This read may exceed the destination size limit. This is both inefficient and can lead to linear read overflows if a source string is not NUL-terminated[1]. Additionally, it returns the size of the source string, not the resulting size of the destination string. In an effort to remove strlcpy() completely[2], replace strlcpy() here with strscpy(). Overflow should be impossible here, but actually check for buffer sizes being identical with BUILD_BUG_ON(), and include a run-time check as well. Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strlcpy [1] Link: https://github.com/KSPP/linux/issues/89 [2] Cc: Martin K. Petersen <[email protected]> Cc: James E.J. Bottomley <[email protected]> Cc: Steffen Maier <[email protected]> Cc: Benjamin Block <[email protected]> Cc: Heiko Carstens <[email protected]> Cc: Vasily Gorbik <[email protected]> Cc: Alexander Gordeev <[email protected]> Cc: Christian Borntraeger <[email protected]> Cc: Sven Schnelle <[email protected]> Cc: Azeem Shaikh <[email protected]> Cc: <[email protected]> Cc: <[email protected]> Signed-off-by: Kees Cook <[email protected]> Link: https://lore.kernel.org/r/[email protected] Acked-by: Benjamin Block <[email protected]> Signed-off-by: Martin K. Petersen <[email protected]>
-rw-r--r--drivers/s390/scsi/zfcp_fc.c15
1 files changed, 13 insertions, 2 deletions
diff --git a/drivers/s390/scsi/zfcp_fc.c b/drivers/s390/scsi/zfcp_fc.c
index 4f0d0e55f0d4..d6516ab00437 100644
--- a/drivers/s390/scsi/zfcp_fc.c
+++ b/drivers/s390/scsi/zfcp_fc.c
@@ -900,8 +900,19 @@ static void zfcp_fc_rspn(struct zfcp_adapter *adapter,
zfcp_fc_ct_ns_init(&rspn_req->ct_hdr, FC_NS_RSPN_ID,
FC_SYMBOLIC_NAME_SIZE);
hton24(rspn_req->rspn.fr_fid.fp_fid, fc_host_port_id(shost));
- len = strlcpy(rspn_req->rspn.fr_name, fc_host_symbolic_name(shost),
- FC_SYMBOLIC_NAME_SIZE);
+
+ BUILD_BUG_ON(sizeof(rspn_req->name) !=
+ sizeof(fc_host_symbolic_name(shost)));
+ BUILD_BUG_ON(sizeof(rspn_req->name) !=
+ type_max(typeof(rspn_req->rspn.fr_name_len)) + 1);
+ len = strscpy(rspn_req->name, fc_host_symbolic_name(shost),
+ sizeof(rspn_req->name));
+ /*
+ * It should be impossible for this to truncate (see BUILD_BUG_ON()
+ * above), but be robust anyway.
+ */
+ if (WARN_ON(len < 0))
+ len = sizeof(rspn_req->name) - 1;
rspn_req->rspn.fr_name_len = len;
sg_init_one(&fc_req->sg_req, rspn_req, sizeof(*rspn_req));