Skip to contentSkip to conversation

wifi: mwifiex: Fix use after free in mwifiex_11n_dispatch_amsdu_pkt()

PATCH In review Dan Carpenter 2 participants 2 messages Jan 8

Subsystems (2)
Kernel janitors Marvell mwifiex wireless driver
lore
Share & tools
lore

Dan Carpenter

author
#0 lore raw

PATCH 1/1 wifi: mwifiex: Fix use after free in mwifiex_11n_dispatch_amsdu_pkt()

The ieee80211_amsdu_to_8023s() function generally frees the skb, so save the skb->len first before it's freed.

Fixes 776f742040ca ("mwifiex: fix AMPDU not setup on TDLS link problem")
Signed-off-by Dan Carpenter <[email protected]>
---
 drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c +2 -1
index 354c5ce66045..90831a1350f5 100644
@@ -25,6 +25,7 @@ static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv,
 
 	if (le16_to_cpu(local_rx_pd->rx_pkt_type) == PKT_TYPE_AMSDU) {
 		struct sk_buff_head list;
+		int skb_len = skb->len;
 		struct sk_buff *rx_skb;
 
 		__skb_queue_head_init(&list);

@@ -44,7 +45,7 @@ static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv,
 			    ntohs(rx_hdr->eth803_hdr.h_proto) == ETH_P_TDLS) {
 				mwifiex_process_tdls_action_frame(priv,
 								  (u8 *)rx_hdr,
-								  skb->len);
+								  skb_len);
 			}
 
 			if (priv->bss_role == MWIFIEX_BSS_ROLE_UAP)
Signature · 1 line
2.51.0
#1 lore raw

drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv, · line 48

Quoted 6 lines · Dan Carpenter
drivers/net/wireless/marvell/mwifiex/11n_rxreorder.c +1 -1
@@ -44,7 +45,7 @@ static int mwifiex_11n_dispatch_amsdu_pkt(struct mwifiex_private *priv,
 			    ntohs(rx_hdr->eth803_hdr.h_proto) == ETH_P_TDLS) {
 				mwifiex_process_tdls_action_frame(priv,
 								  (u8 *)rx_hdr,
-								  skb->len);
+								  skb_len);

I'm 99% sure that should be rx_skb->len at this point, it can't want to handle a single A-MSDU encapsulated action as if it was as long as the entire A-MSDU that held it, that'd likely crash (too), especially if it wasn't the first frame in it.

johannes