All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/22] gcc-7 -Wformat-* warnings
@ 2017-07-14 12:06 Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default Arnd Bergmann
                   ` (22 more replies)
  0 siblings, 23 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
@ 2017-07-14 12:06 ` Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann
                   ` (21 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Masahiro Yamada, Michal Marek
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, linux-kbuild

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 02/22] scsi: megaraid: fix format-overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default Arnd Bergmann
@ 2017-07-14 12:06 ` Arnd Bergmann
  2017-07-14 12:06   ` Arnd Bergmann
                   ` (20 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Kashyap Desai, Sumit Saxena, Shivasharan S,
	James E.J. Bottomley, Martin K. Petersen
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, linux-scsi, x86, Arnd Bergmann,
	megaraidlinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 03/22] scsi: mpt3sas: fix format overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
@ 2017-07-14 12:06   ` Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani, James E.J. Bottomley,
	Martin K. Petersen
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, linux-scsi, x86, Arnd Bergmann,
	Hannes Reinecke, Calvin Owens, Adam Manzanares, Bart Van Assche,
	James Bottomley, MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 03/22] scsi: mpt3sas: fix format overflow warning
@ 2017-07-14 12:06   ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani, James E.J. Bottomley,
	Martin K. Petersen
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, linux-scsi, x86, Arnd Bergmann,
	Hannes Reinecke, Calvin Owens, Adam Manzanares, Bart Van Assche,
	James Bottomley, MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 04/22] scsi: fusion: fix string overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
@ 2017-07-14 12:06   ` Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Sathya Prakash, Chaitra P B, Suganath Prabu Subramani
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Helge Deller,
	MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 04/22] scsi: fusion: fix string overflow warning
@ 2017-07-14 12:06   ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Sathya Prakash, Chaitra P B, Suganath Prabu Subramani
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Helge Deller,
	MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 05/22] scsi: gdth: avoid buffer overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (3 preceding siblings ...)
  2017-07-14 12:06   ` Arnd Bergmann
@ 2017-07-14 12:06 ` Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 06/22] scsi: fnic: fix format string " Arnd Bergmann
                   ` (17 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Achim Leubner, James E.J. Bottomley, Martin K. Petersen
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, linux-scsi, x86, Arnd Bergmann, David Howells

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 06/22] scsi: fnic: fix format string overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (4 preceding siblings ...)
  2017-07-14 12:06 ` [PATCH 05/22] scsi: gdth: avoid buffer " Arnd Bergmann
