[PATCH] wifi: wilc1000: avoid buffer overflow in WID string configuration

[PATCH] wifi: wilc1000: avoid buffer overflow in WID string configuration

[Ajay.Kathat]

Fix the following copy overflow warning identified by Smatch static checker.

 drivers/net/wireless/microchip/wilc1000/wlan_cfg.c:184 wilc_wlan_parse_response_frame()
        error: '__memcpy()' 'cfg->s[i]->str' copy overflow (512 vs 65537)

This patch introduces size check before accessing the memory buffer.
The checks are base on the WID type of received data from the firmware.
For WID string configuration, the size limit is determined by the maximum
element size in 'struct wilc_cfg_str_vals'. Therefore, WILC_MAX_CFG_STR_SIZE
macro is added to configure this size.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Closes: https://lore.kernel.org/linux-wireless/aLFbr9Yu9j_TQTey@stanley.mountain
Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
---
 .../wireless/microchip/wilc1000/wlan_cfg.c    | 23 +++++++++++++++----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c b/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
index 131388886acb..a9a16012f9f3 100644
--- a/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
+++ b/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
@@ -52,6 +52,7 @@ static const struct wilc_cfg_str g_cfg_str[] = {
 #define WILC_RESP_MSG_TYPE_NETWORK_INFO		'N'
 #define WILC_RESP_MSG_TYPE_SCAN_COMPLETE	'S'

+#define WILC_MAX_CFG_STR_SIZE			WILC_MAX_ASSOC_RESP_FRAME_SIZE
 /********************************************
  *
  *      Configuration Functions
@@ -147,44 +148,56 @@ static void wilc_wlan_parse_response_frame(struct wilc *wl, u8 *info, int size)

 		switch (FIELD_GET(WILC_WID_TYPE, wid)) {
 		case WID_CHAR:
+			len = 3;
+			if (len + 2  > size)
+				return;
+
 			while (cfg->b[i].id != WID_NIL && cfg->b[i].id != wid)
 				i++;

 			if (cfg->b[i].id == wid)
 				cfg->b[i].val = info[4];

-			len = 3;
 			break;

 		case WID_SHORT:
+			len = 4;
+			if (len + 2  > size)
+				return;
+
 			while (cfg->hw[i].id != WID_NIL && cfg->hw[i].id != wid)
 				i++;

 			if (cfg->hw[i].id == wid)
 				cfg->hw[i].val = get_unaligned_le16(&info[4]);

-			len = 4;
 			break;

 		case WID_INT:
+			len = 6;
+			if (len + 2  > size)
+				return;
+
 			while (cfg->w[i].id != WID_NIL && cfg->w[i].id != wid)
 				i++;

 			if (cfg->w[i].id == wid)
 				cfg->w[i].val = get_unaligned_le32(&info[4]);

-			len = 6;
 			break;

 		case WID_STR:
+			len = 2 + get_unaligned_le16(&info[2]);
+			if (len > WILC_MAX_CFG_STR_SIZE || (len + 2  > size))
+				return;
+
 			while (cfg->s[i].id != WID_NIL && cfg->s[i].id != wid)
 				i++;

 			if (cfg->s[i].id == wid)
 				memcpy(cfg->s[i].str, &info[2],
-				       get_unaligned_le16(&info[2]) + 2);
+				       len);

-			len = 2 + get_unaligned_le16(&info[2]);
 			break;

 		default:
--
2.34.1

Re: [PATCH] wifi: wilc1000: avoid buffer overflow in WID string configuration

[Ajay.Kathat]

On 8/29/25 11:22, Ajay Kathat - C15481 wrote:
> Fix the following copy overflow warning identified by Smatch static checker.
> 
>  drivers/net/wireless/microchip/wilc1000/wlan_cfg.c:184 wilc_wlan_parse_response_frame()
>         error: '__memcpy()' 'cfg->s[i]->str' copy overflow (512 vs 65537)
> 
> This patch introduces size check before accessing the memory buffer.
> The checks are base on the WID type of received data from the firmware.
> For WID string configuration, the size limit is determined by the maximum
> element size in 'struct wilc_cfg_str_vals'. Therefore, WILC_MAX_CFG_STR_SIZE
> macro is added to configure this size.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-wireless/aLFbr9Yu9j_TQTey@stanley.mountain
> Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---
>  .../wireless/microchip/wilc1000/wlan_cfg.c    | 23 +++++++++++++++----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c b/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
> index 131388886acb..a9a16012f9f3 100644
> --- a/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
> +++ b/drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
> @@ -52,6 +52,7 @@ static const struct wilc_cfg_str g_cfg_str[] = {
>  #define WILC_RESP_MSG_TYPE_NETWORK_INFO		'N'
>  #define WILC_RESP_MSG_TYPE_SCAN_COMPLETE	'S'
> 
> +#define WILC_MAX_CFG_STR_SIZE			WILC_MAX_ASSOC_RESP_FRAME_SIZE
>  /********************************************
>   *
>   *      Configuration Functions
> @@ -147,44 +148,56 @@ static void wilc_wlan_parse_response_frame(struct wilc *wl, u8 *info, int size)
> 
>  		switch (FIELD_GET(WILC_WID_TYPE, wid)) {
>  		case WID_CHAR:
> +			len = 3;
> +			if (len + 2  > size)
> +				return;
> +
>  			while (cfg->b[i].id != WID_NIL && cfg->b[i].id != wid)
>  				i++;
> 
>  			if (cfg->b[i].id == wid)
>  				cfg->b[i].val = info[4];
> 
> -			len = 3;
>  			break;
> 
>  		case WID_SHORT:
> +			len = 4;
> +			if (len + 2  > size)
> +				return;
> +
>  			while (cfg->hw[i].id != WID_NIL && cfg->hw[i].id != wid)
>  				i++;
> 
>  			if (cfg->hw[i].id == wid)
>  				cfg->hw[i].val = get_unaligned_le16(&info[4]);
> 
> -			len = 4;
>  			break;
> 
>  		case WID_INT:
> +			len = 6;
> +			if (len + 2  > size)
> +				return;
> +
>  			while (cfg->w[i].id != WID_NIL && cfg->w[i].id != wid)
>  				i++;
> 
>  			if (cfg->w[i].id == wid)
>  				cfg->w[i].val = get_unaligned_le32(&info[4]);
> 
> -			len = 6;
>  			break;
> 
>  		case WID_STR:
> +			len = 2 + get_unaligned_le16(&info[2]);
> +			if (len > WILC_MAX_CFG_STR_SIZE || (len + 2  > size))
> +				return;

My bad, I did a mistake here. After reviewing the patch again, I realized that
this check can't be against a single maximum value. Instead, it needs to use
the size of specific element within structure received from firmware.
Please ignore this patch. I will update and send the v2 version.

Regards,
Ajay

Re: [PATCH] wifi: wilc1000: avoid buffer overflow in WID string configuration

[Dan Carpenter]

On Fri, Aug 29, 2025 at 06:22:44PM +0000, Ajay.Kathat@microchip.com wrote:
> Fix the following copy overflow warning identified by Smatch static checker.
> 
>  drivers/net/wireless/microchip/wilc1000/wlan_cfg.c:184 wilc_wlan_parse_response_frame()
>         error: '__memcpy()' 'cfg->s[i]->str' copy overflow (512 vs 65537)
> 
> This patch introduces size check before accessing the memory buffer.
> The checks are base on the WID type of received data from the firmware.
> For WID string configuration, the size limit is determined by the maximum
> element size in 'struct wilc_cfg_str_vals'. Therefore, WILC_MAX_CFG_STR_SIZE
> macro is added to configure this size.
> 
> Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
> Closes: https://lore.kernel.org/linux-wireless/aLFbr9Yu9j_TQTey@stanley.mountain
> Suggested-by: Dan Carpenter <dan.carpenter@linaro.org>
> Signed-off-by: Ajay Singh <ajay.kathat@microchip.com>
> ---

Thanks so much!

Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

regards,
dan carpenter