diff mbox series

[net-next] wlcore: fix overlapping snprintf arguments in debugfs

Message ID 20210323125723.1961432-1-arnd@kernel.org
State New
Headers show
Series [net-next] wlcore: fix overlapping snprintf arguments in debugfs | expand

Commit Message

Arnd Bergmann March 23, 2021, 12:57 p.m. UTC
From: Arnd Bergmann <arnd@arndb.de>


gcc complains about undefined behavior in calling snprintf()
with the same buffer as input and output:

drivers/net/wireless/ti/wl18xx/debugfs.c: In function 'diversity_num_of_packets_per_ant_read':
drivers/net/wireless/ti/wl18xx/../wlcore/debugfs.h:86:3: error: 'snprintf' argument 4 overlaps destination object 'buf' [-Werror=restrict]
   86 |   snprintf(buf, sizeof(buf), "%s[%d] = %d\n",  \
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   87 |     buf, i, stats->sub.name[i]);   \
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ti/wl18xx/debugfs.c:24:2: note: in expansion of macro 'DEBUGFS_FWSTATS_FILE_ARRAY'
   24 |  DEBUGFS_FWSTATS_FILE_ARRAY(a, b, c, wl18xx_acx_statistics)
      |  ^~~~~~~~~~~~~~~~~~~~~~~~~~
