[PATCH] staging: rtl8723bs: validate HT capability IE length before use

Ali Ahmet Memis <[email protected]>
Newsgroups org.kernel.vger.stable,dev.linux.lists.linux-staging,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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: [email protected]
Signed-off-by: Ali Ahmet Memis <[email protected]>
---
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.