All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] mac80211: monitor mode injection fixes
@ 2020-07-23  8:51 Mathy Vanhoef
  2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
  0 siblings, 1 reply; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23  8:51 UTC (permalink / raw)
  To: vanhoefm, Johannes Berg, linux-wireless, mathy.vanhoef; +Cc: Mathy Vanhoef

This set of patches fixes some bugs related to frame injection, adds
an existing radiotap flag to avoid sequence number overwrites, avoids
an out-of-bounds memory read when injecting frames, and makes the
usage of certain Tx flags more consistent.

Mathy Vanhoef (6):
  mac80211: never drop injected frames even if normally not allowed
  mac80211: add radiotap flag to prevent sequence number overwrite
  mac80211: do not overwrite the sequence number if requested
  mac80211: use same flag everywhere to avoid sequence number overwrite
  mac80211: remove unused flags argument in transmit functions
  mac80211: parse radiotap header when selecting Tx queue

 include/net/ieee80211_radiotap.h |  1 +
 include/net/mac80211.h           | 11 +++++
 net/mac80211/cfg.c               |  2 +-
 net/mac80211/ieee80211_i.h       | 12 ++---
 net/mac80211/iface.c             | 15 ++++--
 net/mac80211/offchannel.c        |  2 +-
 net/mac80211/rx.c                |  2 +-
 net/mac80211/scan.c              |  7 ++-
 net/mac80211/sta_info.c          |  2 +-
 net/mac80211/tx.c                | 81 +++++++++++++++-----------------
 10 files changed, 72 insertions(+), 63 deletions(-)

-- 
2.27.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed
  2020-07-23  8:51 [PATCH 0/6] mac80211: monitor mode injection fixes Mathy Vanhoef
@ 2020-07-23 10:01 ` Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 2/6] mac80211: add radiotap flag to prevent sequence number overwrite Mathy Vanhoef
                     ` (4 more replies)
  0 siblings, 5 replies; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23 10:01 UTC (permalink / raw)
  To: johannes, linux-wireless; +Cc: Mathy Vanhoef

In ieee80211_tx_dequeue there is a check to see if the dequeued frame
is allowed in the current state. Injected frames that are normally
not allowed are being be dropped here. Fix this by checking if a
frame was injected and if so always allowing it.

