Re: [bug report] wilc1000: move wilc driver out of staging

[<Ajay.Kathat@microchip.com>]

Hi Dan,

Thanks for pointing out this warning along with the analysis.

On 8/29/25 00:50, Dan Carpenter wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> Hello Ajay Singh,
> 
> [ Obviously this bug was in staging as well...  ]
> 
> Commit 5625f965d764 ("wilc1000: move wilc driver out of staging")
> from Jun 25, 2020 (linux-next), leads to the following Smatch static
> checker warning:
> 
>         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)
> 
> drivers/net/wireless/microchip/wilc1000/wlan_cfg.c
>     138 static void wilc_wlan_parse_response_frame(struct wilc *wl, u8 *info, int size)
>     139 {
>     140         u16 wid;
>     141         u32 len = 0, i = 0;
>     142         struct wilc_cfg *cfg = &wl->cfg;
>     143
>     144         while (size > 0) {
>     145                 i = 0;
>     146                 wid = get_unaligned_le16(info);
>     147
>     148                 switch (FIELD_GET(WILC_WID_TYPE, wid)) {
>     149                 case WID_CHAR:
>     150                         while (cfg->b[i].id != WID_NIL && cfg->b[i].id != wid)
>     151                                 i++;
>     152
> 
> This is the rx path and info comes from skb->data so it feels like we
> need more bounds  checking.
> 
>         if (info < 5)
>                 return;
> 

Agree, the read can go beyond the buffer limit so it's safe to add boundary
checks before accessing the 'info' buffer.

>     153                         if (cfg->b[i].id == wid)
>     154                                 cfg->b[i].val = info[4];
>     155
>     156                         len = 3;
>     157                         break;
>     158
>     159                 case WID_SHORT:
> 
>         if (info < 6)
>                 return;
> 
>     160                         while (cfg->hw[i].id != WID_NIL && cfg->hw[i].id != wid)
>     161                                 i++;
>     162
>     163                         if (cfg->hw[i].id == wid)
>     164                                 cfg->hw[i].val = get_unaligned_le16(&info[4]);
>     165
>     166                         len = 4;
>     167                         break;
>     168
>     169                 case WID_INT:
> 
>         if (info < 8)
>                 return;
> 
>     170                         while (cfg->w[i].id != WID_NIL && cfg->w[i].id != wid)
>     171                                 i++;
>     172
>     173                         if (cfg->w[i].id == wid)
>     174                                 cfg->w[i].val = get_unaligned_le32(&info[4]);
>     175
>     176                         len = 6;
>     177                         break;
>     178
>     179                 case WID_STR:
> 
> How many bytes are there in cfg->s[i].str?  Smatch thinks 512, but I
> don't know where Smatch is getting that...
> 
>         len = 2 + get_unaligned_le16(&info[2]);
>         if (len + 2 > SOME_LIMIT)
>                 return;

I think Smatch got 512 value from the below macro.

#define WILC_MAX_ASSOC_RESP_FRAME_SIZE 512

Patch[1] has been submitted to address this warning. I hope this would resolve
the warning.

1.
https://lore.kernel.org/linux-wireless/20250829182241.45150-1-ajay.kathat@microchip.com/


Regards,
Ajay