[PATCH] mac80211: fix paged RX crypto

[PATCH] mac80211: fix paged RX crypto

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

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 <johannes.berg@intel.com>
Acked-by: Zhu Yi <yi.zhu@intel.com>
---
 net/mac80211/rx.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

--- wireless-testing.orig/net/mac80211/rx.c	2010-04-07 11:19:30.000000000 +0200
+++ wireless-testing/net/mac80211/rx.c	2010-04-07 11:20:05.000000000 +0200
@@ -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) &&

Re: [PATCH] mac80211: fix paged RX crypto

[Zhu Yi]

On Wed, 2010-04-07 at 17:26 +0800, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
> 
> 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 <johannes.berg@intel.com>
> Acked-by: Zhu Yi <yi.zhu@intel.com>

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 <yi.zhu@intel.com>

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 14366d4..a8f11f6 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -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);

Re: [PATCH] mac80211: fix paged RX crypto

[Zhu Yi]

On Thu, 2010-04-08 at 11:22 +0800, Zhu Yi wrote:
> On Wed, 2010-04-07 at 17:26 +0800, Johannes Berg wrote:
> > From: Johannes Berg <johannes.berg@intel.com>
> > 
> > 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 <johannes.berg@intel.com>
> > Acked-by: Zhu Yi <yi.zhu@intel.com>
> 
> 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 <yi.zhu@intel.com>

V2: reset hdr.

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 14366d4..23312dd 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -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) &&

Re: [PATCH] mac80211: fix paged RX crypto

[Johannes Berg]

On Thu, 2010-04-08 at 11:36 +0800, Zhu Yi wrote:

> > 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.

> > 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

Re: [PATCH] mac80211: fix paged RX crypto

[Zhu Yi]

On Thu, 2010-04-08 at 14:14 +0800, Johannes Berg wrote:
> On Thu, 2010-04-08 at 11:36 +0800, Zhu Yi wrote:
> 
> > > 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.
> 
> > > 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

Re: [PATCH] mac80211: fix paged RX crypto

[Johannes Berg]

On Thu, 2010-04-08 at 15:14 +0800, Zhu Yi wrote:
> On Thu, 2010-04-08 at 14:14 +0800, Johannes Berg wrote:
> > On Thu, 2010-04-08 at 11:36 +0800, Zhu Yi wrote:
> > 
> > > > 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.
> > 
> > > > 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