Signed-off-by: Mathy Vanhoef <Mathy.Vanhoef@kuleuven.be>
---
 net/mac80211/tx.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 1a2941e52..62d01ab26 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3618,7 +3618,7 @@ struct sk_buff *ieee80211_tx_dequeue(struct ieee80211_hw *hw,
 	tx.skb = skb;
 	tx.sdata = vif_to_sdata(info->control.vif);
 
-	if (txq->sta) {
+	if (txq->sta && !(info->flags & IEEE80211_TX_CTL_INJECTED)) {
 		tx.sta = container_of(txq->sta, struct sta_info, sta);
 		/*
 		 * Drop unicast frames to unauthorised stations unless they are
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/6] mac80211: add radiotap flag to prevent sequence number overwrite
  2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
@ 2020-07-23 10:01   ` Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 3/6] mac80211: do not overwrite the sequence number if requested Mathy Vanhoef
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23 10:01 UTC (permalink / raw)
  To: johannes, linux-wireless; +Cc: Mathy Vanhoef

The radiotap specification contains a flag to indicate that the sequence
number of an injected frame should not be overwritten. Parse this flag
and define and set a corresponding Tx control flag.

Signed-off-by: Mathy Vanhoef <Mathy.Vanhoef@kuleuven.be>
---
 include/net/ieee80211_radiotap.h | 1 +
 include/net/mac80211.h           | 3 +++
 net/mac80211/tx.c                | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/include/net/ieee80211_radiotap.h b/include/net/ieee80211_radiotap.h
index 459d355f6..19c00d100 100644
--- a/include/net/ieee80211_radiotap.h
+++ b/include/net/ieee80211_radiotap.h
@@ -117,6 +117,7 @@ enum ieee80211_radiotap_tx_flags {
 	IEEE80211_RADIOTAP_F_TX_CTS = 0x0002,
 	IEEE80211_RADIOTAP_F_TX_RTS = 0x0004,
 	IEEE80211_RADIOTAP_F_TX_NOACK = 0x0008,
+	IEEE80211_RADIOTAP_F_TX_NOSEQNO = 0x0010,
 };
 
 /* for IEEE80211_RADIOTAP_MCS "have" flags */
diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 11d5610d2..6615fe450 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -825,6 +825,8 @@ enum mac80211_tx_info_flags {
  * @IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP: This frame skips mesh path lookup
  * @IEEE80211_TX_CTRL_HW_80211_ENCAP: This frame uses hardware encapsulation
  *	(header conversion)
+ * @IEEE80211_TX_CTRL_NO_SEQNO: Do not overwrite the sequence number that
+ *	has already been assigned to this frame.
  *
  * These flags are used in tx_info->control.flags.
  */
@@ -836,6 +838,7 @@ enum mac80211_tx_control_flags {
 	IEEE80211_TX_CTRL_FAST_XMIT		= BIT(4),
 	IEEE80211_TX_CTRL_SKIP_MPATH_LOOKUP	= BIT(5),
 	IEEE80211_TX_CTRL_HW_80211_ENCAP	= BIT(6),
+	IEEE80211_TX_CTRL_NO_SEQNO		= BIT(7),
 };
 
 /*
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 62d01ab26..040b0ef18 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2084,6 +2084,8 @@ static bool ieee80211_parse_tx_radiotap(struct ieee80211_local *local,
 			txflags = get_unaligned_le16(iterator.this_arg);
 			if (txflags & IEEE80211_RADIOTAP_F_TX_NOACK)
 				info->flags |= IEEE80211_TX_CTL_NO_ACK;
+			if (txflags & IEEE80211_RADIOTAP_F_TX_NOSEQNO)
+				info->control.flags |= IEEE80211_TX_CTRL_NO_SEQNO;
 			break;
 
 		case IEEE80211_RADIOTAP_RATE:
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/6] mac80211: do not overwrite the sequence number if requested
  2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 2/6] mac80211: add radiotap flag to prevent sequence number overwrite Mathy Vanhoef
@ 2020-07-23 10:01   ` Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 4/6] mac80211: use same flag everywhere to avoid sequence number overwrite Mathy Vanhoef
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23 10:01 UTC (permalink / raw)
  To: johannes, linux-wireless; +Cc: Mathy Vanhoef

Check if the Tx control flag is set to prevent sequence number overwrites,
and if so, do not assign a new sequence number to the transmitted frame.

Signed-off-by: Mathy Vanhoef <Mathy.Vanhoef@kuleuven.be>
---
 net/mac80211/tx.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 040b0ef18..cc5068999 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -824,6 +824,9 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *tx)
 	if (ieee80211_is_qos_nullfunc(hdr->frame_control))
 		return TX_CONTINUE;
 
+	if (info->control.flags & IEEE80211_TX_CTRL_NO_SEQNO)
+		return TX_CONTINUE;
+
 	/*
 	 * Anything but QoS data that has a sequence number field
 	 * (is long enough) gets a sequence number from the global
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/6] mac80211: use same flag everywhere to avoid sequence number overwrite
  2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 2/6] mac80211: add radiotap flag to prevent sequence number overwrite Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 3/6] mac80211: do not overwrite the sequence number if requested Mathy Vanhoef
@ 2020-07-23 10:01   ` Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 5/6] mac80211: remove unused flags argument in transmit functions Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 6/6] mac80211: parse radiotap header when selecting Tx queue Mathy Vanhoef
  4 siblings, 0 replies; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23 10:01 UTC (permalink / raw)
  To: johannes, linux-wireless; +Cc: Mathy Vanhoef

Use the IEEE80211_TX_CTRL_NO_SEQNO flag in ieee80211_tx_info to mark
probe requests whose sequence number must not be overwritten. This
provides consistency with the radiotap flag that can be set to indicate
that the sequence number of an injected frame should not be overwritten.

Signed-off-by: Mathy Vanhoef <Mathy.Vanhoef@kuleuven.be>
---
 net/mac80211/ieee80211_i.h | 1 -
 net/mac80211/scan.c        | 7 +++----
 net/mac80211/tx.c          | 2 --
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index ec1a71ac6..8f1a325e0 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -164,7 +164,6 @@ typedef unsigned __bitwise ieee80211_tx_result;
 #define TX_DROP		((__force ieee80211_tx_result) 1u)
 #define TX_QUEUED	((__force ieee80211_tx_result) 2u)
 
-#define IEEE80211_TX_NO_SEQNO		BIT(0)
 #define IEEE80211_TX_UNICAST		BIT(1)
 #define IEEE80211_TX_PS_BUFFERED	BIT(2)
 
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index ad90bbe57..8ccdafa62 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -591,7 +591,6 @@ static void ieee80211_send_scan_probe_req(struct ieee80211_sub_if_data *sdata,
 					  struct ieee80211_channel *channel)
 {
 	struct sk_buff *skb;
-	u32 txdata_flags = 0;
 
 	skb = ieee80211_build_probe_req(sdata, src, dst, ratemask, channel,
 					ssid, ssid_len,
@@ -600,15 +599,15 @@ static void ieee80211_send_scan_probe_req(struct ieee80211_sub_if_data *sdata,
 	if (skb) {
 		if (flags & IEEE80211_PROBE_FLAG_RANDOM_SN) {
 			struct ieee80211_hdr *hdr = (void *)skb->data;
+			struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 			u16 sn = get_random_u32();
 
-			txdata_flags |= IEEE80211_TX_NO_SEQNO;
+			info->control.flags |= IEEE80211_TX_CTRL_NO_SEQNO;
 			hdr->seq_ctrl =
 				cpu_to_le16(IEEE80211_SN_TO_SEQ(sn));
 		}
 		IEEE80211_SKB_CB(skb)->flags |= tx_flags;
-		ieee80211_tx_skb_tid_band(sdata, skb, 7, channel->band,
-					  txdata_flags);
+		ieee80211_tx_skb_tid_band(sdata, skb, 7, channel->band, 0);
 	}
 }
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index cc5068999..279d1edbb 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -835,8 +835,6 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *tx)
 	 */
 	if (!ieee80211_is_data_qos(hdr->frame_control) ||
 	    is_multicast_ether_addr(hdr->addr1)) {
-		if (tx->flags & IEEE80211_TX_NO_SEQNO)
-			return TX_CONTINUE;
 		/* driver should assign sequence number */
 		info->flags |= IEEE80211_TX_CTL_ASSIGN_SEQ;
 		/* for pure STA mode without beacons, we can do it */
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/6] mac80211: remove unused flags argument in transmit functions
  2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
                     ` (2 preceding siblings ...)
  2020-07-23 10:01   ` [PATCH 4/6] mac80211: use same flag everywhere to avoid sequence number overwrite Mathy Vanhoef
@ 2020-07-23 10:01   ` Mathy Vanhoef
  2020-07-23 10:01   ` [PATCH 6/6] mac80211: parse radiotap header when selecting Tx queue Mathy Vanhoef
  4 siblings, 0 replies; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23 10:01 UTC (permalink / raw)
  To: johannes, linux-wireless; +Cc: Mathy Vanhoef

The flags argument in transmit functions is no longer being used
and can be removed.

Signed-off-by: Mathy Vanhoef <Mathy.Vanhoef@kuleuven.be>
---
 net/mac80211/cfg.c         |  2 +-
 net/mac80211/ieee80211_i.h | 11 +++++------
 net/mac80211/offchannel.c  |  2 +-
 net/mac80211/rx.c          |  2 +-
 net/mac80211/scan.c        |  2 +-
 net/mac80211/sta_info.c    |  2 +-
 net/mac80211/tx.c          | 19 ++++++++-----------
 7 files changed, 18 insertions(+), 22 deletions(-)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9b360544a..06f57b8b8 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -3583,7 +3583,7 @@ static int ieee80211_probe_client(struct wiphy *wiphy, struct net_device *dev,
 	}
 
 	local_bh_disable();
-	ieee80211_xmit(sdata, sta, skb, 0);
+	ieee80211_xmit(sdata, sta, skb);
 	local_bh_enable();
 
 	ret = 0;
diff --git a/net/mac80211/ieee80211_i.h b/net/mac80211/ieee80211_i.h
index 8f1a325e0..0e7ef5d5b 100644
--- a/net/mac80211/ieee80211_i.h
+++ b/net/mac80211/ieee80211_i.h
@@ -1966,12 +1966,11 @@ void ieee80211_regulatory_limit_wmm_params(struct ieee80211_sub_if_data *sdata,
 void ieee80211_set_wmm_default(struct ieee80211_sub_if_data *sdata,
 			       bool bss_notify, bool enable_qos);
 void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
-		    struct sta_info *sta, struct sk_buff *skb,
-		    u32 txdata_flags);
+		    struct sta_info *sta, struct sk_buff *skb);
 
 void __ieee80211_tx_skb_tid_band(struct ieee80211_sub_if_data *sdata,
 				 struct sk_buff *skb, int tid,
-				 enum nl80211_band band, u32 txdata_flags);
+				 enum nl80211_band band);
 
 /* sta_out needs to be checked for ERR_PTR() before using */
 int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
@@ -1981,10 +1980,10 @@ int ieee80211_lookup_ra_sta(struct ieee80211_sub_if_data *sdata,
 static inline void
 ieee80211_tx_skb_tid_band(struct ieee80211_sub_if_data *sdata,
 			  struct sk_buff *skb, int tid,
-			  enum nl80211_band band, u32 txdata_flags)
+			  enum nl80211_band band)
 {
 	rcu_read_lock();
-	__ieee80211_tx_skb_tid_band(sdata, skb, tid, band, txdata_flags);
+	__ieee80211_tx_skb_tid_band(sdata, skb, tid, band);
 	rcu_read_unlock();
 }
 
@@ -2002,7 +2001,7 @@ static inline void ieee80211_tx_skb_tid(struct ieee80211_sub_if_data *sdata,
 	}
 
 	__ieee80211_tx_skb_tid_band(sdata, skb, tid,
-				    chanctx_conf->def.chan->band, 0);
+				    chanctx_conf->def.chan->band);
 	rcu_read_unlock();
 }
 
