All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnd Bergmann <arnd@kernel.org>
To: Hannes Reinecke <hare@suse.de>,
	"James E.J. Bottomley" <jejb@linux.ibm.com>,
	"Martin K. Petersen" <martin.petersen@oracle.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	linux-scsi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH 2/2] libfc: work around Warray-bounds warning
Date: Mon, 26 Oct 2020 17:06:13 +0100	[thread overview]
Message-ID: <20201026160705.3706396-2-arnd@kernel.org> (raw)
In-Reply-To: <20201026160705.3706396-1-arnd@kernel.org>

From: Arnd Bergmann <arnd@arndb.de>

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().

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/libfc/fc_encode.h | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/libfc/fc_encode.h b/drivers/scsi/libfc/fc_encode.h
index 18203cae04b2..602c97a651bc 100644
--- a/drivers/scsi/libfc/fc_encode.h
+++ b/drivers/scsi/libfc/fc_encode.h
@@ -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;
-- 
2.27.0


  reply	other threads:[~2020-10-26 16:07 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-10-26 16:06 [PATCH 1/2] libfc: move scsi/fc_encode.h to libfc Arnd Bergmann
2020-10-26 16:06 ` Arnd Bergmann
2020-10-26 16:06 ` Arnd Bergmann [this message]
2020-10-27  2:39 ` kernel test robot
2020-10-27  2:39   ` kernel test robot
2020-10-30  1:52 ` Martin K. Petersen
2020-10-30  1:52   ` Martin K. Petersen
2020-11-05  4:21 ` Martin K. Petersen
2020-11-05  4:21   ` Martin K. Petersen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20201026160705.3706396-2-arnd@kernel.org \
    --to=arnd@kernel.org \
    --cc=arnd@arndb.de \
    --cc=hare@suse.de \
    --cc=jejb@linux.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.