Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1687288 > unrolled thread

[PATCH 00/22] gcc-7 -Wformat-* warnings

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:10 +0200
    [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:10 +0200
    [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:10 +0200
    [PATCH 07/22] scsi: gdth: increase the procfs event buffer size Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
    [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin  numbers Andy Shevchenko <andriy.shevchenko@linux.intel.com> - 2017-07-14 15:00 +0200
        Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann <arnd@arndb.de> - 2017-07-14 22:00 +0200
    [PATCH 03/22] scsi: mpt3sas: fix format overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
    [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 11/22] net: thunder_bgx: avoid format string overflow  warning Robin Murphy <robin.murphy@arm.com> - 2017-07-14 14:40 +0200
      Re: [PATCH 11/22] net: thunder_bgx: avoid format string overflow  warning David Miller <davem@davemloft.net> - 2017-07-14 18:10 +0200
    [PATCH 10/22] bnx2x: fix format overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 10/22] bnx2x: fix format overflow warning David Miller <davem@davemloft.net> - 2017-07-14 18:10 +0200
    [PATCH 21/22] fscache: fix fscache_objlist_show format processing Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
    [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 15:00 +0200
        Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-17 15:00 +0200
          Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Hans Verkuil <hverkuil@xs4all.nl> - 2017-07-17 15:00 +0200
    [PATCH 04/22] scsi: fusion: fix string overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      RE: [PATCH 04/22] scsi: fusion: fix string overflow warning David Laight <David.Laight@ACULAB.COM> - 2017-07-17 11:20 +0200
        Re: [PATCH 04/22] scsi: fusion: fix string overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-17 14:10 +0200
    [PATCH 15/22] hwmon: applesmc: fix format string overflow Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 15/22] hwmon: applesmc: fix format string overflow Guenter Roeck <linux@roeck-us.net> - 2017-07-14 16:10 +0200
    [PATCH 12/22] vmxnet3: avoid format strint overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 12/22] vmxnet3: avoid format strint overflow warning David Miller <davem@davemloft.net> - 2017-07-14 18:10 +0200
    [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      RE: [PATCH 17/22] platform/x86: alienware-wmi: fix format string  overflow warning <Mario.Limonciello@dell.com> - 2017-07-14 20:40 +0200
      Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string  overflow warning Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-07-14 21:20 +0200
        Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string  overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 21:40 +0200
          Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string  overflow warning Andy Shevchenko <andy.shevchenko@gmail.com> - 2017-07-14 21:50 +0200
    [PATCH 05/22] scsi: gdth: avoid buffer overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
    [PATCH 09/22] net: niu: fix format string overflow warning: Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 09/22] net: niu: fix format string overflow warning: David Miller <davem@davemloft.net> - 2017-07-14 18:10 +0200
    [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning David Miller <davem@davemloft.net> - 2017-07-14 18:10 +0200
    [PATCH 16/22] x86: intel-mid: fix a format string overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
    [PATCH 06/22] scsi: fnic: fix format string overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
    [PATCH 22/22] IB/mlx4: fix sprintf format warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 22/22] IB/mlx4: fix sprintf format warning Leon Romanovsky <leon@kernel.org> - 2017-07-14 15:50 +0200
    [PATCH 20/22] sound: pci: avoid string overflow warnings Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 20/22] sound: pci: avoid string overflow warnings Takashi Iwai <tiwai@suse.de> - 2017-07-14 14:30 +0200
    [PATCH 13/22] liquidio: fix possible eeprom format string overflow Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200
      Re: [PATCH 13/22] liquidio: fix possible eeprom format string  overflow David Miller <davem@davemloft.net> - 2017-07-14 18:10 +0200
    [PATCH 19/22] block: DAC960: shut up format-overflow warning Arnd Bergmann <arnd@arndb.de> - 2017-07-14 14:20 +0200

Page 1 of 3  [1] 2 3  Next page →

#1687288 — [PATCH 00/22] gcc-7 -Wformat-* warnings

This series addresses all warnings that gcc-7 introduces for
-Wformat-overflow= and turns off the -Wformat-truncation by default
(they remain enabled with "make W=1").

The -Wformat-overflow patches take varying approaches:

- When the final use of the buffer is not limited and we print
  into an intermediate variable on the stack, I generally make
  the temporary buffer slightly larger to accomodate all
  theoretically possible values. Usually the code is already
  correct for all expected values, but gcc doesn't see that.

- In some cases, we use a fixed-length buffer as the %s input
  for an sprintf to another buffer of the same length. Here
  I could make the first buffer slightly smaller so that gcc
  can prove the copies to be correct.