diff --git a/net/mac80211/offchannel.c b/net/mac80211/offchannel.c
index db3b8bf75..882319bdb 100644
--- a/net/mac80211/offchannel.c
+++ b/net/mac80211/offchannel.c
@@ -264,7 +264,7 @@ static void ieee80211_handle_roc_started(struct ieee80211_roc_work *roc,
 	if (roc->mgmt_tx_cookie) {
 		if (!WARN_ON(!roc->frame)) {
 			ieee80211_tx_skb_tid_band(roc->sdata, roc->frame, 7,
-						  roc->chan->band, 0);
+						  roc->chan->band);
 			roc->frame = NULL;
 		}
 	} else {
diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 5c5af4b5f..2c88a69b7 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -3591,7 +3591,7 @@ ieee80211_rx_h_action_return(struct ieee80211_rx_data *rx)
 		}
 
 		__ieee80211_tx_skb_tid_band(rx->sdata, nskb, 7,
-					    status->band, 0);
+					    status->band);
 	}
 	dev_kfree_skb(rx->skb);
 	return RX_QUEUED;
diff --git a/net/mac80211/scan.c b/net/mac80211/scan.c
index 8ccdafa62..8c6b4ff7b 100644
--- a/net/mac80211/scan.c
+++ b/net/mac80211/scan.c
@@ -607,7 +607,7 @@ static void ieee80211_send_scan_probe_req(struct ieee80211_sub_if_data *sdata,
 				cpu_to_le16(IEEE80211_SN_TO_SEQ(sn));
 		}
 		IEEE80211_SKB_CB(skb)->flags |= tx_flags;
