2010-12-07 05:13:58

by greearb

[permalink] [raw]
Subject: [PATCH] ath9k: Check for NULL sta in ath_tx_start

From: Ben Greear <[email protected]>

It can be NULL according to docs, and logging showed it
to be NULL in practice.

Signed-off-by: Ben Greear <[email protected]>
---
:100644 100644 8b0b076... 5ddca08... M drivers/net/wireless/ath/ath9k/xmit.c
drivers/net/wireless/ath/ath9k/xmit.c | 5 ++++-
1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/xmit.c b/drivers/net/wireless/ath/ath9k/xmit.c
index 8b0b076..5ddca08 100644
--- a/drivers/net/wireless/ath/ath9k/xmit.c
+++ b/drivers/net/wireless/ath/ath9k/xmit.c
@@ -1764,7 +1764,10 @@ int ath_tx_start(struct ieee80211_hw *hw, struct sk_buff *skb,
int frmlen = skb->len + FCS_LEN;
int q;

- txctl->an = (struct ath_node *)sta->drv_priv;
+ /* NOTE: sta can be NULL according to net/mac80211.h */
+ if (sta)
+ txctl->an = (struct ath_node *)sta->drv_priv;
+
if (info->control.hw_key)
frmlen += info->control.hw_key->icv_len;

--
1.7.2.3



2010-12-07 17:13:47

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On 12/07/2010 12:31 AM, Vasanthakumar Thiagarajan wrote:
> On Tue, Dec 07, 2010 at 01:19:22PM +0530, Ben Greear wrote:
>> On 12/06/2010 11:42 PM, Vasanthakumar Thiagarajan wrote:
>>> On Tue, Dec 07, 2010 at 11:06:24AM +0530, Ben Greear wrote:
>>>> On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
>>>>> On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
>>>>>> From: Ben Greear<[email protected]>
>>>>>>
>>>>>> It can be NULL according to docs, and logging showed it
>>>>>> to be NULL in practice.
>>>>>>
>>>>>> Signed-off-by: Ben Greear<[email protected]>
>>>>>
>>>>> Does this fix an oops? If so can you explain and provide the trace and
>>>>> resubmit and cc stable in the commit log?
>>>>
>>>> I think it fixes the TID corruption I posted about earlier. It seems
>>>> so obvious though, that I'm curious why no one else sees problems,
>>>> and why I didn't see more crashes in my setup.
>>>>
>>>> (The paprd code appears to send with null STA, for instance, and my
>>>> printks showed lots of NULL stas in my 16-sta test setup, though I
>>>> don't think I'm using the paprd code path.)
>>>
>>> paprd is used only with>= AR9003.
>>
>> Whoever coded it up hopefully had that hardware...so why didn't
>> they see lots of crashes?
>
> I myself tested it lot of times, but did not see any crash, weird.

Looks like the offending change when in recently (11/4/10, one of Felix's
patches).

That is probably why no one else is hitting this yet, and it
isn't needed for stable I'm guessing....

Thanks,
Ben

>
> Vasanth
>
>>
>> Thanks,
>> Ben
>>
>>>
>>> Vasanth
>>
>>
>> --
>> Ben Greear<[email protected]>
>> 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 [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com


Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On Tue, Dec 07, 2010 at 10:43:38PM +0530, Ben Greear wrote:
> On 12/07/2010 12:31 AM, Vasanthakumar Thiagarajan wrote:
> > On Tue, Dec 07, 2010 at 01:19:22PM +0530, Ben Greear wrote:
> >> On 12/06/2010 11:42 PM, Vasanthakumar Thiagarajan wrote:
> >>> On Tue, Dec 07, 2010 at 11:06:24AM +0530, Ben Greear wrote:
> >>>> On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
> >>>>> On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
> >>>>>> From: Ben Greear<[email protected]>
> >>>>>>
> >>>>>> It can be NULL according to docs, and logging showed it
> >>>>>> to be NULL in practice.
> >>>>>>
> >>>>>> Signed-off-by: Ben Greear<[email protected]>
> >>>>>
> >>>>> Does this fix an oops? If so can you explain and provide the trace and
> >>>>> resubmit and cc stable in the commit log?
> >>>>
> >>>> I think it fixes the TID corruption I posted about earlier. It seems
> >>>> so obvious though, that I'm curious why no one else sees problems,
> >>>> and why I didn't see more crashes in my setup.
> >>>>
> >>>> (The paprd code appears to send with null STA, for instance, and my
> >>>> printks showed lots of NULL stas in my 16-sta test setup, though I
> >>>> don't think I'm using the paprd code path.)
> >>>
> >>> paprd is used only with>= AR9003.
> >>
> >> Whoever coded it up hopefully had that hardware...so why didn't
> >> they see lots of crashes?
> >
> > I myself tested it lot of times, but did not see any crash, weird.
>
> Looks like the offending change when in recently (11/4/10, one of Felix's
> patches).
>
> That is probably why no one else is hitting this yet, and it
> isn't needed for stable I'm guessing....

Nice, care to send a patch?. Yeah, it is not needed for stable.

Vasanth

2010-12-07 05:21:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On Mon, Dec 6, 2010 at 9:13 PM, <[email protected]> wrote:
> From: Ben Greear <[email protected]>
>
> It can be NULL according to docs, and logging showed it
> to be NULL in practice.
>
> Signed-off-by: Ben Greear <[email protected]>

Does this fix an oops? If so can you explain and provide the trace and
resubmit and cc stable in the commit log?

Luis

2010-12-07 07:49:32

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On 12/06/2010 11:42 PM, Vasanthakumar Thiagarajan wrote:
> On Tue, Dec 07, 2010 at 11:06:24AM +0530, Ben Greear wrote:
>> On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
>>> On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
>>>> From: Ben Greear<[email protected]>
>>>>
>>>> It can be NULL according to docs, and logging showed it
>>>> to be NULL in practice.
>>>>
>>>> Signed-off-by: Ben Greear<[email protected]>
>>>
>>> Does this fix an oops? If so can you explain and provide the trace and
>>> resubmit and cc stable in the commit log?
>>
>> I think it fixes the TID corruption I posted about earlier. It seems
>> so obvious though, that I'm curious why no one else sees problems,
>> and why I didn't see more crashes in my setup.
>>
>> (The paprd code appears to send with null STA, for instance, and my
>> printks showed lots of NULL stas in my 16-sta test setup, though I
>> don't think I'm using the paprd code path.)
>
> paprd is used only with>= AR9003.

Whoever coded it up hopefully had that hardware...so why didn't
they see lots of crashes?

Thanks,
Ben

>
> Vasanth


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

2010-12-07 05:36:30

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
> On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
>> From: Ben Greear<[email protected]>
>>
>> It can be NULL according to docs, and logging showed it
>> to be NULL in practice.
>>
>> Signed-off-by: Ben Greear<[email protected]>
>
> Does this fix an oops? If so can you explain and provide the trace and
> resubmit and cc stable in the commit log?

I think it fixes the TID corruption I posted about earlier. It seems
so obvious though, that I'm curious why no one else sees problems,
and why I didn't see more crashes in my setup.

(The paprd code appears to send with null STA, for instance, and my
printks showed lots of NULL stas in my 16-sta test setup, though I
don't think I'm using the paprd code path.)

I'll test some more and re-post tomorrow if all goes well.

Thanks,
Ben

--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On Tue, Dec 07, 2010 at 11:06:24AM +0530, Ben Greear wrote:
> On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
> > On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
> >> From: Ben Greear<[email protected]>
> >>
> >> It can be NULL according to docs, and logging showed it
> >> to be NULL in practice.
> >>
> >> Signed-off-by: Ben Greear<[email protected]>
> >
> > Does this fix an oops? If so can you explain and provide the trace and
> > resubmit and cc stable in the commit log?
>
> I think it fixes the TID corruption I posted about earlier. It seems
> so obvious though, that I'm curious why no one else sees problems,
> and why I didn't see more crashes in my setup.
>
> (The paprd code appears to send with null STA, for instance, and my
> printks showed lots of NULL stas in my 16-sta test setup, though I
> don't think I'm using the paprd code path.)

paprd is used only with >= AR9003.

Vasanth

2010-12-08 15:41:04

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On 12/07/2010 09:17 PM, Vasanthakumar Thiagarajan wrote:
> On Tue, Dec 07, 2010 at 10:43:38PM +0530, Ben Greear wrote:
>> On 12/07/2010 12:31 AM, Vasanthakumar Thiagarajan wrote:
>>> On Tue, Dec 07, 2010 at 01:19:22PM +0530, Ben Greear wrote:
>>>> On 12/06/2010 11:42 PM, Vasanthakumar Thiagarajan wrote:
>>>>> On Tue, Dec 07, 2010 at 11:06:24AM +0530, Ben Greear wrote:
>>>>>> On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
>>>>>>> On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
>>>>>>>> From: Ben Greear<[email protected]>
>>>>>>>>
>>>>>>>> It can be NULL according to docs, and logging showed it
>>>>>>>> to be NULL in practice.
>>>>>>>>
>>>>>>>> Signed-off-by: Ben Greear<[email protected]>
>>>>>>>
>>>>>>> Does this fix an oops? If so can you explain and provide the trace and
>>>>>>> resubmit and cc stable in the commit log?
>>>>>>
>>>>>> I think it fixes the TID corruption I posted about earlier. It seems
>>>>>> so obvious though, that I'm curious why no one else sees problems,
>>>>>> and why I didn't see more crashes in my setup.
>>>>>>
>>>>>> (The paprd code appears to send with null STA, for instance, and my
>>>>>> printks showed lots of NULL stas in my 16-sta test setup, though I
>>>>>> don't think I'm using the paprd code path.)
>>>>>
>>>>> paprd is used only with>= AR9003.
>>>>
>>>> Whoever coded it up hopefully had that hardware...so why didn't
>>>> they see lots of crashes?
>>>
>>> I myself tested it lot of times, but did not see any crash, weird.
>>
>> Looks like the offending change when in recently (11/4/10, one of Felix's
>> patches).
>>
>> That is probably why no one else is hitting this yet, and it
>> isn't needed for stable I'm guessing....
>
> Nice, care to send a patch?. Yeah, it is not needed for stable.

The same patch I originally posted to to start this thread
should do just fine.

If someone really wants an oops report, I can add an artificial BUG_ON
to catch this, but for whatever reason, it normally just crashes things
weirdly and perhaps sometimes screwed up the file-system, so I'd rather
just have the patch go in as it is..I think it is obviously more correct
than the existing code.

Thanks,
Ben

>
> Vasanth


--
Ben Greear <[email protected]>
Candela Technologies Inc http://www.candelatech.com

Subject: Re: [PATCH] ath9k: Check for NULL sta in ath_tx_start

On Tue, Dec 07, 2010 at 01:19:22PM +0530, Ben Greear wrote:
> On 12/06/2010 11:42 PM, Vasanthakumar Thiagarajan wrote:
> > On Tue, Dec 07, 2010 at 11:06:24AM +0530, Ben Greear wrote:
> >> On 12/06/2010 09:21 PM, Luis R. Rodriguez wrote:
> >>> On Mon, Dec 6, 2010 at 9:13 PM,<[email protected]> wrote:
> >>>> From: Ben Greear<[email protected]>
> >>>>
> >>>> It can be NULL according to docs, and logging showed it
> >>>> to be NULL in practice.
> >>>>
> >>>> Signed-off-by: Ben Greear<[email protected]>
> >>>
> >>> Does this fix an oops? If so can you explain and provide the trace and
> >>> resubmit and cc stable in the commit log?
> >>
> >> I think it fixes the TID corruption I posted about earlier. It seems
> >> so obvious though, that I'm curious why no one else sees problems,
> >> and why I didn't see more crashes in my setup.
> >>
> >> (The paprd code appears to send with null STA, for instance, and my
> >> printks showed lots of NULL stas in my 16-sta test setup, though I
> >> don't think I'm using the paprd code path.)
> >
> > paprd is used only with>= AR9003.
>
> Whoever coded it up hopefully had that hardware...so why didn't
> they see lots of crashes?

I myself tested it lot of times, but did not see any crash, weird.

Vasanth

>
> Thanks,
> Ben
>
> >
> > Vasanth
>
>
> --
> Ben Greear <[email protected]>
> Candela Technologies Inc http://www.candelatech.com