2011-08-16 21:31:03

by Bill Jordan

[permalink] [raw]
Subject: [BUG] ath9k truncated management packets from TKIP connected stations

I'm not quite sure what the correct fix is for this.

Ath9k in AP mode with a TKIP security: If a connected station sends a
management packet, the packet is truncated by 8 bytes before being
delivered to hostapd. This prevents the station from reauthenticating
or connecting to a different SSID on the same radio.

In ath9k_rx_accept, for management packets, strip_mic will be true,
and RX_FLAG_MMIC_STRIPPED will be set in rxs->flag. In
ath9k_rx_skb_postprocess, if ah->sw_mgmt_crypto is set,
RX_FLAG_DECRYPTED will be cleared. However, RX_FLAG_MMIC_STRIPPED will
still be set, so, in ath_rx_tasklet, 8 bytes will be trimmed off the
end of the skb.

I'm thinking that in ath9k_rx_accept, is_valid_tkip ?should also
consider ieee80211_is_mgmt(fc). But this wouldn't take into
consideration ah->sw_mgmt_crypto.

Alternatively, RX_FLAG_MMIC_STRIPPED could be cleared in
ath9k_rx_skb_postprocess when RX_FLAG_DECRYPTED is cleared.

I'm looking for input from someone who understands this code better.

Thanks,
Bill Jordan


2011-08-16 22:52:33

by Felix Fietkau

[permalink] [raw]
Subject: Re: [BUG] ath9k truncated management packets from TKIP connected stations

On 2011-08-16 2:31 PM, Bill Jordan wrote:
> I'm not quite sure what the correct fix is for this.
>
> Ath9k in AP mode with a TKIP security: If a connected station sends a
> management packet, the packet is truncated by 8 bytes before being
> delivered to hostapd. This prevents the station from reauthenticating
> or connecting to a different SSID on the same radio.
>
> In ath9k_rx_accept, for management packets, strip_mic will be true,
> and RX_FLAG_MMIC_STRIPPED will be set in rxs->flag. In
> ath9k_rx_skb_postprocess, if ah->sw_mgmt_crypto is set,
> RX_FLAG_DECRYPTED will be cleared. However, RX_FLAG_MMIC_STRIPPED will
> still be set, so, in ath_rx_tasklet, 8 bytes will be trimmed off the
> end of the skb.
>
> I'm thinking that in ath9k_rx_accept, is_valid_tkip should also
> consider ieee80211_is_mgmt(fc). But this wouldn't take into
> consideration ah->sw_mgmt_crypto.
>
> Alternatively, RX_FLAG_MMIC_STRIPPED could be cleared in
> ath9k_rx_skb_postprocess when RX_FLAG_DECRYPTED is cleared.
>
> I'm looking for input from someone who understands this code better.
We should probably just keep strip_mic set to false for mgmt frames.

- Felix

2011-08-17 20:06:50

by Bill Jordan

[permalink] [raw]
Subject: [PATCH] ath9k: fix MGMT packets when using TKIP

Prevent 8 bytes from being truncated from MGMT packets
when using TKIP.

Signed-off-by: Bill Jordan <[email protected]>
---
drivers/net/wireless/ath/ath9k/recv.c | 3 ++-
1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 7409402..d8737f2 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -824,7 +824,8 @@ static bool ath9k_rx_accept(struct ath_common *common,
is_mc = !!is_multicast_ether_addr(hdr->addr1);
is_valid_tkip = rx_stats->rs_keyix != ATH9K_RXKEYIX_INVALID &&
test_bit(rx_stats->rs_keyix, common->tkip_keymap);
- strip_mic = is_valid_tkip && !(rx_stats->rs_status &
+ strip_mic = is_valid_tkip && !ieee80211_is_mgmt(fc) &&
+ !(rx_stats->rs_status &
(ATH9K_RXERR_DECRYPT | ATH9K_RXERR_CRC | ATH9K_RXERR_MIC));

if (!rx_stats->rs_datalen)
--
1.7.6