public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()
@ 2026-04-20 14:04 Alexandru Hossu
  2026-04-20 14:06 ` Alexandru Hossu
  0 siblings, 1 reply; 8+ messages in thread
From: Alexandru Hossu @ 2026-04-20 14:04 UTC (permalink / raw)
  To: gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter, Alexandru Hossu

From: Alexandru Hossu <a.hossu@student.tudelft.nl>

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.

The parallel function HT_info_handler() already carries the correct guard:

  if (pIE->length > sizeof(struct HT_info_element))
          return;

Apply the same pattern to HT_caps_handler().

Signed-off-by: Alexandru Hossu <a.hossu@student.tudelft.nl>
---
 drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index 6a7c09db4..b75e7f4f8 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -934,6 +934,9 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
 	if (phtpriv->ht_option == false)
 		return;
 
+	if (pIE->length > sizeof(struct HT_caps_element))
+		return;
+
 	pmlmeinfo->HT_caps_enable = 1;
 
 	for (i = 0; i < (pIE->length); i++) {
-- 
2.53.0


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

* Re: [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()
  2026-04-20 14:04 Alexandru Hossu
@ 2026-04-20 14:06 ` Alexandru Hossu
  0 siblings, 0 replies; 8+ messages in thread
From: Alexandru Hossu @ 2026-04-20 14:06 UTC (permalink / raw)
  To: gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter, Alexandru Hossu

Sorry for the noise — the previous series was sent with the wrong
From/Signed-off-by (university address instead of personal Gmail).
Please ignore patches 1/2 and 2/2 from Message-ID
<20260420140432.150431-1-hossu.alexandru@gmail.com>.
Resending the correct series now.

Alexandru

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