drivers/net/wireless/ti/wl18xx/debugfs.c:159:1: note: in expansion of macro 'WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY'
  159 | WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY(diversity, num_of_packets_per_ant,

There are probably other ways of handling the debugfs file, without
using on-stack buffers, but a simple workaround here is to remember the
current position in the buffer and just keep printing in there.

Fixes: bcca1bbdd412 ("wlcore: add debugfs macro to help print fw statistics arrays")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 drivers/net/wireless/ti/wlcore/boot.c    | 13 ++++++++-----
 drivers/net/wireless/ti/wlcore/debugfs.h |  7 ++++---
 2 files changed, 12 insertions(+), 8 deletions(-)

-- 
2.29.2

Comments

Kalle Valo April 7, 2021, 11:41 a.m. UTC | #1
Arnd Bergmann <arnd@kernel.org> writes:

> From: Arnd Bergmann <arnd@arndb.de>

>

> gcc complains about undefined behavior in calling snprintf()

> with the same buffer as input and output:

>

> drivers/net/wireless/ti/wl18xx/debugfs.c: In function

> 'diversity_num_of_packets_per_ant_read':

> drivers/net/wireless/ti/wl18xx/../wlcore/debugfs.h:86:3: error:

> 'snprintf' argument 4 overlaps destination object 'buf'

> [-Werror=restrict]

>    86 |   snprintf(buf, sizeof(buf), "%s[%d] = %d\n",  \

>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>    87 |     buf, i, stats->sub.name[i]);   \

>       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/net/wireless/ti/wl18xx/debugfs.c:24:2: note: in expansion of

> macro 'DEBUGFS_FWSTATS_FILE_ARRAY'

>    24 |  DEBUGFS_FWSTATS_FILE_ARRAY(a, b, c, wl18xx_acx_statistics)

>       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/net/wireless/ti/wl18xx/debugfs.c:159:1: note: in expansion of macro 'WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY'

>   159 | WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY(diversity, num_of_packets_per_ant,

>

> There are probably other ways of handling the debugfs file, without

> using on-stack buffers, but a simple workaround here is to remember the

> current position in the buffer and just keep printing in there.

>

> Fixes: bcca1bbdd412 ("wlcore: add debugfs macro to help print fw statistics arrays")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  drivers/net/wireless/ti/wlcore/boot.c    | 13 ++++++++-----

>  drivers/net/wireless/ti/wlcore/debugfs.h |  7 ++++---


This should go to wireless-drivers-next, not net-next.

-- 
https://patchwork.kernel.org/project/linux-wireless/list/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Kalle Valo April 17, 2021, 6:02 p.m. UTC | #2
Arnd Bergmann <arnd@kernel.org> wrote:

> From: Arnd Bergmann <arnd@arndb.de>

> 

> gcc complains about undefined behavior in calling snprintf()

> with the same buffer as input and output:

> 

> drivers/net/wireless/ti/wl18xx/debugfs.c: In function 'diversity_num_of_packets_per_ant_read':

> drivers/net/wireless/ti/wl18xx/../wlcore/debugfs.h:86:3: error: 'snprintf' argument 4 overlaps destination object 'buf' [-Werror=restrict]

>    86 |   snprintf(buf, sizeof(buf), "%s[%d] = %d\n",  \

>       |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

>    87 |     buf, i, stats->sub.name[i]);   \

>       |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/net/wireless/ti/wl18xx/debugfs.c:24:2: note: in expansion of macro 'DEBUGFS_FWSTATS_FILE_ARRAY'

>    24 |  DEBUGFS_FWSTATS_FILE_ARRAY(a, b, c, wl18xx_acx_statistics)

>       |  ^~~~~~~~~~~~~~~~~~~~~~~~~~

> drivers/net/wireless/ti/wl18xx/debugfs.c:159:1: note: in expansion of macro 'WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY'

>   159 | WL18XX_DEBUGFS_FWSTATS_FILE_ARRAY(diversity, num_of_packets_per_ant,

> 

> There are probably other ways of handling the debugfs file, without

> using on-stack buffers, but a simple workaround here is to remember the

> current position in the buffer and just keep printing in there.

> 

> Fixes: bcca1bbdd412 ("wlcore: add debugfs macro to help print fw statistics arrays")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>


Patch applied to wireless-drivers-next.git, thanks.

7b0e2c4f6be3 wlcore: fix overlapping snprintf arguments in debugfs

-- 
https://patchwork.kernel.org/project/linux-wireless/patch/20210323125723.1961432-1-arnd@kernel.org/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
diff mbox series

Patch

diff --git a/drivers/net/wireless/ti/wlcore/boot.c b/drivers/net/wireless/ti/wlcore/boot.c
index e14d88e558f0..85abd0a2d1c9 100644
--- a/drivers/net/wireless/ti/wlcore/boot.c
+++ b/drivers/net/wireless/ti/wlcore/boot.c
@@ -72,6 +72,7 @@  static int wlcore_validate_fw_ver(struct wl1271 *wl)
 	unsigned int *min_ver = (wl->fw_type == WL12XX_FW_TYPE_MULTI) ?
 		wl->min_mr_fw_ver : wl->min_sr_fw_ver;
 	char min_fw_str[32] = "";
+	int off = 0;
 	int i;
 
 	/* the chip must be exactly equal */
@@ -105,13 +106,15 @@  static int wlcore_validate_fw_ver(struct wl1271 *wl)
 	return 0;
 
 fail:
-	for (i = 0; i < NUM_FW_VER; i++)
+	for (i = 0; i < NUM_FW_VER && off < sizeof(min_fw_str); i++)
 		if (min_ver[i] == WLCORE_FW_VER_IGNORE)
-			snprintf(min_fw_str, sizeof(min_fw_str),
-				  "%s*.", min_fw_str);
+			off += snprintf(min_fw_str + off,
+					sizeof(min_fw_str) - off,
+					"*.");
 		else
-			snprintf(min_fw_str, sizeof(min_fw_str),
-				  "%s%u.", min_fw_str, min_ver[i]);
+			off += snprintf(min_fw_str + off,
+					sizeof(min_fw_str) - off,
+					"%u.", min_ver[i]);
 
 	wl1271_error("Your WiFi FW version (%u.%u.%u.%u.%u) is invalid.\n"
 		     "Please use at least FW %s\n"
diff --git a/drivers/net/wireless/ti/wlcore/debugfs.h b/drivers/net/wireless/ti/wlcore/debugfs.h
index b143293e694f..715edfa5f89f 100644
--- a/drivers/net/wireless/ti/wlcore/debugfs.h
+++ b/drivers/net/wireless/ti/wlcore/debugfs.h
@@ -78,13 +78,14 @@  static ssize_t sub## _ ##name## _read(struct file *file,		\
 	struct wl1271 *wl = file->private_data;				\
 	struct struct_type *stats = wl->stats.fw_stats;			\
 	char buf[DEBUGFS_FORMAT_BUFFER_SIZE] = "";			\
+	int pos = 0;							\
 	int i;								\
 									\
 	wl1271_debugfs_update_stats(wl);				\
 									\
-	for (i = 0; i < len; i++)					\
-		snprintf(buf, sizeof(buf), "%s[%d] = %d\n",		\
-			 buf, i, stats->sub.name[i]);			\
+	for (i = 0; i < len && pos < sizeof(buf); i++)			\
+		pos += snprintf(buf + pos, sizeof(buf),			\
+			 "[%d] = %d\n", i, stats->sub.name[i]);		\
 									\
 	return wl1271_format_buffer(userbuf, count, ppos, "%s", buf);	\
 }									\