From: Luka Gejak <luka.gejak@linux.dev>

When building an association request frame, the driver iterates over
the ies received from the ap. In several places, the driver trusts the
attacker-controlled pIE->length without validating that it meets the
minimum expected size for the respective ie.

For WLAN_EID_HT_CAPABILITY, this causes an oob read of adjacent heap
memory which is then transmitted over the air (remote heap information
disclosure). For WLAN_EID_VENDOR_SPECIFIC, it causes two separate oob
reads: one when checking the 4-byte oui, and another when copying the
14-byte wps ie.

Fix these issues by adding upper-bound checks at the start of the loop
to ensure the ie fits within the buffer, and explicit lower-bound
checks to return a failure if the length is insufficient. For
HT_CAPABILITY, also clamp the length passed to rtw_set_ie() to the
struct size.

Also fix three additional issues discovered during review:
- Missing free of pmgntframe and its xmitbuf before jumping to exit
  in the WLAN_EID_VENDOR_SPECIFIC lower-bound checks.
- In is_ap_in_tkip(), add missing lower-bound checks for the RSN and
  vendor-specific IE data accesses (pre-existing bug).
- Move rtw_buf_update() before dump_mgntframe() to avoid a potential
  use-after-free of pwlanhdr, which points into the mgmt frame buffer
  (pre-existing bug).

Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
---
Changes in v7:
 - Address new sashiko comments.

Changes in v6:
 - Restore full changes history.

Changes in v5:
 - Address sashiko comments.

Changes in v4:
 - Added upper-bound checks at the start of the loop to ensure the ie
  fits within the received buffer, as pointed out by Dan.
 - Updated commit message to reflect the addition of upper-bound checks.

Changes in v3:
 - Switched to fail-fast handling for malformed IEs in issue_assocreq().
 - Fixed HT capability path to use structure-sized output length in 
  rtw_set_ie().
 - Updated commit message to reflect all oob read cases.

Changes in v2:
 - Refactored rtw_set_ie() alignment to follow "open parenthesis" style.
 - Allowed the line length to exceed 100 characters for better
  readability as requested by Greg KH.

 drivers/staging/rtl8723bs/core/rtw_mlme_ext.c | 43 +++++++++++++++++--
 .../staging/rtl8723bs/core/rtw_wlan_util.c    | 13 +++++-
 2 files changed, 50 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
index a86d6f97cf02..43112d14f619 100644
--- a/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
+++ b/drivers/staging/rtl8723bs/core/rtw_mlme_ext.c
@@ -2826,6 +2826,9 @@ void issue_assocreq(struct adapter *padapter)
 		if (pmlmeinfo->network.supported_rates[i] == 0)
 			break;
 
