2008-01-17 15:09:58

by Johannes Berg

[permalink] [raw]
Subject: [PATCH] mac80211: fix RCU locking in __ieee80211_rx_handle_packet

Commit c7a51bda ("mac80211: restructure __ieee80211_rx") extracted
__ieee80211_rx_handle_packet out of __ieee80211_rx and hence changed
the locking rules for __ieee80211_rx_handle_packet(), it is now
invoked under RCU lock. There is, however, one instance left where
it contains an rcu_read_unlock() in an error path, which is a bug.

Signed-off-by: Johannes Berg <[email protected]>
---
sparse actually detected this.... oh well

net/mac80211/rx.c | 1 -
1 file changed, 1 deletion(-)

--- everything.orig/net/mac80211/rx.c 2008-01-16 21:15:17.532295846 +0100
+++ everything/net/mac80211/rx.c 2008-01-16 21:15:44.002272517 +0100
@@ -1730,7 +1730,6 @@ void __ieee80211_rx_handle_packet(struct
ieee80211_invoke_rx_handlers(local, local->rx_handlers, &rx,
rx.sta);
sta_info_put(sta);
- rcu_read_unlock();
return;
}





2008-01-17 15:32:31

by Ron Rindjunsky

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix RCU locking in __ieee80211_rx_handle_packet

>
> sparse actually detected this.... oh well
>

Arrrr... i did the re-structuring of Rx flow and forgot to sparse.
I'll issue a patch to clean up the warnings.

2008-01-17 15:36:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: fix RCU locking in __ieee80211_rx_handle_packet


On Thu, 2008-01-17 at 17:32 +0200, Ron Rindjunsky wrote:
> >
> > sparse actually detected this.... oh well
> >
>
> Arrrr... i did the re-structuring of Rx flow and forgot to sparse.
> I'll issue a patch to clean up the warnings.

No worries, the rest is just harmless missing "static" keywords. I
wonder though, should we get rid of that special case completely? Jiri
intended this as an 'optimisation' but since we rarely even hit the case
(I had to play with monitor interfaces *and* AP mode to hit it!!) it
probably hurts performance more by the extra branches and all that. And
it makes it harder to verify correctness...

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part