2010-04-07 09:26:59

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] 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(-)

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




2010-04-08 03:35:40

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix paged RX crypto

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

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



2010-04-08 07:16:18

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix paged RX crypto

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


2010-04-08 06:14:52

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix paged RX crypto

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


2010-04-08 03:26:04

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix paged RX crypto

On Wed, 2010-04-07 at 17:26 +0800, Johannes Berg wrote:
> 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]>

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

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



2010-04-08 07:14:32

by Zhu Yi

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix paged RX crypto

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