mac80211: fix paged RX crypto

PATCH In review Johannes Berg 2 participants 6 messages Apr 7, 2010 – Apr 8, 2010 mac80211

lore

Share & tools

lore

Johannes Berg

author maintainer

2010-04-07 09:26 UTC #0 lore raw

PATCH 1/1 mac80211: fix paged RX crypto

From: Johannes Berg <[email protected]>

WEP crypto was broken, but upon finding the problem it is evident that other things were broken by the paged RX patch as well.

To fix it, for now move the linearising in front. This means that we linearise all frames, which is not at all what we want, but at least it fixes the problem for now.

Signed-off-by Johannes Berg <[email protected]>

Acked-by Zhu Yi <[email protected]>

---
 net/mac80211/rx.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

net/mac80211/rx.c +6 -4

@@ -820,7 +820,7 @@ ieee80211_rx_h_decrypt(struct ieee80211_
 {
 	struct sk_buff *skb = rx->skb;
 	struct ieee80211_rx_status *status = IEEE80211_SKB_RXCB(skb);
-	struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)skb->data;
+	struct ieee80211_hdr *hdr;
 	int keyidx;
 	int hdrlen;
 	ieee80211_rx_result result = RX_DROP_UNUSABLE;

@@ -861,6 +861,11 @@ ieee80211_rx_h_decrypt(struct ieee80211_
 	if (!(rx->flags & IEEE80211_RX_RA_MATCH))
 		return RX_CONTINUE;
 
+	if (skb_linearize(rx->skb))
+		return RX_DROP_UNUSABLE;
+
+	hdr = (struct ieee80211_hdr *)skb->data;
+
 	/* start without a key */
 	rx->key = NULL;

@@ -944,9 +949,6 @@ ieee80211_rx_h_decrypt(struct ieee80211_
 		return RX_DROP_MONITOR;
 	}
 
-	if (skb_linearize(rx->skb))
-		return RX_DROP_UNUSABLE;
-
 	/* Check for weak IVs if possible */
 	if (rx->sta && rx->key->conf.alg == ALG_WEP &&
 	    ieee80211_is_data(hdr->frame_control) &&

Zhu Yi

2010-04-08 03:26 UTC #1 lore raw

Quoted 6 lines · Johannes Berg

From: Johannes Berg <[email protected]>
WEP crypto was broken, but upon finding the problem it is evident that other things were broken by the paged RX patch as well.

Quoted 7 lines

To fix it, for now move the linearising in front. This means that we linearise all frames, which is not at all what we want, but at least it fixes the problem for now.
Signed-off-by Johannes Berg <[email protected]>
Acked-by Zhu Yi <[email protected]>

I thought it over. We don't need to handle nonlinear skb in ieee80211_get_mmie_keyidx(), because we only need to touch fields out of 802.11 header for management frames, and we have already skb_linearize all management frames before. Now we just need to handle WEP IV correctly. How about this patch?

Signed-off-by Zhu Yi <[email protected]>

net/mac80211/rx.c +6 -4

index 14366d4..a8f11f6 100644
@@ -894,6 +894,7 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 			rx->key = key;
 		return RX_CONTINUE;
 	} else {
+		u8 keyid;
 		/*
 		 * The device doesn't give us the IV so we won't be
 		 * able to look up the key. That's ok though, we

@@ -916,7 +917,8 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 		 * no need to call ieee80211_wep_get_keyidx,
 		 * it verifies a bunch of things we've done already
 		 */
-		keyidx = rx->skb->data[hdrlen + 3] >> 6;
+		skb_copy_bits(rx->skb, hdrlen + 3, &keyid, 1);
+		keyidx = keyid >> 6;
 
 		rx->key = rcu_dereference(rx->sdata->keys[keyidx]);

@@ -937,9 +939,6 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 		return RX_DROP_MONITOR;
 	}
 
-	if (skb_linearize(rx->skb))
-		return RX_DROP_UNUSABLE;
-
 	/* Check for weak IVs if possible */
 	if (rx->sta && rx->key->conf.alg == ALG_WEP &&
 	    ieee80211_is_data(hdr->frame_control) &&

@@ -948,6 +947,9 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 	    ieee80211_wep_is_weak_iv(rx->skb, rx->key))
 		rx->sta->wep_weak_iv_count++;
 
+	if (skb_linearize(rx->skb))
+		return RX_DROP_UNUSABLE;
+
 	switch (rx->key->conf.alg) {
 	case ALG_WEP:
 		result = ieee80211_crypto_wep_decrypt(rx);

Zhu Yi

2010-04-08 03:35 UTC #2 lore raw

Quoted 15 lines · Zhu Yi

Quoted 6 lines · Johannes Berg
From: Johannes Berg <[email protected]>
WEP crypto was broken, but upon finding the problem it is evident that other things were broken by the paged RX patch as well.
Quoted 7 lines
To fix it, for now move the linearising in front. This means that we linearise all frames, which is not at all what we want, but at least it fixes the problem for now.
Signed-off-by Johannes Berg <[email protected]>
Acked-by Zhu Yi <[email protected]>

Quoted 7 lines

I thought it over. We don't need to handle nonlinear skb in ieee80211_get_mmie_keyidx(), because we only need to touch fields out of 802.11 header for management frames, and we have already skb_linearize all management frames before. Now we just need to handle WEP IV correctly. How about this patch?
Signed-off-by Zhu Yi <[email protected]>

V2: reset hdr.

net/mac80211/rx.c +5 -1

index 14366d4..23312dd 100644
@@ -894,6 +894,7 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 			rx->key = key;
 		return RX_CONTINUE;
 	} else {
+		u8 keyid;
 		/*
 		 * The device doesn't give us the IV so we won't be
 		 * able to look up the key. That's ok though, we

@@ -916,7 +917,8 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 		 * no need to call ieee80211_wep_get_keyidx,
 		 * it verifies a bunch of things we've done already
 		 */
-		keyidx = rx->skb->data[hdrlen + 3] >> 6;
+		skb_copy_bits(rx->skb, hdrlen + 3, &keyid, 1);
+		keyidx = keyid >> 6;
 
 		rx->key = rcu_dereference(rx->sdata->keys[keyidx]);

@@ -940,6 +942,8 @@ ieee80211_rx_h_decrypt(struct ieee80211_rx_data *rx)
 	if (skb_linearize(rx->skb))
 		return RX_DROP_UNUSABLE;
 
+	hdr = (struct ieee80211_hdr *)skb->data;
+
 	/* Check for weak IVs if possible */
 	if (rx->sta && rx->key->conf.alg == ALG_WEP &&
 	    ieee80211_is_data(hdr->frame_control) &&

Johannes Berg

author maintainer

2010-04-08 06:14 UTC #3 lore raw

Quoted 4 lines · Zhu Yi

Quoted 4 lines
I thought it over. We don't need to handle nonlinear skb in ieee80211_get_mmie_keyidx(), because we only need to touch fields out of 802.11 header for management frames, and we have already skb_linearize all management frames before.

Good point, forgot about that.

Quoted 2 lines

Quoted 2 lines
Now we just need to handle WEP IV
correctly. How about this patch?

Yeah, seems like it should work, did you test with WEP?

johannes

Zhu Yi

2010-04-08 07:14 UTC #4 lore raw

Quoted 7 lines · Johannes Berg

Quoted 4 lines · Zhu Yi
Quoted 4 lines
I thought it over. We don't need to handle nonlinear skb in ieee80211_get_mmie_keyidx(), because we only need to touch fields out of 802.11 header for management frames, and we have already skb_linearize all management frames before.

Quoted 6 lines

Good point, forgot about that.
Quoted 2 lines
Quoted 2 lines
Now we just need to handle WEP IV
correctly. How about this patch?
Yeah, seems like it should work, did you test with WEP?

Yes, it works for me. Since john has already merged yours, I'll send a incremental one.

Thanks,
-yi

Johannes Berg

author maintainer

2010-04-08 07:16 UTC #5 lore raw

Quoted 10 lines · Zhu Yi

Quoted 9 lines · Johannes Berg
Quoted 4 lines · Zhu Yi
Quoted 4 lines
I thought it over. We don't need to handle nonlinear skb in ieee80211_get_mmie_keyidx(), because we only need to touch fields out of 802.11 header for management frames, and we have already skb_linearize all management frames before.
Good point, forgot about that.

Quoted 7 lines

Quoted 4 lines
Quoted 2 lines
Quoted 2 lines
Now we just need to handle WEP IV
correctly. How about this patch?
Yeah, seems like it should work, did you test with WEP?
Yes, it works for me. Since john has already merged yours, I'll send a incremental one.

Great. Yeah I noticed that too later, thanks.

johannes