-		ieee80211_tx_skb_tid_band(sdata, skb, 7, channel->band, 0);
+		ieee80211_tx_skb_tid_band(sdata, skb, 7, channel->band);
 	}
 }
 
diff --git a/net/mac80211/sta_info.c b/net/mac80211/sta_info.c
index cd8487bc6..6a368f41b 100644
--- a/net/mac80211/sta_info.c
+++ b/net/mac80211/sta_info.c
@@ -1455,7 +1455,7 @@ static void ieee80211_send_null_response(struct sta_info *sta, int tid,
 	}
 
 	info->band = chanctx_conf->def.chan->band;
-	ieee80211_xmit(sdata, sta, skb, 0);
+	ieee80211_xmit(sdata, sta, skb);
 	rcu_read_unlock();
 }
 
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 279d1edbb..e2cb933cd 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1891,7 +1891,7 @@ EXPORT_SYMBOL(ieee80211_tx_prepare_skb);
  */
 static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata,
 			 struct sta_info *sta, struct sk_buff *skb,
-			 bool txpending, u32 txdata_flags)
+			 bool txpending)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_tx_data tx;
@@ -1909,8 +1909,6 @@ static bool ieee80211_tx(struct ieee80211_sub_if_data *sdata,
 	led_len = skb->len;
 	res_prepare = ieee80211_tx_prepare(sdata, &tx, sta, skb);
 
-	tx.flags |= txdata_flags;
-
 	if (unlikely(res_prepare == TX_DROP)) {
 		ieee80211_free_txskb(&local->hw, skb);
 		return true;
@@ -1978,8 +1976,7 @@ static int ieee80211_skb_resize(struct ieee80211_sub_if_data *sdata,
 }
 
 void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
-		    struct sta_info *sta, struct sk_buff *skb,
-		    u32 txdata_flags)
+		    struct sta_info *sta, struct sk_buff *skb)
 {
 	struct ieee80211_local *local = sdata->local;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
@@ -2014,7 +2011,7 @@ void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
 	}
 
 	ieee80211_set_qos_hdr(sdata, skb);
