[PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Alexandru Hossu]

HT_caps_handler() iterates pIE->length bytes and writes into
HT_caps.u.HT_cap[], which is a fixed 26-byte array (sizeof struct
HT_caps_element). Because pIE->length is a raw u8 from an over-the-air
802.11 AssocResponse frame and is never validated, a malicious AP can set
it up to 255, causing up to 229 bytes of out-of-bounds writes into
adjacent fields of struct mlme_ext_info.

Truncate the iteration count to the size of HT_caps.u.HT_cap using
min_t() so that data from a longer-than-expected IE is silently ignored
rather than written out of bounds, preserving interoperability with APs
that pad the element.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index e0d73c267786..3247565f41bd 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -936,7 +936,8 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
 
 	pmlmeinfo->HT_caps_enable = 1;
 
-	for (i = 0; i < (pIE->length); i++) {
+	for (i = 0; i < min_t(unsigned int, pIE->length,
+			       sizeof(pmlmeinfo->HT_caps.u.HT_cap)); i++) {
 		if (i != 2) {
 			/* Commented by Albert 2010/07/12 */
 			/* Got the endian issue here. */
-- 
2.53.0

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Dan Carpenter]

On Mon, Apr 27, 2026 at 10:17:47AM +0200, Alexandru Hossu wrote:
> HT_caps_handler() iterates pIE->length bytes and writes into
> HT_caps.u.HT_cap[], which is a fixed 26-byte array (sizeof struct
> HT_caps_element). Because pIE->length is a raw u8 from an over-the-air
> 802.11 AssocResponse frame and is never validated, a malicious AP can set
> it up to 255, causing up to 229 bytes of out-of-bounds writes into
> adjacent fields of struct mlme_ext_info.
> 
> Truncate the iteration count to the size of HT_caps.u.HT_cap using
> min_t() so that data from a longer-than-expected IE is silently ignored
> rather than written out of bounds, preserving interoperability with APs
> that pad the element.
> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---

We need a little change log here.  I was hoping you would provide
a link to the AI review in the changelog.

I feel like the AI review is probabl wrong.  In this case the
original code corrupted memory so the code didn't "work" before, it
corrupted memory.  But I'm interested to see the AI review.

regards,
dan carpenter

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Alexandru Hossu]

On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> We need a little change log here.  I was hoping you would provide
> a link to the AI review in the changelog.

Hi Dan,

Sorry about the missing changelog, will add it in v3.

For the AI review link, I don't have a direct link to the bot output. What I know is from Greg's reply in the v1 thread on lore.kernel.org, where he said both his fix and mine would break things on some systems according to the review bot and asked me to use truncation instead. I went with min_t() specifically because he asked for it.

You're right that technically early return would have been strictly better than the original, the original was already writing out of bounds so it wasn't working to begin with. But since Greg asked for truncation I kept it that way.

Thank you.

Alexandru

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Dan Carpenter]

On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
> On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> > We need a little change log here.  I was hoping you would provide
> > a link to the AI review in the changelog.
> 
> Hi Dan,
> 
> Sorry about the missing changelog, will add it in v3.
> 
> For the AI review link, I don't have a direct link to the bot output.
> What I know is from Greg's reply in the v1 thread on lore.kernel.org,

What about a link to the email on lore?

> where he said both his fix and mine would break things on some systems
> according to the review bot and asked me to use truncation instead.
> I went with min_t() specifically because he asked for it.
> 
> You're right that technically early return would have been strictly
> better than the original, the original was already writing out of
> bounds so it wasn't working to begin with. But since Greg asked for
> truncation I kept it that way.

This is the path of least resistance, but it's better to push back
on bad advice from AI.  Greg won't be offended.  And if he still
doesn't like after you push back then you can still do the min_t()
version.  Both versions are fine really, but I generally go with
the stricter one when it doesn't break anything that wasn't already
very broken.

Better to use umin() instead of min_t(), btw.

regards,
dan carpenter

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Greg KH]

On Mon, Apr 27, 2026 at 12:48:38PM +0300, Dan Carpenter wrote:
> On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
> > On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> > > We need a little change log here.  I was hoping you would provide
> > > a link to the AI review in the changelog.
> > 
> > Hi Dan,
> > 
> > Sorry about the missing changelog, will add it in v3.
> > 
> > For the AI review link, I don't have a direct link to the bot output.
> > What I know is from Greg's reply in the v1 thread on lore.kernel.org,
> 
> What about a link to the email on lore?

Sorry, I was on a plane with no connectivity to look it up, here's the
AI review for my patch:
	https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh

> > where he said both his fix and mine would break things on some systems
> > according to the review bot and asked me to use truncation instead.
> > I went with min_t() specifically because he asked for it.
> > 
> > You're right that technically early return would have been strictly
> > better than the original, the original was already writing out of
> > bounds so it wasn't working to begin with. But since Greg asked for
> > truncation I kept it that way.
> 
> This is the path of least resistance, but it's better to push back
> on bad advice from AI.  Greg won't be offended.  And if he still
> doesn't like after you push back then you can still do the min_t()
> version.  Both versions are fine really, but I generally go with
> the stricter one when it doesn't break anything that wasn't already
> very broken.

