[PATCH v4 0/2] staging: rtl8723bs: fix OOB write and read in HT_caps_handler and OnAssocRsp

Alexandru Hossu posted 2 patches 1 month, 1 week ago
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(-)
[PATCH v4 0/2] staging: rtl8723bs: fix OOB write and read in HT_caps_handler and OnAssocRsp
Posted by Alexandru Hossu 1 month, 1 week ago
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
Re: [PATCH v4 0/2] staging: rtl8723bs: fix OOB write and read in HT_caps_handler and OnAssocRsp
Posted by Greg KH 1 month ago
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
Re: [PATCH v4 0/2] staging: rtl8723bs: fix OOB write and read in HT_caps_handler and OnAssocRsp
Posted by Greg KH 1 month ago
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