Re: [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie()

[Jeff Johnson <jeff.johnson@oss.qualcomm.com>]

On 7/25/2026 7:20 AM, Deepanshu Kartikey wrote:
> The KASAN allocation trace shows that a malformed IE buffer is
> stored via SIOCSIWGENIE (cfg80211_wext_siwgenie()) without any
> validation. The crash trace shows that a subsequent SIOCSIWESSID
> triggers a connection attempt which calls cfg80211_sme_get_conn_ies()
> to process the stored IE buffer, causing:
> 
>  - An out-of-bounds read in skip_ie() which reads ies[pos+1]
>    (the length byte) past the end of the 1-byte buffer.
> 
>  - An integer underflow in the memcpy size argument when offs
>    returned by ieee80211_ie_split() exceeds ies_len, causing
>    unsigned subtraction to wrap to SIZE_MAX and triggering a
>    fortify panic.
> 
> Fix this by validating the IE buffer in cfg80211_wext_siwgenie()
> before storing it. First reject buffers smaller than 2 bytes since
> a valid IE requires at least a type and length field. Then use
> for_each_element() and for_each_element_completed() to verify all
> elements are well-formed. Return -EINVAL if validation fails.
> 
> Reported-by: syzbot+cc867e537e4bd36f69bb@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=cc867e537e4bd36f69bb
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
> 
> ---
> v2: Use for_each_element() and for_each_element_completed() instead
>     of open-coded validation loop, as suggested by Johannes Berg.
>     Also add explicit ie_len < 2 check to handle the case where
>     for_each_element_completed() returns true for a 1-byte buffer.
> ---
>  net/wireless/wext-sme.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/net/wireless/wext-sme.c b/net/wireless/wext-sme.c
> index 573b6b15a446..51fc2617a1c3 100644
> --- a/net/wireless/wext-sme.c
> +++ b/net/wireless/wext-sme.c
> @@ -319,6 +319,18 @@ int cfg80211_wext_siwgenie(struct net_device *dev,
>  		return 0;
>  
>  	if (ie_len) {
> +		const struct element *elem;
> +
> +		 /* IE must have at least Type + Length bytes */
> +		if (ie_len < 2)
> +			return -EINVAL;

doesn't for_each_element() already handle this?
	     (const u8 *)(_data) + (_datalen) - (const u8 *)_elem >=	\
		(int)sizeof(*_elem) &&					\

> +		for_each_element(elem, extra, ie_len) {
> +			/* nothing */
> +		}
> +
> +		if (!for_each_element_completed(elem, extra, ie_len))
> +			return -EINVAL;

perhaps this sequence should be refactored into a separate helper that is used
here, and by the existing functions that have the same pattern?
validate_beacon_head()
validate_ie_attr()

> +
>  		ie = kmemdup(extra, ie_len, GFP_KERNEL);
>  		if (!ie)
>  			return -ENOMEM;