[PATCH v2 0/2] staging: rtl8723bs: bound two IE parses

[PATCH v2 0/2] staging: rtl8723bs: bound two IE parses

[Ali Ahmet Memis]

v1 put a single check across five call sites and one of them was not an
HT capability site at all. It was the SSID parse in rtw_check_beacon_data(),
so the patch required an SSID of at least sizeof(struct ieee80211_ht_cap),
26 bytes, before copying it. Every SSID shorter than that would have been
dropped, which breaks bringing up an AP with an ordinary name. That was a
regression, not a fix, and I asked for v1 to be dropped here:

  https://lore.kernel.org/all/20260802151916.38931-1-ali@iusegentoo.com/

v2 splits the two apart. Patch 1 is v1 minus that hunk: the four real HT
capability sites, unchanged otherwise. Patch 2 is the check that site
actually needed, which is an upper bound rather than a lower one, since
rtw_get_ie() can hand back an element length of up to 255 for a 32 byte
destination.

I have no rtl8723bs hardware, so both are from reading the driver rather
than from an observed failure. Build tested with CONFIG_RTL8723BS=m.

Ali Ahmet Memis (2):
  staging: rtl8723bs: validate HT capability IE length before use
  staging: rtl8723bs: bound the SSID element length before copying it

 drivers/staging/rtl8723bs/core/rtw_ap.c        | 2 +-
 drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 2 +-
 drivers/staging/rtl8723bs/core/rtw_mlme.c      | 2 +-
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c  | 2 +-
 drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)


base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
-- 
2.55.0

[PATCH v2 1/2] staging: rtl8723bs: validate HT capability IE length before use

[Ali Ahmet Memis]

