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

On Mon, Jul 27, 2026 at 2:18 AM Jeff Johnson
<[email protected]> wrote:
>
> 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: [email protected]
> > Closes: https://syzkaller.appspot.com/bug?extid=cc867e537e4bd36f69bb
> > Signed-off-by: Deepanshu Kartikey <[email protected]>
> >
> > ---
> > 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()
>

Thanks for the suggestion. I will send patch v3.

Thanks
Deepanshu