[PATCH 0/2] hw scan fixes

[PATCH 0/2] hw scan fixes

[Johannes Berg]

I'm rolling these up because without the iwlwifi
fix I'm pretty sure we will also get lockdep
warnings even if the actual deadlock scenario
is rather unlikely to start with.

Technically these are 2.6.35 regressions, but
I guess it's a little late?

johannes

[PATCH 1/2] iwlwifi: fix possible recursive locking deadlock

[Johannes Berg]

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

commit f84b29ec0a1ab767679d3f2428877b65f94bc3ff
Author: Johannes Berg <johannes.berg@intel.com>
Date:   Tue May 18 02:29:13 2010 -0700

    iwlwifi: queue user-initiated scan when doing internal scan

introduced a potential deadlock because it calls
ieee80211_scan_completed() with the priv->mutex
held, but mac80211 may call back into iwlwifi
which would lead to recursive locking. Move this
out from under the mutex.

Cc: stable@kernel.org
Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 drivers/net/wireless/iwlwifi/iwl-core.c |    6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- wireless-testing.orig/drivers/net/wireless/iwlwifi/iwl-core.c	2010-07-30 13:27:51.000000000 +0200
+++ wireless-testing/drivers/net/wireless/iwlwifi/iwl-core.c	2010-07-30 13:27:53.000000000 +0200
@@ -2000,6 +2000,7 @@ void iwl_mac_remove_interface(struct iee
 			      struct ieee80211_vif *vif)
 {
 	struct iwl_priv *priv = hw->priv;
+	bool scan_completed = false;
 
 	IWL_DEBUG_MAC80211(priv, "enter\n");
 
@@ -2013,7 +2014,7 @@ void iwl_mac_remove_interface(struct iee
 	if (priv->vif == vif) {
 		priv->vif = NULL;
 		if (priv->scan_vif == vif) {
-			ieee80211_scan_completed(priv->hw, true);
+			scan_completed = true;
 			priv->scan_vif = NULL;
 			priv->scan_request = NULL;
 		}
@@ -2021,6 +2022,9 @@ void iwl_mac_remove_interface(struct iee
 	}
 	mutex_unlock(&priv->mutex);
 
+	if (scan_completed)
+		ieee80211_scan_completed(priv->hw, true);
+
 	IWL_DEBUG_MAC80211(priv, "leave\n");
 
 }

[PATCH 2/2] mac80211: fix scan locking wrt. hw scan

[Johannes Berg]

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

Releasing the scan mutex while starting scans
can lead to unexpected things happening, so
we shouldn't do that. Fix that and hold the
mutex across the scan triggering.

Signed-off-by: Johannes Berg <johannes.berg@intel.com>
---
 net/mac80211/scan.c |   14 --------------
 1 file changed, 14 deletions(-)

--- wireless-testing.orig/net/mac80211/scan.c	2010-07-30 08:43:30.000000000 +0200
+++ wireless-testing/net/mac80211/scan.c	2010-07-30 08:43:38.000000000 +0200
@@ -401,19 +401,7 @@ static int __ieee80211_start_scan(struct
 	else
 		__set_bit(SCAN_SW_SCANNING, &local->scanning);
 
-	/*
-	 * Kicking off the scan need not be protected,
-	 * only the scan variable stuff, since now
-	 * local->scan_req is assigned and other callers
-	 * will abort their scan attempts.
-	 *
-	 * This avoids too many locking dependencies
-	 * so that the scan completed calls have more
-	 * locking freedom.
-	 */
-
 	ieee80211_recalc_idle(local);
-	mutex_unlock(&local->scan_mtx);
 
 	if (local->ops->hw_scan) {
 		WARN_ON(!ieee80211_prep_hw_scan(local));
@@ -421,8 +409,6 @@ static int __ieee80211_start_scan(struct
 	} else
 		rc = ieee80211_start_sw_scan(local);
 
-	mutex_lock(&local->scan_mtx);
-
 	if (rc) {
 		kfree(local->hw_scan_req);
 		local->hw_scan_req = NULL;

Re: [PATCH 0/2] hw scan fixes

[Johannes Berg]

On Fri, 2010-07-30 at 13:30 +0200, Johannes Berg wrote:

> Technically these are 2.6.35 regressions, but
> I guess it's a little late?

Wait, no, only the iwlwifi one is a regression.

johannes