Re: [PATCH] staging: rtl8723bs: fix buffer overflow and OOB accesses in rtw_check_beacon_data()

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

On Mon, Sep 21, 2026 at 04:37:30PM +0900, Park Tae-sun wrote:
> In rtw_check_beacon_data(), the Information Element (IE) parsing logic
> has boundary validation issues and search window calculations that can
> lead to out-of-bounds accesses:
> 
> First, IEEE 802.11 beacon frames contain 12 bytes of fixed parameters
> (Timestamp, Beacon Interval, Capability Info) before variable IEs start
> at _BEACON_IE_OFFSET_ (12). The function checks len < 0, but if len is
> smaller than _BEACON_IE_OFFSET_, (pbss_network->ie_length -
> _BEACON_IE_OFFSET_) underflows on the u32 field, and reading the beacon
> interval at offset 8 via rtw_get_beacon_interval_from_ie() accesses
> out-of-bounds data if len < 10.
> 
> Second, the manual vendor IE loops for WPA and WMM advance p by
> (ie_len + 2) on non-matching elements, but calculate the limit passed
> to rtw_get_ie() as:
>   (pbss_network->ie_length - _BEACON_IE_OFFSET_ - (ie_len + 2))
> This only subtracts the previous element's length rather than the
> accumulated offset (p - start), causing p + limit to extend past the
> end of the buffer on subsequent iterations. In addition, calling
> memcmp() without checking ie_len can read past short vendor elements.
> 
> Third, in the WMM loop, once the OUI matches, bytes up to *(p + 22)
> are modified without checking whether the IE contains the full
> 24-byte WMM parameter payload (WLAN_WMM_LEN). A truncated element
> leads to out-of-bounds writes.
> 
> Address these by:
>   1. Checking len < _BEACON_IE_OFFSET_ at function entry.
>   2. Using the existing rtw_get_ie_ex() helper which validates
>      element boundaries and OUI lengths before calling memcmp().
>   3. Passing ie_len directly to rtw_parse_wpa_ie() without +2,
>      because rtw_get_ie_ex() already includes the 2-byte header.
>   4. Verifying that the WMM element has at least WLAN_WMM_LEN + 2
>      bytes before modifying the parameter records.
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Park Tae-sun <ts930@dgu.ac.kr>
> ---
>  drivers/staging/rtl8723bs/core/rtw_ap.c | 81 ++++++++++---------------
>  1 file changed, 33 insertions(+), 48 deletions(-)

How did you find this?

How did you test it?

You need a blank line before the Fixes: tag, right?  Didn't checkpatch
catch this?

thanks,

greg k-h