Commit 4280a0a7 authored by Justin Stitt's avatar Justin Stitt Committed by Martin K. Petersen

scsi: message: fusion: Replace deprecated strncpy() with strscpy_pad()

strncpy() is deprecated for use on NUL-terminated destination strings [1]
and as such we should prefer more robust and less ambiguous string
interfaces.

Since all these structs are copied out to userspace let's keep them
NUL-padded by using strscpy_pad() which guarantees NUL-termination of the
destination buffer while also providing the NUL-padding behavior that
strncpy() has.

Let's also opt to use the more idiomatic strscpy() usage of: 'dest, src,
sizeof(dest)' in cases where the compiler can determine the size of the
destination buffer. Do this for all cases of strscpy...() in this file.

To be abundantly sure we don't leak stack data out to user space let's also
change a strscpy() to strscpy_pad(). This strscpy() was introduced in
commit dbe37c71 ("scsi: message: fusion: Replace all non-returning
strlcpy() with strscpy()")

Note that since we are creating these structs with a copy_from_user() and
modifying fields and then copying back out to the user it is probably OK
not to explicitly NUL-pad everything as any data leak is probably just data
from the user themselves. If this is too eager, let's opt for strscpy()
which is still in the spirit of removing deprecated strncpy() usage
treewide.

Link: https://www.kernel.org/doc/html/latest/process/deprecated.html#strncpy-on-nul-terminated-strings [1]
Link: https://github.com/KSPP/linux/issues/90
Cc: linux-hardening@vger.kernel.org
Cc: Kees Cook <keescook@chromium.org>
Signed-off-by: default avatarJustin Stitt <justinstitt@google.com>
Link: https://lore.kernel.org/r/20230927-strncpy-drivers-message-fusion-mptctl-c-v1-1-bb2eddc1743c@google.comReviewed-by: default avatarKees Cook <keescook@chromium.org>
Signed-off-by: default avatarMartin K. Petersen <martin.petersen@oracle.com>
parent 971237b9
...@@ -1328,8 +1328,8 @@ mptctl_getiocinfo (MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size) ...@@ -1328,8 +1328,8 @@ mptctl_getiocinfo (MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size)
/* Set the Version Strings. /* Set the Version Strings.
*/ */
strncpy (karg->driverVersion, MPT_LINUX_PACKAGE_NAME, MPT_IOCTL_VERSION_LENGTH); strscpy_pad(karg->driverVersion, MPT_LINUX_PACKAGE_NAME,
karg->driverVersion[MPT_IOCTL_VERSION_LENGTH-1]='\0'; sizeof(karg->driverVersion));
karg->busChangeEvent = 0; karg->busChangeEvent = 0;
karg->hostId = ioc->pfacts[port].PortSCSIID; karg->hostId = ioc->pfacts[port].PortSCSIID;
...@@ -1493,10 +1493,8 @@ mptctl_readtest (MPT_ADAPTER *ioc, unsigned long arg) ...@@ -1493,10 +1493,8 @@ mptctl_readtest (MPT_ADAPTER *ioc, unsigned long arg)
#else #else
karg.chip_type = ioc->pcidev->device; karg.chip_type = ioc->pcidev->device;
#endif #endif
strncpy (karg.name, ioc->name, MPT_MAX_NAME); strscpy_pad(karg.name, ioc->name, sizeof(karg.name));
karg.name[MPT_MAX_NAME-1]='\0'; strscpy_pad(karg.product, ioc->prod_name, sizeof(karg.product));
strncpy (karg.product, ioc->prod_name, MPT_PRODUCT_LENGTH);
karg.product[MPT_PRODUCT_LENGTH-1]='\0';
/* Copy the data from kernel memory to user memory /* Copy the data from kernel memory to user memory
*/ */
...@@ -2394,7 +2392,7 @@ mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size) ...@@ -2394,7 +2392,7 @@ mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size)
cfg.dir = 0; /* read */ cfg.dir = 0; /* read */
cfg.timeout = 10; cfg.timeout = 10;
strncpy(karg.serial_number, " ", 24); strscpy_pad(karg.serial_number, " ", sizeof(karg.serial_number));
if (mpt_config(ioc, &cfg) == 0) { if (mpt_config(ioc, &cfg) == 0) {
if (cfg.cfghdr.hdr->PageLength > 0) { if (cfg.cfghdr.hdr->PageLength > 0) {
/* Issue the second config page request */ /* Issue the second config page request */
...@@ -2408,8 +2406,9 @@ mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size) ...@@ -2408,8 +2406,9 @@ mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size)
if (mpt_config(ioc, &cfg) == 0) { if (mpt_config(ioc, &cfg) == 0) {
ManufacturingPage0_t *pdata = (ManufacturingPage0_t *) pbuf; ManufacturingPage0_t *pdata = (ManufacturingPage0_t *) pbuf;
if (strlen(pdata->BoardTracerNumber) > 1) { if (strlen(pdata->BoardTracerNumber) > 1) {
strscpy(karg.serial_number, strscpy_pad(karg.serial_number,
pdata->BoardTracerNumber, 24); pdata->BoardTracerNumber,
sizeof(karg.serial_number));
} }
} }
dma_free_coherent(&ioc->pcidev->dev, dma_free_coherent(&ioc->pcidev->dev,
...@@ -2456,7 +2455,7 @@ mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size) ...@@ -2456,7 +2455,7 @@ mptctl_hp_hostinfo(MPT_ADAPTER *ioc, unsigned long arg, unsigned int data_size)
} }
} }
/* /*
* Gather ISTWI(Industry Standard Two Wire Interface) Data * Gather ISTWI(Industry Standard Two Wire Interface) Data
*/ */
if ((mf = mpt_get_msg_frame(mptctl_id, ioc)) == NULL) { if ((mf = mpt_get_msg_frame(mptctl_id, ioc)) == NULL) {
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment