From e007b857e88097c96c45620bf3b04a4e309053d1 Mon Sep 17 00:00:00 2001 From: Eliad Peller Date: Thu, 24 Nov 2011 18:13:56 +0200 Subject: [PATCH 1/6] nl80211: fix MAC address validation MAC addresses have a fixed length. The current policy allows passing < ETH_ALEN bytes, which might result in reading beyond the buffer. Cc: stable@vger.kernel.org Signed-off-by: Eliad Peller Signed-off-by: John W. Linville --- net/wireless/nl80211.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/net/wireless/nl80211.c b/net/wireless/nl80211.c index b3a476fe8272..ffafda5022c2 100644 --- a/net/wireless/nl80211.c +++ b/net/wireless/nl80211.c @@ -89,8 +89,8 @@ static const struct nla_policy nl80211_policy[NL80211_ATTR_MAX+1] = { [NL80211_ATTR_IFINDEX] = { .type = NLA_U32 }, [NL80211_ATTR_IFNAME] = { .type = NLA_NUL_STRING, .len = IFNAMSIZ-1 }, - [NL80211_ATTR_MAC] = { .type = NLA_BINARY, .len = ETH_ALEN }, - [NL80211_ATTR_PREV_BSSID] = { .type = NLA_BINARY, .len = ETH_ALEN }, + [NL80211_ATTR_MAC] = { .len = ETH_ALEN }, + [NL80211_ATTR_PREV_BSSID] = { .len = ETH_ALEN }, [NL80211_ATTR_KEY] = { .type = NLA_NESTED, }, [NL80211_ATTR_KEY_DATA] = { .type = NLA_BINARY, From 24f50a9d165745fd0701c6e089d35f58a229ea69 Mon Sep 17 00:00:00 2001 From: Johannes Berg Date: Thu, 24 Nov 2011 20:06:14 +0100 Subject: [PATCH 2/6] mac80211: don't stop a single aggregation session twice Nikolay noticed (by code review) that mac80211 can attempt to stop an aggregation session while it is already being stopped. So to fix it, check whether stop is already being done and bail out if so. Also move setting the STOPPING state into the lock so things are properly atomic. Cc: stable@vger.kernel.org Reported-by: Nikolay Martynov Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 2ac033989e01..674b345ade81 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -160,6 +160,12 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, return -ENOENT; } + /* if we're already stopping ignore any new requests to stop */ + if (test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) { + spin_unlock_bh(&sta->lock); + return -EALREADY; + } + if (test_bit(HT_AGG_STATE_WANT_START, &tid_tx->state)) { /* not even started yet! */ ieee80211_assign_tid_tx(sta, tid, NULL); @@ -168,6 +174,8 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, return 0; } + set_bit(HT_AGG_STATE_STOPPING, &tid_tx->state); + spin_unlock_bh(&sta->lock); #ifdef CONFIG_MAC80211_HT_DEBUG @@ -175,8 +183,6 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, sta->sta.addr, tid); #endif /* CONFIG_MAC80211_HT_DEBUG */ - set_bit(HT_AGG_STATE_STOPPING, &tid_tx->state); - del_timer_sync(&tid_tx->addba_resp_timer); /* From e55b32c110b025ce07b40227f620e99700bf8741 Mon Sep 17 00:00:00 2001 From: Stanislaw Gruszka Date: Mon, 28 Nov 2011 10:33:40 +0100 Subject: [PATCH 3/6] rtlwifi: fix lps_lock deadlock rtl_lps_leave can be called from interrupt context, so we have to disable interrupts when taking lps_lock. Below is full lockdep info about deadlock: [ 93.815269] ================================= [ 93.815390] [ INFO: inconsistent lock state ] [ 93.815472] 2.6.41.1-3.offch.fc15.x86_64.debug #1 [ 93.815556] --------------------------------- [ 93.815635] inconsistent {SOFTIRQ-ON-W} -> {IN-SOFTIRQ-W} usage. [ 93.815743] swapper/0 [HC0[0]:SC1[1]:HE1:SE0] takes: [ 93.815832] (&(&rtlpriv->locks.lps_lock)->rlock){+.?...}, at: [] rtl_lps_leave+0x26/0x103 [rtlwifi] [ 93.815947] {SOFTIRQ-ON-W} state was registered at: [ 93.815947] [] __lock_acquire+0x369/0xd0c [ 93.815947] [] lock_acquire+0xf3/0x13e [ 93.815947] [] _raw_spin_lock+0x45/0x79 [ 93.815947] [] rtl_swlps_rf_awake+0x5a/0x76 [rtlwifi] [ 93.815947] [] rtl_op_config+0x12a/0x32a [rtlwifi] [ 93.815947] [] ieee80211_hw_config+0x124/0x129 [mac80211] [ 93.815947] [] ieee80211_dynamic_ps_disable_work+0x32/0x47 [mac80211] [ 93.815947] [] process_one_work+0x205/0x3e7 [ 93.815947] [] worker_thread+0xda/0x15d [ 93.815947] [] kthread+0xa8/0xb0 [ 93.815947] [] kernel_thread_helper+0x4/0x10 [ 93.815947] irq event stamp: 547822 [ 93.815947] hardirqs last enabled at (547822): [] _raw_spin_unlock_irqrestore+0x45/0x61 [ 93.815947] hardirqs last disabled at (547821): [] _raw_spin_lock_irqsave+0x22/0x8e [ 93.815947] softirqs last enabled at (547790): [] _local_bh_enable+0x13/0x15 [ 93.815947] softirqs last disabled at (547791): [] call_softirq+0x1c/0x30 [ 93.815947] [ 93.815947] other info that might help us debug this: [ 93.815947] Possible unsafe locking scenario: [ 93.815947] [ 93.815947] CPU0 [ 93.815947] ---- [ 93.815947] lock(&(&rtlpriv->locks.lps_lock)->rlock); [ 93.815947] [ 93.815947] lock(&(&rtlpriv->locks.lps_lock)->rlock); [ 93.815947] [ 93.815947] *** DEADLOCK *** [ 93.815947] [ 93.815947] no locks held by swapper/0. [ 93.815947] [ 93.815947] stack backtrace: [ 93.815947] Pid: 0, comm: swapper Not tainted 2.6.41.1-3.offch.fc15.x86_64.debug #1 [ 93.815947] Call Trace: [ 93.815947] [] print_usage_bug+0x1e7/0x1f8 [ 93.815947] [] ? save_stack_trace+0x2c/0x49 [ 93.815947] [] ? print_irq_inversion_bug.part.18+0x1a0/0x1a0 [ 93.815947] [] mark_lock+0x106/0x220 [ 93.815947] [] __lock_acquire+0x2f5/0xd0c [ 93.815947] [] ? native_sched_clock+0x34/0x36 [ 93.830125] [] ? sched_clock+0x9/0xd [ 93.830125] [] ? sched_clock_local+0x12/0x75 [ 93.830125] [] ? rtl_lps_leave+0x26/0x103 [rtlwifi] [ 93.830125] [] lock_acquire+0xf3/0x13e [ 93.830125] [] ? rtl_lps_leave+0x26/0x103 [rtlwifi] [ 93.830125] [] _raw_spin_lock+0x45/0x79 [ 93.830125] [] ? rtl_lps_leave+0x26/0x103 [rtlwifi] [ 93.830125] [] ? skb_dequeue+0x62/0x6d [ 93.830125] [] rtl_lps_leave+0x26/0x103 [rtlwifi] [ 93.830125] [] _rtl_pci_ips_leave_tasklet+0xe/0x10 [rtlwifi] [ 93.830125] [] tasklet_action+0x8d/0xee [ 93.830125] [] __do_softirq+0x112/0x25a [ 93.830125] [] call_softirq+0x1c/0x30 [ 93.830125] [] do_softirq+0x4b/0xa1 [ 93.830125] [] irq_exit+0x5d/0xcf [ 93.830125] [] do_IRQ+0x8e/0xa5 [ 93.830125] [] common_interrupt+0x73/0x73 [ 93.830125] [] ? trace_hardirqs_off+0xd/0xf [ 93.830125] [] ? intel_idle+0xe5/0x10c [ 93.830125] [] ? intel_idle+0xe1/0x10c [ 93.830125] [] cpuidle_idle_call+0x11c/0x1fe [ 93.830125] [] cpu_idle+0xab/0x101 [ 93.830125] [] rest_init+0xd7/0xde [ 93.830125] [] ? csum_partial_copy_generic+0x16c/0x16c [ 93.830125] [] start_kernel+0x3dd/0x3ea [ 93.830125] [] x86_64_start_reservations+0xaf/0xb3 [ 93.830125] [] ? early_idt_handlers+0x140/0x140 [ 93.830125] [] x86_64_start_kernel+0x102/0x111 Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=755154 Reported-by: vjain02@students.poly.edu Reported-and-tested-by: Oliver Paukstadt Cc: stable@vger.kernel.org Acked-by: Larry Finger Signed-off-by: Stanislaw Gruszka Signed-off-by: John W. Linville --- drivers/net/wireless/rtlwifi/ps.c | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/drivers/net/wireless/rtlwifi/ps.c b/drivers/net/wireless/rtlwifi/ps.c index a693feffbe72..0b04b2e7b597 100644 --- a/drivers/net/wireless/rtlwifi/ps.c +++ b/drivers/net/wireless/rtlwifi/ps.c @@ -394,7 +394,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw) if (mac->link_state != MAC80211_LINKED) return; - spin_lock(&rtlpriv->locks.lps_lock); + spin_lock_irq(&rtlpriv->locks.lps_lock); /* Idle for a while if we connect to AP a while ago. */ if (mac->cnt_after_linked >= 2) { @@ -406,7 +406,7 @@ void rtl_lps_enter(struct ieee80211_hw *hw) } } - spin_unlock(&rtlpriv->locks.lps_lock); + spin_unlock_irq(&rtlpriv->locks.lps_lock); } /*Leave the leisure power save mode.*/ @@ -415,8 +415,9 @@ void rtl_lps_leave(struct ieee80211_hw *hw) struct rtl_priv *rtlpriv = rtl_priv(hw); struct rtl_ps_ctl *ppsc = rtl_psc(rtl_priv(hw)); struct rtl_hal *rtlhal = rtl_hal(rtl_priv(hw)); + unsigned long flags; - spin_lock(&rtlpriv->locks.lps_lock); + spin_lock_irqsave(&rtlpriv->locks.lps_lock, flags); if (ppsc->fwctrl_lps) { if (ppsc->dot11_psmode != EACTIVE) { @@ -437,7 +438,7 @@ void rtl_lps_leave(struct ieee80211_hw *hw) rtl_lps_set_psmode(hw, EACTIVE); } } - spin_unlock(&rtlpriv->locks.lps_lock); + spin_unlock_irqrestore(&rtlpriv->locks.lps_lock, flags); } /* For sw LPS*/ @@ -538,9 +539,9 @@ void rtl_swlps_rf_awake(struct ieee80211_hw *hw) RT_CLEAR_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM); } - spin_lock(&rtlpriv->locks.lps_lock); + spin_lock_irq(&rtlpriv->locks.lps_lock); rtl_ps_set_rf_state(hw, ERFON, RF_CHANGE_BY_PS); - spin_unlock(&rtlpriv->locks.lps_lock); + spin_unlock_irq(&rtlpriv->locks.lps_lock); } void rtl_swlps_rfon_wq_callback(void *data) @@ -573,9 +574,9 @@ void rtl_swlps_rf_sleep(struct ieee80211_hw *hw) if (rtlpriv->link_info.busytraffic) return; - spin_lock(&rtlpriv->locks.lps_lock); + spin_lock_irq(&rtlpriv->locks.lps_lock); rtl_ps_set_rf_state(hw, ERFSLEEP, RF_CHANGE_BY_PS); - spin_unlock(&rtlpriv->locks.lps_lock); + spin_unlock_irq(&rtlpriv->locks.lps_lock); if (ppsc->reg_rfps_level & RT_RF_OFF_LEVL_ASPM && !RT_IN_PS_LEVEL(ppsc, RT_PS_LEVEL_ASPM)) { From a73228124bed4022d4d4c5663d9679ba2fb99c6c Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Sat, 26 Nov 2011 23:37:43 +0100 Subject: [PATCH 4/6] ath9k: Revert change that broke AR928X on Acer Ferrari One Revert a hunk in drivers/net/wireless/ath/ath9k/hw.c introduced by commit 2577c6e8f2320f1d2f09be122efef5b9118efee4 (ath9k_hw: Add support for AR946/8x chipsets) that caused a nasty regression to appear on my Acer Ferrari One (the box locks up entirely at random times after the wireless has been started without any way to get debug information out of it). Signed-off-by: Rafael J. Wysocki Signed-off-by: John W. Linville --- drivers/net/wireless/ath/ath9k/hw.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath9k/hw.c b/drivers/net/wireless/ath/ath9k/hw.c index b479160dc262..7a9c6f7cd7e1 100644 --- a/drivers/net/wireless/ath/ath9k/hw.c +++ b/drivers/net/wireless/ath/ath9k/hw.c @@ -1826,7 +1826,8 @@ static void ath9k_set_power_sleep(struct ath_hw *ah, int setChip) } /* Clear Bit 14 of AR_WA after putting chip into Full Sleep mode. */ - REG_WRITE(ah, AR_WA, ah->WARegVal & ~AR_WA_D3_L1_DISABLE); + if (AR_SREV_9300_20_OR_LATER(ah)) + REG_WRITE(ah, AR_WA, ah->WARegVal & ~AR_WA_D3_L1_DISABLE); } /* From d305a6557b2c4dca0110f05ffe745b1ef94adb80 Mon Sep 17 00:00:00 2001 From: Nikolay Martynov Date: Mon, 28 Nov 2011 09:18:00 +0100 Subject: [PATCH 5/6] mac80211: fix race condition caused by late addBA response If addBA responses comes in just after addba_resp_timer has expired mac80211 will still accept it and try to open the aggregation session. This causes drivers to be confused and in some cases even crash. This patch fixes the race condition and makes sure that if addba_resp_timer has expired addBA response is not longer accepted and we do not try to open half-closed session. Cc: stable@vger.kernel.org Signed-off-by: Nikolay Martynov [some adjustments] Signed-off-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index 674b345ade81..eea6e5c8d168 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -762,11 +762,27 @@ void ieee80211_process_addba_resp(struct ieee80211_local *local, goto out; } - del_timer(&tid_tx->addba_resp_timer); + del_timer_sync(&tid_tx->addba_resp_timer); #ifdef CONFIG_MAC80211_HT_DEBUG printk(KERN_DEBUG "switched off addBA timer for tid %d\n", tid); #endif + + /* + * addba_resp_timer may have fired before we got here, and + * caused WANT_STOP to be set. If the stop then was already + * processed further, STOPPING might be set. + */ + if (test_bit(HT_AGG_STATE_WANT_STOP, &tid_tx->state) || + test_bit(HT_AGG_STATE_STOPPING, &tid_tx->state)) { +#ifdef CONFIG_MAC80211_HT_DEBUG + printk(KERN_DEBUG + "got addBA resp for tid %d but we already gave up\n", + tid); +#endif + goto out; + } + /* * IEEE 802.11-2007 7.3.1.14: * In an ADDBA Response frame, when the Status Code field From 2a1e0fd175dcfd72096ba9291d31e3b1b5342e60 Mon Sep 17 00:00:00 2001 From: Emmanuel Grumbach Date: Sun, 27 Nov 2011 15:29:44 +0200 Subject: [PATCH 6/6] mac80211: fix race between the AGG SM and the Tx data path When a packet is supposed to sent be as an a-MPDU, mac80211 sets IEEE80211_TX_CTL_AMPDU to let the driver know. On the other hand, mac80211 configures the driver for aggregration with the ampdu_action callback. There is race between these two mechanisms since the following scenario can occur when the BA agreement is torn down: Tx softIRQ drv configuration ========== ================= check OPERATIONAL bit Set the TX_CTL_AMPDU bit in the packet clear OPERATIONAL bit stop Tx AGG Pass Tx packet to the driver. In that case the driver would get a packet with TX_CTL_AMPDU set although it has already been notified that the BA session has been torn down. To fix this, we need to synchronize all the Qdisc activity after we cleared the OPERATIONAL bit. After that step, all the following packets will be buffered until the driver reports it is ready to get new packets for this RA / TID. This buffering allows not to run into another race that would send packets with TX_CTL_AMPDU unset while the driver hasn't been requested to tear down the BA session yet. This race occurs in practice and iwlwifi complains with a WARN_ON when it happens. Cc: stable@kernel.org Signed-off-by: Emmanuel Grumbach Reviewed-by: Johannes Berg Signed-off-by: John W. Linville --- net/mac80211/agg-tx.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/net/mac80211/agg-tx.c b/net/mac80211/agg-tx.c index eea6e5c8d168..331472ce038c 100644 --- a/net/mac80211/agg-tx.c +++ b/net/mac80211/agg-tx.c @@ -192,6 +192,20 @@ int ___ieee80211_stop_tx_ba_session(struct sta_info *sta, u16 tid, */ clear_bit(HT_AGG_STATE_OPERATIONAL, &tid_tx->state); + /* + * There might be a few packets being processed right now (on + * another CPU) that have already gotten past the aggregation + * check when it was still OPERATIONAL and consequently have + * IEEE80211_TX_CTL_AMPDU set. In that case, this code might + * call into the driver at the same time or even before the + * TX paths calls into it, which could confuse the driver. + * + * Wait for all currently running TX paths to finish before + * telling the driver. New packets will not go through since + * the aggregation session is no longer OPERATIONAL. + */ + synchronize_net(); + tid_tx->stop_initiator = initiator; tid_tx->tx_stop = tx;