rpms/kernel/F-13 iwlwifi-fix-internal-scan-race.patch, NONE, 1.1 iwlwifi-fix-scan-races.patch, NONE, 1.1 kernel.spec, 1.2040, 1.2041
John W. Linville
linville at fedoraproject.org
Wed May 19 19:32:47 UTC 2010
Author: linville
Update of /cvs/pkgs/rpms/kernel/F-13
In directory cvs01.phx2.fedoraproject.org:/tmp/cvs-serv19333
Modified Files:
kernel.spec
Added Files:
iwlwifi-fix-internal-scan-race.patch
iwlwifi-fix-scan-races.patch
Log Message:
iwlwifi: fix scan races ; iwlwifi: fix internal scan race
iwlwifi-fix-internal-scan-race.patch:
iwl-scan.c | 22 ++++++++++++++++++----
1 file changed, 18 insertions(+), 4 deletions(-)
--- NEW FILE iwlwifi-fix-internal-scan-race.patch ---
>From reinette.chatre at intel.com Thu May 13 17:49:59 2010
Return-path: <reinette.chatre at intel.com>
Envelope-to: linville at tuxdriver.com
Delivery-date: Thu, 13 May 2010 17:49:59 -0400
Received: from mga09.intel.com ([134.134.136.24])
by smtp.tuxdriver.com with esmtp (Exim 4.63)
(envelope-from <reinette.chatre at intel.com>)
id 1OCgI1-0007H3-Eg
for linville at tuxdriver.com; Thu, 13 May 2010 17:49:59 -0400
Received: from orsmga002.jf.intel.com ([10.7.209.21])
by orsmga102.jf.intel.com with ESMTP; 13 May 2010 14:48:04 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="4.53,224,1272870000";
d="scan'208";a="517743256"
Received: from rchatre-desk.amr.corp.intel.com.jf.intel.com (HELO localhost.localdomain) ([134.134.15.94])
by orsmga002.jf.intel.com with ESMTP; 13 May 2010 14:49:12 -0700
From: Reinette Chatre <reinette.chatre at intel.com>
To: linville at tuxdriver.com
Cc: linux-wireless at vger.kernel.org, ipw3945-devel at lists.sourceforge.net, Reinette Chatre <reinette.chatre at intel.com>
Subject: [PATCH 1/2] iwlwifi: fix internal scan race
Date: Thu, 13 May 2010 14:49:44 -0700
Message-Id: <1273787385-9248-2-git-send-email-reinette.chatre at intel.com>
X-Mailer: git-send-email 1.6.3.3
In-Reply-To: <1273787385-9248-1-git-send-email-reinette.chatre at intel.com>
References: <1273787385-9248-1-git-send-email-reinette.chatre at intel.com>
X-Spam-Score: -4.2 (----)
X-Spam-Status: No
Status: RO
Content-Length: 3370
Lines: 91
From: Reinette Chatre <reinette.chatre at intel.com>
It is possible for internal scan to race against itself if the device is
not returning the scan results from first requests. What happens in this
case is the cleanup done during the abort of the first internal scan also
cleans up part of the new scan, causing it to access memory it shouldn't.
Here are details:
* First internal scan is triggered and scan command sent to device.
* After seven seconds there is no scan results so the watchdog timer
triggers a scan abort.
* The scan abort succeeds and a SCAN_COMPLETE_NOTIFICATION is received for
failed scan.
* During processing of SCAN_COMPLETE_NOTIFICATION we clear STATUS_SCANNING
and queue the "scan_completed" work.
** At this time, since the problem that caused the internal scan in first
place is still present, a new internal scan is triggered.
The behavior at this point is a bit different between 2.6.34 and 2.6.35
since 2.6.35 has a lot of this synchronized. The rest of the race
description will thus be generalized.
** As part of preparing for the scan "is_internal_short_scan" is set to
true.
* At this point the completion work for fist scan is run. As part of this
there is some locking missing around the "is_internal_short_scan"
variable and it is set to "false".
** Now the second scan runs and it considers itself a real (not internal0
scan and thus causes problems with wrong memory being accessed.
The fix is twofold.
* Since "is_internal_short_scan" should be protected by mutex, fix this in
scan completion work so that changes to it can be serialized.
* Do not queue a new internal scan if one is in progress.
This fixes https://bugzilla.kernel.org/show_bug.cgi?id=15824
Signed-off-by: Reinette Chatre <reinette.chatre at intel.com>
---
drivers/net/wireless/iwlwifi/iwl-scan.c | 21 ++++++++++++++++++---
1 files changed, 18 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index 2367286..a2c4855 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -560,6 +560,11 @@ static void iwl_bg_start_internal_scan(struct work_struct *work)
mutex_lock(&priv->mutex);
+ if (priv->is_internal_short_scan == true) {
+ IWL_DEBUG_SCAN(priv, "Internal scan already in progress\n");
+ goto unlock;
+ }
+
if (!iwl_is_ready_rf(priv)) {
IWL_DEBUG_SCAN(priv, "not ready or exit pending\n");
goto unlock;
@@ -957,17 +962,27 @@ void iwl_bg_scan_completed(struct work_struct *work)
{
struct iwl_priv *priv =
container_of(work, struct iwl_priv, scan_completed);
+ bool internal = false;
IWL_DEBUG_SCAN(priv, "SCAN complete scan\n");
cancel_delayed_work(&priv->scan_check);
- if (!priv->is_internal_short_scan)
- ieee80211_scan_completed(priv->hw, false);
- else {
+ mutex_lock(&priv->mutex);
+ if (priv->is_internal_short_scan) {
priv->is_internal_short_scan = false;
IWL_DEBUG_SCAN(priv, "internal short scan completed\n");
+ internal = true;
}
+ mutex_unlock(&priv->mutex);
+
+ /*
+ * Do not hold mutex here since this will cause mac80211 to call
+ * into driver again into functions that will attempt to take
+ * mutex.
+ */
+ if (!internal)
+ ieee80211_scan_completed(priv->hw, false);
if (test_bit(STATUS_EXIT_PENDING, &priv->status))
return;
--
1.6.3.3
iwlwifi-fix-scan-races.patch:
iwl-agn.c | 1 +
iwl-core.c | 1 -
iwl-core.h | 2 +-
iwl-dev.h | 1 +
iwl-scan.c | 31 ++++++++++++++++++++-----------
5 files changed, 23 insertions(+), 13 deletions(-)
--- NEW FILE iwlwifi-fix-scan-races.patch ---
commit 88be026490ed89c2ffead81a52531fbac5507e01
Author: Johannes Berg <johannes.berg at intel.com>
Date: Wed Apr 7 00:21:36 2010 -0700
iwlwifi: fix scan races
When an internal scan is started, nothing protects the
is_internal_short_scan variable which can cause crashes,
cf. https://bugzilla.kernel.org/show_bug.cgi?id=15667.
Fix this by making the short scan request use the mutex
for locking, which requires making the request go to a
work struct so that it can sleep.
Reported-by: Peter Zijlstra <peterz at infradead.org>
Signed-off-by: Johannes Berg <johannes.berg at intel.com>
Signed-off-by: Reinette Chatre <reinette.chatre at intel.com>
diff --git a/drivers/net/wireless/iwlwifi/iwl-agn.c b/drivers/net/wireless/iwlwifi/iwl-agn.c
index e4c2e1e..ba0fdba 100644
--- a/drivers/net/wireless/iwlwifi/iwl-agn.c
+++ b/drivers/net/wireless/iwlwifi/iwl-agn.c
@@ -3330,6 +3330,7 @@ static void iwl_cancel_deferred_work(struct iwl_priv *priv)
cancel_delayed_work_sync(&priv->init_alive_start);
cancel_delayed_work(&priv->scan_check);
+ cancel_work_sync(&priv->start_internal_scan);
cancel_delayed_work(&priv->alive_start);
cancel_work_sync(&priv->beacon_update);
del_timer_sync(&priv->statistics_periodic);
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.c b/drivers/net/wireless/iwlwifi/iwl-core.c
index 894bcb8..1459cdb 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.c
+++ b/drivers/net/wireless/iwlwifi/iwl-core.c
@@ -3357,7 +3357,6 @@ static void iwl_force_rf_reset(struct iwl_priv *priv)
*/
IWL_DEBUG_INFO(priv, "perform radio reset.\n");
iwl_internal_short_hw_scan(priv);
- return;
}
diff --git a/drivers/net/wireless/iwlwifi/iwl-core.h b/drivers/net/wireless/iwlwifi/iwl-core.h
index 732590f..36940a9 100644
--- a/drivers/net/wireless/iwlwifi/iwl-core.h
+++ b/drivers/net/wireless/iwlwifi/iwl-core.h
@@ -506,7 +506,7 @@ void iwl_init_scan_params(struct iwl_priv *priv);
int iwl_scan_cancel(struct iwl_priv *priv);
int iwl_scan_cancel_timeout(struct iwl_priv *priv, unsigned long ms);
int iwl_mac_hw_scan(struct ieee80211_hw *hw, struct cfg80211_scan_request *req);
-int iwl_internal_short_hw_scan(struct iwl_priv *priv);
+void iwl_internal_short_hw_scan(struct iwl_priv *priv);
int iwl_force_reset(struct iwl_priv *priv, int mode);
u16 iwl_fill_probe_req(struct iwl_priv *priv, struct ieee80211_mgmt *frame,
const u8 *ie, int ie_len, int left);
diff --git a/drivers/net/wireless/iwlwifi/iwl-dev.h b/drivers/net/wireless/iwlwifi/iwl-dev.h
index 6054c5f..ef1720a 100644
--- a/drivers/net/wireless/iwlwifi/iwl-dev.h
+++ b/drivers/net/wireless/iwlwifi/iwl-dev.h
@@ -1296,6 +1296,7 @@ struct iwl_priv {
struct work_struct tt_work;
struct work_struct ct_enter;
struct work_struct ct_exit;
+ struct work_struct start_internal_scan;
struct tasklet_struct irq_tasklet;
diff --git a/drivers/net/wireless/iwlwifi/iwl-scan.c b/drivers/net/wireless/iwlwifi/iwl-scan.c
index bd2f7c4..5062f4e 100644
--- a/drivers/net/wireless/iwlwifi/iwl-scan.c
+++ b/drivers/net/wireless/iwlwifi/iwl-scan.c
@@ -469,6 +469,8 @@ EXPORT_SYMBOL(iwl_init_scan_params);
static int iwl_scan_initiate(struct iwl_priv *priv)
{
+ WARN_ON(!mutex_is_locked(&priv->mutex));
+
IWL_DEBUG_INFO(priv, "Starting scan...\n");
set_bit(STATUS_SCANNING, &priv->status);
priv->is_internal_short_scan = false;
@@ -546,24 +548,31 @@ EXPORT_SYMBOL(iwl_mac_hw_scan);
* internal short scan, this function should only been called while associated.
* It will reset and tune the radio to prevent possible RF related problem
*/
-int iwl_internal_short_hw_scan(struct iwl_priv *priv)
+void iwl_internal_short_hw_scan(struct iwl_priv *priv)
{
- int ret = 0;
+ queue_work(priv->workqueue, &priv->start_internal_scan);
+}
+
+static void iwl_bg_start_internal_scan(struct work_struct *work)
+{
+ struct iwl_priv *priv =
+ container_of(work, struct iwl_priv, start_internal_scan);
+
+ mutex_lock(&priv->mutex);
if (!iwl_is_ready_rf(priv)) {
- ret = -EIO;
IWL_DEBUG_SCAN(priv, "not ready or exit pending\n");
- goto out;
+ goto unlock;
}
+
if (test_bit(STATUS_SCANNING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Scan already in progress.\n");
- ret = -EAGAIN;
- goto out;
+ goto unlock;
}
+
if (test_bit(STATUS_SCAN_ABORTING, &priv->status)) {
IWL_DEBUG_SCAN(priv, "Scan request while abort pending\n");
- ret = -EAGAIN;
- goto out;
+ goto unlock;
}
priv->scan_bands = 0;
@@ -576,9 +585,8 @@ int iwl_internal_short_hw_scan(struct iwl_priv *priv)
set_bit(STATUS_SCANNING, &priv->status);
priv->is_internal_short_scan = true;
queue_work(priv->workqueue, &priv->request_scan);
-
-out:
- return ret;
+ unlock:
+ mutex_unlock(&priv->mutex);
}
EXPORT_SYMBOL(iwl_internal_short_hw_scan);
@@ -964,6 +972,7 @@ void iwl_setup_scan_deferred_work(struct iwl_priv *priv)
INIT_WORK(&priv->scan_completed, iwl_bg_scan_completed);
INIT_WORK(&priv->request_scan, iwl_bg_request_scan);
INIT_WORK(&priv->abort_scan, iwl_bg_abort_scan);
+ INIT_WORK(&priv->start_internal_scan, iwl_bg_start_internal_scan);
INIT_DELAYED_WORK(&priv->scan_check, iwl_bg_scan_check);
}
EXPORT_SYMBOL(iwl_setup_scan_deferred_work);
Index: kernel.spec
===================================================================
RCS file: /cvs/pkgs/rpms/kernel/F-13/kernel.spec,v
retrieving revision 1.2040
retrieving revision 1.2041
diff -u -p -r1.2040 -r1.2041
--- kernel.spec 19 May 2010 06:16:02 -0000 1.2040
+++ kernel.spec 19 May 2010 19:32:47 -0000 1.2041
@@ -852,6 +852,11 @@ Patch12851: perf-mount-debugfs-automatic
# upstream 5dc6416414fb3ec6e2825fd4d20c8bf1d7fe0395 (#593226)
Patch12900: btrfs-check-for-read-permission-on-src-file-in-clone-ioctl.patch
+# iwlwifi: fix scan races
+Patch12910: iwlwifi-fix-scan-races.patch
+# iwlwifi: fix internal scan race
+Patch12911: iwlwifi-fix-internal-scan-race.patch
+
%endif
BuildRoot: %{_tmppath}/kernel-%{KVERREL}-root
@@ -1555,6 +1560,11 @@ ApplyPatch perf-mount-debugfs-automatica
# upstream 5dc6416414fb3ec6e2825fd4d20c8bf1d7fe0395 (#593226)
ApplyPatch btrfs-check-for-read-permission-on-src-file-in-clone-ioctl.patch
+# iwlwifi: fix scan races
+ApplyPatch iwlwifi-fix-scan-races.patch
+# iwlwifi: fix internal scan race
+ApplyPatch iwlwifi-fix-internal-scan-race.patch
+
# END OF PATCH APPLICATIONS
%endif
@@ -2204,6 +2214,10 @@ fi
# and build.
%changelog
+* Wed May 19 2010 John W. Linville <linville at redhat.com>
+- iwlwifi: fix scan races
+- iwlwifi: fix internal scan race
+
* Wed May 19 2010 Dave Airlie <airlied at redhat.com>
- disable vmwgfx at request of vmware
More information about the scm-commits
mailing list