| Message ID | 20210323125723.1961432-1-arnd@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | [net-next] wlcore: fix overlapping snprintf arguments in debugfs | expand |
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
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 --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); \ } \