[v3] mac80211: fix control port tx status check

Commit Message

Markus Theil June 22, 2020, 12:35 p.m. UTC

The initial control port tx status patch assumed, that
we have IEEE 802.11 frames, but actually ethernet frames
are stored in the ack skb. Fix this by checking for the
correct ethertype and skb protocol 802.3.

Also allow tx status reports for ETH_P_PREAUTH, as preauth
frames can also be send over the nl80211 control port.

Fixes: a7528198add8 ("mac80211: support control port TX status reporting")
Reported-by: Jouni Malinen <[e-mail]>
Signed-off-by: Markus Theil <[e-mail]>
---
v3: also check for ETH_P_PREAUTH
v2: use __be16, as suggested by Johannes Berg
 net/mac80211/status.c | 22 +++++++++++++++-------
 1 file changed, 15 insertions(+), 7 deletions(-)

--
2.27.0

Comments

kernel test robot June 22, 2020, 3:37 p.m. UTC | #1

Hi Markus,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on mac80211/master v5.8-rc2 next-20200622]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use  as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Markus-Theil/mac80211-fix-control-port-tx-status-check/20200622-203641
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
config: c6x-randconfig-s032-20200622 (attached as .config)
compiler: c6x-elf-gcc (GCC) 9.3.0
reproduce:
        # apt-get install sparse
        # sparse version: v0.6.2-dirty
        # save the attached .config to linux build tree
        make W=1 C=1 ARCH=c6x CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__'

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[e-mail]>


sparse warnings: (new ones prefixed by >>)

>> net/mac80211/status.c:642:36: sparse: sparse: incorrect type in initializer (different base types) @@     expected restricted __be16 [usertype] ethertype @@     got int @@
>> net/mac80211/status.c:642:36: sparse:     expected restricted __be16 [usertype] ethertype
>> net/mac80211/status.c:642:36: sparse:     got int

vim +642 net/mac80211/status.c

   623	
   624	static void ieee80211_report_ack_skb(struct ieee80211_local *local,
   625					     struct ieee80211_tx_info *info,
   626					     bool acked, bool dropped)
   627	{
   628		struct sk_buff *skb;
   629		unsigned long flags;
   630	
   631		spin_lock_irqsave(&local->ack_status_lock, flags);
   632		skb = idr_remove(&local->ack_status_frames, info->ack_frame_id);
   633		spin_unlock_irqrestore(&local->ack_status_lock, flags);
   634	
   635		if (!skb)
   636			return;
   637	
   638		if (info->flags & IEEE80211_TX_INTFL_NL80211_FRAME_TX) {
   639			u64 cookie = IEEE80211_SKB_CB(skb)->ack.cookie;
   640			struct ieee80211_sub_if_data *sdata;
   641			struct ieee80211_hdr *hdr = (void *)skb->data;
 > 642			__be16 ethertype = 0xffff;
   643	
   644			if (skb->len >= ETH_HLEN && skb->protocol == cpu_to_be16(ETH_P_802_3))
   645				skb_copy_bits(skb, 2 * ETH_ALEN, &ethertype, ETH_TLEN);
   646	
   647			rcu_read_lock();
   648			sdata = ieee80211_sdata_from_skb(local, skb);
   649			if (sdata) {
   650				if (ethertype == sdata->control_port_protocol ||
   651				    ethertype == cpu_to_be16(ETH_P_PREAUTH))
   652					cfg80211_control_port_tx_status(&sdata->wdev,
   653									cookie,
   654									skb->data,
   655									skb->len,
   656									acked,
   657									GFP_ATOMIC);
   658				else if (ieee80211_is_any_nullfunc(hdr->frame_control))
   659					cfg80211_probe_status(sdata->dev, hdr->addr1,
   660							      cookie, acked,
   661							      info->status.ack_signal,
   662							      info->status.is_valid_ack_signal,
   663							      GFP_ATOMIC);
   664				else if (ieee80211_is_mgmt(hdr->frame_control))
   665					cfg80211_mgmt_tx_status(&sdata->wdev, cookie,
   666								skb->data, skb->len,
   667								acked, GFP_ATOMIC);
   668				else
   669					pr_warn("Unknown status report in ack skb\n");
   670	
   671			}
   672			rcu_read_unlock();
   673	
   674			dev_kfree_skb_any(skb);
   675		} else if (dropped) {
   676			dev_kfree_skb_any(skb);
   677		} else {
   678			/* consumes skb */
   679			skb_complete_wifi_ack(skb, acked);
   680		}
   681	}
   682	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[e-mail]

Patch

diff --git a/net/mac80211/status.c b/net/mac80211/status.c
index 7b1bacac39c6..7d1bc3ca389a 100644
--- a/net/mac80211/status.c
+++ b/net/mac80211/status.c
@@ -639,11 +639,23 @@  static void ieee80211_report_ack_skb(struct ieee80211_local *local,
 		u64 cookie = IEEE80211_SKB_CB(skb)->ack.cookie;
 		struct ieee80211_sub_if_data *sdata;
 		struct ieee80211_hdr *hdr = (void *)skb->data;
+		__be16 ethertype = 0xffff;
+
+		if (skb->len >= ETH_HLEN && skb->protocol == cpu_to_be16(ETH_P_802_3))
+			skb_copy_bits(skb, 2 * ETH_ALEN, &ethertype, ETH_TLEN);

 		rcu_read_lock();
 		sdata = ieee80211_sdata_from_skb(local, skb);
 		if (sdata) {
-			if (ieee80211_is_any_nullfunc(hdr->frame_control))
+			if (ethertype == sdata->control_port_protocol ||
+			    ethertype == cpu_to_be16(ETH_P_PREAUTH))
+				cfg80211_control_port_tx_status(&sdata->wdev,
+								cookie,
+								skb->data,
+								skb->len,
+								acked,
+								GFP_ATOMIC);
+			else if (ieee80211_is_any_nullfunc(hdr->frame_control))
 				cfg80211_probe_status(sdata->dev, hdr->addr1,
 						      cookie, acked,
 						      info->status.ack_signal,
@@ -654,12 +666,8 @@  static void ieee80211_report_ack_skb(struct ieee80211_local *local,
 							skb->data, skb->len,
 							acked, GFP_ATOMIC);
 			else
-				cfg80211_control_port_tx_status(&sdata->wdev,
-								cookie,
-								skb->data,
-								skb->len,
-								acked,
-								GFP_ATOMIC);
+				pr_warn("Unknown status report in ack skb\n");
+
 		}
 		rcu_read_unlock();