wifi: rtw89: fix OOB read in rtw89_core_cancel_6ghz_probe_tx()

From: Tristan Madani <[email protected]>

rtw89_core_cancel_6ghz_probe_tx() computes a pointer to the IE area with

    ies = mgmt->u.beacon.variable;

and then passes skb->len as the IE data length to cfg80211_find_ie():

    ssid_ie = cfg80211_find_ie(WLAN_EID_SSID, ies, skb->len);

skb->len is the total frame length, not the length of the IE portion. The IEs start at offset 36 (24-byte header + 12-byte fixed fields), so the correct IE length is skb->len minus that offset. Passing the full frame length causes cfg80211_find_ie() to walk up to 36 bytes past the end of the skb data buffer, triggering a slab-out-of-bounds read.

Additionally, the function does not verify that the frame is long enough to contain the fixed beacon/probe-response fields before computing the IE pointer.

Fix by returning early when skb->len is shorter than the variable-IE offset, and pass the correct IE data length to cfg80211_find_ie().

---
 drivers/net/wireless/realtek/rtw89/core.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

index 68dad6090f87e..7b46715ea12db 100644
@@ -2532,9 +2532,11 @@ static void rtw89_core_cancel_6ghz_probe_tx(struct rtw89_dev *rtwdev,
 {
 	struct ieee80211_rx_status *rx_status = IEEE80211_SKB_RXCB(skb);
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
+	size_t hdr_len = offsetof(struct ieee80211_mgmt, u.beacon.variable);
 	struct list_head *pkt_list = rtwdev->scan_info.pkt_list;
 	struct rtw89_pktofld_info *info;
 	const u8 *ies = mgmt->u.beacon.variable, *ssid_ie;
+	size_t ie_len;
 	bool queue_work = false;
 
 	if (rx_status->band != NL80211_BAND_6GHZ)

@@ -2545,7 +2547,12 @@ static void rtw89_core_cancel_6ghz_probe_tx(struct rtw89_dev *rtwdev,
 		return;
 	}
 
-	ssid_ie = cfg80211_find_ie(WLAN_EID_SSID, ies, skb->len);
+	if (skb->len < hdr_len)
+		return;
+
+	ie_len = skb->len - hdr_len;
+
+	ssid_ie = cfg80211_find_ie(WLAN_EID_SSID, ies, ie_len);
 
 	list_for_each_entry(info, &pkt_list[NL80211_BAND_6GHZ], list) {
 		if (ether_addr_equal(info->bssid, mgmt->bssid)) {

2.47.3

From: Tristan Madani <[email protected]>

__rtw89_wow_parse_akm() computes a pointer to the IE area with

    ies = mgmt->u.assoc_req.variable;

and then passes skb->len as the IE data length to cfg80211_find_ie():

    rsn = cfg80211_find_ie(WLAN_EID_RSN, ies, skb->len);

skb->len is the total frame length, not the length of the IE portion. The IEs in an association request start at offset 28 (24-byte header + 4-byte fixed fields), so the correct IE length is skb->len minus that offset. Passing the full frame length causes cfg80211_find_ie() to walk up to 28 bytes past the end of the skb data buffer, triggering a slab-out-of-bounds read.

Fix by validating the minimum frame length and passing the correct IE data length to cfg80211_find_ie().

---
 drivers/net/wireless/realtek/rtw89/wow.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

index 8dadd8df4fc65..82b74ded68631 100644
@@ -16,10 +16,16 @@ void __rtw89_wow_parse_akm(struct rtw89_dev *rtwdev, struct sk_buff *skb)
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)skb->data;
 	struct rtw89_wow_param *rtw_wow = &rtwdev->wow;
+	size_t hdr_len = offsetof(struct ieee80211_mgmt, u.assoc_req.variable);
 	const u8 *rsn, *ies = mgmt->u.assoc_req.variable;
 	struct rtw89_rsn_ie *rsn_ie;
 
-	rsn = cfg80211_find_ie(WLAN_EID_RSN, ies, skb->len);
+	if (skb->len < hdr_len)
+		return;
+
+	rsn = cfg80211_find_ie(WLAN_EID_RSN, ies,
+			       skb->len - hdr_len);
+
 	if (!rsn)
 		return;
 

2.47.3

The length passed to cfg80211_find_ie() is indeed wrong and the fix looks correct to me. I did try to reproduce the reported slab-out-of-bounds though, and I don't think it is reachable. If I'm wrong about this, I'd be glad to be corrected.

cfg80211_find_ie() walks the elements via for_each_element(), which is bounded by _data + _datalen and never dereferences past it. With ies = skb->data + 28 and len = skb->len, the highest byte the walk can touch is skb->data + 28 + skb->len - 1, i.e. exactly 28 bytes past skb_tail_pointer(), as you describe.

What follows skb_tail_pointer(), however, is the skb tailroom and then struct skb_shared_info, and both live inside the same allocation. sizeof(struct skb_shared_info) is 320 bytes on x86_64, so even with zero tailroom the 28-byte overread stays within the object and KASAN has nothing to report.

Measured on RTL8851BE (rtw89_8851be) with a kprobe on __rtw89_wow_parse_akm(), on a real association request:

    skb->len   tailroom   overread   slack to end of allocation
    160        1552       28         1872

So the defect is a read of uninitialised memory inside the skb rather than an out-of-bounds access -- KMSAN territory, not KASAN. The practical consequence is that on a network with no RSN IE (an open BSS, where mac80211 emits no RSN element at all), the walk continues into the tailroom and may match a bogus element with id 48, after which rtw_wow->akm is set from garbage.

That is still worth fixing, but it may be worth rewording the commit message, since the "slab-out-of-bounds" wording is what justifies the
Cc: stable here. The same reasoning applies to patch 1/2.

One more thing in the same function, which this patch does not address:

	rsn_ie = (struct rtw89_rsn_ie *)rsn;
	rtw_wow->akm = rsn_ie->akm_cipher_suite.type;

struct rtw89_rsn_ie is 20 bytes and akm_cipher_suite.type sits at offset 19, but cfg80211_find_ie() only validates the element header. A minimal RSN element with datalen = 2 (version only) makes that read land 15 bytes past the end of the element, and with the length fix applied it can still reach past the end of the frame when the element sits at the tail. Checking rsn[1] before the cast would close that too.

Thanks,
Maxim