[PATCH] mac80211: fix sdata locking around __ieee80211_request_smps

[PATCH] mac80211: fix sdata locking around __ieee80211_request_smps

[Johannes Berg]

From: Johannes Berg <johannes.berg@intel.com>

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 <kubakici@wp.pl>
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 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

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

[Jakub Kicinski]

On Mon,  3 Jun 2013 13:53:20 +0200, Johannes Berg wrote:
> From: Johannes Berg <johannes.berg@intel.com>
>
> 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 <kubakici@wp.pl>
> Signed-off-by: Johannes Berg <johannes.berg@intel.com>
> ---
>  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