-	ieee80211_tx(sdata, sta, skb, false, txdata_flags);
+	ieee80211_tx(sdata, sta, skb, false);
 }
 
 static bool ieee80211_parse_tx_radiotap(struct ieee80211_local *local,
@@ -2349,7 +2346,7 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb,
 	if (!ieee80211_parse_tx_radiotap(local, skb))
 		goto fail_rcu;
 
-	ieee80211_xmit(sdata, NULL, skb, 0);
+	ieee80211_xmit(sdata, NULL, skb);
 	rcu_read_unlock();
 
 	return NETDEV_TX_OK;
@@ -4011,7 +4008,7 @@ void __ieee80211_subif_start_xmit(struct sk_buff *skb,
 
 		ieee80211_tx_stats(dev, skb->len);
 
-		ieee80211_xmit(sdata, sta, skb, 0);
+		ieee80211_xmit(sdata, sta, skb);
 	}
 	goto out;
  out_free:
@@ -4377,7 +4374,7 @@ static bool ieee80211_tx_pending_skb(struct ieee80211_local *local,
 			return true;
 		}
 		info->band = chanctx_conf->def.chan->band;
-		result = ieee80211_tx(sdata, NULL, skb, true, 0);
+		result = ieee80211_tx(sdata, NULL, skb, true);
 	} else if (info->control.flags & IEEE80211_TX_CTRL_HW_80211_ENCAP) {
 		if (ieee80211_lookup_ra_sta(sdata, skb, &sta)) {
 			dev_kfree_skb(skb);
@@ -5336,7 +5333,7 @@ EXPORT_SYMBOL(ieee80211_unreserve_tid);
 
 void __ieee80211_tx_skb_tid_band(struct ieee80211_sub_if_data *sdata,
 				 struct sk_buff *skb, int tid,
-				 enum nl80211_band band, u32 txdata_flags)
+				 enum nl80211_band band)
 {
 	int ac = ieee80211_ac_from_tid(tid);
 
@@ -5353,7 +5350,7 @@ void __ieee80211_tx_skb_tid_band(struct ieee80211_sub_if_data *sdata,
 	 */
 	local_bh_disable();
 	IEEE80211_SKB_CB(skb)->band = band;
-	ieee80211_xmit(sdata, NULL, skb, txdata_flags);
+	ieee80211_xmit(sdata, NULL, skb);
 	local_bh_enable();
 }
 
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 6/6] mac80211: parse radiotap header when selecting Tx queue
  2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
                     ` (3 preceding siblings ...)
  2020-07-23 10:01   ` [PATCH 5/6] mac80211: remove unused flags argument in transmit functions Mathy Vanhoef
@ 2020-07-23 10:01   ` Mathy Vanhoef
  4 siblings, 0 replies; 7+ messages in thread
From: Mathy Vanhoef @ 2020-07-23 10:01 UTC (permalink / raw)
  To: johannes, linux-wireless; +Cc: Mathy Vanhoef

Already parse the radiotap header in ieee80211_monitor_select_queue.
In a subsequent commit this will allow us to add a radiotap flag that
influences the queue on which injected packets will be sent.

This also fixes the incomplete validation of the injected frame in
ieee80211_monitor_select_queue: currently an out of bounds memory
access may occur in in the called function ieee80211_select_queue_80211
if the 802.11 header is too small.

Note that in ieee80211_monitor_start_xmit the radiotap header is parsed
again, which is necessairy because ieee80211_monitor_select_queue is not
always called beforehand.

Signed-off-by: Mathy Vanhoef <Mathy.Vanhoef@kuleuven.be>
---
 include/net/mac80211.h |  8 +++++++
 net/mac80211/iface.c   | 15 ++++++++----
 net/mac80211/tx.c      | 54 +++++++++++++++++++-----------------------
 3 files changed, 43 insertions(+), 34 deletions(-)

diff --git a/include/net/mac80211.h b/include/net/mac80211.h
index 6615fe450..4e23ad385 100644
--- a/include/net/mac80211.h
+++ b/include/net/mac80211.h
@@ -6238,6 +6238,14 @@ bool ieee80211_tx_prepare_skb(struct ieee80211_hw *hw,
 			      struct ieee80211_vif *vif, struct sk_buff *skb,
 			      int band, struct ieee80211_sta **sta);
 
+/**
+ * Sanity-check and parse the radiotap header of injected frames
+ * @skb: packet injected by userspace
+ * @dev: the &struct device of this 802.11 device
+ */
+bool ieee80211_parse_tx_radiotap(struct sk_buff *skb,
+				 struct net_device *dev);
+
 /**
  * struct ieee80211_noa_data - holds temporary data for tracking P2P NoA state
  *
diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index f900c84fb..132ff7678 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -1183,17 +1183,24 @@ static u16 ieee80211_monitor_select_queue(struct net_device *dev,
 {
 	struct ieee80211_sub_if_data *sdata = IEEE80211_DEV_TO_SUB_IF(dev);
 	struct ieee80211_local *local = sdata->local;
+	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_hdr *hdr;
-	struct ieee80211_radiotap_header *rtap = (void *)skb->data;
+	int len_rthdr;
 
 	if (local->hw.queues < IEEE80211_NUM_ACS)
 		return 0;
 
-	if (skb->len < 4 ||
-	    skb->len < le16_to_cpu(rtap->it_len) + 2 /* frame control */)
+	/* reset flags and info before parsing radiotap header */
+	memset(info, 0, sizeof(*info));
+
+	if (!ieee80211_parse_tx_radiotap(skb, dev))
 		return 0; /* doesn't matter, frame will be dropped */
 
-	hdr = (void *)((u8 *)skb->data + le16_to_cpu(rtap->it_len));
+	len_rthdr = ieee80211_get_radiotap_len(skb->data);
+	hdr = (struct ieee80211_hdr *)(skb->data + len_rthdr);
+	if (skb->len < len_rthdr + 2 ||
+	    skb->len < len_rthdr + ieee80211_hdrlen(hdr->frame_control))
+		return 0; /* doesn't matter, frame will be dropped */
 
 	return ieee80211_select_queue_80211(sdata, skb, hdr);
 }
diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index e2cb933cd..dac73baeb 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -2014,9 +2014,10 @@ void ieee80211_xmit(struct ieee80211_sub_if_data *sdata,
 	ieee80211_tx(sdata, sta, skb, false);
 }
 
-static bool ieee80211_parse_tx_radiotap(struct ieee80211_local *local,
-					struct sk_buff *skb)
+bool ieee80211_parse_tx_radiotap(struct sk_buff *skb,
+				 struct net_device *dev)
 {
+	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_radiotap_iterator iterator;
 	struct ieee80211_radiotap_header *rthdr =
 		(struct ieee80211_radiotap_header *) skb->data;
@@ -2035,6 +2036,18 @@ static bool ieee80211_parse_tx_radiotap(struct ieee80211_local *local,
 	u8 vht_mcs = 0, vht_nss = 0;
 	int i;
 
+	/* check for not even having the fixed radiotap header part */
+	if (unlikely(skb->len < sizeof(struct ieee80211_radiotap_header)))
+		return false; /* too short to be possibly valid */
+
+	/* is it a header version we can trust to find length from? */
+	if (unlikely(rthdr->it_version))
+		return false; /* only version 0 is supported */
+
+	/* does the skb contain enough to deliver on the alleged length? */
+	if (unlikely(skb->len < ieee80211_get_radiotap_len(skb->data)))
+		return false; /* skb too short for claimed rt header extent */
+
 	info->flags |= IEEE80211_TX_INTFL_DONT_ENCRYPT |
 		       IEEE80211_TX_CTL_DONTFRAG;
 
@@ -2188,13 +2201,6 @@ static bool ieee80211_parse_tx_radiotap(struct ieee80211_local *local,
 						     local->hw.max_rate_tries);
 	}
 
-	/*
-	 * remove the radiotap header
-	 * iterator->_max_length was sanity-checked against
-	 * skb->len by iterator init
-	 */
-	skb_pull(skb, iterator._max_length);
-
 	return true;
 }
 
@@ -2203,8 +2209,6 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb,
 {
 	struct ieee80211_local *local = wdev_priv(dev->ieee80211_ptr);
 	struct ieee80211_chanctx_conf *chanctx_conf;
-	struct ieee80211_radiotap_header *prthdr =
-		(struct ieee80211_radiotap_header *)skb->data;
 	struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
 	struct ieee80211_hdr *hdr;
 	struct ieee80211_sub_if_data *tmp_sdata, *sdata;
@@ -2212,21 +2216,17 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb,
 	u16 len_rthdr;
 	int hdrlen;
 
-	/* check for not even having the fixed radiotap header part */
-	if (unlikely(skb->len < sizeof(struct ieee80211_radiotap_header)))
-		goto fail; /* too short to be possibly valid */
+	memset(info, 0, sizeof(*info));
+	info->flags = IEEE80211_TX_CTL_REQ_TX_STATUS |
+		      IEEE80211_TX_CTL_INJECTED;
 
-	/* is it a header version we can trust to find length from? */
-	if (unlikely(prthdr->it_version))
-		goto fail; /* only version 0 is supported */
+	/* Sanity-check and process the injection radiotap header */
+	if (!ieee80211_parse_tx_radiotap(skb, dev))
+		goto fail;
 
-	/* then there must be a radiotap header with a length we can use */
+	/* we now know there is a radiotap header with a length we can use */
 	len_rthdr = ieee80211_get_radiotap_len(skb->data);
 
-	/* does the skb contain enough to deliver on the alleged length? */
-	if (unlikely(skb->len < len_rthdr))
-		goto fail; /* skb too short for claimed rt header extent */
-
 	/*
 	 * fix up the pointers accounting for the radiotap
 	 * header still being in there.  We are being given
@@ -2272,11 +2272,6 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb,
 		skb->priority = *p & IEEE80211_QOS_CTL_TAG1D_MASK;
 	}
 
-	memset(info, 0, sizeof(*info));
-
-	info->flags = IEEE80211_TX_CTL_REQ_TX_STATUS |
-		      IEEE80211_TX_CTL_INJECTED;
-
 	rcu_read_lock();
 
 	/*
@@ -2342,9 +2337,8 @@ netdev_tx_t ieee80211_monitor_start_xmit(struct sk_buff *skb,
 
 	info->band = chandef->chan->band;
 
-	/* process and remove the injection radiotap header */
-	if (!ieee80211_parse_tx_radiotap(local, skb))
-		goto fail_rcu;
+	/* remove the injection radiotap header */
+	skb_pull(skb, len_rthdr);
 
 	ieee80211_xmit(sdata, NULL, skb);
 	rcu_read_unlock();
-- 
2.27.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-07-23 10:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-23  8:51 [PATCH 0/6] mac80211: monitor mode injection fixes Mathy Vanhoef
2020-07-23 10:01 ` [PATCH 1/6] mac80211: never drop injected frames even if normally not allowed Mathy Vanhoef
2020-07-23 10:01   ` [PATCH 2/6] mac80211: add radiotap flag to prevent sequence number overwrite Mathy Vanhoef
2020-07-23 10:01   ` [PATCH 3/6] mac80211: do not overwrite the sequence number if requested Mathy Vanhoef
2020-07-23 10:01   ` [PATCH 4/6] mac80211: use same flag everywhere to avoid sequence number overwrite Mathy Vanhoef
2020-07-23 10:01   ` [PATCH 5/6] mac80211: remove unused flags argument in transmit functions Mathy Vanhoef
2020-07-23 10:01   ` [PATCH 6/6] mac80211: parse radiotap header when selecting Tx queue Mathy Vanhoef

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.