History

scsi: ipr: Work around fortify-string warning

The ipr_log_vpd_compact() function triggers a fortified memcpy() warning about a potential string overflow with all versions of clang: In file included from drivers/scsi/ipr.c:43: In file included from include/linux/string.h:254: include/linux/fortify-string.h:520:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning] __write_overflow_field(p_size_field, size); ^ include/linux/fortify-string.h:520:4: error: call to '__write_overflow_field' declared with 'warning' attribute: detected write beyond size of field (1st parameter); maybe use struct_group()? [-Werror,-Wattribute-warning] 2 errors generated. I don't see anything actually wrong with the function, but this is the only instance I can reproduce of the fortification going wrong in the kernel at the moment, so the easiest solution may be to rewrite the function into something that does not trigger the warning. Instead of having a combined buffer for vendor/device/serial strings, use three separate local variables and just truncate the whitespace individually.

Arnd Bergmann committed 3 years ago

Commit ee4e7df1 parent 3a2d1ef1 file · +21 −20Browse files

Unified · Split

drivers/scsi/ipr.c+21−20
@@ -1516,23 +1516,22 @@ static void ipr_process_ccn(struct ipr_cmnd *ipr_cmd) · 1515 unmodified lines
15161516 }
15171517
15181518 /**
1519− * strip_and_pad_whitespace - Strip and pad trailing whitespace.
1520− * @i: index into buffer
1521− * @buf: string to modify
1519+ * strip_whitespace - Strip and pad trailing whitespace.
1520+ * @i: size of buffer
1521+ * @buf: string to modify
15221522 *
1523− * This function will strip all trailing whitespace, pad the end
1524− * of the string with a single space, and NULL terminate the string.
1523+ * This function will strip all trailing whitespace and
1524+ * NUL terminate the string.
15251525 *
1526− * Return value:
1527− * new length of string
15281526 **/
1529−static int strip_and_pad_whitespace(int i, char *buf)
1527+static void strip_whitespace(int i, char *buf)
15301528 {
1529+ if (i < 1)
1530+ return;
1531+ i--;
15311532 while (i && buf[i] == ' ')
15321533 i--;
1533− buf[i+1] = ' ';
1534− buf[i+2] = '\0';
1535− return i + 2;
1534+ buf[i+1] = '\0';
15361535 }
15371536
15381537 /**
@@ -1547,19 +1546,21 @@ static int strip_and_pad_whitespace(int i, char *buf) · 8 unmodified lines
15471546 static void ipr_log_vpd_compact(char *prefix, struct ipr_hostrcb *hostrcb,
15481547 struct ipr_vpd *vpd)
15491548 {
1550− char buffer[IPR_VENDOR_ID_LEN + IPR_PROD_ID_LEN + IPR_SERIAL_NUM_LEN + 3];
1551− int i = 0;
1549+ char vendor_id[IPR_VENDOR_ID_LEN + 1];
1550+ char product_id[IPR_PROD_ID_LEN + 1];
1551+ char sn[IPR_SERIAL_NUM_LEN + 1];
15521552
1553− memcpy(buffer, vpd->vpids.vendor_id, IPR_VENDOR_ID_LEN);
1554− i = strip_and_pad_whitespace(IPR_VENDOR_ID_LEN - 1, buffer);
1553+ memcpy(vendor_id, vpd->vpids.vendor_id, IPR_VENDOR_ID_LEN);
1554+ strip_whitespace(IPR_VENDOR_ID_LEN, vendor_id);
15551555
1556− memcpy(&buffer[i], vpd->vpids.product_id, IPR_PROD_ID_LEN);
1557− i = strip_and_pad_whitespace(i + IPR_PROD_ID_LEN - 1, buffer);
1556+ memcpy(product_id, vpd->vpids.product_id, IPR_PROD_ID_LEN);
1557+ strip_whitespace(IPR_PROD_ID_LEN, product_id);
15581558
1559− memcpy(&buffer[i], vpd->sn, IPR_SERIAL_NUM_LEN);
1560− buffer[IPR_SERIAL_NUM_LEN + i] = '\0';
1559+ memcpy(sn, vpd->sn, IPR_SERIAL_NUM_LEN);
1560+ strip_whitespace(IPR_SERIAL_NUM_LEN, sn);
15611561
1562− ipr_hcam_err(hostrcb, "%s VPID/SN: %s\n", prefix, buffer);
1562+ ipr_hcam_err(hostrcb, "%s VPID/SN: %s %s %s\n", prefix,
1563+ vendor_id, product_id, sn);
15631564 }
15641565
15651566 /**