Agreed, it takes a lot to offend me these days :)

thanks,

greg k-h

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Dan Carpenter]

On Mon, Apr 27, 2026 at 05:11:19AM -0600, Greg KH wrote:
> On Mon, Apr 27, 2026 at 12:48:38PM +0300, Dan Carpenter wrote:
> > On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
> > > On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> > > > We need a little change log here.  I was hoping you would provide
> > > > a link to the AI review in the changelog.
> > > 
> > > Hi Dan,
> > > 
> > > Sorry about the missing changelog, will add it in v3.
> > > 
> > > For the AI review link, I don't have a direct link to the bot output.
> > > What I know is from Greg's reply in the v1 thread on lore.kernel.org,
> > 
> > What about a link to the email on lore?
> 
> Sorry, I was on a plane with no connectivity to look it up, here's the
> AI review for my patch:
> 	https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
> 

Ah.  Very good.  That's fair enough then.  The AI is very convincing.

regards,
dan carpenter

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Greg KH]

On Mon, Apr 27, 2026 at 03:58:23PM +0300, Dan Carpenter wrote:
> On Mon, Apr 27, 2026 at 05:11:19AM -0600, Greg KH wrote:
> > On Mon, Apr 27, 2026 at 12:48:38PM +0300, Dan Carpenter wrote:
> > > On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
> > > > On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> > > > > We need a little change log here.  I was hoping you would provide
> > > > > a link to the AI review in the changelog.
> > > > 
> > > > Hi Dan,
> > > > 
> > > > Sorry about the missing changelog, will add it in v3.
> > > > 
> > > > For the AI review link, I don't have a direct link to the bot output.
> > > > What I know is from Greg's reply in the v1 thread on lore.kernel.org,
> > > 
> > > What about a link to the email on lore?
> > 
> > Sorry, I was on a plane with no connectivity to look it up, here's the
> > AI review for my patch:
> > 	https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
> > 
> 
> Ah.  Very good.  That's fair enough then.  The AI is very convincing.

Yes, but is it correct?  That's the problem with these tools :)

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Luka Gejak]

On April 27, 2026 3:11:28 PM GMT+02:00, Greg KH <gregkh@linuxfoundation.org> wrote:
>On Mon, Apr 27, 2026 at 03:58:23PM +0300, Dan Carpenter wrote:
>> On Mon, Apr 27, 2026 at 05:11:19AM -0600, Greg KH wrote:
>> > On Mon, Apr 27, 2026 at 12:48:38PM +0300, Dan Carpenter wrote:
>> > > On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
>> > > > On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
>> > > > > We need a little change log here.  I was hoping you would provide
>> > > > > a link to the AI review in the changelog.
>> > > > 
>> > > > Hi Dan,
>> > > > 
>> > > > Sorry about the missing changelog, will add it in v3.
>> > > > 
>> > > > For the AI review link, I don't have a direct link to the bot output.
>> > > > What I know is from Greg's reply in the v1 thread on lore.kernel.org,
>> > > 
>> > > What about a link to the email on lore?
>> > 
>> > Sorry, I was on a plane with no connectivity to look it up, here's the
>> > AI review for my patch:
>> > 	https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
>> > 
>> 
>> Ah.  Very good.  That's fair enough then.  The AI is very convincing.
>
>Yes, but is it correct?  That's the problem with these tools :)
>

Hi Greg, Dan,
I have reviewed this patch. While it successfully prevents the oob 
write, it unfortunately introduces a functional regression. By 
enforcing if (pIE->length > sizeof(pmlmeinfo->HT_caps)) and returning 
early, the driver bypasses setting pmlmeinfo->HT_caps_enable = 1;. If 
future 802.11 standards (or non-standard vendors) append extra bytes 
to the ht capability ie, completely discarding the ie will silently 
disable high throughput mode and degrade performance. To fix the oob 
write without breaking functionality, a better approach would be using
min_t() to cap the length and only process the valid portion.
Separately, while auditing this path, I noticed two pre-existing oob 
read vulnerabilities:
1. The GET_HT_CAPABILITY_ELE_RX_STBC(pIE->data) macro 
(drivers/staging/rtl8723bs/include/rtw_ht.h:81) unconditionally reads 
from pIE->data[1]. If a malicious packet provides a pIE->length of 0 
or 1, the driver will blindly read past the end of the ie's data. 
2. In OnAssocRsp() (drivers/staging/rtl8723bs/core/rtw_mlme_ext.c), 
the ie iteration loop for (i = ...; i < pkt_len;) does not verify 
if i + 2 <= pkt_len or if i + 2 + pIE->length <= pkt_len before 
invoking handlers. Therefore, pIE->data isn't guaranteed to be bounded
within the received packet buffer, allowing oob reads simply by 
overflowing pkt_len from the air.
I recommend revising this patch to use min_t() to avoid the ht mode 
regression. Since the oob reads are pre-existing issues outside the 
scope of fixing this oob write, they should be addressed in separate 
patches, which I am happy to submit :).
Best regards,
Luka Gejak

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Dan Carpenter]