- In cases where the output buffer is required to have a fixed
  length, I use snprintf() instead of sprintf(). This turns
  the overflow warning into a truncation warning that is then
  ignored. Here it would be much nicer to have a way to tell
  the compiler what the maximum expected length is, but I
  couldn't figure out a way to actually shut up the truncation
  warning completely. Any ideas would be welcome.

Please review and apply as needed.

      Arnd

Arnd Bergmann (22):
  kbuild: disable -Wformat-truncation warnings by default
  scsi: megaraid: fix format-overflow warning
  scsi: mpt3sas: fix format overflow warning
  scsi: fusion: fix string overflow warning
  scsi: gdth: avoid buffer overflow warning
  scsi: fnic: fix format string overflow warning
  scsi: gdth: increase the procfs event buffer size
  isdn: divert: fix sprintf buffer overflow warning
  net: niu: fix format string overflow warning:
  bnx2x: fix format overflow warning
  net: thunder_bgx: avoid format string overflow warning
  vmxnet3: avoid format strint overflow warning
  liquidio: fix possible eeprom format string overflow
  [media] usbvision-i2c: fix format overflow warning
  hwmon: applesmc: fix format string overflow
  x86: intel-mid: fix a format string overflow warning
  platform/x86: alienware-wmi: fix format string overflow warning
  gpio: acpi: fix string overflow for large pin numbers
  block: DAC960: shut up format-overflow warning
  sound: pci: avoid string overflow warnings
  fscache: fix fscache_objlist_show format processing
  IB/mlx4: fix sprintf format warning

 .../intel-mid/device_libs/platform_max7315.c       |  6 ++++--
 drivers/block/DAC960.c                             | 12 +++++++----
 drivers/gpio/gpiolib-acpi.c                        |  2 +-
 drivers/hwmon/applesmc.c                           |  2 +-
 drivers/infiniband/hw/mlx4/sysfs.c                 |  2 +-
 drivers/isdn/divert/isdn_divert.c                  | 25 +++++++++++-----------
 drivers/media/usb/usbvision/usbvision-i2c.c        |  4 ++--
 drivers/message/fusion/mptbase.c                   |  2 +-
 .../net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c    |  3 ++-
 drivers/net/ethernet/cavium/liquidio/lio_ethtool.c |  2 +-
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c  |  2 +-
 drivers/net/ethernet/sun/niu.c                     |  4 ++--
 drivers/net/vmxnet3/vmxnet3_int.h                  |  2 +-
 drivers/platform/x86/alienware-wmi.c               |  2 +-
 drivers/scsi/fnic/fnic.h                           |  2 +-
 drivers/scsi/gdth.c                                |  2 +-
 drivers/scsi/gdth_proc.c                           |  2 +-
 drivers/scsi/megaraid.c                            |  6 ++++--
 drivers/scsi/mpt3sas/mpt3sas_base.h                |  2 +-
 fs/fscache/object-list.c                           |  3 ++-
 scripts/Makefile.extrawarn                         |  3 +++
 sound/pci/mixart/mixart.h                          |  4 ++--
 sound/pci/pcxhr/pcxhr.h                            |  4 ++--
 sound/pci/rme9652/hdspm.c                          |  2 +-
 24 files changed, 57 insertions(+), 43 deletions(-)

-- 
2.9.0

[toc] | [next] | [standalone]

#1687295 — [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default

With x86 allmodconfig, we currently get 233 -Wformat-truncation warnings,
which makes the entire warnings rather useless.

This turns off the warning by default, unless we specify W=1 or higher

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 scripts/Makefile.extrawarn | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index fb3522fd8702..4b63c2f71adb 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -67,5 +67,8 @@ KBUILD_CFLAGS += $(call cc-disable-warning, format)
 KBUILD_CFLAGS += $(call cc-disable-warning, sign-compare)
 KBUILD_CFLAGS += $(call cc-disable-warning, format-zero-length)
 KBUILD_CFLAGS += $(call cc-disable-warning, uninitialized)
+else
+# noisy gcc-7 warnings
+KBUILD_CFLAGS += $(call cc-option,-Wformat-truncation=0)
 endif
 endif
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687299 — [PATCH 02/22] scsi: megaraid: fix format-overflow warning

gcc-7 complains that the firmware version strings might overflow
for some values:

