[PATCH rtw-next 0/3] wifi: rtl8xxxu/rtlwifi: validate action frame size

[PATCH rtw-next 0/3] wifi: rtl8xxxu/rtlwifi: validate action frame size

[Ping-Ke Shih]

As mentioned by Johannes [1], some where of rtl8xxxu and rtlwifi check
fields of action frame without checking size. Refine the code to check
size by IEEE80211_MIN_ACTION_SIZE() before using.

[1] https://lore.kernel.org/linux-wireless/d4e7f2ec4a7def5820eb41e84ed0815e159cf39b.camel@sipsolutions.net/

Ping-Ke Shih (3):
  wifi: rtl8xxxu: validate action frame size before using in
    rtl8xxxu_dump_action()
  wifi: rtlwifi: validate action frame size in rtl_action_proc()
  wifi: rtlwifi: validate action frame size before using in
    _rtl_pci_tx_isr()

 drivers/net/wireless/realtek/rtl8xxxu/core.c | 13 +++++++++++--
 drivers/net/wireless/realtek/rtlwifi/base.c  | 17 ++++++++++-------
 drivers/net/wireless/realtek/rtlwifi/pci.c   |  7 ++++---
 3 files changed, 25 insertions(+), 12 deletions(-)


base-commit: fa489a77e3267e05df95db96ba98e141ec07cbd9
-- 
2.25.1

[PATCH rtw-next 1/3] wifi: rtl8xxxu: validate action frame size before using in rtl8xxxu_dump_action()

[Ping-Ke Shih]

The rtl8xxxu_dump_action() is to print action frames when turning on
debug mask. Validate the skb->len size to prevent potential broken in
monitor mode injection.

