2013-06-03 11:53:27

by Johannes Berg

Subject: [PATCH] mac80211: fix sdata locking around __ieee80211_request_smps

From: Johannes Berg <[email protected]>

My cfg80211/mac80211 locking unification broke the sdata
locking in ieee80211_set_power_mgmt, it needs to acquire
the lock for __ieee80211_request_smps(). Add the locking.

Reported-by: Jakub Kicinski <[email protected]>
Signed-off-by: Johannes Berg <[email protected]>
---
net/mac80211/cfg.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
index 9034da1..3062210 100644
--- a/net/mac80211/cfg.c
+++ b/net/mac80211/cfg.c
@@ -2378,7 +2378,9 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
local->dynamic_ps_forced_timeout = timeout;

/* no change, but if automatic follow powersave */
+ sdata_lock(sdata);
__ieee80211_request_smps(sdata, sdata->u.mgd.req_smps);
+ sdata_unlock(sdata);

if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);
--
1.8.0

2013-06-03 13:05:10

by Jakub Kicinski

Subject: Re: [PATCH] mac80211: fix sdata locking around __ieee80211_request_smps

On Mon, 3 Jun 2013 13:53:20 +0200, Johannes Berg wrote:
> From: Johannes Berg <[email protected]>
>
> My cfg80211/mac80211 locking unification broke the sdata
> locking in ieee80211_set_power_mgmt, it needs to acquire
> the lock for __ieee80211_request_smps(). Add the locking.
>
> Reported-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Johannes Berg <[email protected]>
> ---
> net/mac80211/cfg.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/net/mac80211/cfg.c b/net/mac80211/cfg.c
> index 9034da1..3062210 100644
> --- a/net/mac80211/cfg.c
> +++ b/net/mac80211/cfg.c
> @@ -2378,7 +2378,9 @@ static int ieee80211_set_power_mgmt(struct wiphy *wiphy, struct net_device *dev,
> local->dynamic_ps_forced_timeout = timeout;
>
> /* no change, but if automatic follow powersave */
> + sdata_lock(sdata);
> __ieee80211_request_smps(sdata, sdata->u.mgd.req_smps);
> + sdata_unlock(sdata);
>
> if (local->hw.flags & IEEE80211_HW_SUPPORTS_DYNAMIC_PS)
> ieee80211_hw_config(local, IEEE80211_CONF_CHANGE_PS);

I can confirm that this patch fixes the reported warning.

I had to double check that sdata_lock takes the wdev mutex
though, because lockdep assert on cfg.c:2321 looks like this:

lockdep_assert_held(&sdata->wdev.mtx)

instead of:

sdata_assert_lock(&sdata)

just saying... :-)

-- kuba