2010-04-30 22:47:24

by Abhijeet Kolekar

[permalink] [raw]
Subject: [PATCH V2] mac80211: fix paged defragmentation

Paged RX skb patch broke the defragmentation. We need to read hdr again
after linearization.

It fixes following bug
http://bugzilla.intellinuxwireless.org/show_bug.cgi?id=2194

Signed-off-by: Zhu, Yi <[email protected]>
Signed-off-by: Abhijeet Kolekar <[email protected]>
---
v2: Zhu yi suggested following fix.
net/mac80211/rx.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
index 72efbd8..e7217e2 100644
--- a/net/mac80211/rx.c
+++ b/net/mac80211/rx.c
@@ -1255,6 +1255,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
if (skb_linearize(rx->skb))
return RX_DROP_UNUSABLE;

+ hdr = (struct ieee80211_hdr *)rx->skb->data;
seq = (sc & IEEE80211_SCTL_SEQ) >> 4;

if (frag == 0) {
--
1.6.3.3



2010-05-03 18:15:11

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH V2] mac80211: fix paged defragmentation

On Fri, Apr 30, 2010 at 07:49:52PM -0400, Ben Gamari wrote:
> On Fri, 30 Apr 2010 15:57:59 -0700, Abhijeet Kolekar <[email protected]> wrote:
> >
> > diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> > index 72efbd8..e7217e2 100644
> > --- a/net/mac80211/rx.c
> > +++ b/net/mac80211/rx.c
> > @@ -1255,6 +1255,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
> > if (skb_linearize(rx->skb))
> > return RX_DROP_UNUSABLE;
> >
> > + hdr = (struct ieee80211_hdr *)rx->skb->data;
> > seq = (sc & IEEE80211_SCTL_SEQ) >> 4;
> >
> It seems to me that this might deserve a comment describing exactly why
> hdr needs to be set twice in one function. To the uninformed outsider
> the code simply seems redundant. It's unclear semantics like this that
> could cause nasty issues when someone goes back for housecleaning. Just
> a thought.

Not only that, but is there something we need to do to make sure the
compiler doesn't think it can optimize away the second assignment
of hdr?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-04-30 23:49:59

by Ben Gamari

[permalink] [raw]
Subject: Re: [PATCH V2] mac80211: fix paged defragmentation

On Fri, 30 Apr 2010 15:57:59 -0700, Abhijeet Kolekar <[email protected]> wrote:
>
> diff --git a/net/mac80211/rx.c b/net/mac80211/rx.c
> index 72efbd8..e7217e2 100644
> --- a/net/mac80211/rx.c
> +++ b/net/mac80211/rx.c
> @@ -1255,6 +1255,7 @@ ieee80211_rx_h_defragment(struct ieee80211_rx_data *rx)
> if (skb_linearize(rx->skb))
> return RX_DROP_UNUSABLE;
>
> + hdr = (struct ieee80211_hdr *)rx->skb->data;
> seq = (sc & IEEE80211_SCTL_SEQ) >> 4;
>
It seems to me that this might deserve a comment describing exactly why
hdr needs to be set twice in one function. To the uninformed outsider
the code simply seems redundant. It's unclear semantics like this that
could cause nasty issues when someone goes back for housecleaning. Just
a thought.

- Ben