Compile tested only.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtl8xxxu/core.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtl8xxxu/core.c b/drivers/net/wireless/realtek/rtl8xxxu/core.c
index f20fade0c099..508137e4a87a 100644
--- a/drivers/net/wireless/realtek/rtl8xxxu/core.c
+++ b/drivers/net/wireless/realtek/rtl8xxxu/core.c
@@ -5126,7 +5126,7 @@ static void rtl8xxxu_tx_complete(struct urb *urb)
 }
 
 static void rtl8xxxu_dump_action(struct device *dev,
-				 struct ieee80211_hdr *hdr)
+				 struct ieee80211_hdr *hdr, unsigned int skb_len)
 {
 	struct ieee80211_mgmt *mgmt = (struct ieee80211_mgmt *)hdr;
 	u16 cap, timeout;
@@ -5134,8 +5134,14 @@ static void rtl8xxxu_dump_action(struct device *dev,
 	if (!(rtl8xxxu_debug & RTL8XXXU_DEBUG_ACTION))
 		return;
 
+	if (skb_len < IEEE80211_MIN_ACTION_SIZE(action_code))
+		return;
+
 	switch (mgmt->u.action.action_code) {
 	case WLAN_ACTION_ADDBA_RESP:
+		if (skb_len < IEEE80211_MIN_ACTION_SIZE(addba_resp))
+			break;
+
 		cap = le16_to_cpu(mgmt->u.action.addba_resp.capab);
 		timeout = le16_to_cpu(mgmt->u.action.addba_resp.timeout);
 		dev_info(dev, "WLAN_ACTION_ADDBA_RESP: "
@@ -5148,6 +5154,9 @@ static void rtl8xxxu_dump_action(struct device *dev,
 			 le16_to_cpu(mgmt->u.action.addba_resp.status));
 		break;
 	case WLAN_ACTION_ADDBA_REQ:
+		if (skb_len < IEEE80211_MIN_ACTION_SIZE(addba_req))
+			break;
+
 		cap = le16_to_cpu(mgmt->u.action.addba_req.capab);
 		timeout = le16_to_cpu(mgmt->u.action.addba_req.timeout);
 		dev_info(dev, "WLAN_ACTION_ADDBA_REQ: "
@@ -5437,7 +5446,7 @@ static void rtl8xxxu_tx(struct ieee80211_hw *hw,
 	}
 
 	if (ieee80211_is_action(hdr->frame_control))
-		rtl8xxxu_dump_action(dev, hdr);
+		rtl8xxxu_dump_action(dev, hdr, skb->len);
 
 	tx_info->rate_driver_data[0] = hw;
 
-- 
2.25.1

[PATCH rtw-next 2/3] wifi: rtlwifi: validate action frame size in rtl_action_proc()

[Ping-Ke Shih]

Since action frames might be malformed from RX or injected TX, validate
the size before using.

More, use struct ieee80211_mgmt to access fields of action frames instead
of counting barely.

Tested with ping on RTL8723BE. The add BA request action frames of TX/RX
can be handled properly.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtlwifi/base.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/base.c b/drivers/net/wireless/realtek/rtlwifi/base.c
index aad377864e73..9e98c01bb90e 100644
--- a/drivers/net/wireless/realtek/rtlwifi/base.c
+++ b/drivers/net/wireless/realtek/rtlwifi/base.c
@@ -1369,18 +1369,19 @@ bool rtl_action_proc(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
 	struct rtl_mac *mac = rtl_mac(rtl_priv(hw));
 	struct ieee80211_hdr *hdr = rtl_get_hdr(skb);
 	struct rtl_priv *rtlpriv = rtl_priv(hw);
+	struct ieee80211_mgmt *mgmt;
 	__le16 fc = rtl_get_fc(skb);
-	u8 *act = (u8 *)(((u8 *)skb->data + MAC80211_3ADDR_LEN));
-	u8 category;
 
 	if (!ieee80211_is_action(fc))
 		return true;
 
-	category = *act;
-	act++;
-	switch (category) {
+	mgmt = (void *)skb->data;
+	if (skb->len < IEEE80211_MIN_ACTION_SIZE(action_code))
+		return true;
+
+	switch (mgmt->u.action.category) {
 	case ACT_CAT_BA:
-		switch (*act) {
+		switch (mgmt->u.action.action_code) {
 		case ACT_ADDBAREQ:
 			if (mac->act_scanning)
 				return false;
@@ -1394,9 +1395,11 @@ bool rtl_action_proc(struct ieee80211_hw *hw, struct sk_buff *skb, u8 is_tx)
 				struct ieee80211_sta *sta = NULL;
 				struct rtl_sta_info *sta_entry = NULL;
 				struct rtl_tid_data *tid_data;
-				struct ieee80211_mgmt *mgmt = (void *)skb->data;
 				u16 capab = 0, tid = 0;
 
+				if (skb->len < IEEE80211_MIN_ACTION_SIZE(addba_req))
+					return true;
+
 				rcu_read_lock();
 				sta = rtl_find_sta(hw, hdr->addr3);
 				if (sta == NULL) {
-- 
2.25.1

[PATCH rtw-next 3/3] wifi: rtlwifi: validate action frame size before using in _rtl_pci_tx_isr()

[Ping-Ke Shih]

Since TX action frames might be malformed, validate the size before using.

Tested on RTL8723BE.

Signed-off-by: Ping-Ke Shih <pkshih@realtek.com>
---
 drivers/net/wireless/realtek/rtlwifi/pci.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/net/wireless/realtek/rtlwifi/pci.c b/drivers/net/wireless/realtek/rtlwifi/pci.c
index 9cc0a871ea3c..73018a0498b4 100644
--- a/drivers/net/wireless/realtek/rtlwifi/pci.c
+++ b/drivers/net/wireless/realtek/rtlwifi/pci.c
@@ -505,9 +505,10 @@ static void _rtl_pci_tx_isr(struct ieee80211_hw *hw, int prio)
 			}
 		}
 		if (ieee80211_is_action(fc)) {
-			struct ieee80211_mgmt *action_frame =
-				(struct ieee80211_mgmt *)skb->data;
-			if (action_frame->u.action.action_code ==
+			struct ieee80211_mgmt *action_frame = (void *)skb->data;
+
+			if (skb->len >= IEEE80211_MIN_ACTION_SIZE(action_code) &&
+			    action_frame->u.action.action_code ==
 			    WLAN_HT_ACTION_SMPS) {
 				dev_kfree_skb(skb);
 				goto tx_status_ok;
-- 
2.25.1