mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie()
@ 2026-07-25 14:20 Deepanshu Kartikey
  2026-07-26 20:48 ` Jeff Johnson
  0 siblings, 1 reply; 4+ messages in thread
From: Deepanshu Kartikey @ 2026-07-25 14:20 UTC (permalink / raw)
  To: johannes
  Cc: linux-wireless, linux-kernel, Deepanshu Kartikey,
	syzbot+cc867e537e4bd36f69bb

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;
+		for_each_element(elem, extra, ie_len) {
+			/* nothing */
+		}
+
+		if (!for_each_element_completed(elem, extra, ie_len))
+			return -EINVAL;
+
 		ie = kmemdup(extra, ie_len, GFP_KERNEL);
 		if (!ie)
 			return -ENOMEM;
-- 
2.43.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie()
  2026-07-25 14:20 [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie() Deepanshu Kartikey
@ 2026-07-26 20:48 ` Jeff Johnson
  2026-07-29  1:41   ` Deepanshu Kartikey
  0 siblings, 1 reply; 4+ messages in thread
From: Jeff Johnson @ 2026-07-26 20:48 UTC (permalink / raw)
  To: Deepanshu Kartikey, johannes
  Cc: linux-wireless, linux-kernel, syzbot+cc867e537e4bd36f69bb

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;


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie()
  2026-07-26 20:48 ` Jeff Johnson
@ 2026-07-29  1:41   ` Deepanshu Kartikey
  2026-07-29  6:34     ` Johannes Berg
  0 siblings, 1 reply; 4+ messages in thread
From: Deepanshu Kartikey @ 2026-07-29  1:41 UTC (permalink / raw)
  To: Jeff Johnson
  Cc: johannes, linux-wireless, linux-kernel, syzbot+cc867e537e4bd36f69bb

On Mon, Jul 27, 2026 at 2:18 AM Jeff Johnson
<jeff.johnson@oss.qualcomm.com> 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: 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()
>

Thanks for the suggestion. I will send patch v3.

Thanks
Deepanshu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie()
  2026-07-29  1:41   ` Deepanshu Kartikey
@ 2026-07-29  6:34     ` Johannes Berg
  0 siblings, 0 replies; 4+ messages in thread
From: Johannes Berg @ 2026-07-29  6:34 UTC (permalink / raw)
  To: Deepanshu Kartikey, Jeff Johnson
  Cc: linux-wireless, linux-kernel, syzbot+cc867e537e4bd36f69bb

On Wed, 2026-07-29 at 07:11 +0530, Deepanshu Kartikey wrote:
> 
> Thanks for the suggestion. I will send patch v3.

I applied (a modified version of) v2 yesterday, so I'm not going to take
v3 - and anyway the bigger cleanup isn't great for wireless as a fix
right now. We can revisit this in wireless-next.

johannes

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-29  6:34 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-07-25 14:20 [PATCH v2] wifi: cfg80211: validate IEs in cfg80211_wext_siwgenie() Deepanshu Kartikey
2026-07-26 20:48 ` Jeff Johnson
2026-07-29  1:41   ` Deepanshu Kartikey
2026-07-29  6:34     ` Johannes Berg

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®