On Mon, Apr 27, 2026 at 07:11:28AM -0600, Greg KH wrote:
> On Mon, Apr 27, 2026 at 03:58:23PM +0300, Dan Carpenter wrote:
> > On Mon, Apr 27, 2026 at 05:11:19AM -0600, Greg KH wrote:
> > > On Mon, Apr 27, 2026 at 12:48:38PM +0300, Dan Carpenter wrote:
> > > > On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
> > > > > On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> > > > > > We need a little change log here.  I was hoping you would provide
> > > > > > a link to the AI review in the changelog.
> > > > > 
> > > > > Hi Dan,
> > > > > 
> > > > > Sorry about the missing changelog, will add it in v3.
> > > > > 
> > > > > For the AI review link, I don't have a direct link to the bot output.
> > > > > What I know is from Greg's reply in the v1 thread on lore.kernel.org,
> > > > 
> > > > What about a link to the email on lore?
> > > 
> > > Sorry, I was on a plane with no connectivity to look it up, here's the
> > > AI review for my patch:
> > > 	https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
> > > 
> > 
> > Ah.  Very good.  That's fair enough then.  The AI is very convincing.
> 
> Yes, but is it correct?  That's the problem with these tools :)

If we go with this approach then probably we should probably
change HT_info_handler() to match as well?

regards,
dan carpenter

Re: [PATCH v2 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Dan Carpenter]

On Mon, Apr 27, 2026 at 05:45:11PM +0300, Dan Carpenter wrote:
> On Mon, Apr 27, 2026 at 07:11:28AM -0600, Greg KH wrote:
> > On Mon, Apr 27, 2026 at 03:58:23PM +0300, Dan Carpenter wrote:
> > > On Mon, Apr 27, 2026 at 05:11:19AM -0600, Greg KH wrote:
> > > > On Mon, Apr 27, 2026 at 12:48:38PM +0300, Dan Carpenter wrote:
> > > > > On Mon, Apr 27, 2026 at 02:28:39AM -0700, Alexandru Hossu wrote:
> > > > > > On Mon, Apr 27, 2026 at 11:17 AM, Dan Carpenter wrote:
> > > > > > > We need a little change log here.  I was hoping you would provide
> > > > > > > a link to the AI review in the changelog.
> > > > > > 
> > > > > > Hi Dan,
> > > > > > 
> > > > > > Sorry about the missing changelog, will add it in v3.
> > > > > > 
> > > > > > For the AI review link, I don't have a direct link to the bot output.
> > > > > > What I know is from Greg's reply in the v1 thread on lore.kernel.org,
> > > > > 
> > > > > What about a link to the email on lore?
> > > > 
> > > > Sorry, I was on a plane with no connectivity to look it up, here's the
> > > > AI review for my patch:
> > > > 	https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
> > > > 
> > > 
> > > Ah.  Very good.  That's fair enough then.  The AI is very convincing.
> > 
> > Yes, but is it correct?  That's the problem with these tools :)
> 
> If we go with this approach then probably we should probably
> change HT_info_handler() to match as well?

No, sorry, I think HT_info_handler() is fine.  I don't love that it
doesn't return error codes, but since we set pmlmeinfo->HT_info_enable
on success that kind of works as error handling.

regards,
dan carpenter

[PATCH v2 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop

[Alexandru Hossu]

The IE parsing loop in OnAssocRsp() advances by (pIE->length + 2) each
iteration but only guards on i < pkt_len. When a malicious AP sends an
AssocResponse whose last IE has only one byte remaining in the frame
(the element_id byte lands at pkt_len-1), the loop reads pIE->length
from pframe[pkt_len], which is one byte past the allocated receive buffer.

Additionally, even when the header bytes are in bounds, pIE->length
itself can extend the data window beyond pkt_len, silently passing a
truncated IE to the handler functions.

Add two guards at the top of the loop body:
  1. Break if fewer than sizeof(*pIE) bytes remain (can't read header).
  2. Break if the IE's declared data extends past pkt_len.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index c646dc2a1741..68ce422305ed 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -1406,7 +1406,11 @@ unsigned int OnAssocRsp(struct adapter *padapter, union recv_frame *precv_frame)
 	/* to handle HT, WMM, rate adaptive, update MAC reg */
 	/* for not to handle the synchronous IO in the tasklet */
 	for (i = (6 + WLAN_HDR_A3_LEN); i < pkt_len;) {
+		if (i + sizeof(*pIE) > pkt_len)
+			break;
 		pIE = (struct ndis_80211_var_ie *)(pframe + i);
+		if (i + sizeof(*pIE) + pIE->length > pkt_len)
+			break;
 
 		switch (pIE->element_id) {
 		case WLAN_EID_VENDOR_SPECIFIC:
-- 
2.53.0