Four sites locate the HT capability element with rtw_get_ie() and then
read through it without checking that the element is long enough:

	p = rtw_get_ie(..., WLAN_EID_HT_CAPABILITY, &len, ...);
	if (p && len > 0) {
		pht_cap = (struct ieee80211_ht_cap *)(p + 2);
		ht_cap_info = le16_to_cpu(pht_cap->cap_info);

rtw_get_ie() only bounds the element against the end of the IE buffer, so
len is whatever the sender put in the length byte. A beacon or probe
response carrying a one byte HT capability element passes len > 0 and the
driver then reads two bytes of cap_info, and in rtw_update_ht_cap() the
ampdu_params_info byte after that, from beyond the element.

An HT capability element is a fixed 26 bytes, so require that much before
dereferencing it. The frames come from the air, so the length is not
under local control.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 2 +-
 drivers/staging/rtl8723bs/core/rtw_mlme.c      | 2 +-
 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c  | 2 +-
 drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
index 863ddf846218..2e66a6e86a32 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ieee80211.c
@@ -1094,7 +1094,7 @@ void rtw_get_bcn_info(struct wlan_network *pnetwork)
 	/* get bwmode and ch_offset */
 	/* parsing HT_CAP_IE */
 	p = rtw_get_ie(pnetwork->network.ies + _FIXED_IE_LENGTH_, WLAN_EID_HT_CAPABILITY, &len, pnetwork->network.ie_length - _FIXED_IE_LENGTH_);
-	if (p && len > 0) {
+	if (p && len >= sizeof(struct ieee80211_ht_cap)) {
 		pht_cap = (struct ieee80211_ht_cap *)(p + 2);
 		pnetwork->bcn_info.ht_cap_info = le16_to_cpu(pht_cap->cap_info);
 	} else {
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme.c b/drivers/staging/rtl8723bs/core/rtw_mlme.c
index 1196ec011455..03dd4b5e94d6 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme.c
@@ -2416,7 +2416,7 @@ void rtw_update_ht_cap(struct adapter *padapter, u8 *pie, uint ie_len, u8 channe
 	/* check Max Rx A-MPDU Size */
 	len = 0;
 	p = rtw_get_ie(pie + sizeof(struct ndis_802_11_fix_ie), WLAN_EID_HT_CAPABILITY, &len, ie_len - sizeof(struct ndis_802_11_fix_ie));
-	if (p && len > 0) {
+	if (p && len >= sizeof(struct ieee80211_ht_cap)) {
 		pht_capie = (struct ieee80211_ht_cap *)(p + 2);
 		max_ampdu_sz = (pht_capie->ampdu_params_info & IEEE80211_HT_CAP_AMPDU_FACTOR);
 		max_ampdu_sz = 1 << (max_ampdu_sz + 3); /*  max_ampdu_sz (kbytes); */
diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index a443b3530fb9..c884700d6e0d 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -3934,7 +3934,7 @@ u8 collect_bss_info(struct adapter *padapter, union recv_frame *precv_frame, str
 		struct mlme_priv *pmlmepriv = &padapter->mlmepriv;
 
 		p = rtw_get_ie(bssid->ies + ie_offset, WLAN_EID_HT_CAPABILITY, &len, bssid->ie_length - ie_offset);
-		if (p && len > 0) {
+		if (p && len >= sizeof(struct HT_caps_element)) {
 			struct HT_caps_element	*pHT_caps;
 
 			pHT_caps = (struct HT_caps_element	*)(p + 2);
diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index a4de538722b5..7fd032b89429 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -1130,7 +1130,7 @@ int rtw_check_bcn_info(struct adapter *Adapter, u8 *pframe, u32 packet_len)
 	/* check bw and channel offset */
 	/* parsing HT_CAP_IE */
 	p = rtw_get_ie(bssid->ies + _FIXED_IE_LENGTH_, WLAN_EID_HT_CAPABILITY, &len, bssid->ie_length - _FIXED_IE_LENGTH_);
-	if (p && len > 0) {
+	if (p && len >= sizeof(struct ieee80211_ht_cap)) {
 		pht_cap = (struct ieee80211_ht_cap *)(p + 2);
 		ht_cap_info = le16_to_cpu(pht_cap->cap_info);
 	} else {
-- 
2.55.0

[PATCH v2 2/2] staging: rtl8723bs: bound the SSID element length before copying it

[Ali Ahmet Memis]

rtw_check_beacon_data() copies the SSID element straight into a fixed
32 byte array:

	p = rtw_get_ie(ie + _BEACON_IE_OFFSET_, WLAN_EID_SSID, &ie_len, ...);
	if (p && ie_len > 0) {
		memset(&pbss_network->ssid, 0, sizeof(struct ndis_802_11_ssid));
		memcpy(pbss_network->ssid.ssid, (p + 2), ie_len);

rtw_get_ie() writes the raw element length byte to *len and only limits
it against the end of the IE buffer:

	tmp = *(p + 1);
	if (i + 2 + tmp > limit)
		break;
	if (*p == index) {
		*len = tmp;

so ie_len can be up to 255, while the destination is

	struct ndis_802_11_ssid {
		u32  ssid_length;
		u8   ssid[32];
	};

and the only length check the function does beforehand is len <= MAX_IE_SZ
on the whole buffer. An SSID element longer than 32 bytes therefore
overruns ssid[] and the members of struct wlan_bssid_ex that follow it in
pmlmepriv->cur_network.network.

The beacon comes from cfg80211 start_ap and change_beacon, so it needs
CAP_NET_ADMIN and a beacon that hostapd would not normally build, but
nothing stops it. Skip the copy when the element does not fit, which is
what already happens when the element is absent.

Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 drivers/staging/rtl8723bs/core/rtw_ap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 065850a9e894..62f420636485 100644
--- a/drivers/staging/rtl8723bs/core/rtw_ap.c
+++ b/drivers/staging/rtl8723bs/core/rtw_ap.c
@@ -802,7 +802,7 @@ int rtw_check_beacon_data(struct adapter *padapter, u8 *pbuf,  int len)
 		       WLAN_EID_SSID,
 		       &ie_len,
 		       (pbss_network->ie_length - _BEACON_IE_OFFSET_));
-	if (p && ie_len > 0) {
+	if (p && ie_len > 0 && ie_len <= sizeof(pbss_network->ssid.ssid)) {
 		memset(&pbss_network->ssid, 0, sizeof(struct ndis_802_11_ssid));
 		memcpy(pbss_network->ssid.ssid, (p + 2), ie_len);
 		pbss_network->ssid.ssid_length = ie_len;
-- 
2.55.0

Re: [PATCH v2 1/2] staging: rtl8723bs: validate HT capability IE length before use

[Greg Kroah-Hartman]

On Sun, Aug 02, 2026 at 03:35:08PM +0000, Ali Ahmet Memis wrote:
> Four sites locate the HT capability element with rtw_get_ie() and then
> read through it without checking that the element is long enough:
> 
> 	p = rtw_get_ie(..., WLAN_EID_HT_CAPABILITY, &len, ...);
> 	if (p && len > 0) {
> 		pht_cap = (struct ieee80211_ht_cap *)(p + 2);
> 		ht_cap_info = le16_to_cpu(pht_cap->cap_info);
> 
> rtw_get_ie() only bounds the element against the end of the IE buffer, so
> len is whatever the sender put in the length byte. A beacon or probe
> response carrying a one byte HT capability element passes len > 0 and the
> driver then reads two bytes of cap_info, and in rtw_update_ht_cap() the
> ampdu_params_info byte after that, from beyond the element.
> 
> An HT capability element is a fixed 26 bytes, so require that much before
> dereferencing it. The frames come from the air, so the length is not
> under local control.
> 
> Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
> ---
>  drivers/staging/rtl8723bs/core/rtw_ieee80211.c | 2 +-
>  drivers/staging/rtl8723bs/core/rtw_mlme.c      | 2 +-
>  drivers/staging/rtl8723bs/core/rtw_mlme_ext.c  | 2 +-
>  drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)

What tool did you use to find and fix these and how were they tested?

thanks,

greg k-h