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

[PATCH v2] 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 splitting ieee80211_csa_update_counter into
locked and unlocked variants and use the latter in rcu locked section.

v2: Use splitted functions instead of direct counter decrement.

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

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 8df1342..312b199 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -3216,6 +3216,16 @@ static void ieee80211_set_csa(struct ieee80211_sub_if_data *sdata,
 	rcu_read_unlock();
 }
 
+static u8 __ieee80211_csa_update_counter(struct beacon_data *beacon)
+{
+	beacon->csa_current_counter--;
+
+	/* the counter should never reach 0 */
+	WARN_ON_ONCE(!beacon->csa_current_counter);
+
+	return beacon->csa_current_counter;
+}
+
 u8 ieee80211_csa_update_counter(struct ieee80211_vif *vif)
 {
 	struct ieee80211_sub_if_data *sdata = vif_to_sdata(vif);
@@ -3234,11 +3244,7 @@ u8 ieee80211_csa_update_counter(struct ieee80211_vif *vif)
 	if (!beacon)
 		goto unlock;
 
-	beacon->csa_current_counter--;
-
-	/* the counter should never reach 0 */
-	WARN_ON_ONCE(!beacon->csa_current_counter);
-	count = beacon->csa_current_counter;
+	count = __ieee80211_csa_update_counter(beacon);
 
 unlock:
 	rcu_read_unlock();
@@ -3338,7 +3344,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 		if (beacon) {
 			if (beacon->csa_counter_offsets[0]) {
 				if (!is_template)
-					ieee80211_csa_update_counter(vif);
+					__ieee80211_csa_update_counter(beacon);
 
 				ieee80211_set_csa(sdata, beacon);
 			}
@@ -3384,7 +3390,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 
 		if (beacon->csa_counter_offsets[0]) {
 			if (!is_template)
-				ieee80211_csa_update_counter(vif);
+				__ieee80211_csa_update_counter(beacon);
 
 			ieee80211_set_csa(sdata, beacon);
 		}
@@ -3414,7 +3420,7 @@ __ieee80211_beacon_get(struct ieee80211_hw *hw,
 				 * for now we leave it consistent with overall
 				 * mac80211's behavior.
 				 */
-				ieee80211_csa_update_counter(vif);
+				__ieee80211_csa_update_counter(beacon);
 
 			ieee80211_set_csa(sdata, beacon);
 		}
-- 
1.9.1

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

[Johannes Berg]

On Mon, 2015-06-01 at 16:51 +0200, Wojciech Dubowik wrote:
> We call rcu locked ieee80211_csa_update_counter from already locked
> section. Fix it by splitting ieee80211_csa_update_counter into
> locked and unlocked variants and use the latter in rcu locked section.

The subject of this patch implies there's a bug ("double locking", which
I'd interpret as "nested locking") and that there's something to fix.

At best, this is an optimisation, can you please label it as such?

johannes