Re: [PATCH v3 1/2] mwifiex: Use a define for firmware version string length
From: Brian Norris <briannorris@chromium.org>
Date: 2021-11-03 17:28:21
Also in:
lkml, netdev
On Wed, Nov 03, 2021 at 06:10:54PM +0100, Jonas Dreßler wrote:
Since the version string we get from the firmware is always 128 characters long, use a define for this size instead of having the number 128 copied all over the place. Signed-off-by: Jonas Dreßler <redacted>
Thanks for this patch. For the series: Reviewed-by: Brian Norris <briannorris@chromium.org>
quoted hunk ↗ jump to hunk
--- drivers/net/wireless/marvell/mwifiex/fw.h | 4 +++- drivers/net/wireless/marvell/mwifiex/main.h | 2 +- drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c | 5 +++-- 3 files changed, 7 insertions(+), 4 deletions(-)diff --git a/drivers/net/wireless/marvell/mwifiex/fw.h b/drivers/net/wireless/marvell/mwifiex/fw.h index 2ff23ab259ab..63c25c69ed2b 100644 --- a/drivers/net/wireless/marvell/mwifiex/fw.h +++ b/drivers/net/wireless/marvell/mwifiex/fw.h@@ -2071,9 +2071,11 @@ struct mwifiex_ie_types_robust_coex { __le32 mode; } __packed; +#define MWIFIEX_VERSION_STR_LENGTH 128 + struct host_cmd_ds_version_ext { u8 version_str_sel; - char version_str[128]; + char version_str[MWIFIEX_VERSION_STR_LENGTH]; } __packed; struct host_cmd_ds_mgmt_frame_reg {diff --git a/drivers/net/wireless/marvell/mwifiex/main.h b/drivers/net/wireless/marvell/mwifiex/main.h index 90012cbcfd15..65609ea2327e 100644 --- a/drivers/net/wireless/marvell/mwifiex/main.h +++ b/drivers/net/wireless/marvell/mwifiex/main.h@@ -646,7 +646,7 @@ struct mwifiex_private { struct wireless_dev wdev; struct mwifiex_chan_freq_power cfp; u32 versionstrsel; - char version_str[128]; + char version_str[MWIFIEX_VERSION_STR_LENGTH]; #ifdef CONFIG_DEBUG_FS struct dentry *dfs_dev_dir; #endifdiff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c index 6b5d35d9e69f..20b69a37f9e1 100644 --- a/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmdresp.c@@ -711,8 +711,9 @@ static int mwifiex_ret_ver_ext(struct mwifiex_private *priv, if (version_ext) { version_ext->version_str_sel = ver_ext->version_str_sel; memcpy(version_ext->version_str, ver_ext->version_str, - sizeof(char) * 128); - memcpy(priv->version_str, ver_ext->version_str, 128); + MWIFIEX_VERSION_STR_LENGTH); + memcpy(priv->version_str, ver_ext->version_str, + MWIFIEX_VERSION_STR_LENGTH);
Not related to your patch, but this highlights that nobody is ensuring this string is 0-terminated, and various other places (notably, *not* your patch 2!) assume that it is. We should either fix those to use an snprintf()/length-restricted variant, or else just force: priv->version_str[MWIFIEX_VERSION_STR_LENGTH - 1] = '\0'; here. But that's a separate issue/patch. I can cook one on top of your series when it gets merged if you don't want to. Brian
} return 0; } -- 2.33.1