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@xxxxxxxxxxxxx/
Regards,
Ajay