@ 2017-07-14 12:06 ` Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 07/22] scsi: gdth: increase the procfs event buffer size Arnd Bergmann
                   ` (16 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Satish Kharat, Sesidhar Baddela, Karan Tilak Kumar,
	James E.J. Bottomley, Martin K. Petersen
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, linux-scsi, x86, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 07/22] scsi: gdth: increase the procfs event buffer size
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (5 preceding siblings ...)
  2017-07-14 12:06 ` [PATCH 06/22] scsi: fnic: fix format string " Arnd Bergmann
@ 2017-07-14 12:06 ` Arnd Bergmann
  2017-07-14 12:07 ` [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning Arnd Bergmann
                   ` (15 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:06 UTC (permalink / raw)
  To: linux-kernel, Achim Leubner, James E.J. Bottomley, Martin K. Petersen
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, linux-scsi, x86, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (6 preceding siblings ...)
  2017-07-14 12:06 ` [PATCH 07/22] scsi: gdth: increase the procfs event buffer size Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 16:03   ` David Miller
  2017-07-14 12:07 ` [PATCH 09/22] net: niu: fix format string overflow warning: Arnd Bergmann
                   ` (14 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Karsten Keil, Geliang Tang, David S. Miller, Arnd Bergmann
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	James E . J . Bottomley, Martin K . Petersen, linux-scsi, x86

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 09/22] net: niu: fix format string overflow warning:
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (7 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 16:03   ` David Miller
  2017-07-14 12:07 ` [PATCH 10/22] bnx2x: fix format overflow warning Arnd Bergmann
                   ` (13 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, David S. Miller, Philippe Reynes, Arnd Bergmann
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	James E . J . Bottomley, Martin K . Petersen, linux-scsi, x86,
	Eric Dumazet, stephen hemminger, Jarod Wilson

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 10/22] bnx2x: fix format overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (8 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 09/22] net: niu: fix format string overflow warning: Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 16:03   ` David Miller
  2017-07-14 12:07   ` Arnd Bergmann
                   ` (12 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Yuval Mintz, Ariel Elior, everest-linux-l2
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Colin Ian King, Philippe Reynes

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default Arnd Bergmann
@ 2017-07-14 12:07   ` Arnd Bergmann
  2017-07-14 12:06   ` Arnd Bergmann
                     ` (20 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Sunil Goutham, Robert Richter
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Thanneeru Srinivasulu,
	George Cherian, Vadim Lomovtsev, Radha Mohan Chintakuntla,
	linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
@ 2017-07-14 12:07   ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Sunil Goutham, Robert Richter
  Cc: George Cherian, James E . J . Bottomley, linux-scsi,
	Martin K . Petersen, Greg Kroah-Hartman, x86,
	Radha Mohan Chintakuntla, Vadim Lomovtsev, Arnd Bergmann,
	linux-arm-kernel, netdev, Thanneeru Srinivasulu, akpm,
	Linus Torvalds, David S . Miller, Guenter Roeck

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
@ 2017-07-14 12:07   ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 12/22] vmxnet3: avoid format strint overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (10 preceding siblings ...)
  2017-07-14 12:07   ` Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 16:04   ` David Miller
  2017-07-14 12:07 ` [PATCH 13/22] liquidio: fix possible eeprom format string overflow Arnd Bergmann
                   ` (10 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Shrikrishna Khare, VMware, Inc.
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 13/22] liquidio: fix possible eeprom format string overflow
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (11 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 12/22] vmxnet3: avoid format strint " Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 16:04   ` David Miller
  2017-07-14 12:07 ` [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann
                   ` (9 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Derek Chickles, Satanand Burla, Felix Manlunas,
	Raghu Vatsavayi
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Weilin Chang, Prasad Kanneganti

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (12 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 13/22] liquidio: fix possible eeprom format string overflow Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-17 12:53   ` Hans Verkuil
  2017-07-14 12:07 ` [PATCH 15/22] hwmon: applesmc: fix format string overflow Arnd Bergmann
                   ` (8 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Hans Verkuil, Mauro Carvalho Chehab
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Sakari Ailus, linux-media

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 15/22] hwmon: applesmc: fix format string overflow
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (13 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 14:06   ` Guenter Roeck
  2017-07-14 12:07 ` [PATCH 16/22] x86: intel-mid: fix a format string overflow warning Arnd Bergmann
                   ` (7 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Henrik Rydberg, Jean Delvare, Guenter Roeck
  Cc: Greg Kroah-Hartman, Linus Torvalds, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, linux-hwmon

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


^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 16/22] x86: intel-mid: fix a format string overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (14 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 15/22] hwmon: applesmc: fix format string overflow Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 12:07 ` [PATCH 17/22] platform/x86: alienware-wmi: fix " Arnd Bergmann
                   ` (6 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Thomas Gleixner, Ingo Molnar, H. Peter Anvin, x86
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, Arnd Bergmann

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (15 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 16/22] x86: intel-mid: fix a format string overflow warning Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 18:30     ` Mario.Limonciello
  2017-07-14 19:18   ` Andy Shevchenko
  2017-07-14 12:07 ` [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann
                   ` (5 subsequent siblings)
  22 siblings, 2 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Darren Hart, Andy Shevchenko
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Mario Limonciello, Arvind Yadav,
	platform-driver-x86

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (16 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 17/22] platform/x86: alienware-wmi: fix " Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 12:52   ` Andy Shevchenko
  2017-07-14 12:07 ` [PATCH 19/22] block: DAC960: shut up format-overflow warning Arnd Bergmann
                   ` (4 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Mika Westerberg, Andy Shevchenko, Linus Walleij
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Hans de Goede, Rafael J. Wysocki,
	Wei Yongjun, linux-gpio, linux-acpi

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


^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 19/22] block: DAC960: shut up format-overflow warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (17 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 14:04     ` Jens Axboe
  2017-07-14 12:07   ` Arnd Bergmann
                   ` (3 subsequent siblings)
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Jens Axboe, Andrew Morton
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Uwe Kleine-König

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 20/22] sound: pci: avoid string overflow warnings
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
@ 2017-07-14 12:07   ` Arnd Bergmann
  2017-07-14 12:06 ` [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann
                     ` (21 subsequent siblings)
  22 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Jaroslav Kysela, Takashi Iwai
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Geliang Tang, Takashi Sakamoto,
	alsa-devel

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 20/22] sound: pci: avoid string overflow warnings
@ 2017-07-14 12:07   ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Jaroslav Kysela, Takashi Iwai
  Cc: alsa-devel, James E . J . Bottomley, linux-scsi,
	Martin K . Petersen, Greg Kroah-Hartman, x86, Takashi Sakamoto,
	Geliang Tang, Arnd Bergmann, netdev, akpm, Linus Torvalds,
	David S . Miller, Guenter Roeck

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 21/22] fscache: fix fscache_objlist_show format processing
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (19 preceding siblings ...)
  2017-07-14 12:07   ` Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-09-04 18:29   ` Jérémy Lefaure
  2017-07-14 12:07 ` [PATCH 22/22] IB/mlx4: fix sprintf format warning Arnd Bergmann
  2017-07-25  1:48 ` [PATCH 00/22] gcc-7 -Wformat-* warnings Martin K. Petersen
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, David Howells
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, James Morris, linux-cachefs

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 22/22] IB/mlx4: fix sprintf format warning
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (20 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 21/22] fscache: fix fscache_objlist_show format processing Arnd Bergmann
@ 2017-07-14 12:07 ` Arnd Bergmann
  2017-07-14 13:48   ` Leon Romanovsky
  2017-07-25  1:48 ` [PATCH 00/22] gcc-7 -Wformat-* warnings Martin K. Petersen
  22 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 12:07 UTC (permalink / raw)
  To: linux-kernel, Yishai Hadas, Doug Ledford, Sean Hefty, Hal Rosenstock
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Arnd Bergmann, Leon Romanovsky, Adit Ranadive,
	Or Gerlitz, Steve Wise, linux-rdma

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 20/22] sound: pci: avoid string overflow warnings
  2017-07-14 12:07   ` Arnd Bergmann
@ 2017-07-14 12:28     ` Takashi Iwai
  -1 siblings, 0 replies; 66+ messages in thread
From: Takashi Iwai @ 2017-07-14 12:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Jaroslav Kysela, linux-kernel, alsa-devel, David S . Miller,
	Geliang Tang, x86, akpm, Linus Torvalds, James E . J . Bottomley,
	Greg Kroah-Hartman, Martin K . Petersen, Guenter Roeck,
	Takashi Sakamoto, linux-scsi, netdev

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 20/22] sound: pci: avoid string overflow warnings
@ 2017-07-14 12:28     ` Takashi Iwai
  0 siblings, 0 replies; 66+ messages in thread
From: Takashi Iwai @ 2017-07-14 12:28 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: alsa-devel, James E . J . Bottomley, linux-scsi,
	Martin K . Petersen, Geliang Tang, x86, linux-kernel, netdev,
	Greg Kroah-Hartman, akpm, Takashi Sakamoto, Linus Torvalds,
	David S . Miller, Guenter Roeck

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
  2017-07-14 12:07   ` Arnd Bergmann
@ 2017-07-14 12:33     ` Robin Murphy
  -1 siblings, 0 replies; 66+ messages in thread
From: Robin Murphy @ 2017-07-14 12:33 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, Sunil Goutham, Robert Richter
  Cc: George Cherian, James E . J . Bottomley, linux-scsi,
	Martin K . Petersen, Greg Kroah-Hartman, x86,
	Radha Mohan Chintakuntla, Vadim Lomovtsev, linux-arm-kernel,
	netdev, Thanneeru Srinivasulu, akpm, Linus Torvalds,
	David S . Miller, Guenter Roeck

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
@ 2017-07-14 12:33     ` Robin Murphy
  0 siblings, 0 replies; 66+ messages in thread
From: Robin Murphy @ 2017-07-14 12:33 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers
  2017-07-14 12:07 ` [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann
@ 2017-07-14 12:52   ` Andy Shevchenko
  2017-07-14 19:59       ` Arnd Bergmann
  0 siblings, 1 reply; 66+ messages in thread
From: Andy Shevchenko @ 2017-07-14 12:52 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, Mika Westerberg, Linus Walleij
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Hans de Goede, Rafael J. Wysocki, Wei Yongjun,
	linux-gpio, linux-acpi

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 22/22] IB/mlx4: fix sprintf format warning
  2017-07-14 12:07 ` [PATCH 22/22] IB/mlx4: fix sprintf format warning Arnd Bergmann
@ 2017-07-14 13:48   ` Leon Romanovsky
  0 siblings, 0 replies; 66+ messages in thread
From: Leon Romanovsky @ 2017-07-14 13:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, Yishai Hadas, Doug Ledford, Sean Hefty,
	Hal Rosenstock, Greg Kroah-Hartman, Linus Torvalds,
	Guenter Roeck, akpm, netdev, David S . Miller,
	James E . J . Bottomley, Martin K . Petersen, linux-scsi, x86,
	Adit Ranadive, Or Gerlitz, Steve Wise, linux-rdma

[-- Attachment #1: Type: text/plain, Size: 1214 bytes --]

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>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 19/22] block: DAC960: shut up format-overflow warning
  2017-07-14 12:07 ` [PATCH 19/22] block: DAC960: shut up format-overflow warning Arnd Bergmann
@ 2017-07-14 14:04     ` Jens Axboe
  0 siblings, 0 replies; 66+ messages in thread
From: Jens Axboe @ 2017-07-14 14:04 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, Andrew Morton
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Uwe Kleine-König

On 07/14/2017 06:07 AM, Arnd Bergmann wrote:
> 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.

Thanks Arnd, added for 4.14.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 19/22] block: DAC960: shut up format-overflow warning
@ 2017-07-14 14:04     ` Jens Axboe
  0 siblings, 0 replies; 66+ messages in thread
From: Jens Axboe @ 2017-07-14 14:04 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, Andrew Morton
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Uwe Kleine-König

On 07/14/2017 06:07 AM, Arnd Bergmann wrote:
> 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.

Thanks Arnd, added for 4.14.

-- 
Jens Axboe

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 15/22] hwmon: applesmc: fix format string overflow
  2017-07-14 12:07 ` [PATCH 15/22] hwmon: applesmc: fix format string overflow Arnd Bergmann
@ 2017-07-14 14:06   ` Guenter Roeck
  0 siblings, 0 replies; 66+ messages in thread
From: Guenter Roeck @ 2017-07-14 14:06 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, Henrik Rydberg, Jean Delvare
  Cc: Greg Kroah-Hartman, Linus Torvalds, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, linux-hwmon

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


^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning
  2017-07-14 12:07 ` [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning Arnd Bergmann
@ 2017-07-14 16:03   ` David Miller
  0 siblings, 0 replies; 66+ messages in thread
From: David Miller @ 2017-07-14 16:03 UTC (permalink / raw)
  To: arnd
  Cc: linux-kernel, isdn, geliangtang, gregkh, torvalds, linux, akpm,
	netdev, jejb, martin.petersen, linux-scsi, x86

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 09/22] net: niu: fix format string overflow warning:
  2017-07-14 12:07 ` [PATCH 09/22] net: niu: fix format string overflow warning: Arnd Bergmann
@ 2017-07-14 16:03   ` David Miller
  0 siblings, 0 replies; 66+ messages in thread
From: David Miller @ 2017-07-14 16:03 UTC (permalink / raw)
  To: arnd
  Cc: linux-kernel, tremyfr, gregkh, torvalds, linux, akpm, netdev,
	jejb, martin.petersen, linux-scsi, x86, edumazet, stephen, jarod

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 10/22] bnx2x: fix format overflow warning
  2017-07-14 12:07 ` [PATCH 10/22] bnx2x: fix format overflow warning Arnd Bergmann
@ 2017-07-14 16:03   ` David Miller
  0 siblings, 0 replies; 66+ messages in thread
From: David Miller @ 2017-07-14 16:03 UTC (permalink / raw)
  To: arnd
  Cc: linux-kernel, Yuval.Mintz, ariel.elior, everest-linux-l2, gregkh,
	torvalds, linux, akpm, netdev, jejb, martin.petersen, linux-scsi,
	x86, colin.king, tremyfr

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
  2017-07-14 12:07   ` Arnd Bergmann
@ 2017-07-14 16:03     ` David Miller
  -1 siblings, 0 replies; 66+ messages in thread
From: David Miller @ 2017-07-14 16:03 UTC (permalink / raw)
  To: arnd
  Cc: linux-kernel, sgoutham, rric, gregkh, torvalds, linux, akpm,
	netdev, jejb, martin.petersen, linux-scsi, x86, tsrinivasulu,
	george.cherian, Vadim.Lomovtsev, rchintakuntla, linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* [PATCH 11/22] net: thunder_bgx: avoid format string overflow warning
@ 2017-07-14 16:03     ` David Miller
  0 siblings, 0 replies; 66+ messages in thread
From: David Miller @ 2017-07-14 16:03 UTC (permalink / raw)
  To: linux-arm-kernel

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 12/22] vmxnet3: avoid format strint overflow warning
  2017-07-14 12:07 ` [PATCH 12/22] vmxnet3: avoid format strint " Arnd Bergmann
@ 2017-07-14 16:04   ` David Miller
  0 siblings, 0 replies; 66+ messages in thread
From: David Miller @ 2017-07-14 16:04 UTC (permalink / raw)
  To: arnd
  Cc: linux-kernel, skhare, pv-drivers, gregkh, torvalds, linux, akpm,
	netdev, jejb, martin.petersen, linux-scsi, x86

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 13/22] liquidio: fix possible eeprom format string overflow
  2017-07-14 12:07 ` [PATCH 13/22] liquidio: fix possible eeprom format string overflow Arnd Bergmann
@ 2017-07-14 16:04   ` David Miller
  2017-07-14 22:40       ` Burla, Satananda
  0 siblings, 1 reply; 66+ messages in thread
From: David Miller @ 2017-07-14 16:04 UTC (permalink / raw)
  To: arnd
  Cc: linux-kernel, derek.chickles, satananda.burla, felix.manlunas,
	raghu.vatsavayi, gregkh, torvalds, linux, akpm, netdev, jejb,
	martin.petersen, linux-scsi, x86, weilin.chang,
	prasad.kanneganti

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.

^ permalink raw reply	[flat|nested] 66+ messages in thread

* RE: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning
  2017-07-14 12:07 ` [PATCH 17/22] platform/x86: alienware-wmi: fix " Arnd Bergmann
@ 2017-07-14 18:30     ` Mario.Limonciello
  2017-07-14 19:18   ` Andy Shevchenko
  1 sibling, 0 replies; 66+ messages in thread
From: Mario.Limonciello @ 2017-07-14 18:30 UTC (permalink / raw)
  To: arnd, linux-kernel, dvhart, andy
  Cc: gregkh, torvalds, linux, akpm, netdev, davem, jejb,
	martin.petersen, linux-scsi, x86, arvind.yadav.cs,
	platform-driver-x86

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* RE: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning
@ 2017-07-14 18:30     ` Mario.Limonciello
  0 siblings, 0 replies; 66+ messages in thread
From: Mario.Limonciello @ 2017-07-14 18:30 UTC (permalink / raw)
  To: arnd, linux-kernel, dvhart, andy
  Cc: gregkh, torvalds, linux, akpm, netdev, davem, jejb,
	martin.petersen, linux-scsi, x86, arvind.yadav.cs,
	platform-driver-x86

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning
  2017-07-14 12:07 ` [PATCH 17/22] platform/x86: alienware-wmi: fix " Arnd Bergmann
  2017-07-14 18:30     ` Mario.Limonciello
@ 2017-07-14 19:18   ` Andy Shevchenko
  2017-07-14 19:37     ` Arnd Bergmann
  1 sibling, 1 reply; 66+ messages in thread
From: Andy Shevchenko @ 2017-07-14 19:18 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, Darren Hart, Andy Shevchenko, Greg Kroah-Hartman,
	Linus Torvalds, Guenter Roeck, Andrew Morton, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Mario Limonciello, Arvind Yadav,
	Platform Driver

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning
  2017-07-14 19:18   ` Andy Shevchenko
@ 2017-07-14 19:37     ` Arnd Bergmann
  2017-07-14 19:49       ` Andy Shevchenko
  0 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 19:37 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: linux-kernel, Darren Hart, Andy Shevchenko, Greg Kroah-Hartman,
	Linus Torvalds, Guenter Roeck, Andrew Morton, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Mario Limonciello, Arvind Yadav,
	Platform Driver

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 17/22] platform/x86: alienware-wmi: fix format string overflow warning
  2017-07-14 19:37     ` Arnd Bergmann
@ 2017-07-14 19:49       ` Andy Shevchenko
  0 siblings, 0 replies; 66+ messages in thread
