1. Patchew
  2. linux
  3. View series

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

Park Tae-sun posted 1 patch 3 days, 12 hours ago 
drivers/staging/rtl8723bs/core/rtw_ap.c | 81 ++++++++++---------------
1 file changed, 33 insertions(+), 48 deletions(-)
[PATCH] staging: rtl8723bs: fix buffer overflow and OOB accesses in rtw_check_beacon_data()
Posted by Park Tae-sun 3 days, 12 hours ago 
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(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 4728f62a1557..70e95b07ca2a 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -771,7 +771,7 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 	if (!check_fwstate(pmlmepriv, WIFI_AP_STATE))
 		return _FAIL;
 
-	if (len < 0 || len > MAX_IE_SZ)
+	if (len < _BEACON_IE_OFFSET_ || len > MAX_IE_SZ)
 		return _FAIL;
 
 	pbss_network->ie_length = len;
@@ -885,61 +885,46 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 	group_cipher = 0; pairwise_cipher = 0;
 	psecuritypriv->wpa_group_cipher = _NO_PRIVACY_;
 	psecuritypriv->wpa_pairwise_cipher = _NO_PRIVACY_;
-	for (p = ie + _BEACON_IE_OFFSET_; ; p += (ie_len + 2)) {
-		p = rtw_get_ie(p,
-			       WLAN_EID_VENDOR_SPECIFIC,
-			       &ie_len,
-			       (pbss_network->ie_length - _BEACON_IE_OFFSET_ - (ie_len + 2)));
-		if ((p) && (!memcmp(p + 2, OUI1, 4))) {
-			if (rtw_parse_wpa_ie(p,
-					     ie_len + 2,
-					     &group_cipher,
-					     &pairwise_cipher,
-					     NULL) == _SUCCESS) {
-				psecuritypriv->dot11_auth_algrthm = dot11_auth_algrthm_8021x;
-
-				psecuritypriv->dot8021xalg = 1;/* psk,  todo:802.1x */
-
-				psecuritypriv->wpa_psk |= BIT(0);
-
-				psecuritypriv->wpa_group_cipher = group_cipher;
-				psecuritypriv->wpa_pairwise_cipher = pairwise_cipher;
-			}
+	p = rtw_get_ie_ex(ie + _BEACON_IE_OFFSET_,
+			  pbss_network->ie_length - _BEACON_IE_OFFSET_,
+			  WLAN_EID_VENDOR_SPECIFIC,
+			  OUI1, sizeof(OUI1), NULL, &ie_len);
+	if (p) {
+		if (rtw_parse_wpa_ie(p,
+				     ie_len,
+				     &group_cipher,
+				     &pairwise_cipher,
+				     NULL) == _SUCCESS) {
+			psecuritypriv->dot11_auth_algrthm = dot11_auth_algrthm_8021x;
 
-			break;
-		}
+			psecuritypriv->dot8021xalg = 1;/* psk,  todo:802.1x */
 
-		if (!p || ie_len == 0)
-			break;
+			psecuritypriv->wpa_psk |= BIT(0);
+
+			psecuritypriv->wpa_group_cipher = group_cipher;
+			psecuritypriv->wpa_pairwise_cipher = pairwise_cipher;
+		}
 	}
 
 	/* wmm */
 	ie_len = 0;
 	pmlmepriv->qospriv.qos_option = 0;
 	if (pregistrypriv->wmm_enable) {
-		for (p = ie + _BEACON_IE_OFFSET_; ; p += (ie_len + 2)) {
-			p = rtw_get_ie(p,
-				       WLAN_EID_VENDOR_SPECIFIC,
-				       &ie_len,
-				       (pbss_network->ie_length -
-					_BEACON_IE_OFFSET_ - (ie_len + 2)));
-			if ((p) && !memcmp(p + 2, WMM_PARA_IE, 6)) {
-				pmlmepriv->qospriv.qos_option = 1;
-
-				*(p + 8) |= BIT(7);/* QoS Info, support U-APSD */
-
-				/* disable all ACM bits since the WMM admission */
-				/* control is not supported */
-				*(p + 10) &= ~BIT(4); /* BE */
-				*(p + 14) &= ~BIT(4); /* BK */
-				*(p + 18) &= ~BIT(4); /* VI */
-				*(p + 22) &= ~BIT(4); /* VO */
-
-				break;
-			}
-
-			if (!p || ie_len == 0)
-				break;
+		p = rtw_get_ie_ex(ie + _BEACON_IE_OFFSET_,
+				  pbss_network->ie_length - _BEACON_IE_OFFSET_,
+				  WLAN_EID_VENDOR_SPECIFIC,
+				  WMM_PARA_IE, sizeof(WMM_PARA_IE), NULL, &ie_len);
+		if (p && ie_len >= WLAN_WMM_LEN + 2) {
+			pmlmepriv->qospriv.qos_option = 1;
+
+			*(p + 8) |= BIT(7);/* QoS Info, support U-APSD */
+
+			/* disable all ACM bits since the WMM admission */
+			/* control is not supported */
+			*(p + 10) &= ~BIT(4); /* BE */
+			*(p + 14) &= ~BIT(4); /* BK */
+			*(p + 18) &= ~BIT(4); /* VI */
+			*(p + 22) &= ~BIT(4); /* VO */
 		}
 	}
 
-- 
2.43.0
Re: [PATCH] staging: rtl8723bs: fix buffer overflow and OOB accesses in rtw_check_beacon_data()
Posted by Greg Kroah-Hartman 3 days, 11 hours ago 
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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.