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
        Re: [PATCH 20/22] sound: pci: avoid string overflow warnings Arnd Bergmann <arnd@arndb.de> - 2017-07-18 14:00 +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
    Re: [PATCH 00/22] gcc-7 -Wformat-* warnings "Martin K. Petersen" <martin.petersen@oracle.com> - 2017-07-25 03:50 +0200

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

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

On Fri, 14 Jul 2017 14:07:12 +0200,
Arnd Bergmann wrote:
> 
> 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>

Thanks for the patch.  I have seen it but ignored, so far, as not sure
which action is the best.  An alternative solution is to use
snprintf() blindly, for example.

For mixart, it's even better to drop mgr->shortname[] and longname[]
assignment.  The shortname is the fixed string, and the longname is
used only at copying to card->longname, so we can create a string
there from the scratch.


Takashi

> ---
>  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]

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

On Fri, Jul 14, 2017 at 2:28 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Fri, 14 Jul 2017 14:07:12 +0200,
>
> Thanks for the patch.  I have seen it but ignored, so far, as not sure
> which action is the best.  An alternative solution is to use
> snprintf() blindly, for example.
>
> For mixart, it's even better to drop mgr->shortname[] and longname[]
> assignment.  The shortname is the fixed string, and the longname is
> used only at copying to card->longname, so we can create a string
> there from the scratch.

I've done that now, and tried to be a little smarter with the other
conversions. I also found related problems in ISA drivers after
randconfig testing and fixed those as well.

Sent a 7-patch series now as a replacement.

       Arnd

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

#1687325 — [PATCH 13/22] liquidio: fix possible eeprom format string overflow

gcc reports that the temporary buffer for computing the
string length may be too small here:

drivers/net/ethernet/cavium/liquidio/lio_ethtool.c: In function 'lio_get_eeprom_len':
/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:345:21: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
  len = sprintf(buf, "boardname:%s serialnum:%s maj:%lld min:%lld\n",
                     ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:345:6: note: 'sprintf' output between 35 and 167 bytes into a destination of size 128
  len = sprintf(buf, "boardname:%s serialnum:%s maj:%lld min:%lld\n",

This extends it to 192 bytes, which is certainly enough. As far
as I could tell, there are no other constraints that require a specific
maximum size.

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

diff --git a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
index 28ecda3d3404..ebd353bc78ff 100644
--- a/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
+++ b/drivers/net/ethernet/cavium/liquidio/lio_ethtool.c
@@ -335,7 +335,7 @@ lio_ethtool_get_channels(struct net_device *dev,
 
 static int lio_get_eeprom_len(struct net_device *netdev)
 {
-	u8 buf[128];
+	u8 buf[192];
 	struct lio *lio = GET_LIO(netdev);
 	struct octeon_device *oct_dev = lio->oct_dev;
 	struct octeon_board_info *board_info;
-- 
2.9.0

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

#1687541 — Re: [PATCH 13/22] liquidio: fix possible eeprom format string overflow

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

> gcc reports that the temporary buffer for computing the
> string length may be too small here:
> 
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c: In function 'lio_get_eeprom_len':
> /drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:345:21: error: 'sprintf' may write a terminating nul past the end of the destination [-Werror=format-overflow=]
>   len = sprintf(buf, "boardname:%s serialnum:%s maj:%lld min:%lld\n",
>                      ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:345:6: note: 'sprintf' output between 35 and 167 bytes into a destination of size 128
>   len = sprintf(buf, "boardname:%s serialnum:%s maj:%lld min:%lld\n",
> 
> This extends it to 192 bytes, which is certainly enough. As far
> as I could tell, there are no other constraints that require a specific
> maximum size.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Applied.

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

#1687326 — [PATCH 19/22] block: DAC960: shut up format-overflow warning

gcc-7 points out that a large controller number would overflow the
string length for the procfs name and the firmware version string:

drivers/block/DAC960.c: In function 'DAC960_Probe':
drivers/block/DAC960.c:6591:38: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
drivers/block/DAC960.c: In function 'DAC960_V1_ReadControllerConfiguration':
drivers/block/DAC960.c:1681:40: error: '%02d' directive writing between 2 and 3 bytes into a region of size between 2 and 5 [-Werror=format-overflow=]
drivers/block/DAC960.c:1681:40: note: directive argument in the range [0, 255]
drivers/block/DAC960.c:1681:3: note: 'sprintf' output between 10 and 14 bytes into a destination of size 12

Both of these seem appropriately sized, and using snprintf()
instead of sprintf() improves this by ensuring that even
incorrect data won't cause undefined behavior here.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 drivers/block/DAC960.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/block/DAC960.c b/drivers/block/DAC960.c
index 245a879b036e..255591ab3716 100644
--- a/drivers/block/DAC960.c
+++ b/drivers/block/DAC960.c
@@ -1678,9 +1678,12 @@ static bool DAC960_V1_ReadControllerConfiguration(DAC960_Controller_T
       Enquiry2->FirmwareID.FirmwareType = '0';
       Enquiry2->FirmwareID.TurnID = 0;
     }
-  sprintf(Controller->FirmwareVersion, "%d.%02d-%c-%02d",
-	  Enquiry2->FirmwareID.MajorVersion, Enquiry2->FirmwareID.MinorVersion,
-	  Enquiry2->FirmwareID.FirmwareType, Enquiry2->FirmwareID.TurnID);
+  snprintf(Controller->FirmwareVersion, sizeof(Controller->FirmwareVersion),
+	   "%d.%02d-%c-%02d",
+	   Enquiry2->FirmwareID.MajorVersion,
+	   Enquiry2->FirmwareID.MinorVersion,
+	   Enquiry2->FirmwareID.FirmwareType,
+	   Enquiry2->FirmwareID.TurnID);
   if (!((Controller->FirmwareVersion[0] == '5' &&
 	 strcmp(Controller->FirmwareVersion, "5.06") >= 0) ||
 	(Controller->FirmwareVersion[0] == '4' &&
@@ -6588,7 +6591,8 @@ static void DAC960_CreateProcEntries(DAC960_Controller_T *Controller)
 			    &dac960_proc_fops);
 	}
 
-	sprintf(Controller->ControllerName, "c%d", Controller->ControllerNumber);
+	snprintf(Controller->ControllerName, sizeof(Controller->ControllerName),
+		 "c%d", Controller->ControllerNumber);
 	ControllerProcEntry = proc_mkdir(Controller->ControllerName,
 					 DAC960_ProcDirectoryEntry);
 	proc_create_data("initial_status", 0, ControllerProcEntry, &dac960_initial_status_proc_fops, Controller);
-- 
2.9.0

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

#1695337

Arnd,

> 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").

Applied the SCSI patches. Thanks!

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [standalone]

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

Back to top | Article view | linux.kernel

csiph-web