From: Andy Shevchenko @ 2017-07-14 19:49 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, Darren Hart, Andy Shevchenko, Greg Kroah-Hartman,
	Linus Torvalds, Guenter Roeck, Andrew Morton, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Mario Limonciello, Arvind Yadav,
	Platform Driver

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers
  2017-07-14 12:52   ` Andy Shevchenko
  2017-07-14 19:59       ` Arnd Bergmann
@ 2017-07-14 19:59       ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 19:59 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linux Kernel Mailing List, Mika Westerberg, Linus Walleij,
	Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, Andrew Morton,
	Networking, David S . Miller, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, the arch/x86 maintainers,
	Hans de Goede, Rafael J. Wysocki, Wei Yongjun, linux-gpio, ACPI

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers
@ 2017-07-14 19:59       ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 19:59 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linux Kernel Mailing List, Mika Westerberg, Linus Walleij,
	Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, Andrew Morton,
	Networking, David S . Miller, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, the arch/x86 maintainers,
	Hans de Goede, Rafael J. Wysocki, Wei Yongjun, linux-gpio,
	ACPI Devel Maling List

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers
@ 2017-07-14 19:59       ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-14 19:59 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Linux Kernel Mailing List, Mika Westerberg, Linus Walleij,
	Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, Andrew Morton,
	Networking, David S . Miller, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, the arch/x86 maintainers,
	Hans de Goede, Rafael J. Wysocki, Wei Yongjun, linux-gpio, ACPI

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 13/22] liquidio: fix possible eeprom format string overflow
  2017-07-14 16:04   ` David Miller
  2017-07-14 22:40       ` Burla, Satananda
