[Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch

[Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch

[Dave Taht]

yea! (I think)


---------- Forwarded message ----------
From: Michal Kazior <michal.kazior@tieto.com>
Date: Thu, Jan 12, 2017 at 6:28 AM
Subject: [PATCH] mac80211: prevent skb/txq mismatch
To: johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org, greearb@candelatech.com,
mohammed@qti.qualcomm.com, Michal Kazior <michal.kazior@tieto.com>


Station structure is considered as not uploaded
(to driver) until drv_sta_state() finishes. This
call is however done after the structure is
attached to mac80211 internal lists and hashes.
This means mac80211 can lookup (and use) station
structure before it is uploaded to a driver.

If this happens (structure exists, but
sta->uploaded is false) fast_tx path can still be
taken. Deep in the fastpath call the sta->uploaded
is checked against to derive "pubsta" argument for
ieee80211_get_txq(). If sta->uploaded is false
(and sta is actually non-NULL) ieee80211_get_txq()
effectively downgraded to vif->txq.

At first glance this may look innocent but coerces
mac80211 into a state that is almost guaranteed
(codel may drop offending skb) to crash because a
station-oriented skb gets queued up on
vif-oriented txq. The ieee80211_tx_dequeue() ends
up looking at info->control.flags and tries to use
txq->sta which in the fail case is NULL.

It's probably pointless to pretend one can
downgrade skb from sta-txq to vif-txq.

Only drivers using wake_tx_queue were affected.

Example crash dump before fix:

 Unable to handle kernel paging request at virtual address ffffe26c
 PC is at ieee80211_tx_dequeue+0x204/0x690 [mac80211]
 [<bf4252a4>] (ieee80211_tx_dequeue [mac80211]) from
 [<bf4b1388>] (ath10k_mac_tx_push_txq+0x54/0x1c0 [ath10k_core])
 [<bf4b1388>] (ath10k_mac_tx_push_txq [ath10k_core]) from
 [<bf4bdfbc>] (ath10k_htt_txrx_compl_task+0xd78/0x11d0 [ath10k_core])
 [<bf4bdfbc>] (ath10k_htt_txrx_compl_task [ath10k_core])
 [<bf51c5a4>] (ath10k_pci_napi_poll+0x54/0xe8 [ath10k_pci])
 [<bf51c5a4>] (ath10k_pci_napi_poll [ath10k_pci]) from
 [<c0572e90>] (net_rx_action+0xac/0x160)

Reported-by: Mohammed Shafi Shajakhan <mohammed@qti.qualcomm.com>
Signed-off-by: Michal Kazior <michal.kazior@tieto.com>
---
 net/mac80211/tx.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 4dea18be385c..c77fcf83d004 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -1244,13 +1244,16 @@ ieee80211_tx_prepare(struct
ieee80211_sub_if_data *sdata,

 static struct txq_info *ieee80211_get_txq(struct ieee80211_local *local,
                                          struct ieee80211_vif *vif,
-                                         struct ieee80211_sta *pubsta,
+                                         struct sta_info *sta,
                                          struct sk_buff *skb)
 {
        struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
        struct ieee80211_tx_info *info = IEEE80211_SKB_CB(skb);
        struct ieee80211_txq *txq = NULL;

+       if (sta && !sta->uploaded)
+               return NULL;
+
        if ((info->flags & IEEE80211_TX_CTL_SEND_AFTER_DTIM) ||
            (info->control.flags & IEEE80211_TX_CTRL_PS_RESPONSE))
                return NULL;
@@ -1258,10 +1261,10 @@ static struct txq_info
*ieee80211_get_txq(struct ieee80211_local *local,
        if (!ieee80211_is_data(hdr->frame_control))
                return NULL;

-       if (pubsta) {
+       if (sta) {
                u8 tid = skb->priority & IEEE80211_QOS_CTL_TID_MASK;

-               txq = pubsta->txq[tid];
+               txq = sta->sta.txq[tid];
        } else if (vif) {
                txq = vif->txq;
        }
@@ -1504,23 +1507,17 @@ static bool ieee80211_queue_skb(struct
ieee80211_local *local,
        struct fq *fq = &local->fq;
        struct ieee80211_vif *vif;
        struct txq_info *txqi;
-       struct ieee80211_sta *pubsta;

        if (!local->ops->wake_tx_queue ||
            sdata->vif.type == NL80211_IFTYPE_MONITOR)
                return false;

-       if (sta && sta->uploaded)
-               pubsta = &sta->sta;
-       else
-               pubsta = NULL;
-
        if (sdata->vif.type == NL80211_IFTYPE_AP_VLAN)
                sdata = container_of(sdata->bss,
                                     struct ieee80211_sub_if_data, u.ap);

        vif = &sdata->vif;
-       txqi = ieee80211_get_txq(local, vif, pubsta, skb);
+       txqi = ieee80211_get_txq(local, vif, sta, skb);

        if (!txqi)
                return false;
--
2.1.4



-- 
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org

Re: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch

[Jon Pike]

[-- Attachment #1: Type: text/plain, Size: 2454 bytes --]

I'm pretty clueless on this,  but as a single data point, my Archer C7 has
abt 21 days of uptime now, since my last update, as the single router in a
4 person household.  I've been running cake/piece of cake the whole time.

Standard disclaimers apply as to clueless user not knowing if his FW
version has anything to do with the issue in question, or if its connected
to other things like whatever is going on with the airtime fairness.  But
the comment in the third paragraph sounded interesting in light of my lack
of a crash and running a SQM method, FWIW.

My last update was a sysupgrade to LEDE Reboot SNAPSHOT r2687-dc5f496
pretty sure date was 12-27-16.


Today's Topics:

   1. Fwd: [PATCH] mac80211: prevent skb/txq mismatch (Dave Taht)


---------- Forwarded message ----------
From: Dave Taht <dave.taht@gmail.com>
To: make-wifi-fast@lists.bufferbloat.net
Cc:
Date: Thu, 12 Jan 2017 10:44:12 -0800
Subject: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch
yea! (I think)


---------- Forwarded message ----------
From: Michal Kazior <michal.kazior@tieto.com>
Date: Thu, Jan 12, 2017 at 6:28 AM
Subject: [PATCH] mac80211: prevent skb/txq mismatch
To: johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org, greearb@candelatech.com,
mohammed@qti.qualcomm.com, Michal Kazior <michal.kazior@tieto.com>


Station structure is considered as not uploaded
(to driver) until drv_sta_state() finishes. This
call is however done after the structure is
attached to mac80211 internal lists and hashes.
This means mac80211 can lookup (and use) station
structure before it is uploaded to a driver.

If this happens (structure exists, but
sta->uploaded is false) fast_tx path can still be
taken. Deep in the fastpath call the sta->uploaded
is checked against to derive "pubsta" argument for
ieee80211_get_txq(). If sta->uploaded is false
(and sta is actually non-NULL) ieee80211_get_txq()
effectively downgraded to vif->txq.

At first glance this may look innocent but coerces
mac80211 into a state that is almost guaranteed
(codel may drop offending skb) to crash because a
station-oriented skb gets queued up on
vif-oriented txq. The ieee80211_tx_dequeue() ends
up looking at info->control.flags and tries to use
txq->sta which in the fail case is NULL.
..........

_______________________________________________
Make-wifi-fast mailing list
Make-wifi-fast@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/make-wifi-fast

[-- Attachment #2: Type: text/html, Size: 3918 bytes --]

Re: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch

[Jon Pike]

[-- Attachment #1: Type: text/plain, Size: 3842 bytes --]

And,  it is probably worth mentioning that I also have IPV6 routing
disabled, after what seemed like an issue with a local device being overly
chatty.

Have also reread a bit,  and see that Jan 3 or 4 seems to be the date of
ATF removal, so I assume I still have it.

On Jan 18, 2017 9:00 AM, <make-wifi-fast-request@lists.bufferbloat.net>
wrote:

Send Make-wifi-fast mailing list submissions to
        make-wifi-fast@lists.bufferbloat.net

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.bufferbloat.net/listinfo/make-wifi-fast
or, via email, send a message with subject or body 'help' to
        make-wifi-fast-request@lists.bufferbloat.net

You can reach the person managing the list at
        make-wifi-fast-owner@lists.bufferbloat.net

When replying, please edit your Subject line so it is more specific
than "Re: Contents of Make-wifi-fast digest..."

Today's Topics:

   1. Re: Fwd: [PATCH] mac80211: prevent skb/txq mismatch (Jon Pike)


---------- Forwarded message ----------
From: Jon Pike <jonpike54@gmail.com>
To: make-wifi-fast@lists.bufferbloat.net
Cc:
Date: Tue, 17 Jan 2017 22:58:30 -0800
Subject: Re: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq
mismatch
I'm pretty clueless on this,  but as a single data point, my Archer C7 has
abt 21 days of uptime now, since my last update, as the single router in a
4 person household.  I've been running cake/piece of cake the whole time.

Standard disclaimers apply as to clueless user not knowing if his FW
version has anything to do with the issue in question, or if its connected
to other things like whatever is going on with the airtime fairness.  But
the comment in the third paragraph sounded interesting in light of my lack
of a crash and running a SQM method, FWIW.

My last update was a sysupgrade to LEDE Reboot SNAPSHOT r2687-dc5f496
pretty sure date was 12-27-16.


Today's Topics:

   1. Fwd: [PATCH] mac80211: prevent skb/txq mismatch (Dave Taht)


---------- Forwarded message ----------
From: Dave Taht <dave.taht@gmail.com>
To: make-wifi-fast@lists.bufferbloat.net
Cc:
Date: Thu, 12 Jan 2017 10:44:12 -0800
Subject: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch
yea! (I think)


---------- Forwarded message ----------
From: Michal Kazior <michal.kazior@tieto.com>
Date: Thu, Jan 12, 2017 at 6:28 AM
Subject: [PATCH] mac80211: prevent skb/txq mismatch
To: johannes@sipsolutions.net
Cc: linux-wireless@vger.kernel.org, greearb@candelatech.com,
mohammed@qti.qualcomm.com, Michal Kazior <michal.kazior@tieto.com>


Station structure is considered as not uploaded
(to driver) until drv_sta_state() finishes. This
call is however done after the structure is
attached to mac80211 internal lists and hashes.
This means mac80211 can lookup (and use) station
structure before it is uploaded to a driver.

If this happens (structure exists, but
sta->uploaded is false) fast_tx path can still be
taken. Deep in the fastpath call the sta->uploaded
is checked against to derive "pubsta" argument for
ieee80211_get_txq(). If sta->uploaded is false
(and sta is actually non-NULL) ieee80211_get_txq()
effectively downgraded to vif->txq.

At first glance this may look innocent but coerces
mac80211 into a state that is almost guaranteed
(codel may drop offending skb) to crash because a
station-oriented skb gets queued up on
vif-oriented txq. The ieee80211_tx_dequeue() ends
up looking at info->control.flags and tries to use
txq->sta which in the fail case is NULL.
..........

_______________________________________________
Make-wifi-fast mailing list
Make-wifi-fast@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/make-wifi-fast



_______________________________________________
Make-wifi-fast mailing list
Make-wifi-fast@lists.bufferbloat.net
https://lists.bufferbloat.net/listinfo/make-wifi-fast

[-- Attachment #2: Type: text/html, Size: 6565 bytes --]

Re: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch

[Dave Taht]

If everybody could track their experiences here, that would help.

https://bugs.lede-project.org/index.php?do=details&task_id=368


On Wed, Jan 18, 2017 at 9:38 AM, Jon Pike <jonpike54@gmail.com> wrote:
> And,  it is probably worth mentioning that I also have IPV6 routing
> disabled, after what seemed like an issue with a local device being overly
> chatty.
>
> Have also reread a bit,  and see that Jan 3 or 4 seems to be the date of ATF
> removal, so I assume I still have it.
>
> On Jan 18, 2017 9:00 AM, <make-wifi-fast-request@lists.bufferbloat.net>
> wrote:
>
> Send Make-wifi-fast mailing list submissions to
>         make-wifi-fast@lists.bufferbloat.net
>
> To subscribe or unsubscribe via the World Wide Web, visit
>         https://lists.bufferbloat.net/listinfo/make-wifi-fast
> or, via email, send a message with subject or body 'help' to
>         make-wifi-fast-request@lists.bufferbloat.net
>
> You can reach the person managing the list at
>         make-wifi-fast-owner@lists.bufferbloat.net
>
> When replying, please edit your Subject line so it is more specific
> than "Re: Contents of Make-wifi-fast digest..."
>
> Today's Topics:
>
>    1. Re: Fwd: [PATCH] mac80211: prevent skb/txq mismatch (Jon Pike)
>
>
>
> ---------- Forwarded message ----------
> From: Jon Pike <jonpike54@gmail.com>
> To: make-wifi-fast@lists.bufferbloat.net
> Cc:
> Date: Tue, 17 Jan 2017 22:58:30 -0800
> Subject: Re: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq
> mismatch
> I'm pretty clueless on this,  but as a single data point, my Archer C7 has
> abt 21 days of uptime now, since my last update, as the single router in a 4
> person household.  I've been running cake/piece of cake the whole time.
>
> Standard disclaimers apply as to clueless user not knowing if his FW version
> has anything to do with the issue in question, or if its connected to other
> things like whatever is going on with the airtime fairness.  But the comment
> in the third paragraph sounded interesting in light of my lack of a crash
> and running a SQM method, FWIW.
>
> My last update was a sysupgrade to LEDE Reboot SNAPSHOT r2687-dc5f496 pretty
> sure date was 12-27-16.
>
>
> Today's Topics:
>
>    1. Fwd: [PATCH] mac80211: prevent skb/txq mismatch (Dave Taht)
>
>
> ---------- Forwarded message ----------
> From: Dave Taht <dave.taht@gmail.com>
> To: make-wifi-fast@lists.bufferbloat.net
> Cc:
> Date: Thu, 12 Jan 2017 10:44:12 -0800
> Subject: [Make-wifi-fast] Fwd: [PATCH] mac80211: prevent skb/txq mismatch
> yea! (I think)
>
>
> ---------- Forwarded message ----------
> From: Michal Kazior <michal.kazior@tieto.com>
> Date: Thu, Jan 12, 2017 at 6:28 AM
> Subject: [PATCH] mac80211: prevent skb/txq mismatch
> To: johannes@sipsolutions.net
> Cc: linux-wireless@vger.kernel.org, greearb@candelatech.com,
> mohammed@qti.qualcomm.com, Michal Kazior <michal.kazior@tieto.com>
>
>
> Station structure is considered as not uploaded
> (to driver) until drv_sta_state() finishes. This
> call is however done after the structure is
> attached to mac80211 internal lists and hashes.
> This means mac80211 can lookup (and use) station
> structure before it is uploaded to a driver.
>
> If this happens (structure exists, but
> sta->uploaded is false) fast_tx path can still be
> taken. Deep in the fastpath call the sta->uploaded
> is checked against to derive "pubsta" argument for
> ieee80211_get_txq(). If sta->uploaded is false
> (and sta is actually non-NULL) ieee80211_get_txq()
> effectively downgraded to vif->txq.
>
> At first glance this may look innocent but coerces
> mac80211 into a state that is almost guaranteed
> (codel may drop offending skb) to crash because a
> station-oriented skb gets queued up on
> vif-oriented txq. The ieee80211_tx_dequeue() ends
> up looking at info->control.flags and tries to use
> txq->sta which in the fail case is NULL.
> ..........
>
> _______________________________________________
> Make-wifi-fast mailing list
> Make-wifi-fast@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/make-wifi-fast
>
>
>
> _______________________________________________
> Make-wifi-fast mailing list
> Make-wifi-fast@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/make-wifi-fast
>
>
>
> _______________________________________________
> Make-wifi-fast mailing list
> Make-wifi-fast@lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/make-wifi-fast



-- 
Dave Täht
Let's go make home routers and wifi faster! With better software!
http://blog.cerowrt.org