Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

scsi: libfc: Work around -Warray-bounds warning

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>

authored by

Arnd Bergmann and committed by
Martin K. Petersen
8fd9efca e31ac898

+19 -11
+19 -11
drivers/scsi/libfc/fc_encode.h
··· 163 163 return 0; 164 164 } 165 165 166 + static inline void fc_ct_ms_fill_attr(struct fc_fdmi_attr_entry *entry, 167 + const char *in, size_t len) 168 + { 169 + int copied = strscpy(entry->value, in, len); 170 + if (copied > 0) 171 + memset(entry->value, copied, len - copied); 172 + } 173 + 166 174 /** 167 175 * fc_ct_ms_fill() - Fill in a mgmt service request frame 168 176 * @lport: local port. ··· 240 232 put_unaligned_be16(FC_FDMI_HBA_ATTR_MANUFACTURER, 241 233 &entry->type); 242 234 put_unaligned_be16(len, &entry->len); 243 - strncpy((char *)&entry->value, 235 + fc_ct_ms_fill_attr(entry, 244 236 fc_host_manufacturer(lport->host), 245 237 FC_FDMI_HBA_ATTR_MANUFACTURER_LEN); 246 238 ··· 252 244 put_unaligned_be16(FC_FDMI_HBA_ATTR_SERIALNUMBER, 253 245 &entry->type); 254 246 put_unaligned_be16(len, &entry->len); 255 - strncpy((char *)&entry->value, 247 + fc_ct_ms_fill_attr(entry, 256 248 fc_host_serial_number(lport->host), 257 249 FC_FDMI_HBA_ATTR_SERIALNUMBER_LEN); 258 250 ··· 264 256 put_unaligned_be16(FC_FDMI_HBA_ATTR_MODEL, 265 257 &entry->type); 266 258 put_unaligned_be16(len, &entry->len); 267 - strncpy((char *)&entry->value, 259 + fc_ct_ms_fill_attr(entry, 268 260 fc_host_model(lport->host), 269 261 FC_FDMI_HBA_ATTR_MODEL_LEN); 270 262 ··· 276 268 put_unaligned_be16(FC_FDMI_HBA_ATTR_MODELDESCRIPTION, 277 269 &entry->type); 278 270 put_unaligned_be16(len, &entry->len); 279 - strncpy((char *)&entry->value, 271 + fc_ct_ms_fill_attr(entry, 280 272 fc_host_model_description(lport->host), 281 273 FC_FDMI_HBA_ATTR_MODELDESCR_LEN); 282 274 ··· 288 280 put_unaligned_be16(FC_FDMI_HBA_ATTR_HARDWAREVERSION, 289 281 &entry->type); 290 282 put_unaligned_be16(len, &entry->len); 291 - strncpy((char *)&entry->value, 283 + fc_ct_ms_fill_attr(entry, 292 284 fc_host_hardware_version(lport->host), 293 285 FC_FDMI_HBA_ATTR_HARDWAREVERSION_LEN); 294 286 ··· 300 292 put_unaligned_be16(FC_FDMI_HBA_ATTR_DRIVERVERSION, 301 293 &entry->type); 302 294 put_unaligned_be16(len, &entry->len); 303 - strncpy((char *)&entry->value, 295 + fc_ct_ms_fill_attr(entry, 304 296 fc_host_driver_version(lport->host), 305 297 FC_FDMI_HBA_ATTR_DRIVERVERSION_LEN); 306 298 ··· 312 304 put_unaligned_be16(FC_FDMI_HBA_ATTR_OPTIONROMVERSION, 313 305 &entry->type); 314 306 put_unaligned_be16(len, &entry->len); 315 - strncpy((char *)&entry->value, 307 + fc_ct_ms_fill_attr(entry, 316 308 fc_host_optionrom_version(lport->host), 317 309 FC_FDMI_HBA_ATTR_OPTIONROMVERSION_LEN); 318 310 ··· 324 316 put_unaligned_be16(FC_FDMI_HBA_ATTR_FIRMWAREVERSION, 325 317 &entry->type); 326 318 put_unaligned_be16(len, &entry->len); 327 - strncpy((char *)&entry->value, 319 + fc_ct_ms_fill_attr(entry, 328 320 fc_host_firmware_version(lport->host), 329 321 FC_FDMI_HBA_ATTR_FIRMWAREVERSION_LEN); 330 322 ··· 419 411 &entry->type); 420 412 put_unaligned_be16(len, &entry->len); 421 413 /* Use the sysfs device name */ 422 - strncpy((char *)&entry->value, 414 + fc_ct_ms_fill_attr(entry, 423 415 dev_name(&lport->host->shost_gendev), 424 416 strnlen(dev_name(&lport->host->shost_gendev), 425 417 FC_FDMI_PORT_ATTR_HOSTNAME_LEN)); ··· 433 425 &entry->type); 434 426 put_unaligned_be16(len, &entry->len); 435 427 if (strlen(fc_host_system_hostname(lport->host))) 436 - strncpy((char *)&entry->value, 428 + fc_ct_ms_fill_attr(entry, 437 429 fc_host_system_hostname(lport->host), 438 430 strnlen(fc_host_system_hostname(lport->host), 439 431 FC_FDMI_PORT_ATTR_HOSTNAME_LEN)); 440 432 else 441 - strncpy((char *)&entry->value, 433 + fc_ct_ms_fill_attr(entry, 442 434 init_utsname()->nodename, 443 435 FC_FDMI_PORT_ATTR_HOSTNAME_LEN); 444 436 break;