@ 2017-07-14 22:40       ` Burla, Satananda
  0 siblings, 0 replies; 66+ messages in thread
From: Burla, Satananda @ 2017-07-14 22:40 UTC (permalink / raw)
  To: Chickles, Derek, Manlunas, Felix
  Cc: arnd, linux-kernel, Chickles, Derek, Manlunas, Felix, Vatsavayi,
	Raghu, gregkh, torvalds, linux, akpm, netdev, jejb,
	martin.petersen, linux-scsi, x86, Chang, Weilin, Kanneganti,
	Prasad

The 07/14/2017 09:04, David Miller wrote:
> 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.
I had raised a bug for this earlier and attached a patch as well. 

http://cabugzilla1.caveonetworks.com/octeon_bugzilla/show_bug.cgi?id=26421


-- 
Regards
Satanand

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 13/22] liquidio: fix possible eeprom format string overflow
@ 2017-07-14 22:40       ` Burla, Satananda
  0 siblings, 0 replies; 66+ messages in thread
From: Burla, Satananda @ 2017-07-14 22:40 UTC (permalink / raw)
  To: Chickles, Derek, Manlunas, Felix
  Cc: arnd, linux-kernel, Chickles, Derek, Manlunas, Felix, Vatsavayi,
	Raghu, gregkh, torvalds, linux, akpm, netdev, jejb,
	martin.petersen, linux-scsi, x86, Chang, Weilin,

