This patch was committed last night but contains a bug: I forgot to adjust
error paths to avoid a double-free. Please check the follow-up fix below. Ok?
(This double-free wouldn't trigger under normal conditions even in -current,
unless we receive crafted packets that would also need to pass decryption
if we're not on an open network)
On Wed, Dec 09, 2020 at 03:34:05PM +0100, Stefan Sperling wrote:
> With A-MSDUs enabled in -current people are seeing a lot of
>
> input packet decapsulations failed
>
> events in netstat -W iwm0.
>
> This fixes the issue on iwm 8265 for me. There are between 6 and 8 bytes of
> trailing data in the A-MSDU frame buffer. This results in a decap failure
> being counted when m_pullup() fails to fetch ETHER_HDR_LEN + LLC_SNAPFRAMELEN
> bytes.
>
> I suppose such bytes are padding left by the hardware, most likely due to
> decryption. It should be safe to ignore these. A TCP stream that uses
> A-MSDUs looks clean, so we're not losing any packets. This problem is
> just a cosmetic one.
>
> ok?
> iwm(4) 9260+ and iwx(4) have another issue because these devices actually
> decapsulate A-MSDUs in hardware and our code doesn't handle that yet.
> I will send a separate patch for this soon.
Regarding the above paragraph, I have changed my mind and am shelving
further work on A-MSDU support for now due to lack of time. I will try
to pick this up again later. I would have time to review patches, though,
if anyone else wants to continue this work.
Double-free fix:
diff 22c79bfc5a343ef1dfe575157d8cd8a0dc743910 /usr/src
blob - 924608b6724ca4d3939200dd9f1bc19024870f42
file + sys/net80211/ieee80211_input.c
--- sys/net80211/ieee80211_input.c
+++ sys/net80211/ieee80211_input.c
@@ -1158,7 +1158,7 @@ ieee80211_amsdu_decap(struct ieee80211com *ic, struct
m = m_pullup(m, ETHER_HDR_LEN + LLC_SNAPFRAMELEN);
if (m == NULL) {
ic->ic_stats.is_rx_decap++;
- break;
+ return;
}
eh = mtod(m, struct ether_header *);
/* examine 802.3 header */
@@ -1168,7 +1168,7 @@ ieee80211_amsdu_decap(struct ieee80211com *ic, struct
/* stop processing A-MSDU subframes */
ic->ic_stats.is_rx_decap++;
m_freem(m);
- break;
+ return;
}
llc = (struct llc *)&eh[1];
/* examine 802.2 LLC header */
@@ -1192,7 +1192,7 @@ ieee80211_amsdu_decap(struct ieee80211com *ic, struct
DPRINTF(("A-MSDU subframe too long (%d)\n", len));
ic->ic_stats.is_rx_decap++;
m_freem(m);
- break;
+ return;
}
/* "detach" our A-MSDU subframe from the others */
@@ -1201,7 +1201,7 @@ ieee80211_amsdu_decap(struct ieee80211com *ic, struct
/* stop processing A-MSDU subframes */
ic->ic_stats.is_rx_decap++;
m_freem(m);
- break;
+ return;
}
ieee80211_enqueue_data(ic, m, ni, mcast, ml);
