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 2 of 3 — ← Prev page 1 [2] 3  Next page →

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

On Mon, Jul 17, 2017 at 11:17 AM, David Laight <David.Laight@aculab.com> wrote:
> 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.

Ok, fair enough, I'll send a new version right away.

      Arnd

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

#1687311 — [PATCH 15/22] hwmon: applesmc: fix format string overflow

gcc-7 warns that the key might exceed five bytes for lage index
values:

drivers/hwmon/applesmc.c: In function 'applesmc_show_fan_position':
drivers/hwmon/applesmc.c:906:18: error: '%d' directive writing between 1 and 5 bytes into a region of size 4 [-Werror=format-overflow=]
sprintf(newkey, FAN_ID_FMT, to_index(attr));
                ^~~~~~~
drivers/hwmon/applesmc.c:906:18: note: directive argument in the range [0, 65535]
drivers/hwmon/applesmc.c:906:2: note: 'sprintf' output between 5 and 9 bytes into a destination of size 5

As the key is required to be four characters plus trailing zero,
we know that the index has to be small here. I'm using snprintf()
to avoid the warning. This would truncate the string instead of
overflowing.

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

diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
index 0af7fd311979..515163b9a89f 100644
--- a/drivers/hwmon/applesmc.c
+++ b/drivers/hwmon/applesmc.c
@@ -903,7 +903,7 @@ static ssize_t applesmc_show_fan_position(struct device *dev,
 	char newkey[5];
 	u8 buffer[17];
 
-	sprintf(newkey, FAN_ID_FMT, to_index(attr));
+	snprintf(newkey, sizeof(newkey), FAN_ID_FMT, to_index(attr));
 
 	ret = applesmc_read_key(newkey, buffer, 16);
 	buffer[16] = 0;
-- 
2.9.0

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

#1687462 — Re: [PATCH 15/22] hwmon: applesmc: fix format string overflow

On 07/14/2017 05:07 AM, Arnd Bergmann wrote:
> gcc-7 warns that the key might exceed five bytes for lage index
> values:
> 
> drivers/hwmon/applesmc.c: In function 'applesmc_show_fan_position':
> drivers/hwmon/applesmc.c:906:18: error: '%d' directive writing between 1 and 5 bytes into a region of size 4 [-Werror=format-overflow=]
> sprintf(newkey, FAN_ID_FMT, to_index(attr));
>                  ^~~~~~~
> drivers/hwmon/applesmc.c:906:18: note: directive argument in the range [0, 65535]
> drivers/hwmon/applesmc.c:906:2: note: 'sprintf' output between 5 and 9 bytes into a destination of size 5
> 
> As the key is required to be four characters plus trailing zero,
> we know that the index has to be small here. I'm using snprintf()
> to avoid the warning. This would truncate the string instead of
> overflowing.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

I submitted a more comprehensive patch a couple of days ago. There are other similar
sprintf() calls in the driver which gcc doesn't report.

Guenter

> ---
>   drivers/hwmon/applesmc.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/hwmon/applesmc.c b/drivers/hwmon/applesmc.c
> index 0af7fd311979..515163b9a89f 100644
> --- a/drivers/hwmon/applesmc.c
> +++ b/drivers/hwmon/applesmc.c
> @@ -903,7 +903,7 @@ static ssize_t applesmc_show_fan_position(struct device *dev,
>   	char newkey[5];
>   	u8 buffer[17];
>   
> -	sprintf(newkey, FAN_ID_FMT, to_index(attr));
> +	snprintf(newkey, sizeof(newkey), FAN_ID_FMT, to_index(attr));
>   
>   	ret = applesmc_read_key(newkey, buffer, 16);
>   	buffer[16] = 0;
> 

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

#1687314 — [PATCH 12/22] vmxnet3: avoid format strint overflow warning

gcc-7 notices that "-event-%d" could be more than 11 characters long
if we had larger 'vector' numbers:

drivers/net/vmxnet3/vmxnet3_drv.c: In function 'vmxnet3_activate_dev':
drivers/net/vmxnet3/vmxnet3_drv.c:2095:40: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
sprintf(intr->event_msi_vector_name, "%s-event-%d",
                                     ^~~~~~~~~~~~~
drivers/net/vmxnet3/vmxnet3_drv.c:2095:3: note: 'sprintf' output between 9 and 33 bytes into a destination of size 32

The current code is safe, but making the string a little longer
is harmless and lets gcc see that it's ok.

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

diff --git a/drivers/net/vmxnet3/vmxnet3_int.h b/drivers/net/vmxnet3/vmxnet3_int.h
index ba1c9f93592b..9c51b8be0038 100644
--- a/drivers/net/vmxnet3/vmxnet3_int.h
+++ b/drivers/net/vmxnet3/vmxnet3_int.h
@@ -311,7 +311,7 @@ struct vmxnet3_intr {
 	u8  num_intrs;			/* # of intr vectors */
 	u8  event_intr_idx;		/* idx of the intr vector for event */
 	u8  mod_levels[VMXNET3_LINUX_MAX_MSIX_VECT]; /* moderation level */
-	char	event_msi_vector_name[IFNAMSIZ+11];
+	char	event_msi_vector_name[IFNAMSIZ+17];
 #ifdef CONFIG_PCI_MSI
 	struct msix_entry msix_entries[VMXNET3_LINUX_MAX_MSIX_VECT];
 #endif
-- 
2.9.0

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

#1687538 — Re: [PATCH 12/22] vmxnet3: avoid format strint overflow warning

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

> gcc-7 notices that "-event-%d" could be more than 11 characters long
> if we had larger 'vector' numbers:
> 
> drivers/net/vmxnet3/vmxnet3_drv.c: In function 'vmxnet3_activate_dev':
> drivers/net/vmxnet3/vmxnet3_drv.c:2095:40: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
> sprintf(intr->event_msi_vector_name, "%s-event-%d",
>                                      ^~~~~~~~~~~~~
> drivers/net/vmxnet3/vmxnet3_drv.c:2095:3: note: 'sprintf' output between 9 and 33 bytes into a destination of size 32
> 
> The current code is safe, but making the string a little longer
> is harmless and lets gcc see that it's ok.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

#1687315 — [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning

gcc points out a possible format string overflow for a large value of 'zone':

drivers/platform/x86/alienware-wmi.c: In function 'alienware_wmi_init':
drivers/platform/x86/alienware-wmi.c:461:24: error: '%02X' directive writing between 2 and 8 bytes into a region of size 6 [-Werror=format-overflow=]
   sprintf(buffer, "zone%02X", i);
                        ^~~~
drivers/platform/x86/alienware-wmi.c:461:19: note: directive argument in the range [0, 2147483646]
   sprintf(buffer, "zone%02X", i);
                   ^~~~~~~~~~
drivers/platform/x86/alienware-wmi.c:461:3: note: 'sprintf' output between 7 and 13 bytes into a destination of size 10

While the zone should never be that large, it's easy to make the
buffer a few bytes longer so gcc can prove this to be safe.

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

diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
index 0831b428c217..acc01242da82 100644
--- a/drivers/platform/x86/alienware-wmi.c
+++ b/drivers/platform/x86/alienware-wmi.c
@@ -421,7 +421,7 @@ static DEVICE_ATTR(lighting_control_state, 0644, show_control_state,
 static int alienware_zone_init(struct platform_device *dev)
 {
 	int i;
-	char buffer[10];
+	char buffer[13];
 	char *name;
 
 	if (interface == WMAX) {
-- 
2.9.0

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

#1687602 — RE: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning

> -----Original Message-----
> From: Arnd Bergmann [mailto:arnd@arndb.de]
> Sent: Friday, July 14, 2017 7:07 AM
> To: linux-kernel@vger.kernel.org; Darren Hart <dvhart@infradead.org>; Andy
> Shevchenko <andy@infradead.org>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>; Linus Torvalds
> <torvalds@linux-foundation.org>; Guenter Roeck <linux@roeck-us.net>;
> akpm@linux-foundation.org; netdev@vger.kernel.org; David S . Miller
> <davem@davemloft.net>; James E . J . Bottomley <jejb@linux.vnet.ibm.com>;
> Martin K . Petersen <martin.petersen@oracle.com>; linux-scsi@vger.kernel.org;
> x86@kernel.org; Arnd Bergmann <arnd@arndb.de>; Limonciello, Mario
> <Mario_Limonciello@Dell.com>; Arvind Yadav <arvind.yadav.cs@gmail.com>;
> platform-driver-x86@vger.kernel.org
> Subject: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow
> warning
> 
> gcc points out a possible format string overflow for a large value of 'zone':
> 
> drivers/platform/x86/alienware-wmi.c: In function 'alienware_wmi_init':
> drivers/platform/x86/alienware-wmi.c:461:24: error: '%02X' directive writing
> between 2 and 8 bytes into a region of size 6 [-Werror=format-overflow=]
>    sprintf(buffer, "zone%02X", i);
>                         ^~~~
> drivers/platform/x86/alienware-wmi.c:461:19: note: directive argument in the
> range [0, 2147483646]
>    sprintf(buffer, "zone%02X", i);
>                    ^~~~~~~~~~
> drivers/platform/x86/alienware-wmi.c:461:3: note: 'sprintf' output between 7 and
> 13 bytes into a destination of size 10
> 
> While the zone should never be that large, it's easy to make the
> buffer a few bytes longer so gcc can prove this to be safe.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/platform/x86/alienware-wmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/alienware-wmi.c
> b/drivers/platform/x86/alienware-wmi.c
> index 0831b428c217..acc01242da82 100644
> --- a/drivers/platform/x86/alienware-wmi.c
> +++ b/drivers/platform/x86/alienware-wmi.c
> @@ -421,7 +421,7 @@ static DEVICE_ATTR(lighting_control_state, 0644,
> show_control_state,
>  static int alienware_zone_init(struct platform_device *dev)
>  {
>  	int i;
> -	char buffer[10];
> +	char buffer[13];
>  	char *name;
> 
>  	if (interface == WMAX) {
> --
> 2.9.0

LGTM,  Thanks.

Signed-off-by: Mario Limonciello <mario.limonciello@dell.com>

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

#1687612 — Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning

On Fri, Jul 14, 2017 at 3:07 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> gcc points out a possible format string overflow for a large value of 'zone':
>
> drivers/platform/x86/alienware-wmi.c: In function 'alienware_wmi_init':
> drivers/platform/x86/alienware-wmi.c:461:24: error: '%02X' directive writing between 2 and 8 bytes into a region of size 6 [-Werror=format-overflow=]
>    sprintf(buffer, "zone%02X", i);
>                         ^~~~
> drivers/platform/x86/alienware-wmi.c:461:19: note: directive argument in the range [0, 2147483646]
>    sprintf(buffer, "zone%02X", i);
>                    ^~~~~~~~~~
> drivers/platform/x86/alienware-wmi.c:461:3: note: 'sprintf' output between 7 and 13 bytes into a destination of size 10
>
> While the zone should never be that large, it's easy to make the
> buffer a few bytes longer so gcc can prove this to be safe.

Please, be a bit smarter on such fixes.

Here we need to convert

int i;

to

u8 i;

I will take it after addressing above.

P.S. You may do this change across the file.

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/platform/x86/alienware-wmi.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/x86/alienware-wmi.c b/drivers/platform/x86/alienware-wmi.c
> index 0831b428c217..acc01242da82 100644
> --- a/drivers/platform/x86/alienware-wmi.c
> +++ b/drivers/platform/x86/alienware-wmi.c
> @@ -421,7 +421,7 @@ static DEVICE_ATTR(lighting_control_state, 0644, show_control_state,
>  static int alienware_zone_init(struct platform_device *dev)
>  {
>         int i;
> -       char buffer[10];
> +       char buffer[13];
>         char *name;
>
>         if (interface == WMAX) {
> --
> 2.9.0
>



-- 
With Best Regards,
Andy Shevchenko

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

#1687621 — Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning

On Fri, Jul 14, 2017 at 9:18 PM, Andy Shevchenko
<andy.shevchenko@gmail.com> wrote:
> On Fri, Jul 14, 2017 at 3:07 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>> gcc points out a possible format string overflow for a large value of 'zone':
>>
>> drivers/platform/x86/alienware-wmi.c: In function 'alienware_wmi_init':
>> drivers/platform/x86/alienware-wmi.c:461:24: error: '%02X' directive writing between 2 and 8 bytes into a region of size 6 [-Werror=format-overflow=]
>>    sprintf(buffer, "zone%02X", i);
>>                         ^~~~
>> drivers/platform/x86/alienware-wmi.c:461:19: note: directive argument in the range [0, 2147483646]
>>    sprintf(buffer, "zone%02X", i);
>>                    ^~~~~~~~~~
>> drivers/platform/x86/alienware-wmi.c:461:3: note: 'sprintf' output between 7 and 13 bytes into a destination of size 10
>>
>> While the zone should never be that large, it's easy to make the
>> buffer a few bytes longer so gcc can prove this to be safe.
>
> Please, be a bit smarter on such fixes.
>
> Here we need to convert
>
> int i;
>
> to
>
> u8 i;

That was my first impulse, but then I decided not to change the
idiomatic 'int i' for the index variable to 'u8' as that would be
less idiomatic.

> I will take it after addressing above.
>
> P.S. You may do this change across the file.

How about changing it to 'u8 zone'?

     Arnd

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

#1687626 — Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning

On Fri, Jul 14, 2017 at 10:37 PM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Jul 14, 2017 at 9:18 PM, Andy Shevchenko
> <andy.shevchenko@gmail.com> wrote:
>> On Fri, Jul 14, 2017 at 3:07 PM, Arnd Bergmann <arnd@arndb.de> wrote:
>>> gcc points out a possible format string overflow for a large value of 'zone':

>> Here we need to convert
>>
>> int i;
>>
>> to
>>
>> u8 i;
>
> That was my first impulse, but then I decided not to change the
> idiomatic 'int i' for the index variable to 'u8' as that would be
> less idiomatic.
>
>> I will take it after addressing above.
>>
>> P.S. You may do this change across the file.
>
> How about changing it to 'u8 zone'?

I'm ultimately fine with that (just gentle reminder you might fix all
3 occurrences of it in that driver).

-- 
With Best Regards,
Andy Shevchenko

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

#1687317 — [PATCH 05/22] scsi: gdth: avoid buffer overflow warning

gcc notices that we would overflow the buffer for the
inquiry of the product name if we have too many adapters:

drivers/scsi/gdth.c: In function 'gdth_next':
drivers/scsi/gdth.c:2357:29: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
         sprintf(inq.product,"Host Drive  #%02d",t);
                             ^~~~~~~~~~~~~~~~~~~
drivers/scsi/gdth.c:2357:9: note: 'sprintf' output between 16 and 17 bytes into a destination of size 16
         sprintf(inq.product,"Host Drive  #%02d",t);

This won't happen in practice, so just use snprintf to
truncate the string.

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

diff --git a/drivers/scsi/gdth.c b/drivers/scsi/gdth.c
index facc7271f932..a4473356a9dc 100644
--- a/drivers/scsi/gdth.c
+++ b/drivers/scsi/gdth.c
@@ -2354,7 +2354,7 @@ static int gdth_internal_cache_cmd(gdth_ha_str *ha, Scsi_Cmnd *scp)
         inq.resp_aenc = 2;
         inq.add_length= 32;
         strcpy(inq.vendor,ha->oem_name);
-        sprintf(inq.product,"Host Drive  #%02d",t);
+        snprintf(inq.product, sizeof(inq.product), "Host Drive  #%02d",t);
         strcpy(inq.revision,"   ");
         gdth_copy_internal_data(ha, scp, (char*)&inq, sizeof(gdth_inq_data));
         break;
-- 
2.9.0

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

#1687318 — [PATCH 09/22] net: niu: fix format string overflow warning:

We get a warning for the port_name string that might be longer than
six characters if we had more than 10 ports:

drivers/net/ethernet/sun/niu.c: In function 'niu_put_parent':
drivers/net/ethernet/sun/niu.c:9563:21: error: '%d' directive writing between 1 and 3 bytes into a region of size 2 [-Werror=format-overflow=]
  sprintf(port_name, "port%d", port);
                     ^~~~~~~~
drivers/net/ethernet/sun/niu.c:9563:21: note: directive argument in the range [0, 255]
drivers/net/ethernet/sun/niu.c:9563:2: note: 'sprintf' output between 6 and 8 bytes into a destination of size 6
  sprintf(port_name, "port%d", port);
  ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/sun/niu.c: In function 'niu_pci_init_one':
drivers/net/ethernet/sun/niu.c:9538:22: error: '%d' directive writing between 1 and 3 bytes into a region of size 2 [-Werror=format-overflow=]
   sprintf(port_name, "port%d", port);
                      ^~~~~~~~
drivers/net/ethernet/sun/niu.c:9538:22: note: directive argument in the range [0, 255]
drivers/net/ethernet/sun/niu.c:9538:3: note: 'sprintf' output between 6 and 8 bytes into a destination of size 6

While we know that the port number is small, there is no harm in
making the format string two bytes longer to avoid the warning.

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

diff --git a/drivers/net/ethernet/sun/niu.c b/drivers/net/ethernet/sun/niu.c
index 46cb7f8955a2..4bb04aaf9650 100644
--- a/drivers/net/ethernet/sun/niu.c
+++ b/drivers/net/ethernet/sun/niu.c
@@ -9532,7 +9532,7 @@ static struct niu_parent *niu_get_parent(struct niu *np,
 		p = niu_new_parent(np, id, ptype);
 
 	if (p) {
-		char port_name[6];
+		char port_name[8];
 		int err;
 
 		sprintf(port_name, "port%d", port);
@@ -9553,7 +9553,7 @@ static void niu_put_parent(struct niu *np)
 {
 	struct niu_parent *p = np->parent;
 	u8 port = np->port;
-	char port_name[6];
+	char port_name[8];
 
 	BUG_ON(!p || p->ports[port] != np);
 
-- 
2.9.0

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

#1687542 — Re: [PATCH 09/22] net: niu: fix format string overflow warning:

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

> We get a warning for the port_name string that might be longer than
> six characters if we had more than 10 ports:
> 
> drivers/net/ethernet/sun/niu.c: In function 'niu_put_parent':
> drivers/net/ethernet/sun/niu.c:9563:21: error: '%d' directive writing between 1 and 3 bytes into a region of size 2 [-Werror=format-overflow=]
>   sprintf(port_name, "port%d", port);
>                      ^~~~~~~~
> drivers/net/ethernet/sun/niu.c:9563:21: note: directive argument in the range [0, 255]
> drivers/net/ethernet/sun/niu.c:9563:2: note: 'sprintf' output between 6 and 8 bytes into a destination of size 6
>   sprintf(port_name, "port%d", port);
>   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/sun/niu.c: In function 'niu_pci_init_one':
> drivers/net/ethernet/sun/niu.c:9538:22: error: '%d' directive writing between 1 and 3 bytes into a region of size 2 [-Werror=format-overflow=]
>    sprintf(port_name, "port%d", port);
>                       ^~~~~~~~
> drivers/net/ethernet/sun/niu.c:9538:22: note: directive argument in the range [0, 255]
> drivers/net/ethernet/sun/niu.c:9538:3: note: 'sprintf' output between 6 and 8 bytes into a destination of size 6
> 
> While we know that the port number is small, there is no harm in
> making the format string two bytes longer to avoid the warning.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

#1687319 — [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning

One string we pass into the cs->info buffer might be too long,
as pointed out by gcc:

drivers/isdn/divert/isdn_divert.c: In function 'll_callback':
drivers/isdn/divert/isdn_divert.c:488:22: error: '%d' directive writing between 1 and 3 bytes into a region of size between 1 and 69 [-Werror=format-overflow=]
 sprintf(cs->info, "%d 0x%lx %s %s %s %s 0x%x 0x%x %d %d %s\n",
                   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/isdn/divert/isdn_divert.c:488:22: note: directive argument in the range [0, 255]
drivers/isdn/divert/isdn_divert.c:488:4: note: 'sprintf' output 25 or more bytes (assuming 129) into a destination of size 90

This is unlikely to actually cause problems, so let's use snprintf
as a simple workaround to shut  up the warning and truncate the
buffer instead.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/isdn/divert/isdn_divert.c | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/drivers/isdn/divert/isdn_divert.c b/drivers/isdn/divert/isdn_divert.c
index 060d357f107f..6f423bc49d0d 100644
--- a/drivers/isdn/divert/isdn_divert.c
+++ b/drivers/isdn/divert/isdn_divert.c
@@ -485,18 +485,19 @@ static int isdn_divert_icall(isdn_ctrl *ic)
 				cs->deflect_dest[0] = '\0';
 				retval = 4; /* only proceed */
 			}
-			sprintf(cs->info, "%d 0x%lx %s %s %s %s 0x%x 0x%x %d %d %s\n",
-				cs->akt_state,
-				cs->divert_id,
-				divert_if.drv_to_name(cs->ics.driver),
-				(ic->command == ISDN_STAT_ICALLW) ? "1" : "0",
-				cs->ics.parm.setup.phone,
-				cs->ics.parm.setup.eazmsn,
-				cs->ics.parm.setup.si1,
-				cs->ics.parm.setup.si2,
-				cs->ics.parm.setup.screen,
-				dv->rule.waittime,
-				cs->deflect_dest);
+			snprintf(cs->info, sizeof(cs->info),
+				 "%d 0x%lx %s %s %s %s 0x%x 0x%x %d %d %s\n",
+				 cs->akt_state,
+				 cs->divert_id,
+				 divert_if.drv_to_name(cs->ics.driver),
+				 (ic->command == ISDN_STAT_ICALLW) ? "1" : "0",
+				 cs->ics.parm.setup.phone,
+				 cs->ics.parm.setup.eazmsn,
+				 cs->ics.parm.setup.si1,
+				 cs->ics.parm.setup.si2,
+				 cs->ics.parm.setup.screen,
+				 dv->rule.waittime,
+				 cs->deflect_dest);
 			if ((dv->rule.action == DEFLECT_REPORT) ||
 			    (dv->rule.action == DEFLECT_REJECT)) {
 				put_info_buffer(cs->info);
-- 
2.9.0

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

#1687533 — Re: [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning

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

> One string we pass into the cs->info buffer might be too long,
> as pointed out by gcc:
> 
> drivers/isdn/divert/isdn_divert.c: In function 'll_callback':
> drivers/isdn/divert/isdn_divert.c:488:22: error: '%d' directive writing between 1 and 3 bytes into a region of size between 1 and 69 [-Werror=format-overflow=]
>  sprintf(cs->info, "%d 0x%lx %s %s %s %s 0x%x 0x%x %d %d %s\n",
>                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/isdn/divert/isdn_divert.c:488:22: note: directive argument in the range [0, 255]
> drivers/isdn/divert/isdn_divert.c:488:4: note: 'sprintf' output 25 or more bytes (assuming 129) into a destination of size 90
> 
> This is unlikely to actually cause problems, so let's use snprintf
> as a simple workaround to shut  up the warning and truncate the
> buffer instead.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

#1687320 — [PATCH 16/22] x86: intel-mid: fix a format string overflow warning

We have space for exactly one character for the index in "max7315_%d_base",
but as gcc points out having more would cause an string overflow:

arch/x86/platform/intel-mid/device_libs/platform_max7315.c: In function 'max7315_platform_data':
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26: error: '%d' directive writing between 1 and 11 bytes into a region of size 9 [-Werror=format-overflow=]
   sprintf(base_pin_name, "max7315_%d_base", nr);
                          ^~~~~~~~~~~~~~~~~
arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:26: note: directive argument in the range [-2147483647, 2147483647]
arm-soc/arch/x86/platform/intel-mid/device_libs/platform_max7315.c:41:3: note: 'sprintf' output between 15 and 25 bytes into a destination of size 17
   sprintf(base_pin_name, "max7315_%d_base", nr);

This makes it use an snprintf() to truncate the string if that happened
rather than overflowing the stack.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 arch/x86/platform/intel-mid/device_libs/platform_max7315.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
index 6e075afa7877..58337b2bc682 100644
--- a/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
+++ b/arch/x86/platform/intel-mid/device_libs/platform_max7315.c
@@ -38,8 +38,10 @@ static void __init *max7315_platform_data(void *info)
 	 */
 	strcpy(i2c_info->type, "max7315");
 	if (nr++) {
-		sprintf(base_pin_name, "max7315_%d_base", nr);
-		sprintf(intr_pin_name, "max7315_%d_int", nr);
+		snprintf(base_pin_name, sizeof(base_pin_name),
+			 "max7315_%d_base", nr);
+		snprintf(intr_pin_name, sizeof(intr_pin_name),
+			 "max7315_%d_int", nr);
 	} else {
 		strcpy(base_pin_name, "max7315_base");
 		strcpy(intr_pin_name, "max7315_int");
-- 
2.9.0

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

#1687321 — [PATCH 06/22] scsi: fnic: fix format string overflow warning

The MSI interrupt name can require 11 bytes in addition to the device name,
for a total of 23 bytes:

drivers/scsi/fnic/fnic_isr.c: In function 'fnic_request_intr':
drivers/scsi/fnic/fnic_isr.c:192:4: error: '-fcs-rq' directive writing 7 bytes into a region of size between 5 and 16 [-Werror=format-overflow=]
    "%.11s-fcs-rq", fnic->name);
drivers/scsi/fnic/fnic_isr.c:206:3: note: 'sprintf' output between 12 and 23 bytes into a destination of size 16
   sprintf(fnic->msix[FNIC_MSIX_ERR_NOTIFY].devname,
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    "%.11s-err-notify", fnic->name);

This extends the buffer to fit any possible value.

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

diff --git a/drivers/scsi/fnic/fnic.h b/drivers/scsi/fnic/fnic.h
index 67aab965c0f4..d094ba59ed15 100644
--- a/drivers/scsi/fnic/fnic.h
+++ b/drivers/scsi/fnic/fnic.h
@@ -180,7 +180,7 @@ enum fnic_msix_intr_index {
 
 struct fnic_msix_entry {
 	int requested;
-	char devname[IFNAMSIZ];
+	char devname[IFNAMSIZ + 11];
 	irqreturn_t (*isr)(int, void *);
 	void *devid;
 };
-- 
2.9.0

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

#1687322 — [PATCH 22/22] IB/mlx4: fix sprintf format warning

gcc-7 points out that a negative port_num value would overflow
the string buffer:

drivers/infiniband/hw/mlx4/sysfs.c: In function 'mlx4_ib_device_register_sysfs':
drivers/infiniband/hw/mlx4/sysfs.c:251:16: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
drivers/infiniband/hw/mlx4/sysfs.c:251:2: note: 'sprintf' output between 2 and 11 bytes into a destination of size 10
drivers/infiniband/hw/mlx4/sysfs.c:303:17: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
drivers/infiniband/hw/mlx4/sysfs.c:303:3: note: 'sprintf' output between 2 and 11 bytes into a destination of size 10

While we should be able to assume that port_num is positive here,
making the buffer one byte longer has no downsides and avoids the
warning.

Fixes: c1e7e466120b ("IB/mlx4: Add iov directory in sysfs under the ib device")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/infiniband/hw/mlx4/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/infiniband/hw/mlx4/sysfs.c b/drivers/infiniband/hw/mlx4/sysfs.c
index 0ba5ba7540c8..e219093d2764 100644
--- a/drivers/infiniband/hw/mlx4/sysfs.c
+++ b/drivers/infiniband/hw/mlx4/sysfs.c
@@ -221,7 +221,7 @@ void del_sysfs_port_mcg_attr(struct mlx4_ib_dev *device, int port_num,
 static int add_port_entries(struct mlx4_ib_dev *device, int port_num)
 {
 	int i;
-	char buff[10];
+	char buff[11];
 	struct mlx4_ib_iov_port *port = NULL;
 	int ret = 0 ;
 	struct ib_port_attr attr;
-- 
2.9.0

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

#1687453 — Re: [PATCH 22/22] IB/mlx4: fix sprintf format warning

[Multipart message — attachments visible in raw view] — view raw

On Fri, Jul 14, 2017 at 02:07:14PM +0200, Arnd Bergmann wrote:
> gcc-7 points out that a negative port_num value would overflow
> the string buffer:
>
> drivers/infiniband/hw/mlx4/sysfs.c: In function 'mlx4_ib_device_register_sysfs':
> drivers/infiniband/hw/mlx4/sysfs.c:251:16: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
> drivers/infiniband/hw/mlx4/sysfs.c:251:2: note: 'sprintf' output between 2 and 11 bytes into a destination of size 10
> drivers/infiniband/hw/mlx4/sysfs.c:303:17: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
> drivers/infiniband/hw/mlx4/sysfs.c:303:3: note: 'sprintf' output between 2 and 11 bytes into a destination of size 10
>
> While we should be able to assume that port_num is positive here,
> making the buffer one byte longer has no downsides and avoids the
> warning.
>
> Fixes: c1e7e466120b ("IB/mlx4: Add iov directory in sysfs under the ib device")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  drivers/infiniband/hw/mlx4/sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>

Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>

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

#1687323 — [PATCH 20/22] sound: pci: avoid string overflow warnings

With gcc-7, we get various warnings about a possible string overflow:

sound/pci/rme9652/hdspm.c: In function 'snd_hdspm_create_alsa_devices':
sound/pci/rme9652/hdspm.c:2123:17: error: ' MIDIoverMADI' directive writing 13 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]
sound/pci/pcxhr/pcxhr.c: In function 'pcxhr_probe':
sound/pci/pcxhr/pcxhr.c:1647:28: error: ' [PCM #' directive writing 7 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]
sound/pci/mixart/mixart.c: In function 'snd_mixart_probe':
sound/pci/mixart/mixart.c:1353:28: error: ' [PCM #' directive writing 7 bytes into a region of size between 1 and 32 [-Werror=format-overflow=]
   sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
                            ^~~~~~~~~~~~~~
sound/pci/mixart/mixart.c:1353:28: note: using the range [-2147483648, 2147483647] for directive argument
sound/pci/mixart/mixart.c:1353:3: note: 'sprintf' output between 10 and 51 bytes into a destination of size 32
   sprintf(card->shortname, "%s [PCM #%d]", mgr->shortname, i);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
sound/pci/mixart/mixart.c:1354:27: error: ' [PCM #' directive writing 7 bytes into a region of size between 1 and 80 [-Werror=format-overflow=]
   sprintf(card->longname, "%s [PCM #%d]", mgr->longname, i);
                           ^~~~~~~~~~~~~~
sound/pci/mixart/mixart.c:1354:27: note: using the range [-2147483648, 2147483647] for directive argument
sound/pci/mixart/mixart.c:1354:3: note: 'sprintf' output between 10 and 99 bytes into a destination of size 80

I have checked these all and found that the driver-private
shortname strings for mixart and pcxhr are longer than necessary,
and making them shorter will be safe while also making it clear
that no overflow can happen when they get passed as a substring
into the card shortname.

For hdspm, we have a local buffer of the same size as its substring.
In this case, making the buffer a little longer is safe as the
functions that take it as an argument all use length checking and
the strings we pass into it are actually short enough.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 sound/pci/mixart/mixart.h | 4 ++--
 sound/pci/pcxhr/pcxhr.h   | 4 ++--
 sound/pci/rme9652/hdspm.c | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/sound/pci/mixart/mixart.h b/sound/pci/mixart/mixart.h
index 426743871540..c8309e327663 100644
--- a/sound/pci/mixart/mixart.h
+++ b/sound/pci/mixart/mixart.h
@@ -75,8 +75,8 @@ struct mixart_mgr {
 	struct mem_area mem[2];
 
 	/* share the name */
-	char shortname[32];         /* short name of this soundcard */
-	char longname[80];          /* name of this soundcard */
+	char shortname[16];         /* short name of this soundcard */
+	char longname[40];          /* name of this soundcard */
 
 	/* one and only blocking message or notification may be pending  */
 	u32 pending_event;
diff --git a/sound/pci/pcxhr/pcxhr.h b/sound/pci/pcxhr/pcxhr.h
index 9e39e509a3ef..4909a43ce3d9 100644
--- a/sound/pci/pcxhr/pcxhr.h
+++ b/sound/pci/pcxhr/pcxhr.h
@@ -75,8 +75,8 @@ struct pcxhr_mgr {
 	unsigned long port[3];
 
 	/* share the name */
-	char shortname[32];		/* short name of this soundcard */
-	char longname[96];		/* name of this soundcard */
+	char shortname[16];		/* short name of this soundcard */
+	char longname[40];		/* name of this soundcard */
 
 	struct pcxhr_rmh *prmh;
 
diff --git a/sound/pci/rme9652/hdspm.c b/sound/pci/rme9652/hdspm.c
index 254c3d040118..a1cbf5938a0e 100644
--- a/sound/pci/rme9652/hdspm.c
+++ b/sound/pci/rme9652/hdspm.c
@@ -2061,7 +2061,7 @@ static int snd_hdspm_create_midi(struct snd_card *card,
 				 struct hdspm *hdspm, int id)
 {
 	int err;
-	char buf[32];
+	char buf[64];
 
 	hdspm->midi[id].id = id;
 	hdspm->midi[id].hdspm = hdspm;
-- 
2.9.0

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

Page 2 of 3 — ← Prev page 1 [2] 3  Next page →

Back to top | Article view | linux.kernel

csiph-web