The 07/14/2017 09:04, David Miller wrote:
> 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.
I had raised a bug for this earlier and attached a patch as well. 

http://cabugzilla1.caveonetworks.com/octeon_bugzilla/show_bug.cgi?id=26421


-- 
Regards
Satanand

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 13/22] liquidio: fix possible eeprom format string overflow
@ 2017-07-14 22:40       ` Burla, Satananda
  0 siblings, 0 replies; 66+ messages in thread
From: Burla, Satananda @ 2017-07-14 22:40 UTC (permalink / raw)
  Cc: arnd, linux-kernel, Chickles, Derek, Manlunas, Felix, Vatsavayi,
	Raghu, gregkh, torvalds, linux, akpm, netdev, jejb,
	martin.petersen, linux-scsi, x86, Chang, Weilin, Kanneganti

The 07/14/2017 09:04, David Miller wrote:
> 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.
I had raised a bug for this earlier and attached a patch as well. 

http://cabugzilla1.caveonetworks.com/octeon_bugzilla/show_bug.cgi?id=26421


-- 
Regards
Satanand

^ permalink raw reply	[flat|nested] 66+ messages in thread

* RE: [PATCH 04/22] scsi: fusion: fix string overflow warning
  2017-07-14 12:06   ` Arnd Bergmann
