2015-06-01 14:51:46

by Wojciech Dubowik

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

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 <[email protected]>
---
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

2015-06-09 19:46:21

by Johannes Berg

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

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