drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 4 ++++ drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-)
v3, addressing Dan Carpenter's and Luka Gejak's feedback on v2. Greg, please drop your patch and take this one instead. Changes from v2: - 1/2: switch from min_t() to umin() (Dan Carpenter) - 1/2: keep truncation approach rather than reverting to early return; early return bypasses HT_caps_enable = 1, silently disabling HT mode for APs that append extra bytes to the HT Capabilities IE (Luka Gejak, AI review) - 1/2: expand commit message to document the early return tradeoff - 1/2: add changelog with links to AI review and Greg's v1 reply 2/2 is unchanged from v2. Alexandru Hossu (2): staging: rtl8723bs: fix OOB write in HT_caps_handler() staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 4 ++++ drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) -- 2.53.0
v4, addressing the sashiko review comments on v3.
Regarding your questions:
The two patches to drop from your tree are the ones applied from v2:
41a866092f09 ("staging: rtl8723bs: fix OOB write in HT_caps_handler()")
e36c54247447 ("staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop")
v4 supersedes both.
Regarding hardware: I do not have rtl8723bs hardware available. The
patches are derived from reading the code, cross-checking against the
802.11 spec, and comparing against the existing HT_info_handler() guard
pattern in the same file.
What changed in v4:
Patch 1 (HT_caps_handler):
The v3 umin() loop bounded the write side correctly, but three macros
that run after the loop access pIE->data[0] and pIE->data[1]
unconditionally. If pIE->length is 0 or 1 those reads go out of
bounds. Added if (pIE->length < 2) return; placed after
HT_caps_enable = 1 so that HT negotiation is not regressed.
Patch 2 (OnAssocRsp):
Two additional issues found by sashiko:
- The fixed-field reads (capability, status, AID) at
pframe + WLAN_HDR_A3_LEN + {0,2,4} run without any minimum frame
length check. Added if (pkt_len < WLAN_HDR_A3_LEN + 6) return _FAIL.
- The WMM OUI comparison (memcmp of 6 bytes) ran without checking
pIE->length >= 6. An IE with length < 6 at the end of the packet
caused the memcmp to read into adjacent frame data. Added
pIE->length >= 6 guard.
Alexandru Hossu (2):
staging: rtl8723bs: fix OOB write and read in HT_caps_handler()
staging: rtl8723bs: fix OOB reads in OnAssocRsp() IE parsing
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 10 +++++++++-
drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 6 +++++-
2 files changed, 14 insertions(+), 2 deletions(-)
--
2.53.0
On Tue, May 05, 2026 at 07:22:12PM +0200, Alexandru Hossu wrote: > v4, addressing the sashiko review comments on v3. What about these review comments: https://sashiko.dev/#/patchset/20260505172214.3650398-1-hossu.alexandru@gmail.com thanks, greg k-h
On Tue, May 05, 2026 at 07:22:12PM +0200, Alexandru Hossu wrote:
> v4, addressing the sashiko review comments on v3.
>
> Regarding your questions:
>
> The two patches to drop from your tree are the ones applied from v2:
>
> 41a866092f09 ("staging: rtl8723bs: fix OOB write in HT_caps_handler()")
I have no such git id in my tree, where is this coming from?
> e36c54247447 ("staging: rtl8723bs: fix OOB read in OnAssocRsp() IE loop")
Same here, where is that git id in my tree? What branch?
totally confused.
> v4 supersedes both.
What happened to v3?
> Regarding hardware: I do not have rtl8723bs hardware available. The
> patches are derived from reading the code, cross-checking against the
> 802.11 spec, and comparing against the existing HT_info_handler() guard
> pattern in the same file.
>
> What changed in v4:
>
> Patch 1 (HT_caps_handler):
> The v3 umin() loop bounded the write side correctly, but three macros
> that run after the loop access pIE->data[0] and pIE->data[1]
> unconditionally. If pIE->length is 0 or 1 those reads go out of
> bounds. Added if (pIE->length < 2) return; placed after
> HT_caps_enable = 1 so that HT negotiation is not regressed.
>
> Patch 2 (OnAssocRsp):
> Two additional issues found by sashiko:
> - The fixed-field reads (capability, status, AID) at
> pframe + WLAN_HDR_A3_LEN + {0,2,4} run without any minimum frame
> length check. Added if (pkt_len < WLAN_HDR_A3_LEN + 6) return _FAIL.
> - The WMM OUI comparison (memcmp of 6 bytes) ran without checking
> pIE->length >= 6. An IE with length < 6 at the end of the packet
> caused the memcmp to read into adjacent frame data. Added
> pIE->length >= 6 guard.
what changed in the previous versions? You have to list them all.
And you have 3 different sets of patches I see, why is this not all one
big series? What is the order of these different sets?
really really confused now...
greg k-h
HT_caps_handler() iterates over pIE->length bytes and writes into
HT_caps.u.HT_cap[], a fixed array of sizeof(struct HT_caps_element)
bytes. pIE->length is a raw u8 from an over-the-air 802.11
Association Response frame and is never validated before the loop. A
malicious AP can set it to 255, writing up to 229 bytes past the end
of the array into adjacent fields of struct mlme_ext_info.
Additionally, after the loop the function calls three macros that
unconditionally read pIE->data[0] and pIE->data[1]:
GET_HT_CAPABILITY_ELE_LDPC_CAP(pIE->data) -- reads data[0], bit 0
GET_HT_CAPABILITY_ELE_TX_STBC(pIE->data) -- reads data[0], bit 7
GET_HT_CAPABILITY_ELE_RX_STBC(pIE->data) -- reads data[1], bits 0-1
If a malicious AP sends an HT Capabilities IE with pIE->length less
than 2, both bytes the macros need are outside the IE payload,
causing an out-of-bounds read.
Fix both issues:
- Set HT_caps_enable = 1 first so HT negotiation is not regressed.
- Return early if pIE->length < 2 to protect the macro reads.
- Use umin() in the loop to bound the write side.
The parallel HT_info_handler() already guards against oversized IEs.
This patch applies the same discipline to HT_caps_handler().
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v4:
- Add pIE->length < 2 guard after HT_caps_enable = 1 to prevent OOB
reads from GET_HT_CAPABILITY_ELE_LDPC_CAP/TX_STBC/RX_STBC macros
that access pIE->data[0] and pIE->data[1] unconditionally.
Caught by sashiko review of v3.
- Use umin() in the loop bound to cap writes at
sizeof(HT_caps.u.HT_cap) without bypassing HT_caps_enable.
Changes in v3:
- Switch from min_t() to umin() (Dan Carpenter).
- Keep truncation approach rather than early return so HT_caps_enable
is always set before the length check (Luka Gejak, AI review).
Changes in v2:
- Replace early return before HT_caps_enable = 1 with umin()
truncation so HT mode is not disabled for APs with oversized IEs.
drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index 6a7c09db4cd9..98aa50357e96 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -936,7 +936,11 @@ 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++) {
+ if (pIE->length < 2)
+ return;
+
+ 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.0Three out-of-bounds read paths in OnAssocRsp():
1. Missing minimum frame length check before fixed field reads.
Before entering the IE loop the function reads capability, status,
and AID from fixed offsets relative to pframe + WLAN_HDR_A3_LEN
(at offsets +0, +2, and +4 respectively, so 6 bytes total). There
is no check that pkt_len is large enough to cover these fields. A
malicious AP can send a truncated Association Response frame shorter
than WLAN_HDR_A3_LEN + 6 bytes, causing out-of-bounds reads and
loading garbage into MLME state variables.
2. IE header and payload may extend past the packet end.
The IE loop advances by pIE->length + 2 per iteration but only
guards on i < pkt_len. When the last IE has only one byte left in
the frame, the loop reads pIE->length from pframe[pkt_len], one
byte past the receive buffer. Even when the header bytes are in
bounds, pIE->length can point the data window past pkt_len, silently
passing a truncated IE to the handler functions.
3. WMM OUI comparison reads 6 bytes past a possibly short IE payload.
For WLAN_EID_VENDOR_SPECIFIC, the code calls memcmp(pIE->data,
WMM_PARA_OUI, 6) without checking that pIE->length is at least 6.
An attacker can craft a vendor-specific IE at the end of the frame
with pIE->length smaller than 6. The existing IE bounds check only
confirms the declared payload fits within pkt_len, not that it is
large enough for the 6-byte OUI comparison.
Fix all three:
- Return _FAIL immediately if pkt_len < WLAN_HDR_A3_LEN + 6.
- Add two guards in the IE loop: break if fewer than sizeof(*pIE)
bytes remain, and break if the declared IE payload extends past
pkt_len.
- Guard the WMM OUI comparison with pIE->length >= 6.
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Alexandru Hossu <hossu.alexandru@gmail.com>
---
Changes in v4:
- Add pkt_len < WLAN_HDR_A3_LEN + 6 check before reading the three
fixed fields (capability, status, AID) to prevent OOB reads from
truncated frames. Caught by sashiko review of v3.
- Add pIE->length >= 6 guard before the 6-byte WMM OUI memcmp to
prevent reading past a short IE payload. Caught by sashiko.
Changes in v2:
- Add IE header bounds check: break if i + sizeof(*pIE) > pkt_len.
- Add IE payload bounds check: break if the declared IE data extends
past pkt_len.
drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index 5f00fe282d1b..84cc814f069c 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -1379,6 +1379,9 @@ unsigned int OnAssocRsp(struct adapter *padapter, union recv_frame *precv_frame)
timer_delete_sync(&pmlmeext->link_timer);
+ if (pkt_len < WLAN_HDR_A3_LEN + 6)
+ return _FAIL;
+
/* status */
status = le16_to_cpu(*(__le16 *)(pframe + WLAN_HDR_A3_LEN + 2));
if (status > 0) {
@@ -1400,11 +1403,16 @@ 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:
- if (!memcmp(pIE->data, WMM_PARA_OUI, 6)) /* WMM */
+ if (pIE->length >= 6 &&
+ !memcmp(pIE->data, WMM_PARA_OUI, 6)) /* WMM */
WMM_param_handler(padapter, pIE);
break;
--
2.53.0On Tue, Apr 28, 2026 at 11:16:19AM +0200, Alexandru Hossu wrote: > v3, addressing Dan Carpenter's and Luka Gejak's feedback on v2. > > Greg, please drop your patch and take this one instead. What patch should I drop? What is the git id of it? And what about these review comments: https://sashiko.dev/#/patchset/20260428091621.739680-1-hossu.alexandru@gmail.com And have you tested this on real hardware? thanks, greg k-h
© 2016 - 2026 Red Hat, Inc.