@ 2017-07-17  9:17     ` David Laight
  -1 siblings, 0 replies; 66+ messages in thread
From: David Laight @ 2017-07-17  9:17 UTC (permalink / raw)
  To: 'Arnd Bergmann',
	linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Helge Deller, MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* RE: [PATCH 04/22] scsi: fusion: fix string overflow warning
@ 2017-07-17  9:17     ` David Laight
  0 siblings, 0 replies; 66+ messages in thread
From: David Laight @ 2017-07-17  9:17 UTC (permalink / raw)
  To: 'Arnd Bergmann',
	linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Helge Deller, MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 04/22] scsi: fusion: fix string overflow warning
  2017-07-17  9:17     ` David Laight
  (?)
@ 2017-07-17 12:00       ` Arnd Bergmann
  -1 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-17 12:00 UTC (permalink / raw)
  To: David Laight
  Cc: linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani, Greg Kroah-Hartman, Linus Torvalds,
	Guenter Roeck, akpm, netdev, David S . Miller,
	James E . J . Bottomley, Martin K . Petersen, linux-scsi, x86,
	Helge Deller, MPT-FusionLinux.pdl

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 04/22] scsi: fusion: fix string overflow warning
@ 2017-07-17 12:00       ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-17 12:00 UTC (permalink / raw)
  To: David Laight
  Cc: linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani, Greg Kroah-Hartman, Linus Torvalds,
	Guenter Roeck, akpm, netdev, David S . Miller,
	James E . J . Bottomley, Martin K . Petersen, linux-scsi, x86,
	Helge Deller, MPT-FusionLinux.pdl@broadcom.com

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 04/22] scsi: fusion: fix string overflow warning
@ 2017-07-17 12:00       ` Arnd Bergmann
  0 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-17 12:00 UTC (permalink / raw)
  To: David Laight
  Cc: linux-kernel, Sathya Prakash, Chaitra P B,
	Suganath Prabu Subramani, Greg Kroah-Hartman, Linus Torvalds,
	Guenter Roeck, akpm, netdev, David S . Miller,
	James E . J . Bottomley, Martin K . Petersen, linux-scsi, x86,
	Helge Deller, MPT-FusionLinux.pdl@broadcom.com

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning
  2017-07-14 12:07 ` [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann
@ 2017-07-17 12:53   ` Hans Verkuil
  2017-07-17 12:57     ` Arnd Bergmann
  0 siblings, 1 reply; 66+ messages in thread
From: Hans Verkuil @ 2017-07-17 12:53 UTC (permalink / raw)
  To: Arnd Bergmann, linux-kernel, Mauro Carvalho Chehab
  Cc: Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, akpm, netdev,
	David S . Miller, James E . J . Bottomley, Martin K . Petersen,
	linux-scsi, x86, Sakari Ailus, linux-media

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning
  2017-07-17 12:53   ` Hans Verkuil
@ 2017-07-17 12:57     ` Arnd Bergmann
  2017-07-17 12:59       ` Hans Verkuil
  0 siblings, 1 reply; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-17 12:57 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Linux Kernel Mailing List, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, Andrew Morton,
	Networking, David S . Miller, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, the arch/x86 maintainers,
	Sakari Ailus, Linux Media Mailing List

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning
  2017-07-17 12:57     ` Arnd Bergmann
@ 2017-07-17 12:59       ` Hans Verkuil
  0 siblings, 0 replies; 66+ messages in thread
From: Hans Verkuil @ 2017-07-17 12:59 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Linux Kernel Mailing List, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck, Andrew Morton,
	Networking, David S . Miller, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, the arch/x86 maintainers,
	Sakari Ailus, Linux Media Mailing List

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 20/22] sound: pci: avoid string overflow warnings
  2017-07-14 12:28     ` Takashi Iwai
  (?)
@ 2017-07-18 11:52     ` Arnd Bergmann
  -1 siblings, 0 replies; 66+ messages in thread
From: Arnd Bergmann @ 2017-07-18 11:52 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, Linux Kernel Mailing List, alsa-devel,
	David S . Miller, Geliang Tang, the arch/x86 maintainers,
	Andrew Morton, Linus Torvalds, James E . J . Bottomley,
	Greg Kroah-Hartman, Martin K . Petersen, Guenter Roeck,
	Takashi Sakamoto, linux-scsi, Networking

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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 00/22] gcc-7 -Wformat-* warnings
  2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
                   ` (21 preceding siblings ...)
  2017-07-14 12:07 ` [PATCH 22/22] IB/mlx4: fix sprintf format warning Arnd Bergmann
@ 2017-07-25  1:48 ` Martin K. Petersen
  22 siblings, 0 replies; 66+ messages in thread
From: Martin K. Petersen @ 2017-07-25  1:48 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: linux-kernel, Greg Kroah-Hartman, Linus Torvalds, Guenter Roeck,
	akpm, netdev, David S . Miller, James E . J . Bottomley,
	Martin K . Petersen, linux-scsi, x86


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

^ permalink raw reply	[flat|nested] 66+ messages in thread

* Re: [PATCH 21/22] fscache: fix fscache_objlist_show format processing
  2017-07-14 12:07 ` [PATCH 21/22] fscache: fix fscache_objlist_show format processing Arnd Bergmann
@ 2017-09-04 18:29   ` Jérémy Lefaure
  0 siblings, 0 replies; 66+ messages in thread
From: Jérémy Lefaure @ 2017-09-04 18:29 UTC (permalink / raw)
  To: Arnd Bergmann, Andrew Morton, Stephen Rothwell, David Howells
  Cc: linux-cachefs, linux-kernel, dhowells

> 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>
> ---
Hi,
I sent a patch to fix this issue in April [1]. It was accepted by David
Howells [2]. I don't know why it wasn't upstreamed.

>  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;
>  		}
In my patch I didn't use snprintf (which is fine) but I used the
hexadecimal value (as it is in the documentation [3]). Is it too late
to change this patch ? If it is, I can send a patch to use an hex value.

