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

[PATCH v3 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
umin() 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. An early return on oversized IEs was considered
but rejected: it would bypass the pmlmeinfo->HT_caps_enable = 1
assignment that precedes the loop, silently disabling HT mode for APs
that append extra bytes to the HT Capabilities IE.

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v3:
- Use umin() instead of min_t() (Dan Carpenter)
- Keep truncation approach; early return would bypass
  HT_caps_enable = 1, disabling HT mode for vendor-padded IEs
  (Luka Gejak, AI review)
- Expand commit message to document the early return tradeoff
- Add changelog

AI review (flagged early return as regression-inducing):
  https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
Greg KH v1 reply (requested truncation over early return):
  https://lore.kernel.org/linux-staging/2026042630-tightness-runner-2121@gregkh/

 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..dd34f229df12 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 < umin(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 v3 1/2] staging: rtl8723bs: fix OOB write in HT_caps_handler()

[Luka Gejak]

On April 28, 2026 11:16:20 AM GMT+02:00, Alexandru Hossu <hossu.alexandru@gmail.com> 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
>umin() 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. An early return on oversized IEs was considered
>but rejected: it would bypass the pmlmeinfo->HT_caps_enable = 1
>assignment that precedes the loop, silently disabling HT mode for APs
>that append extra bytes to the HT Capabilities IE.
>
>Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>Cc: stable@vger.kernel.org
>Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
>---
>Changes in v3:
>- Use umin() instead of min_t() (Dan Carpenter)
>- Keep truncation approach; early return would bypass
>  HT_caps_enable = 1, disabling HT mode for vendor-padded IEs
>  (Luka Gejak, AI review)
>- Expand commit message to document the early return tradeoff
>- Add changelog
>
>AI review (flagged early return as regression-inducing):
>  https://sashiko.dev/#/patchset/2026041408-grill-mahogany-d1e3%40gregkh
>Greg KH v1 reply (requested truncation over early return):
>  https://lore.kernel.org/linux-staging/2026042630-tightness-runner-2121@gregkh/
>
> 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..dd34f229df12 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 < umin(pIE->length,
>+			     sizeof(pmlmeinfo->HT_caps.u.HT_cap)); i++) {
> 		if (i != 2) {
> 			/* Commented by Albert 2010/07/12 */
> 			/* Got the endian issue here. */

LGTM,

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

Best regards,
Luka Gejak

[PATCH v3 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

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

[Luka Gejak]

On April 28, 2026 11:16:21 AM GMT+02:00, Alexandru Hossu <hossu.alexandru@gmail.com> 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 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:

LGTM,

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

Best regards,
Luka Gejak