[PATCH] mac80211: Fix double locking on CSA counter update

[PATCH] mac80211: Fix double locking on CSA counter update

[Wojciech Dubowik]

We call rcu locked ieee80211_csa_update_counter from
already locked section. Fix it by decrementing counter
directly instead of calling ieee80211_csa_update_counter.

Signed-off-by: Wojciech Dubowik <Wojciech.Dubowik@neratec.com>
---
 net/mac80211/tx.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 8df1342..9233559 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3338,7 +3338,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 		if (beacon) {
 			if (beacon->csa_counter_offsets[0]) {
 				if (!is_template)
-					ieee80211_csa_update_counter(vif);
+					WARN_ON_ONCE(beacon->csa_current_counter--);
 
 				ieee80211_set_csa(sdata, beacon);
 			}
@@ -3384,7 +3384,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 
 		if (beacon->csa_counter_offsets[0]) {
 			if (!is_template)
-				ieee80211_csa_update_counter(vif);
+				WARN_ON_ONCE(beacon->csa_current_counter--);
 
 			ieee80211_set_csa(sdata, beacon);
 		}
@@ -3414,7 +3414,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 				 * for now we leave it consistent with overall
 				 * mac80211's behavior.
 				 */
-				ieee80211_csa_update_counter(vif);
+				WARN_ON_ONCE(beacon->csa_current_counter--);
 
 			ieee80211_set_csa(sdata, beacon);
 		}
-- 
1.9.1

Re: [PATCH] mac80211: Fix double locking on CSA counter update

[Julian Calaby]

Hi Wojciech,

On Mon, Jun 1, 2015 at 11:54 PM, Wojciech Dubowik
<Wojciech.Dubowik@neratec.com> wrote:
> We call rcu locked ieee80211_csa_update_counter from
> already locked section. Fix it by decrementing counter
> directly instead of calling ieee80211_csa_update_counter.

Stupid question: wouldn't it be better to split the work from
ieee80211_csa_update_counter() into a separate function without
locking and call that instead?

Thanks,

-- 
Julian Calaby

Email: julian.calaby@gmail.com
Profile: http://www.google.com/profiles/julian.calaby/

Re: [PATCH] mac80211: Fix double locking on CSA counter update

[Wojciech Dubowik]

On 01/06/15 16:13, Julian Calaby wrote:
> Hi Wojciech,
>
> On Mon, Jun 1, 2015 at 11:54 PM, Wojciech Dubowik
> <Wojciech.Dubowik@neratec.com> wrote:
>> We call rcu locked ieee80211_csa_update_counter from
>> already locked section. Fix it by decrementing counter
>> directly instead of calling ieee80211_csa_update_counter.
> Stupid question: wouldn't it be better to split the work from
> ieee80211_csa_update_counter() into a separate function without
> locking and call that instead?
Yes. It would be better for maintenance. It's just that they will
have different input parameters

ieee80211_csa_update_counter(struct ieee80211_vif *
__ieee80211_csa_update_counter(struct beacon_data *

as it doesn't make sense to dereference beacon twice.

I guess it's not a problem?
Wojtek
>
> Thanks,
>

Re: [PATCH] mac80211: Fix double locking on CSA counter update

[Johannes Berg]

On Mon, 2015-06-01 at 16:23 +0200, Wojciech Dubowik wrote:
> On 01/06/15 16:13, Julian Calaby wrote:
> > Hi Wojciech,
> >
> > On Mon, Jun 1, 2015 at 11:54 PM, Wojciech Dubowik
> > <Wojciech.Dubowik@neratec.com> wrote:
> >> We call rcu locked ieee80211_csa_update_counter from
> >> already locked section. Fix it by decrementing counter
> >> directly instead of calling ieee80211_csa_update_counter.
> > Stupid question: wouldn't it be better to split the work from
> > ieee80211_csa_update_counter() into a separate function without
> > locking and call that instead?
> Yes. It would be better for maintenance. It's just that they will
> have different input parameters
> 
> ieee80211_csa_update_counter(struct ieee80211_vif *
> __ieee80211_csa_update_counter(struct beacon_data *
> 
> as it doesn't make sense to dereference beacon twice.
> 
> I guess it's not a problem?

Seems fine to me.

johannes