Re: [PATCH 1/2] mac80211: fix txq queue related crashes

[Ben Greear]

On 01/25/2016 10:35 PM, Michal Kazior wrote:

On 25 January 2016 at 18:59, Ben Greear  wrote:

On 01/21/2016 05:23 AM, Michal Kazior wrote:


The driver can access the queue simultanously
while mac80211 tears down the interface. Without
spinlock protection this could lead to corrupting
sk_buff_head and subsequently to an invalid
pointer dereference.


Hard to know for certain, but this *appears* to fix the unexpectedly large
amount of CE/AXI ath10k firmware crashes that we saw in the 4.2 kernel (4.0
previously
ran much better han 4.2 for us).


That's impossible.

Without wake_tx_queue() txqs aren't even allocated (sdata->vif.txq is NULL).


You are right.  But while testing, one of my guys did find a way to reproduce 
the
crash very quickly in 4.2.  Happens fastest when I use the HTT-MGT variant
of my firmware, but same firmware works good-ish in 4.0.  Seems I have something
to bisect now if I can get a minimal patch to apply each time to enable my
htt-mgt firmware feature...

The latest test case is to just to change the channel of the AP while station
is connected.  Station sends some null-funcs, firmware resets it's low-level
stuff a bunch because it doesn't get AKCs, then CE/AXI crashes.  Could be
my firmware or kernel modifications of course, though similar crash scenarios 
have been seen forever
in all sorts of firmwares and kernels.

Thanks,
Ben


--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mac80211: fix txq queue related crashes

[Johannes Berg]

On Thu, 2016-01-21 at 14:23 +0100, Michal Kazior wrote:
> The driver can access the queue simultanously
> while mac80211 tears down the interface. Without
> spinlock protection this could lead to corrupting
> sk_buff_head and subsequently to an invalid
> pointer dereference.
> 
Applied.

johannes
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mac80211: fix txq queue related crashes

[Michal Kazior]

On 25 January 2016 at 18:59, Ben Greear  wrote:
> On 01/21/2016 05:23 AM, Michal Kazior wrote:
>>
>> The driver can access the queue simultanously
>> while mac80211 tears down the interface. Without
>> spinlock protection this could lead to corrupting
>> sk_buff_head and subsequently to an invalid
>> pointer dereference.
>
> Hard to know for certain, but this *appears* to fix the unexpectedly large
> amount of CE/AXI ath10k firmware crashes that we saw in the 4.2 kernel (4.0
> previously
> ran much better han 4.2 for us).

That's impossible.

Without wake_tx_queue() txqs aren't even allocated (sdata->vif.txq is NULL).


Michał
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 1/2] mac80211: fix txq queue related crashes

[Ben Greear]

On 01/21/2016 05:23 AM, Michal Kazior wrote:

The driver can access the queue simultanously
while mac80211 tears down the interface. Without
spinlock protection this could lead to corrupting
sk_buff_head and subsequently to an invalid
pointer dereference.


Hard to know for certain, but this *appears* to fix the unexpectedly large
amount of CE/AXI ath10k firmware crashes that we saw in the 4.2 kernel (4.0 
previously
ran much better han 4.2 for us).

We'll continue testing, in case we are just getting lucky so far.

Thanks,
Ben



Fixes: ba8c3d6f16a1 ("mac80211: add an intermediate software queue 
implementation")
Signed-off-by: Michal Kazior 
---
  net/mac80211/iface.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 33ae3c81bfc5..0451f120746e 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -977,7 +977,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);

+   spin_lock_bh(>queue.lock);
ieee80211_purge_tx_queue(>hw, >queue);
+   spin_unlock_bh(>queue.lock);
+
atomic_set(>txqs_len[txqi->txq.ac], 0);
}





--
Ben Greear 
Candela Technologies Inc  http://www.candelatech.com

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 1/2] mac80211: fix txq queue related crashes

[Michal Kazior]

The driver can access the queue simultanously
while mac80211 tears down the interface. Without
spinlock protection this could lead to corrupting
sk_buff_head and subsequently to an invalid
pointer dereference.

Fixes: ba8c3d6f16a1 ("mac80211: add an intermediate software queue 
implementation")
Signed-off-by: Michal Kazior 
---
 net/mac80211/iface.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mac80211/iface.c b/net/mac80211/iface.c
index 33ae3c81bfc5..0451f120746e 100644
--- a/net/mac80211/iface.c
+++ b/net/mac80211/iface.c
@@ -977,7 +977,10 @@ static void ieee80211_do_stop(struct ieee80211_sub_if_data 
*sdata,
if (sdata->vif.txq) {
struct txq_info *txqi = to_txq_info(sdata->vif.txq);
 
+   spin_lock_bh(>queue.lock);
ieee80211_purge_tx_queue(>hw, >queue);
+   spin_unlock_bh(>queue.lock);
+
atomic_set(>txqs_len[txqi->txq.ac], 0);
}
 
-- 
2.1.4

--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html