On Sun, Jan 13, 2019 at 11:50:35PM +0200, Lauri Tirkkonen wrote:
> Hi, (disclaimer: I know basically nothing about 802.11)
>
> I noticed on my AP a high counter on netstat -W "input unencrypted
> packets with wep/wpa config discarded", aka is_rx_unencrypted. After
> investigation it looked like all of these were frames with type Data,
> but with the "No data" bit set in FC0. Per IEEE's 80211-2016.pdf
> 9.2.4.1.9 (page 644) the Protected bit is set to 0 for these frames, so
> don't insist on them being encrypted. (See also 9.2.4.1.3, p. 640, about
> bit 6 (ie. FC0_SUBTYPE_NODATA) implying no Frame Body).
Thank you for tracking this problem down.
Your diff is not correct. This part introduces a memory leak because
the mbuf is not going to be freed anymore:
> @@ -411,6 +412,12 @@ ieee80211_input(struct ifnet *ifp, struct mbuf *m,
> struct ieee80211_node *ni,
> /* protection is on for Rx */
> if (!(rxi->rxi_flags & IEEE80211_RXI_HWDEC)) {
> if (!(wh->i_fc[1] & IEEE80211_FC1_PROTECTED)) {
> + /*
> + * 9.2.4.1.9 frames without data are
> + * not protected
> + */
> + if (!hasdata)
> + return;
This should say 'goto out' instead of 'return'.
I'd like to propose a more general solution:
The diff below improves naming of so far unused frame subtype constants
and makes it more obvious which subtypes do not carry data, it attributes
"no data" frames to a more suitable stat counter, and it drops them early.
Index: ieee80211.h
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211.h,v
retrieving revision 1.60
diff -u -p -r1.60 ieee80211.h
--- ieee80211.h 2 Jul 2017 14:48:19 -0000 1.60
+++ ieee80211.h 14 Jan 2019 13:05:08 -0000
@@ -140,13 +140,13 @@ struct ieee80211_htframe_addr4 { /* 11n
#define IEEE80211_FC0_SUBTYPE_CF_END_ACK 0xf0
/* for TYPE_DATA (bit combination) */
#define IEEE80211_FC0_SUBTYPE_DATA 0x00
-#define IEEE80211_FC0_SUBTYPE_CF_ACK 0x10
-#define IEEE80211_FC0_SUBTYPE_CF_POLL 0x20
-#define IEEE80211_FC0_SUBTYPE_CF_ACPL 0x30
+#define IEEE80211_FC0_SUBTYPE_DATA_CF_ACK 0x10
+#define IEEE80211_FC0_SUBTYPE_DATA_CF_POLL 0x20
+#define IEEE80211_FC0_SUBTYPE_CF_ACK_POLL 0x30
#define IEEE80211_FC0_SUBTYPE_NODATA 0x40
-#define IEEE80211_FC0_SUBTYPE_CFACK 0x50
-#define IEEE80211_FC0_SUBTYPE_CFPOLL 0x60
-#define IEEE80211_FC0_SUBTYPE_CF_ACK_CF_ACK 0x70
+#define IEEE80211_FC0_SUBTYPE_CF_ACK_NODATA 0x50
+#define IEEE80211_FC0_SUBTYPE_CF_POLL_NODATA 0x60
+#define IEEE80211_FC0_SUBTYPE_CF_ACK_CF_POLL 0x70
#define IEEE80211_FC0_SUBTYPE_QOS 0x80
#define IEEE80211_FC1_DIR_MASK 0x03
Index: ieee80211_input.c
===================================================================
RCS file: /cvs/src/sys/net80211/ieee80211_input.c,v
retrieving revision 1.202
diff -u -p -r1.202 ieee80211_input.c
--- ieee80211_input.c 7 Aug 2018 18:13:14 -0000 1.202
+++ ieee80211_input.c 14 Jan 2019 13:19:30 -0000
@@ -202,6 +202,19 @@ ieee80211_input(struct ifnet *ifp, struc
goto err;
}
}
+
+ /*
+ * "no data" frames are used for various MAC coordination functions,
+ * particularly in the context of QoS. We do not implement related
+ * features yet so do not process "no data" frames any further.
+ */
+ if (subtype & (IEEE80211_FC0_SUBTYPE_NODATA |
+ IEEE80211_FC0_SUBTYPE_CF_POLL_NODATA |
+ IEEE80211_FC0_SUBTYPE_CF_ACK_NODATA)) {
+ ic->ic_stats.is_rx_badsubtype++;
+ goto out;
+ }
+
if ((hasqos = ieee80211_has_qos(wh))) {
qos = ieee80211_get_qos(wh);
tid = qos & IEEE80211_QOS_TID;
@@ -211,7 +224,6 @@ ieee80211_input(struct ifnet *ifp, struc
}
if (type == IEEE80211_FC0_TYPE_DATA && hasqos &&
- (subtype & IEEE80211_FC0_SUBTYPE_NODATA) == 0 &&
!(rxi->rxi_flags & IEEE80211_RXI_AMPDU_DONE)) {
int ba_state = ni->ni_rx_ba[tid].ba_state;
