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
next prev parent reply other threads:[~2020-10-26 16:07 UTC|newest]
Thread overview: 4+ 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 [this message]
2020-10-30 1:52 ` 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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).