Re: [PATCH 2/2] mac80211: Fixing Races for skipping tailroom reservation

[Johannes Berg <johannes@sipsolutions.net>]

On Wed, 2011-06-22 at 18:28 +0530, Yogesh Ashok Powar wrote:
> On Wed, Jun 22, 2011 at 05:49:25AM -0700, Johannes Berg wrote:
> > On Wed, 2011-06-22 at 18:01 +0530, Yogesh Ashok Powar wrote:
> > > > Will work on some other logic.
> > > 
> > > Following is the complete V2-patch
> > > 
> > > v2 changes: a) Moved counter++ before __ieee80211_key_replace in
> > > 		key_link()
> > > 	    b) Moved crypto_tx_tailroom_needed_cnt to sdata resolve
> > > 	        issue with multiple sdata instances in hw reset.
> > 
> > Looks good. Now I'm just worried about memory and compiler barriers that
> > may be needed so the counter update doesn't move after anything else...
> > Hmm.
> 
> I some how feel that synchronize_net may be replaced with
> synchronize_rcu and still the race wont happen.
> 
> So the new logic would be 
> - counter++		<-- Here keys are not added or deleted
> 			    so rcu readers wont have problem
> 			    with extra space allocated.
> 
> - synchronize_rcu	<-- This will flush existing readers. Again
> 			    new readers will have no problem with extra
> 			    space allocated.
> 
> Let me know your opinion on this.

It doesn't really matter -- synchronize_net() is exactly the same as
synchronize_rcu().

What I'm worried about is that there's no memory barrier after the
counter update, so how do we know it cannot happen after
synchronize_rcu()? I think we need something like rcu_assign_pointer()
but I don't see rcu_assign_index() (any more?)

johannes