* [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®