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(-)
rtw_get_ie() reports the length byte straight out of the frame and does
not impose a minimum, so a received HT capability element can be any
size from 0 to 255. Five callers only test that the length is non zero
and then cast the element body to a fixed layout:
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);
cap_info is 16 bits, so a element declaring a length of 1 makes the
driver read one byte beyond the element. rtw_update_ht_cap() goes
further and takes ampdu_params_info at offset 2, and
rtw_check_beacon_data() writes back into cap_info.
Mostly this only produces a wrong cap_info, because the element area is
the fixed 768 byte ies[] array and the extra byte is still inside it.
rtw_check_bcn_info() is different: its struct wlan_bssid_ex comes from
kzalloc() and ies[] is the last member, so a beacon that fills the area
to MAX_IE_SZ and ends with a truncated HT capability element reads one
byte past the allocation.
Require the full element before dereferencing it, which is what
mac80211 does in ieee802_11_parse_elems_full():
if (elen >= sizeof(struct ieee80211_ht_cap))
elems->ht_cap_elem = (void *)pos;
The neighbouring HT operation blocks read infos[0] only and are left
alone, a length of 1 is enough for them.
Fixes: 554c0a3abf21 ("staging: Add rtl8723bs sdio wifi driver")
Cc: stable@vger.kernel.org
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
Found by auditing the IE parsers in this driver for length checks that do
not cover the structure being cast onto the element body.
Tested by lifting rtw_get_ie() and the rtw_check_bcn_info() caller into a
userspace harness under ASan, with the element area as an exact 768 byte
allocation so the redzone sits where ies[] ends, and a beacon whose last
element is a HT capability with a declared length of 1:
HT cap IE at offset 765, body ends at 768, area ends at 768
ERROR: AddressSanitizer: unknown-crash
READ of size 2 at 0x7d028e5e037f
#0 in main harness.c:106
0x7d028e5e0380 is located 0 bytes after 768-byte region
With the check changed to len >= sizeof(struct ieee80211_ht_cap) the
element is rejected and ASan is quiet. I do not have RTL8723BS hardware,
so this is the parsing code exercised out of tree rather than a live
driver run; happy to redo it another way if you would rather see that.
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(-)
diff --git a/drivers/staging/rtl8723bs/core/rtw_ap.c b/drivers/staging/rtl8723bs/core/rtw_ap.c
index 065850a9e894..b3c14e62312e 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 >= sizeof(struct ieee80211_ht_cap)) {
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;
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 {
base-commit: 2d2338c93da79b3bfe4b6099a931d9468d539952
--
2.55.0
Please drop this one. Four of the five hunks are right, the first one is
not, and it is a regression rather than a fix.
The rtw_ap.c hunk is not an HT capability site at all:
p = rtw_get_ie(ie + _BEACON_IE_OFFSET_,
WLAN_EID_SSID,
&ie_len,
(pbss_network->ie_length - _BEACON_IE_OFFSET_));
if (p && ie_len >= sizeof(struct ieee80211_ht_cap)) {
memcpy(pbss_network->ssid.ssid, (p + 2), ie_len);
That is the SSID, and I put an HT capability sized lower bound on it.
sizeof(struct ieee80211_ht_cap) is 26, SSIDs are 0 to 32 bytes, so every
SSID shorter than 26 characters now fails the test and the copy and the
ssid_length assignment are skipped. rtw_check_beacon_data() is the AP
setup path, so this breaks bringing up an AP with an ordinary SSID. I
should have checked which element each site fetches instead of assuming
all five were the same.
While looking at it, the check that site actually needs is the opposite
bound. rtw_get_ie() writes the raw IE length byte to *len and only limits
it against the remaining 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 rtw_check_beacon_data() validates nothing but len <= MAX_IE_SZ before
that memcpy. An SSID element longer than 32 bytes overruns ssid[] inside
pmlmepriv->cur_network.network. It comes in through cfg80211 start_ap and
change_beacon, so it needs CAP_NET_ADMIN and a beacon that hostapd would
not normally build, but the bound is missing.
I will send a v2 with only the four HT capability sites, and the SSID
length check separately, since it is a different bug in the other
direction.
Sorry for the noise.
--
Ali
© 2016 - 2026 Red Hat, Inc.