[PATCH V2] mac80211: fix paged defragmentation

[PATCH V2] mac80211: fix paged defragmentation

[Abhijeet Kolekar]

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 <yi.zhu@intel.com>
Signed-off-by: Abhijeet Kolekar <abhijeet.kolekar@intel.com>
---
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

Re: [PATCH V2] mac80211: fix paged defragmentation

[Ben Gamari]

On Fri, 30 Apr 2010 15:57:59 -0700, Abhijeet Kolekar <abhijeet.kolekar@intel.com> 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

Re: [PATCH V2] mac80211: fix paged defragmentation

[John W. Linville]

On Fri, Apr 30, 2010 at 07:49:52PM -0400, Ben Gamari wrote:
> On Fri, 30 Apr 2010 15:57:59 -0700, Abhijeet Kolekar <abhijeet.kolekar@intel.com> 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
linville@tuxdriver.com			might be all we have.  Be ready.