* [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()
@ 2026-04-20 14:08 Alexandru Hossu
  2026-04-20 14:08 ` [PATCH 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop Alexandru Hossu
  2026-04-21 14:40 ` [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Luka Gejak
  0 siblings, 2 replies; 8+ messages in thread
From: Alexandru Hossu @ 2026-04-20 14:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter, 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.

The parallel function HT_info_handler() already carries the correct guard:

  if (pIE->length > sizeof(struct HT_info_element))
          return;

Apply the same pattern to HT_caps_handler().

Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
 drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index 6a7c09db4..b75e7f4f8 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -934,6 +934,9 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
 	if (phtpriv->ht_option == false)
 		return;
 
+	if (pIE->length > sizeof(struct HT_caps_element))
+		return;
+
 	pmlmeinfo->HT_caps_enable = 1;
 
 	for (i = 0; i < (pIE->length); i++) {
-- 
2.53.0


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

* [PATCH 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop
  2026-04-20 14:08 [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Alexandru Hossu
@ 2026-04-20 14:08 ` Alexandru Hossu
  2026-04-21 14:43   ` Luka Gejak
  2026-04-21 14:40 ` [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Luka Gejak
  1 sibling, 1 reply; 8+ messages in thread
From: Alexandru Hossu @ 2026-04-20 14:08 UTC (permalink / raw)
  To: gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter, 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 the header).
  2. Break if the IE's declared data extends past pkt_len.

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 5f00fe282..9666226a6 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -1400,7 +1400,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


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

* Re: [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()
  2026-04-20 14:08 [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Alexandru Hossu
  2026-04-20 14:08 ` [PATCH 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop Alexandru Hossu
@ 2026-04-21 14:40 ` Luka Gejak
  2026-04-21 14:45   ` Luka Gejak
  2026-04-26 19:26   ` Greg KH
  1 sibling, 2 replies; 8+ messages in thread
From: Luka Gejak @ 2026-04-21 14:40 UTC (permalink / raw)
  To: Alexandru Hossu, gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter

On Mon Apr 20, 2026 at 4:08 PM CEST, 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.
>
> The parallel function HT_info_handler() already carries the correct guard:
>
>   if (pIE->length > sizeof(struct HT_info_element))
>           return;
>
> Apply the same pattern to HT_caps_handler().
>
> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> index 6a7c09db4..b75e7f4f8 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> @@ -934,6 +934,9 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
>  	if (phtpriv->ht_option == false)
>  		return;
>  
> +	if (pIE->length > sizeof(struct HT_caps_element))
> +		return;
> +
>  	pmlmeinfo->HT_caps_enable = 1;
>  
>  	for (i = 0; i < (pIE->length); i++) {

Hi Alexandru,
this fix has been made already by Greg HK therefore this patch is 
unnecessary. You can see his patch at [1].
Best regards,
Luka Gejak

[1]: https://lore.kernel.org/linux-staging/2026041408-grill-mahogany-d1e3@gregkh/

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

* Re: [PATCH 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop
  2026-04-20 14:08 ` [PATCH 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop Alexandru Hossu
@ 2026-04-21 14:43   ` Luka Gejak
  0 siblings, 0 replies; 8+ messages in thread
From: Luka Gejak @ 2026-04-21 14:43 UTC (permalink / raw)
  To: Alexandru Hossu, gregkh; +Cc: linux-staging, linux-kernel, dan.carpenter

On Mon Apr 20, 2026 at 4:08 PM CEST, Alexandru Hossu wrote:
> 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 the header).
>   2. Break if the IE's declared data extends past pkt_len.
>
> 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 5f00fe282..9666226a6 100644
> --- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> +++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
> @@ -1400,7 +1400,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:

LGTM.

Reviewed-by: Luka Gejak <luka.gejak@linux.dev>

Best regards,
Luka Gejak

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

* Re: [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()
  2026-04-21 14:40 ` [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Luka Gejak
@ 2026-04-21 14:45   ` Luka Gejak
  2026-04-26 19:26   ` Greg KH
  1 sibling, 0 replies; 8+ messages in thread
From: Luka Gejak @ 2026-04-21 14:45 UTC (permalink / raw)
  To: Luka Gejak, Alexandru Hossu, gregkh
  Cc: linux-staging, linux-kernel, dan.carpenter

On Tue Apr 21, 2026 at 4:40 PM CEST, Luka Gejak wrote:
> On Mon Apr 20, 2026 at 4:08 PM CEST, 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.
>>
>> The parallel function HT_info_handler() already carries the correct guard:
>>
>>   if (pIE->length > sizeof(struct HT_info_element))
>>           return;
>>
>> Apply the same pattern to HT_caps_handler().
>>
>> Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
>> ---
>>  drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
>> index 6a7c09db4..b75e7f4f8 100644
>> --- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
>> +++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
>> @@ -934,6 +934,9 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
>>  	if (phtpriv->ht_option == false)
>>  		return;
>>  
>> +	if (pIE->length > sizeof(struct HT_caps_element))
>> +		return;
>> +
>>  	pmlmeinfo->HT_caps_enable = 1;
>>  
>>  	for (i = 0; i < (pIE->length); i++) {
>
> Hi Alexandru,
> this fix has been made already by Greg HK therefore this patch is 
					^^^^ *KH (Kroah-Hartman)
> unnecessary. You can see his patch at [1].
> Best regards,
> Luka Gejak
>
> [1]: https://lore.kernel.org/linux-staging/2026041408-grill-mahogany-d1e3@gregkh/


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

* Re: [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()
  2026-04-21 14:40 ` [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Luka Gejak
  2026-04-21 14:45   ` Luka Gejak
@ 2026-04-26 19:26   ` Greg KH
  1 sibling, 0 replies; 8+ messages in thread
From: Greg KH @ 2026-04-26 19:26 UTC (permalink / raw)
  To: Luka Gejak; +Cc: Alexandru Hossu, linux-staging, linux-kernel, dan.carpenter

On Tue, Apr 21, 2026 at 04:40:17PM +0200, Luka Gejak wrote:
> On Mon Apr 20, 2026 at 4:08 PM CEST, 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.
> >
> > The parallel function HT_info_handler() already carries the correct guard:
> >
> >   if (pIE->length > sizeof(struct HT_info_element))
> >           return;
> >
> > Apply the same pattern to HT_caps_handler().
> >
> > Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
> > ---
> >  drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> > index 6a7c09db4..b75e7f4f8 100644
> > --- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> > +++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
> > @@ -934,6 +934,9 @@ void HT_caps_handler(struct adapter *padapter, struct ndis_80211_var_ie *pIE)
> >  	if (phtpriv->ht_option == false)
> >  		return;
> >  
> > +	if (pIE->length > sizeof(struct HT_caps_element))
> > +		return;
> > +
> >  	pmlmeinfo->HT_caps_enable = 1;
> >  
> >  	for (i = 0; i < (pIE->length); i++) {
> 
> Hi Alexandru,
> this fix has been made already by Greg HK therefore this patch is 
> unnecessary. You can see his patch at [1].
> Best regards,
> Luka Gejak
> 
> [1]: https://lore.kernel.org/linux-staging/2026041408-grill-mahogany-d1e3@gregkh/

Yeah, and we both got it wrong, if we do this, this will break things on
some systems according to the ai review bot.  So we need to just
truncate the data, not abort.

Alexandru, want to fix this up in your version and send it?  If so, I'll
drop mine.

thanks,

greg k-h

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

end of thread, other threads:[~2026-04-27  3:50 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 14:08 [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Alexandru Hossu
2026-04-20 14:08 ` [PATCH 2/2] staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop Alexandru Hossu
2026-04-21 14:43   ` Luka Gejak
2026-04-21 14:40 ` [PATCH 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler() Luka Gejak
2026-04-21 14:45   ` Luka Gejak
2026-04-26 19:26   ` Greg KH
  -- strict thread matches above, loose matches on Subject: below --
2026-04-20 14:04 Alexandru Hossu
2026-04-20 14:06 ` Alexandru Hossu

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