drivers/scsi/megaraid.c: In function 'megaraid_probe_one':
drivers/scsi/megaraid.c:314:33: error: '%d' directive writing between 1 and 2 bytes into a region of size between 1 and 2 [-Werror=format-overflow=]
drivers/scsi/megaraid.c:314:33: note: directive argument in the range [0, 15]
drivers/scsi/megaraid.c:314:3: note: 'sprintf' output between 7 and 9 bytes into a destination of size 7
drivers/scsi/megaraid.c:320:35: error: '%d' directive writing between 1 and 2 bytes into a region of size between 1 and 2 [-Werror=format-overflow=]
drivers/scsi/megaraid.c:320:35: note: directive argument in the range [0, 15]
drivers/scsi/megaraid.c:320:3: note: 'sprintf' output between 7 and 9 bytes into a destination of size 7

This makes the code use a truncating snprintf() instead, which shuts
up that warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/megaraid.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/megaraid.c b/drivers/scsi/megaraid.c
index 3c63c292cb92..7195cff51d4c 100644
--- a/drivers/scsi/megaraid.c
+++ b/drivers/scsi/megaraid.c
@@ -311,13 +311,15 @@ mega_query_adapter(adapter_t *adapter)
 	   right 8 bits making them zero. This 0 value was hardcoded to fix
 	   sparse warnings. */
 	if (adapter->product_info.subsysvid == PCI_VENDOR_ID_HP) {
-		sprintf (adapter->fw_version, "%c%d%d.%d%d",
+		snprintf(adapter->fw_version, sizeof(adapter->fw_version),
+			 "%c%d%d.%d%d",
 			 adapter->product_info.fw_version[2],
 			 0,
 			 adapter->product_info.fw_version[1] & 0x0f,
 			 0,
 			 adapter->product_info.fw_version[0] & 0x0f);
-		sprintf (adapter->bios_version, "%c%d%d.%d%d",
+		snprintf(adapter->bios_version, sizeof(adapter->fw_version),
+			 "%c%d%d.%d%d",
 			 adapter->product_info.bios_version[2],
 			 0,
 			 adapter->product_info.bios_version[1] & 0x0f,
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687303 — [PATCH 07/22] scsi: gdth: increase the procfs event buffer size

We print a 256 byte event string into a buffer that is only 161
bytes long, this is clearly wrong:

drivers/scsi/gdth_proc.c: In function 'gdth_show_info':
drivers/scsi/gdth.c:3660:41: error: '%s' directive writing up to 255 bytes into a region of size between 141 and 150 [-Werror=format-overflow=]
             sprintf(buffer,"Adapter %d: %s\n",
                                         ^~
/git/arm-soc/drivers/scsi/gdth.c:3660:13: note: 'sprintf' output between 13 and 277 bytes into a destination of size 161
             sprintf(buffer,"Adapter %d: %s\n",
             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                 dvr->eu.async.ionode,dvr->event_string);
                 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

gcc calculates that the worst case buffer size would be 277 bytes,
so we can use that.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/gdth_proc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/gdth_proc.c b/drivers/scsi/gdth_proc.c
index be609db66807..d08b2716752c 100644
--- a/drivers/scsi/gdth_proc.c
+++ b/drivers/scsi/gdth_proc.c
@@ -147,7 +147,7 @@ int gdth_show_info(struct seq_file *m, struct Scsi_Host *host)
 
     gdth_cmd_str *gdtcmd;
     gdth_evt_str *estr;
-    char hrec[161];
+    char hrec[277];
 
     char *buf;
     gdth_dskstat_str *pds;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687304 — [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers

gcc-7 notices that the pin_table is an array of 16-bit numbers,
but we assume it can be printed as a two-character hexadecimal
string:

drivers/gpio/gpiolib-acpi.c: In function 'acpi_gpiochip_request_interrupt':
drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
   sprintf(ev_name, "_%c%02X",
                        ^~~~
drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the range [0, 65535]
   sprintf(ev_name, "_%c%02X",
                    ^~~~~~~~~
drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5 and 7 bytes into a destination of size 5
   sprintf(ev_name, "_%c%02X",
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~
    agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    pin);
    ~~~~

This can't be right, so this changes it to truncate the number to
an 8-bit pin number.

Fixes: 0d1c28a449c6 ("gpiolib-acpi: Add ACPI5 event model support to gpio.")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/gpio/gpiolib-acpi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
index c9b42dd12dfa..c3faea724af8 100644
--- a/drivers/gpio/gpiolib-acpi.c
+++ b/drivers/gpio/gpiolib-acpi.c
@@ -205,7 +205,7 @@ static acpi_status acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
 		char ev_name[5];
 		sprintf(ev_name, "_%c%02X",
 			agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
-			pin);
+			(u8)pin);
 		if (ACPI_SUCCESS(acpi_get_handle(handle, ev_name, &evt_handle)))
 			handler = acpi_gpio_irq_handler;
 	}
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687381 — Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers

On Fri, 2017-07-14 at 14:07 +0200, Arnd Bergmann wrote:
> gcc-7 notices that the pin_table is an array of 16-bit numbers,
> but we assume it can be printed as a two-character hexadecimal
> string:
> 
> drivers/gpio/gpiolib-acpi.c: In function
> 'acpi_gpiochip_request_interrupt':
> drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing
> between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
>    sprintf(ev_name, "_%c%02X",
>                         ^~~~
> drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the
> range [0, 65535]
>    sprintf(ev_name, "_%c%02X",
>                     ^~~~~~~~~
> drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5
> and 7 bytes into a destination of size 5
>    sprintf(ev_name, "_%c%02X",
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>     agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>     pin);
>     ~~~~


This is obviously a false positive warning.

Here we have
int pin = u16 pin_table[0] <= 255 (implying >= 0).

I see few options how to make it more clear
1) your proposal;
2) use "%02hhX" instead;
3) use if (ret >= 0 && ret <= 255) condition.

I would choose one of the 2-3.

In case gcc will complain about 3), file a bug to gcc crazy warning.

> 
> This can't be right, so this changes it to truncate the number to
> an 8-bit pin number.
> 
> Fixes: 0d1c28a449c6 ("gpiolib-acpi: Add ACPI5 event model support to
> gpio.")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/gpio/gpiolib-acpi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c
> index c9b42dd12dfa..c3faea724af8 100644
> --- a/drivers/gpio/gpiolib-acpi.c
> +++ b/drivers/gpio/gpiolib-acpi.c
> @@ -205,7 +205,7 @@ static acpi_status
> acpi_gpiochip_request_interrupt(struct acpi_resource *ares,
>  		char ev_name[5];
>  		sprintf(ev_name, "_%c%02X",
>  			agpio->triggering == ACPI_EDGE_SENSITIVE ?
> 'E' : 'L',
> -			pin);
> +			(u8)pin);
>  		if (ACPI_SUCCESS(acpi_get_handle(handle, ev_name,
> &evt_handle)))
>  			handler = acpi_gpio_irq_handler;
>  	}

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

[toc] | [prev] | [next] | [standalone]

#1687632 — Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers

On Fri, Jul 14, 2017 at 2:52 PM, Andy Shevchenko
<andriy.shevchenko@linux.intel.com> wrote:
> On Fri, 2017-07-14 at 14:07 +0200, Arnd Bergmann wrote:
>> gcc-7 notices that the pin_table is an array of 16-bit numbers,
>> but we assume it can be printed as a two-character hexadecimal
>> string:
>>
>> drivers/gpio/gpiolib-acpi.c: In function
>> 'acpi_gpiochip_request_interrupt':
>> drivers/gpio/gpiolib-acpi.c:206:24: warning: '%02X' directive writing
>> between 2 and 4 bytes into a region of size 3 [-Wformat-overflow=]
>>    sprintf(ev_name, "_%c%02X",
>>                         ^~~~
>> drivers/gpio/gpiolib-acpi.c:206:20: note: directive argument in the
>> range [0, 65535]
>>    sprintf(ev_name, "_%c%02X",
>>                     ^~~~~~~~~
>> drivers/gpio/gpiolib-acpi.c:206:3: note: 'sprintf' output between 5
>> and 7 bytes into a destination of size 5
>>    sprintf(ev_name, "_%c%02X",
>>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~
>>     agpio->triggering == ACPI_EDGE_SENSITIVE ? 'E' : 'L',
>>     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>>     pin);
>>     ~~~~
>
>
> This is obviously a false positive warning.
>
> Here we have
> int pin = u16 pin_table[0] <= 255 (implying >= 0).
>
> I see few options how to make it more clear
> 1) your proposal;
> 2) use "%02hhX" instead;
> 3) use if (ret >= 0 && ret <= 255) condition.
>
> I would choose one of the 2-3.
>
> In case gcc will complain about 3), file a bug to gcc crazy warning.

Makes sense. I didn't remember the syntax for 2) and couldn't find
it in the man page when I first looked. This seems like a good solution
here.

I'm pretty sure I tried 3) a few times when the warning first showed
up last year, but couldn't get that to work. Filing a gcc bug also seems
like a good idea, but I should first see if it's already fixed. The version
I use for testing at the moment is from late April, and others may
have complained about that already.

      Arnd

[toc] | [prev] | [next] | [standalone]

#1687305 — [PATCH 03/22] scsi: mpt3sas: fix format overflow warning

We print the driver name into one string and then add and ID
and copy it into a second string of the same length, at which
point gcc complains about a possible overflow:

drivers/scsi/mpt3sas/mpt3sas_scsih.c: In function '_scsih_probe':
drivers/scsi/mpt3sas/mpt3sas_scsih.c:8884:21: error: '_cm' directive writing 3 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]
printf(ioc->name, "%s_cm%d", ioc->driver_name, ioc->id);
                  ^~~~~~~~~
drivers/scsi/mpt3sas/mpt3sas_scsih.c:8884:21: note: directive argument in the range [0, 255]
drivers/scsi/mpt3sas/mpt3sas_scsih.c:8884:2: note: 'sprintf' output between 5 and 38 bytes into a destination of size 32
  sprintf(ioc->name, "%s_cm%d", ioc->driver_name, ioc->id);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Making the first string shorter is sufficient to avoid the
warning here, as we know it can only contain either "mpt2sas"
or "mpt3sas".

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/scsi/mpt3sas/mpt3sas_base.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/mpt3sas/mpt3sas_base.h b/drivers/scsi/mpt3sas/mpt3sas_base.h
index 099ab4ca7edf..a77bb7dc12b1 100644
--- a/drivers/scsi/mpt3sas/mpt3sas_base.h
+++ b/drivers/scsi/mpt3sas/mpt3sas_base.h
@@ -970,7 +970,7 @@ struct MPT3SAS_ADAPTER {
 	u8		id;
 	int		cpu_count;
 	char		name[MPT_NAME_LENGTH];
-	char		driver_name[MPT_NAME_LENGTH];
+	char		driver_name[MPT_NAME_LENGTH - 8];
 	char		tmp_string[MPT_STRING_LENGTH];
 	struct pci_dev	*pdev;
 	Mpi2SystemInterfaceRegs_t __iomem *chip;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687306 — [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning

gcc warns that the temporary buffer might be too small here:

drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function 'bgx_probe':
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:16: error: '%d' directive writing between 1 and 10 bytes into a region of size between 9 and 11 [-Werror=format-overflow=]
sprintf(str, "BGX%d LMAC%d mode", bgx->bgx_id, lmacid);
             ^~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:16: note: directive argument in the range [0, 2147483647]
drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:3: note: 'sprintf' output between 16 and 27 bytes into a destination of size 20

This probably can't happen, but it can't hurt to make it long
enough for the theoretical limit.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
index a0ca68ce3fbb..79112563a25a 100644
--- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
+++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
@@ -1008,7 +1008,7 @@ static void bgx_print_qlm_mode(struct bgx *bgx, u8 lmacid)
 {
 	struct device *dev = &bgx->pdev->dev;
 	struct lmac *lmac;
-	char str[20];
+	char str[27];
 
 	if (!bgx->is_dlm && lmacid)
 		return;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687344 — Re: [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning

On 14/07/17 13:07, Arnd Bergmann wrote:
> gcc warns that the temporary buffer might be too small here:
> 
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function 'bgx_probe':
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:16: error: '%d' directive writing between 1 and 10 bytes into a region of size between 9 and 11 [-Werror=format-overflow=]
> sprintf(str, "BGX%d LMAC%d mode", bgx->bgx_id, lmacid);
>              ^~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:16: note: directive argument in the range [0, 2147483647]
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:3: note: 'sprintf' output between 16 and 27 bytes into a destination of size 20
> 
> This probably can't happen, but it can't hurt to make it long
> enough for the theoretical limit.

Probably indeed - both bgx_id and lmacid are u8 here, which would make
the maximum length of that string, including null terminator, exactly 20
characters.

So in this case the warning is not only silly, it's actively wrong;
sure, the arguments themselves are being promoted to ints at that point,
but GCC *knows* the original type, or it couldn't have generated the
correct code for the call :/

Robin.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/net/ethernet/cavium/thunder/thunder_bgx.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> index a0ca68ce3fbb..79112563a25a 100644
> --- a/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> +++ b/drivers/net/ethernet/cavium/thunder/thunder_bgx.c
> @@ -1008,7 +1008,7 @@ static void bgx_print_qlm_mode(struct bgx *bgx, u8 lmacid)
>  {
>  	struct device *dev = &bgx->pdev->dev;
>  	struct lmac *lmac;
> -	char str[20];
> +	char str[27];
>  
>  	if (!bgx->is_dlm && lmacid)
>  		return;
> 

[toc] | [prev] | [next] | [standalone]

#1687534 — Re: [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 14 Jul 2017 14:07:03 +0200

> gcc warns that the temporary buffer might be too small here:
> 
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c: In function 'bgx_probe':
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:16: error: '%d' directive writing between 1 and 10 bytes into a region of size between 9 and 11 [-Werror=format-overflow=]
> sprintf(str, "BGX%d LMAC%d mode", bgx->bgx_id, lmacid);
>              ^~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:16: note: directive argument in the range [0, 2147483647]
> drivers/net/ethernet/cavium/thunder/thunder_bgx.c:1020:3: note: 'sprintf' output between 16 and 27 bytes into a destination of size 20
> 
> This probably can't happen, but it can't hurt to make it long
> enough for the theoretical limit.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

[toc] | [prev] | [next] | [standalone]

#1687307 — [PATCH 10/22] bnx2x: fix format overflow warning

gcc notices that large queue numbers would overflow the queue name
string:

drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c: In function 'bnx2x_get_strings':
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3165:25: error: '%d' directive writing between 1 and 10 bytes into a region of size 5 [-Werror=format-overflow=]
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3165:25: note: directive argument in the range [0, 2147483647]
drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3165:5: note: 'sprintf' output between 2 and 11 bytes into a destination of size 5

There is a hard limit in place that makes the number at most two
digits, so the code is fine. This changes it to use snprintf()
to truncate instead of overflowing, which shuts up that warning.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
index 21bc4bed6b26..1e33abde4a3e 100644
--- a/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
+++ b/drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c
@@ -3162,7 +3162,8 @@ static void bnx2x_get_strings(struct net_device *dev, u32 stringset, u8 *buf)
 		if (is_multi(bp)) {
 			for_each_eth_queue(bp, i) {
 				memset(queue_name, 0, sizeof(queue_name));
-				sprintf(queue_name, "%d", i);
+				snprintf(queue_name, sizeof(queue_name),
+					 "%d", i);
 				for (j = 0; j < BNX2X_NUM_Q_STATS; j++)
 					snprintf(buf + (k + j)*ETH_GSTRING_LEN,
 						ETH_GSTRING_LEN,
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687539 — Re: [PATCH 10/22] bnx2x: fix format overflow warning

From: Arnd Bergmann <arnd@arndb.de>
Date: Fri, 14 Jul 2017 14:07:02 +0200

> gcc notices that large queue numbers would overflow the queue name
> string:
> 
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c: In function 'bnx2x_get_strings':
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3165:25: error: '%d' directive writing between 1 and 10 bytes into a region of size 5 [-Werror=format-overflow=]
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3165:25: note: directive argument in the range [0, 2147483647]
> drivers/net/ethernet/broadcom/bnx2x/bnx2x_ethtool.c:3165:5: note: 'sprintf' output between 2 and 11 bytes into a destination of size 5
> 
> There is a hard limit in place that makes the number at most two
> digits, so the code is fine. This changes it to use snprintf()
> to truncate instead of overflowing, which shuts up that warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

[toc] | [prev] | [next] | [standalone]

#1687308 — [PATCH 21/22] fscache: fix fscache_objlist_show format processing

gcc points out a minor bug in the handling of unknown
cookie types, which could result in a string overflow
when the integer is copied into a 3-byte string:

fs/fscache/object-list.c: In function 'fscache_objlist_show':
fs/fscache/object-list.c:265:19: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
 sprintf(_type, "%02u", cookie->def->type);
                ^~~~~~
fs/fscache/object-list.c:265:4: note: 'sprintf' output between 3 and 4 bytes into a destination of size 3

This is currently harmless as no code sets a type other
than 0 or 1, but it makes sense to use snprintf() here
to avoid overflowing the array if that changes.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 fs/fscache/object-list.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fscache/object-list.c b/fs/fscache/object-list.c
index 67f940892ef8..b5ab06fabc60 100644
--- a/fs/fscache/object-list.c
+++ b/fs/fscache/object-list.c
@@ -262,7 +262,8 @@ static int fscache_objlist_show(struct seq_file *m, void *v)
 			type = "DT";
 			break;
 		default:
-			sprintf(_type, "%02u", cookie->def->type);
+			snprintf(_type, sizeof(_type), "%02u",
+				 cookie->def->type);
 			type = _type;
 			break;
 		}
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1687309 — [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning

gcc-7 notices that we copy a fixed length string into another
string of the same size, with additional characters:

drivers/media/usb/usbvision/usbvision-i2c.c: In function 'usbvision_i2c_register':
drivers/media/usb/usbvision/usbvision-i2c.c:190:36: error: '%d' directive writing between 1 and 11 bytes into a region of size between 0 and 47 [-Werror=format-overflow=]
  sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
                                    ^~~~~~~~~~
drivers/media/usb/usbvision/usbvision-i2c.c:190:2: note: 'sprintf' output between 4 and 76 bytes into a destination of size 48

We know this is fine as the template name is always "usbvision", so
we can easily avoid the warning by using this as the format string
directly.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/media/usb/usbvision/usbvision-i2c.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/usbvision/usbvision-i2c.c b/drivers/media/usb/usbvision/usbvision-i2c.c
index fdf6b6e285da..aae9f69884da 100644
--- a/drivers/media/usb/usbvision/usbvision-i2c.c
+++ b/drivers/media/usb/usbvision/usbvision-i2c.c
@@ -187,8 +187,8 @@ int usbvision_i2c_register(struct usb_usbvision *usbvision)
 
 	usbvision->i2c_adap = i2c_adap_template;
 
-	sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
-		usbvision->dev->bus->busnum, usbvision->dev->devpath);
+	sprintf(usbvision->i2c_adap.name, "usbvision-%d-%s",
+		 usbvision->dev->bus->busnum, usbvision->dev->devpath);
 	PDEBUG(DBG_I2C, "Adaptername: %s", usbvision->i2c_adap.name);
 	usbvision->i2c_adap.dev.parent = &usbvision->dev->dev;
 
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1689006 — Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning

On 14/07/17 14:07, Arnd Bergmann wrote:
> gcc-7 notices that we copy a fixed length string into another
> string of the same size, with additional characters:
> 
> drivers/media/usb/usbvision/usbvision-i2c.c: In function 'usbvision_i2c_register':
> drivers/media/usb/usbvision/usbvision-i2c.c:190:36: error: '%d' directive writing between 1 and 11 bytes into a region of size between 0 and 47 [-Werror=format-overflow=]
>   sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
>                                     ^~~~~~~~~~
> drivers/media/usb/usbvision/usbvision-i2c.c:190:2: note: 'sprintf' output between 4 and 76 bytes into a destination of size 48
> 
> We know this is fine as the template name is always "usbvision", so
> we can easily avoid the warning by using this as the format string
> directly.

Hmm, how about replacing sprintf by snprintf? That feels a lot safer (this is very
old code, it's not surprising it is still using sprintf).

Regards,

	Hans

> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/media/usb/usbvision/usbvision-i2c.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/usb/usbvision/usbvision-i2c.c b/drivers/media/usb/usbvision/usbvision-i2c.c
> index fdf6b6e285da..aae9f69884da 100644
> --- a/drivers/media/usb/usbvision/usbvision-i2c.c
> +++ b/drivers/media/usb/usbvision/usbvision-i2c.c
> @@ -187,8 +187,8 @@ int usbvision_i2c_register(struct usb_usbvision *usbvision)
>  
>  	usbvision->i2c_adap = i2c_adap_template;
>  
> -	sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
> -		usbvision->dev->bus->busnum, usbvision->dev->devpath);
> +	sprintf(usbvision->i2c_adap.name, "usbvision-%d-%s",
> +		 usbvision->dev->bus->busnum, usbvision->dev->devpath);
>  	PDEBUG(DBG_I2C, "Adaptername: %s", usbvision->i2c_adap.name);
>  	usbvision->i2c_adap.dev.parent = &usbvision->dev->dev;
>  
> 

[toc] | [prev] | [next] | [standalone]

#1689008 — Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning

On Mon, Jul 17, 2017 at 2:53 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 14/07/17 14:07, Arnd Bergmann wrote:
>> gcc-7 notices that we copy a fixed length string into another
>> string of the same size, with additional characters:
>>
>> drivers/media/usb/usbvision/usbvision-i2c.c: In function 'usbvision_i2c_register':
>> drivers/media/usb/usbvision/usbvision-i2c.c:190:36: error: '%d' directive writing between 1 and 11 bytes into a region of size between 0 and 47 [-Werror=format-overflow=]
>>   sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
>>                                     ^~~~~~~~~~
>> drivers/media/usb/usbvision/usbvision-i2c.c:190:2: note: 'sprintf' output between 4 and 76 bytes into a destination of size 48
>>
>> We know this is fine as the template name is always "usbvision", so
>> we can easily avoid the warning by using this as the format string
>> directly.
>
> Hmm, how about replacing sprintf by snprintf? That feels a lot safer (this is very
> old code, it's not surprising it is still using sprintf).

With snprintf(), you will still get a -Wformat-truncation warning. One
of my patches
disables that warning by default, but Mauro likes build-testing with
"make W=1", so
it would still show up then.

However, we can do both: replace the string and use snprintf().

       Arnd

[toc] | [prev] | [next] | [standalone]

#1689012 — Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning

On 17/07/17 14:57, Arnd Bergmann wrote:
> On Mon, Jul 17, 2017 at 2:53 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 14/07/17 14:07, Arnd Bergmann wrote:
>>> gcc-7 notices that we copy a fixed length string into another
>>> string of the same size, with additional characters:
>>>
>>> drivers/media/usb/usbvision/usbvision-i2c.c: In function 'usbvision_i2c_register':
>>> drivers/media/usb/usbvision/usbvision-i2c.c:190:36: error: '%d' directive writing between 1 and 11 bytes into a region of size between 0 and 47 [-Werror=format-overflow=]
>>>   sprintf(usbvision->i2c_adap.name, "%s-%d-%s", i2c_adap_template.name,
>>>                                     ^~~~~~~~~~
>>> drivers/media/usb/usbvision/usbvision-i2c.c:190:2: note: 'sprintf' output between 4 and 76 bytes into a destination of size 48
>>>
>>> We know this is fine as the template name is always "usbvision", so
>>> we can easily avoid the warning by using this as the format string
>>> directly.
>>
>> Hmm, how about replacing sprintf by snprintf? That feels a lot safer (this is very
>> old code, it's not surprising it is still using sprintf).
> 
> With snprintf(), you will still get a -Wformat-truncation warning. One
> of my patches
> disables that warning by default, but Mauro likes build-testing with
> "make W=1", so
> it would still show up then.
> 
> However, we can do both: replace the string and use snprintf().

Yes please!

Regards,

	Hans

[toc] | [prev] | [next] | [standalone]

#1687310 — [PATCH 04/22] scsi: fusion: fix string overflow warning

gcc points out a theorerical string overflow:

drivers/message/fusion/mptbase.c: In function 'mpt_detach':
drivers/message/fusion/mptbase.c:2103:17: error: '%s' directive writing up to 31 bytes into a region of size 28 [-Werror=format-overflow=]
sprintf(pname, MPT_PROCFS_MPTBASEDIR "/%s/summary", ioc->name);
               ^~~~~
drivers/message/fusion/mptbase.c:2103:2: note: 'sprintf' output between 13 and 44 bytes into a destination of size 32

We can simply double the size of the local buffer here to be on the
safe side.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/message/fusion/mptbase.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/message/fusion/mptbase.c b/drivers/message/fusion/mptbase.c
index 62cff5afc6bd..46b67a67edc8 100644
--- a/drivers/message/fusion/mptbase.c
+++ b/drivers/message/fusion/mptbase.c
@@ -2079,7 +2079,7 @@ void
 mpt_detach(struct pci_dev *pdev)
 {
 	MPT_ADAPTER 	*ioc = pci_get_drvdata(pdev);
-	char pname[32];
+	char pname[64];
 	u8 cb_idx;
 	unsigned long flags;
 	struct workqueue_struct *wq;
-- 
2.9.0

[toc] | [prev] | [next] | [standalone]

#1688793 — RE: [PATCH 04/22] scsi: fusion: fix string overflow warning

From: Arnd Bergmann
> Sent: 14 July 2017 13:07
> gcc points out a theorerical string overflow:
> 
> drivers/message/fusion/mptbase.c: In function 'mpt_detach':
> drivers/message/fusion/mptbase.c:2103:17: error: '%s' directive writing up to 31 bytes into a region
> of size 28 [-Werror=format-overflow=]
> sprintf(pname, MPT_PROCFS_MPTBASEDIR "/%s/summary", ioc->name);
>                ^~~~~
> drivers/message/fusion/mptbase.c:2103:2: note: 'sprintf' output between 13 and 44 bytes into a
> destination of size 32
> 
> We can simply double the size of the local buffer here to be on the
> safe side.

I think I'd change it to snprintf() as well.
Saves any worries if ioc->name isn't '\0' terminated.

	David

[toc] | [prev] | [next] | [standalone]

Page 1 of 3  [1] 2 3  Next page →

Back to top | Article view | linux.kernel

csiph-web