Thank you,
Jérémy

[1]: https://marc.info/?l=linux-kernel&m=149263432022839&w=4
[2]: https://marc.info/?l=linux-kernel&m=149330544916184&w=4
[3]: see Documentation/filesystems/caching/fscache.txt

^ permalink raw reply	[flat|nested] 66+ messages in thread

end of thread, other threads:[~2017-09-04 18:34 UTC | newest]

Thread overview: 66+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-14 12:06 [PATCH 00/22] gcc-7 -Wformat-* warnings Arnd Bergmann
2017-07-14 12:06 ` [PATCH 01/22] kbuild: disable -Wformat-truncation warnings by default Arnd Bergmann
2017-07-14 12:06 ` [PATCH 02/22] scsi: megaraid: fix format-overflow warning Arnd Bergmann
2017-07-14 12:06 ` [PATCH 03/22] scsi: mpt3sas: fix format overflow warning Arnd Bergmann
2017-07-14 12:06   ` Arnd Bergmann
2017-07-14 12:06 ` [PATCH 04/22] scsi: fusion: fix string " Arnd Bergmann
2017-07-14 12:06   ` Arnd Bergmann
2017-07-17  9:17   ` David Laight
2017-07-17  9:17     ` David Laight
2017-07-17 12:00     ` Arnd Bergmann
2017-07-17 12:00       ` Arnd Bergmann
2017-07-17 12:00       ` Arnd Bergmann
2017-07-14 12:06 ` [PATCH 05/22] scsi: gdth: avoid buffer " Arnd Bergmann
2017-07-14 12:06 ` [PATCH 06/22] scsi: fnic: fix format string " Arnd Bergmann
2017-07-14 12:06 ` [PATCH 07/22] scsi: gdth: increase the procfs event buffer size Arnd Bergmann
2017-07-14 12:07 ` [PATCH 08/22] isdn: divert: fix sprintf buffer overflow warning Arnd Bergmann
2017-07-14 16:03   ` David Miller
2017-07-14 12:07 ` [PATCH 09/22] net: niu: fix format string overflow warning: Arnd Bergmann
2017-07-14 16:03   ` David Miller
2017-07-14 12:07 ` [PATCH 10/22] bnx2x: fix format overflow warning Arnd Bergmann
2017-07-14 16:03   ` David Miller
2017-07-14 12:07 ` [PATCH 11/22] net: thunder_bgx: avoid format string " Arnd Bergmann
2017-07-14 12:07   ` Arnd Bergmann
2017-07-14 12:07   ` Arnd Bergmann
2017-07-14 12:33   ` Robin Murphy
2017-07-14 12:33     ` Robin Murphy
2017-07-14 16:03   ` David Miller
2017-07-14 16:03     ` David Miller
2017-07-14 12:07 ` [PATCH 12/22] vmxnet3: avoid format strint " Arnd Bergmann
2017-07-14 16:04   ` David Miller
2017-07-14 12:07 ` [PATCH 13/22] liquidio: fix possible eeprom format string overflow Arnd Bergmann
2017-07-14 16:04   ` David Miller
2017-07-14 22:40     ` Burla, Satananda
2017-07-14 22:40       ` Burla, Satananda
2017-07-14 22:40       ` Burla, Satananda
2017-07-14 12:07 ` [PATCH 14/22] [media] usbvision-i2c: fix format overflow warning Arnd Bergmann
2017-07-17 12:53   ` Hans Verkuil
2017-07-17 12:57     ` Arnd Bergmann
2017-07-17 12:59       ` Hans Verkuil
2017-07-14 12:07 ` [PATCH 15/22] hwmon: applesmc: fix format string overflow Arnd Bergmann
2017-07-14 14:06   ` Guenter Roeck
2017-07-14 12:07 ` [PATCH 16/22] x86: intel-mid: fix a format string overflow warning Arnd Bergmann
2017-07-14 12:07 ` [PATCH 17/22] platform/x86: alienware-wmi: fix " Arnd Bergmann
2017-07-14 18:30   ` Mario.Limonciello
2017-07-14 18:30     ` Mario.Limonciello
2017-07-14 19:18   ` Andy Shevchenko
2017-07-14 19:37     ` Arnd Bergmann
2017-07-14 19:49       ` Andy Shevchenko
2017-07-14 12:07 ` [PATCH 18/22] gpio: acpi: fix string overflow for large pin numbers Arnd Bergmann
2017-07-14 12:52   ` Andy Shevchenko
2017-07-14 19:59     ` Arnd Bergmann
2017-07-14 19:59       ` Arnd Bergmann
2017-07-14 19:59       ` Arnd Bergmann
2017-07-14 12:07 ` [PATCH 19/22] block: DAC960: shut up format-overflow warning Arnd Bergmann
2017-07-14 14:04   ` Jens Axboe
2017-07-14 14:04     ` Jens Axboe
2017-07-14 12:07 ` [PATCH 20/22] sound: pci: avoid string overflow warnings Arnd Bergmann
2017-07-14 12:07   ` Arnd Bergmann
2017-07-14 12:28   ` Takashi Iwai
2017-07-14 12:28     ` Takashi Iwai
2017-07-18 11:52     ` Arnd Bergmann
2017-07-14 12:07 ` [PATCH 21/22] fscache: fix fscache_objlist_show format processing Arnd Bergmann
2017-09-04 18:29   ` Jérémy Lefaure
2017-07-14 12:07 ` [PATCH 22/22] IB/mlx4: fix sprintf format warning Arnd Bergmann
2017-07-14 13:48   ` Leon Romanovsky
2017-07-25  1:48 ` [PATCH 00/22] gcc-7 -Wformat-* warnings Martin K. Petersen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.