scsi: libfc: Work around -Warray-bounds warning
authorArnd Bergmann <arnd@arndb.de>
Mon, 26 Oct 2020 16:06:13 +0000 (17:06 +0100)
committerMartin K. Petersen <martin.petersen@oracle.com>
Fri, 30 Oct 2020 01:49:25 +0000 (21:49 -0400)
Building libfc with gcc -Warray-bounds identifies a number of cases in one
file where a strncpy() is performed into a single-byte character array:

In file included from include/linux/bitmap.h:9,
                 from include/linux/cpumask.h:12,
                 from include/linux/smp.h:13,
                 from include/linux/lockdep.h:14,
                 from include/linux/spinlock.h:59,
                 from include/linux/debugobjects.h:6,
                 from include/linux/timer.h:8,
                 from include/scsi/libfc.h:11,
                 from drivers/scsi/libfc/fc_elsct.c:17:
In function 'strncpy',
    inlined from 'fc_ct_ms_fill.constprop' at drivers/scsi/libfc/fc_encode.h:235:3:
include/linux/string.h:290:30: warning: '__builtin_strncpy' offset [56, 135] from the object at 'pp' is out of the bounds of referenced subobject 'value' with type '__u8[1]' {aka 'unsigned char[1]'} at offset 56 [-Warray-bounds]
  290 | #define __underlying_strncpy __builtin_strncpy
      |                              ^
include/linux/string.h:300:9: note: in expansion of macro '__underlying_strncpy'
  300 |  return __underlying_strncpy(p, q, size);
      |         ^~~~~~~~~~~~~~~~~~~~

This is not a bug because the 1-byte array is used as an odd way to express
a variable-length data field here. I tried to convert it to a
flexible-array member, but in the end could not figure out why the
sizeof(struct fc_fdmi_???) are used the way they are, and how to properly
convert those.

Work around this instead by abstracting the string copy in a slightly
higher-level function fc_ct_hdr_fill() helper that strscpy() and memset()
to achieve the same result as strncpy() but does not require a
zero-terminated input and does not get checked for the array overflow
because gcc (so far) does not understand the behavior of strscpy().

Link: https://lore.kernel.org/r/20201026160705.3706396-2-arnd@kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Martin K. Petersen <martin.petersen@oracle.com>
drivers/scsi/libfc/fc_encode.h

index 18203cae04b2ce9dc12c6a9df7f05d3fe8b59cf9..602c97a651bc04298259b564ab872b99af807db2 100644 (file)
@@ -163,6 +163,14 @@ static inline int fc_ct_ns_fill(struct fc_lport *lport,
        return 0;
 }
 
+static inline void fc_ct_ms_fill_attr(struct fc_fdmi_attr_entry *entry,
+                                   const char *in, size_t len)
+{
+       int copied = strscpy(entry->value, in, len);
+       if (copied > 0)
+               memset(entry->value, copied, len - copied);
+}
+
 /**
  * fc_ct_ms_fill() - Fill in a mgmt service request frame
  * @lport: local port.
@@ -232,7 +240,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_MANUFACTURER,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_manufacturer(lport->host),
                        FC_FDMI_HBA_ATTR_MANUFACTURER_LEN);
 
@@ -244,7 +252,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_SERIALNUMBER,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_serial_number(lport->host),
                        FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN);
 
@@ -256,7 +264,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_MODEL,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_model(lport->host),
                        FC_FDMI_HBA_ATTR_MODEL_LEN);
 
@@ -268,7 +276,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_MODELDESCRIPTION,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_model_description(lport->host),
                        FC_FDMI_HBA_ATTR_MODELDESCR_LEN);
 
@@ -280,7 +288,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_HARDWAREVERSION,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_hardware_version(lport->host),
                        FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN);
 
@@ -292,7 +300,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_DRIVERVERSION,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_driver_version(lport->host),
                        FC_FDMI_HBA_ATTR_DRIVERVERSION_LEN);
 
@@ -304,7 +312,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_OPTIONROMVERSION,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_optionrom_version(lport->host),
                        FC_FDMI_HBA_ATTR_OPTIONROMVERSION_LEN);
 
@@ -316,7 +324,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                put_unaligned_be16(FC_FDMI_HBA_ATTR_FIRMWAREVERSION,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        fc_host_firmware_version(lport->host),
                        FC_FDMI_HBA_ATTR_FIRMWAREVERSION_LEN);
 
@@ -411,7 +419,7 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
                /* Use the sysfs device name */
-               strncpy((char *)&entry->value,
+               fc_ct_ms_fill_attr(entry,
                        dev_name(&lport->host->shost_gendev),
                        strnlen(dev_name(&lport->host->shost_gendev),
                                FC_FDMI_PORT_ATTR_HOSTNAME_LEN));
@@ -425,12 +433,12 @@ static inline int fc_ct_ms_fill(struct fc_lport *lport,
                                   &entry->type);
                put_unaligned_be16(len, &entry->len);
                if (strlen(fc_host_system_hostname(lport->host)))
-                       strncpy((char *)&entry->value,
+                       fc_ct_ms_fill_attr(entry,
                                fc_host_system_hostname(lport->host),
                                strnlen(fc_host_system_hostname(lport->host),
                                        FC_FDMI_PORT_ATTR_HOSTNAME_LEN));
                else
-                       strncpy((char *)&entry->value,
+                       fc_ct_ms_fill_attr(entry,
                                init_utsname()->nodename,
                                FC_FDMI_PORT_ATTR_HOSTNAME_LEN);
                break;