[PATCH] wifi: wilc1000: fix u8 overflow in SSID scan buffer size calculation

[PATCH] wifi: wilc1000: fix u8 overflow in SSID scan buffer size calculation

[Yasuaki Torimaru]

The variable valuesize is declared as u8 but accumulates the total
length of all SSIDs to scan. Each SSID contributes up to 33 bytes
(IEEE80211_MAX_SSID_LEN + 1), and with WILC_MAX_NUM_PROBED_SSID (10)
SSIDs the total can reach 330, which wraps around to 74 when stored
in a u8.

This causes kmalloc to allocate only 75 bytes while the subsequent
memcpy writes up to 331 bytes into the buffer, resulting in a 256-byte
heap buffer overflow.

Widen valuesize from u8 to u32 to accommodate the full range.

Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
Cc: stable@vger.kernel.org
Signed-off-by: Yasuaki Torimaru <yasuakitorimaru@gmail.com>
---
 drivers/net/wireless/microchip/wilc1000/hif.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/wireless/microchip/wilc1000/hif.c b/drivers/net/wireless/microchip/wilc1000/hif.c
index f354b11cb919..944b2a812b63 100644
--- a/drivers/net/wireless/microchip/wilc1000/hif.c
+++ b/drivers/net/wireless/microchip/wilc1000/hif.c
@@ -163,7 +163,7 @@ int wilc_scan(struct wilc_vif *vif, u8 scan_source,
 	u32 index = 0;
 	u32 i, scan_timeout;
 	u8 *buffer;
-	u8 valuesize = 0;
+	u32 valuesize = 0;
 	u8 *search_ssid_vals = NULL;
 	const u8 ch_list_len = request->n_channels;
 	struct host_if_drv *hif_drv = vif->hif_drv;
-- 
2.50.1

Re: [PATCH] wifi: wilc1000: fix u8 overflow in SSID scan buffer size calculation

[Jeff Johnson]

On 3/24/2026 3:06 AM, Yasuaki Torimaru wrote:
> The variable valuesize is declared as u8 but accumulates the total
> length of all SSIDs to scan. Each SSID contributes up to 33 bytes
> (IEEE80211_MAX_SSID_LEN + 1), and with WILC_MAX_NUM_PROBED_SSID (10)
> SSIDs the total can reach 330, which wraps around to 74 when stored
> in a u8.
> 
> This causes kmalloc to allocate only 75 bytes while the subsequent
> memcpy writes up to 331 bytes into the buffer, resulting in a 256-byte
> heap buffer overflow.
> 
> Widen valuesize from u8 to u32 to accommodate the full range.
> 
> Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Yasuaki Torimaru <yasuakitorimaru@gmail.com>

Reviewed-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>

Another thing to note is it is very strange that the struct wid that defines
the TLV format uses a signed type for both the TLV length and payload pointer:
	s32 size;
	s8 *val;

I don't think I've ever seen this in a TLV representation!

> ---
>  drivers/net/wireless/microchip/wilc1000/hif.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/net/wireless/microchip/wilc1000/hif.c b/drivers/net/wireless/microchip/wilc1000/hif.c
> index f354b11cb919..944b2a812b63 100644
> --- a/drivers/net/wireless/microchip/wilc1000/hif.c
> +++ b/drivers/net/wireless/microchip/wilc1000/hif.c
> @@ -163,7 +163,7 @@ int wilc_scan(struct wilc_vif *vif, u8 scan_source,
>  	u32 index = 0;
>  	u32 i, scan_timeout;
>  	u8 *buffer;
> -	u8 valuesize = 0;
> +	u32 valuesize = 0;
>  	u8 *search_ssid_vals = NULL;
>  	const u8 ch_list_len = request->n_channels;
>  	struct host_if_drv *hif_drv = vif->hif_drv;

Re: [PATCH] wifi: wilc1000: fix u8 overflow in SSID scan buffer size calculation

[Yasuaki Torimaru]

On Tue, 25 Mar 2026, Jeff Johnson wrote:
  > Reviewed-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
  >
  > Another thing to note is it is very strange that the struct wid that defines
  > the TLV format uses a signed type for both the TLV length and
payload pointer:
  >         s32 size;
  >         s8 *val;
  >
  > I don't think I've ever seen this in a TLV representation!

  Thank you for the review.

  Good point — signed types for TLV length and payload are indeed unusual
  and could mask subtle sign-extension bugs. I'll look into a follow-up
  cleanup patch for struct wid once this fix lands.

  Thanks,
  Yasuaki

2026年3月24日(火) 23:50 Jeff Johnson <jeff.johnson@oss.qualcomm.com>:
>
> On 3/24/2026 3:06 AM, Yasuaki Torimaru wrote:
> > The variable valuesize is declared as u8 but accumulates the total
> > length of all SSIDs to scan. Each SSID contributes up to 33 bytes
> > (IEEE80211_MAX_SSID_LEN + 1), and with WILC_MAX_NUM_PROBED_SSID (10)
> > SSIDs the total can reach 330, which wraps around to 74 when stored
> > in a u8.
> >
> > This causes kmalloc to allocate only 75 bytes while the subsequent
> > memcpy writes up to 331 bytes into the buffer, resulting in a 256-byte
> > heap buffer overflow.
> >
> > Widen valuesize from u8 to u32 to accommodate the full range.
> >
> > Fixes: c5c77ba18ea6 ("staging: wilc1000: Add SDIO/SPI 802.11 driver")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Yasuaki Torimaru <yasuakitorimaru@gmail.com>
>
> Reviewed-by: Jeff Johnson <jeff.johnson@oss.qualcomm.com>
>
> Another thing to note is it is very strange that the struct wid that defines
> the TLV format uses a signed type for both the TLV length and payload pointer:
>         s32 size;
>         s8 *val;
>
> I don't think I've ever seen this in a TLV representation!
>
> > ---
> >  drivers/net/wireless/microchip/wilc1000/hif.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/net/wireless/microchip/wilc1000/hif.c b/drivers/net/wireless/microchip/wilc1000/hif.c
> > index f354b11cb919..944b2a812b63 100644
> > --- a/drivers/net/wireless/microchip/wilc1000/hif.c
> > +++ b/drivers/net/wireless/microchip/wilc1000/hif.c
> > @@ -163,7 +163,7 @@ int wilc_scan(struct wilc_vif *vif, u8 scan_source,
> >       u32 index = 0;
> >       u32 i, scan_timeout;
> >       u8 *buffer;
> > -     u8 valuesize = 0;
> > +     u32 valuesize = 0;
> >       u8 *search_ssid_vals = NULL;
> >       const u8 ch_list_len = request->n_channels;
> >       struct host_if_drv *hif_drv = vif->hif_drv;
>