ath9k_htc: fix skb_under_panic error

Commit Message

Helmut Schaa June 5, 2013, 2:24 p.m. UTC

On Tue, Jun 4, 2013 at 8:37 PM, Oleksij Rempel <[e-mail]> wrote:
> This error seems to be really rare, and we do not know real couse of it.
> But, in any case, we should check size of head before reducing it.

Mind to try the (completely untested) patch against wireless-testing instead?
Helmut

---
Subject: [PATCH] ath9k_htc: Restore skb headroom when returning skb to mac80211

ath9k_htc adds padding between the 802.11 header and the payload during
TX by moving the header. When handing the frame back to mac80211 for TX
status handling the header is not moved back into its original position.
This can result in a too small skb headroom when entering ath9k_htc
again (due to a soft retransmission for example) causing an
skb_under_panic oops.

Fix this by moving the 802.11 header back into its original position
before returning the frame to mac80211 as other drivers like rt2x00
or ath5k do.

Signed-off-by: Helmut Schaa <[e-mail]>
---
 drivers/net/wireless/ath/ath9k/htc_drv_txrx.c |   11 +++++++++++
 1 file changed, 11 insertions(+)

     if (slot < 0) {
@@ -504,6 +506,15 @@ send_mac80211:

     ath9k_htc_tx_clear_slot(priv, slot);

+    /* Remove padding before handing frame back to mac80211 */
+    hdr = (struct ieee80211_hdr *) skb->data;
+    padpos = ieee80211_hdrlen(hdr->frame_control);
+    padsize = padpos & 3;
+    if (padsize && skb->len > padpos + padsize) {
+        memmove(skb->data + padsize, skb->data, padpos);
+        skb_pull(skb, padsize);
+    }
+
     /* Send status to mac80211 */
     ieee80211_tx_status(priv->hw, skb);
 }
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Marc Kleine-Budde June 5, 2013, 2:26 p.m. UTC | #1

On 06/05/2013 04:24 PM, Helmut Schaa wrote:
> On Tue, Jun 4, 2013 at 8:37 PM, Oleksij Rempel <[e-mail]> wrote:
>> This error seems to be really rare, and we do not know real couse of it.
>> But, in any case, we should check size of head before reducing it.
> 
> Mind to try the (completely untested) patch against wireless-testing instead?
> Helmut

I will do, however I'm not in range of that USB wireless adapter for
about 1,5 weeks.

Marc

Oleksij Rempel June 5, 2013, 2:46 p.m. UTC | #2

Am 05.06.2013 16:26, schrieb Marc Kleine-Budde:
> On 06/05/2013 04:24 PM, Helmut Schaa wrote:
>> On Tue, Jun 4, 2013 at 8:37 PM, Oleksij Rempel <[e-mail]> wrote:
>>> This error seems to be really rare, and we do not know real couse of it.
>>> But, in any case, we should check size of head before reducing it.
>>
>> Mind to try the (completely untested) patch against wireless-testing instead?
>> Helmut
>
> I will do, however I'm not in range of that USB wireless adapter for
> about 1,5 weeks.

Helmut, thank you for patch!

i'll do regression test, but not week long test. So i probably won't 
reproduce this issue.

Oleksij Rempel June 5, 2013, 5:03 p.m. UTC | #3

Am 05.06.2013 16:46, schrieb Oleksij Rempel:
> Am 05.06.2013 16:26, schrieb Marc Kleine-Budde:
>> On 06/05/2013 04:24 PM, Helmut Schaa wrote:
>>> On Tue, Jun 4, 2013 at 8:37 PM, Oleksij Rempel
>>> <[e-mail]> wrote:
>>>> This error seems to be really rare, and we do not know real couse of
>>>> it.
>>>> But, in any case, we should check size of head before reducing it.
>>>
>>> Mind to try the (completely untested) patch against wireless-testing
>>> instead?
>>> Helmut
>>
>> I will do, however I'm not in range of that USB wireless adapter for
>> about 1,5 weeks.
>
> Helmut, thank you for patch!
>
> i'll do regression test, but not week long test. So i probably won't
> reproduce this issue.

I was running two stream netperf test for 2 hours without visible 
regressions.

Helmut Schaa June 6, 2013, 11:48 a.m. UTC | #4

On Wed, Jun 5, 2013 at 7:03 PM, Oleksij Rempel <[e-mail]> wrote:
> I was running two stream netperf test for 2 hours without visible
> regressions.

With or without your pskb_expand_head patch applied?

Thanks,
Helmut
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to [e-mail]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Oleksij Rempel June 6, 2013, 12:06 p.m. UTC | #5

Am 06.06.2013 13:48, schrieb Helmut Schaa:
> On Wed, Jun 5, 2013 at 7:03 PM, Oleksij Rempel <[e-mail]> wrote:
>> I was running two stream netperf test for 2 hours without visible
>> regressions.
>
> With or without your pskb_expand_head patch applied?
>
> Thanks,
> Helmut
>

whithout my patch, on to of wireless-testing master git. But i didn't 
had this problem before. So, may be this scenario was not used by me.
Shouldn't we actually have this check from my patch, to avoid other oopses?

Marc Kleine-Budde Aug. 16, 2013, 7:32 p.m. UTC | #6

Hello,

On 06/05/2013 04:24 PM, Helmut Schaa wrote:
> On Tue, Jun 4, 2013 at 8:37 PM, Oleksij Rempel <[e-mail]> wrote:
>> This error seems to be really rare, and we do not know real couse of it.
>> But, in any case, we should check size of head before reducing it.
> 
> Mind to try the (completely untested) patch against wireless-testing instead?
> Helmut

I'm running a kernel with a slightly modified version of that patch for
4 weeks without problems so far. I'll send a mail with that patch.

Marc

Patch

diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
index e602c95..666cfb6 100644
--- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
+++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
@@ -448,6 +448,8 @@  static void ath9k_htc_tx_process(struct
ath9k_htc_priv *priv,
     struct ieee80211_conf *cur_conf = &priv->hw->conf;
     bool txok;
     int slot;
+    struct ieee80211_hdr *hdr;
+    int padpos, padsize;

     slot = strip_drv_header(priv, skb);