+		if (index >= NumRates)
+			break;
+
 		/*  Check if the AP's supported rates are also supported by STA. */
 		for (j = 0; j < sta_bssrate_len; j++) {
 			 /*  Avoid the proprietary data rate (22Mbps) of Handlink WSG-4000 AP */
@@ -2855,10 +2858,28 @@ void issue_assocreq(struct adapter *padapter)
 
 	/* vendor specific IE, such as WPA, WMM, WPS */
 	for (i = sizeof(struct ndis_802_11_fix_ie); i < pmlmeinfo->network.ie_length;) {
+		if (i + 2 > pmlmeinfo->network.ie_length) {
+			rtw_free_xmitbuf(pxmitpriv, pmgntframe->pxmitbuf);
+			rtw_free_xmitframe(pxmitpriv, pmgntframe);
+			goto exit;
+		}
+
 		pIE = (struct ndis_80211_var_ie *)(pmlmeinfo->network.ies + i);
 
+		if (pIE->length > pmlmeinfo->network.ie_length - i - 2) {
+			rtw_free_xmitbuf(pxmitpriv, pmgntframe->pxmitbuf);
+			rtw_free_xmitframe(pxmitpriv, pmgntframe);
+			goto exit;
+		}
+
 		switch (pIE->element_id) {
 		case WLAN_EID_VENDOR_SPECIFIC:
+			if (pIE->length < 4) {
+				rtw_free_xmitbuf(pxmitpriv, pmgntframe->pxmitbuf);
+				rtw_free_xmitframe(pxmitpriv, pmgntframe);
+				goto exit;
+			}
+
 			if ((!memcmp(pIE->data, RTW_WPA_OUI, 4)) ||
 					(!memcmp(pIE->data, WMM_OUI, 4)) ||
 					(!memcmp(pIE->data, WPS_OUI, 4))) {
@@ -2870,6 +2891,12 @@ void issue_assocreq(struct adapter *padapter)
 					 * extensions information to AP
 					 */
 
+					if (pIE->length < 14) {
+						rtw_free_xmitbuf(pxmitpriv, pmgntframe->pxmitbuf);
+						rtw_free_xmitframe(pxmitpriv, pmgntframe);
+						goto exit;
+					}
+
 					vs_ie_length = 14;
 				}
 
@@ -2883,8 +2910,17 @@ void issue_assocreq(struct adapter *padapter)
 		case WLAN_EID_HT_CAPABILITY:
 			if (padapter->mlmepriv.htpriv.ht_option) {
 				if (!(is_ap_in_tkip(padapter))) {
+					if (pIE->length < sizeof(struct HT_caps_element)) {
+						rtw_free_xmitbuf(pxmitpriv, pmgntframe->pxmitbuf);
+						rtw_free_xmitframe(pxmitpriv, pmgntframe);
+						goto exit;
+					}
+
 					memcpy(&(pmlmeinfo->HT_caps), pIE->data, sizeof(struct HT_caps_element));
-					pframe = rtw_set_ie(pframe, WLAN_EID_HT_CAPABILITY, pIE->length, (u8 *)(&(pmlmeinfo->HT_caps)), &(pattrib->pktlen));
+					pframe = rtw_set_ie(pframe, WLAN_EID_HT_CAPABILITY,
+							    sizeof(struct HT_caps_element),
+							    (u8 *)&pmlmeinfo->HT_caps,
+							    &pattrib->pktlen);
 				}
 			}
 			break;
@@ -2903,15 +2939,14 @@ void issue_assocreq(struct adapter *padapter)
 	if (pmlmeinfo->assoc_AP_vendor == HT_IOT_PEER_REALTEK)
 		pframe = rtw_set_ie(pframe, WLAN_EID_VENDOR_SPECIFIC, 6, REALTEK_96B_IE, &(pattrib->pktlen));
 
+	rtw_buf_update(&pmlmepriv->assoc_req, &pmlmepriv->assoc_req_len, (u8 *)pwlanhdr, pattrib->pktlen);
 	pattrib->last_txcmdsz = pattrib->pktlen;
 	dump_mgntframe(padapter, pmgntframe);
 
 	ret = _SUCCESS;
 
 exit:
-	if (ret == _SUCCESS)
-		rtw_buf_update(&pmlmepriv->assoc_req, &pmlmepriv->assoc_req_len, (u8 *)pwlanhdr, pattrib->pktlen);
-	else
+	if (ret != _SUCCESS)
 		rtw_buf_free(&pmlmepriv->assoc_req, &pmlmepriv->assoc_req_len);
 }
 
diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
index 1d37c2d5b10d..b8e88aaad897 100644
--- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
+++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c
@@ -1301,17 +1301,26 @@ unsigned int is_ap_in_tkip(struct adapter *padapter)
 
 	if (rtw_get_capability((struct wlan_bssid_ex *)cur_network) & WLAN_CAPABILITY_PRIVACY) {
 		for (i = sizeof(struct ndis_802_11_fix_ie); i < pmlmeinfo->network.ie_length;) {
+			if (i + 2 > pmlmeinfo->network.ie_length)
+				return false;
+
 			pIE = (struct ndis_80211_var_ie *)(pmlmeinfo->network.ies + i);
 
+			if (pIE->length > pmlmeinfo->network.ie_length - i - 2)
+				return false;
+
 			switch (pIE->element_id) {
 			case WLAN_EID_VENDOR_SPECIFIC:
-				if ((!memcmp(pIE->data, RTW_WPA_OUI, 4)) && (!memcmp((pIE->data + 12), WPA_TKIP_CIPHER, 4)))
+				if (pIE->length >= 16 &&
+				    (!memcmp(pIE->data, RTW_WPA_OUI, 4)) &&
+				    (!memcmp((pIE->data + 12), WPA_TKIP_CIPHER, 4)))
 					return true;
 
 				break;
 
 			case WLAN_EID_RSN:
-				if (!memcmp((pIE->data + 8), RSN_TKIP_CIPHER, 4))
+				if (pIE->length >= 12 &&
+				    !memcmp((pIE->data + 8), RSN_TKIP_CIPHER, 4))
 					return true;
 				break;
 
-- 
2.54.0

On Sat, May 23, 2026 at 03:13:31PM +0200, luka.gejak@linux.dev wrote:
> From: Luka Gejak <luka.gejak@linux.dev>
> 
> When building an association request frame, the driver iterates over
> the ies received from the ap. In several places, the driver trusts the
> attacker-controlled pIE->length without validating that it meets the
> minimum expected size for the respective ie.
> 
> For WLAN_EID_HT_CAPABILITY, this causes an oob read of adjacent heap
> memory which is then transmitted over the air (remote heap information
> disclosure). For WLAN_EID_VENDOR_SPECIFIC, it causes two separate oob
> reads: one when checking the 4-byte oui, and another when copying the
> 14-byte wps ie.
> 
> Fix these issues by adding upper-bound checks at the start of the loop
> to ensure the ie fits within the buffer, and explicit lower-bound
> checks to return a failure if the length is insufficient. For
> HT_CAPABILITY, also clamp the length passed to rtw_set_ie() to the
> struct size.
> 
> Also fix three additional issues discovered during review:
> - Missing free of pmgntframe and its xmitbuf before jumping to exit
>   in the WLAN_EID_VENDOR_SPECIFIC lower-bound checks.
> - In is_ap_in_tkip(), add missing lower-bound checks for the RSN and
>   vendor-specific IE data accesses (pre-existing bug).
> - Move rtw_buf_update() before dump_mgntframe() to avoid a potential
>   use-after-free of pwlanhdr, which points into the mgmt frame buffer
>   (pre-existing bug).

When you say "also" that implies you need to break this patch up into
smaller pieces, right?  Please do so.

> 
> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
> Cc: stable@vger.kernel.org
> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
> ---
> Changes in v7:
>  - Address new sashiko comments.
> 

That does not say _what_ you did, only that you did _something_.  Please
be more specific.

thanks,

greg k-h

Hi Greg,
On May 23, 2026 3:44:58 PM GMT+02:00, Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
>On Sat, May 23, 2026 at 03:13:31PM +0200, luka.gejak@linux.dev wrote:
>> From: Luka Gejak <luka.gejak@linux.dev>

...

>> Also fix three additional issues discovered during review:
>> - Missing free of pmgntframe and its xmitbuf before jumping to exit
>>   in the WLAN_EID_VENDOR_SPECIFIC lower-bound checks.
>> - In is_ap_in_tkip(), add missing lower-bound checks for the RSN and
>>   vendor-specific IE data accesses (pre-existing bug).
>> - Move rtw_buf_update() before dump_mgntframe() to avoid a potential
>>   use-after-free of pwlanhdr, which points into the mgmt frame buffer
>>   (pre-existing bug).
>
>When you say "also" that implies you need to break this patch up into
>smaller pieces, right?  Please do so.
>

Well, I just addressed sashiko comments on my patch, so I thought to 
keep it one patch as it was review of if.

>> 
>> Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Luka Gejak <luka.gejak@linux.dev>
>> ---
>> Changes in v7:
>>  - Address new sashiko comments.
>> 
>
>That does not say _what_ you did, only that you did _something_.  Please
>be more specific.

"Also" part is what was changed due to sashiko's request. Should I 
move it here, provide link to sashiko or write it here completely.

>
>thanks,
>
>greg k-h

Best regards,
Luka Gejak