2010-12-01 11:00:19

by Senthil Balasubramanian

[permalink] [raw]
Subject: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
this status for MIC failed frames is buggy.

Due to this, MIC failure events for broadcast frames are not sent to
supplicant resulted in AP disconnecting the STA.

Able to pass Wifi Test case 5.2.18 with this fix.

Cc: Stable <[email protected]> (2.6.36+)
Cc: Kyungwan Nam <[email protected]>
Signed-off-by: Senthil Balasubramanian <[email protected]>
---
v2 -- addressed invalid keyix overrun in tkip_keymap.

drivers/net/wireless/ath/ath9k/mac.c | 3 +--
drivers/net/wireless/ath/ath9k/recv.c | 13 +++++++++++--
2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/mac.c b/drivers/net/wireless/ath/ath9k/mac.c
index b04b37b..7978b27 100644
--- a/drivers/net/wireless/ath/ath9k/mac.c
+++ b/drivers/net/wireless/ath/ath9k/mac.c
@@ -702,8 +702,7 @@ int ath9k_hw_rxprocdesc(struct ath_hw *ah, struct ath_desc *ds,
rs->rs_phyerr = phyerr;
} else if (ads.ds_rxstatus8 & AR_DecryptCRCErr)
rs->rs_status |= ATH9K_RXERR_DECRYPT;
- else if ((ads.ds_rxstatus8 & AR_MichaelErr) &&
- rs->rs_keyix != ATH9K_RXKEYIX_INVALID)
+ else if (ads.ds_rxstatus8 & AR_MichaelErr)
rs->rs_status |= ATH9K_RXERR_MIC;
else if (ads.ds_rxstatus8 & AR_KeyMiss)
rs->rs_status |= ATH9K_RXERR_DECRYPT;
diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 262c815..6c0c796 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -840,6 +840,10 @@ static bool ath9k_rx_accept(struct ath_common *common,
struct ath_rx_status *rx_stats,
bool *decrypt_error)
{
+#define is_mc_or_valid_tkip_keyix ((is_mc || \
+ (rx_stats->rs_keyix != ATH9K_RXKEYIX_INVALID && \
+ test_bit(rx_stats->rs_keyix, common->tkip_keymap))))
+
struct ath_hw *ah = common->ah;
__le16 fc;
u8 rx_status_len = ah->caps.rx_status_len;
@@ -881,15 +885,18 @@ static bool ath9k_rx_accept(struct ath_common *common,
if (rx_stats->rs_status & ATH9K_RXERR_DECRYPT) {
*decrypt_error = true;
} else if (rx_stats->rs_status & ATH9K_RXERR_MIC) {
+ bool is_mc;
/*
* The MIC error bit is only valid if the frame
* is not a control frame or fragment, and it was
* decrypted using a valid TKIP key.
*/
+ is_mc = !!is_multicast_ether_addr(hdr->addr1);
+
if (!ieee80211_is_ctl(fc) &&
!ieee80211_has_morefrags(fc) &&
!(le16_to_cpu(hdr->seq_ctrl) & IEEE80211_SCTL_FRAG) &&
- test_bit(rx_stats->rs_keyix, common->tkip_keymap))
+ is_mc_or_valid_tkip_keyix)
rxs->flag |= RX_FLAG_MMIC_ERROR;
else
rx_stats->rs_status &= ~ATH9K_RXERR_MIC;
@@ -1037,9 +1044,11 @@ static void ath9k_rx_skb_postprocess(struct ath_common *common,
int hdrlen, padpos, padsize;
u8 keyix;
__le16 fc;
+ bool is_mc;

/* see if any padding is done by the hw and remove it */
hdr = (struct ieee80211_hdr *) skb->data;
+ is_mc = !!is_multicast_ether_addr(hdr->addr1);
hdrlen = ieee80211_get_hdrlen_from_skb(skb);
fc = hdr->frame_control;
padpos = ath9k_cmn_padpos(hdr->frame_control);
@@ -1060,7 +1069,7 @@ static void ath9k_rx_skb_postprocess(struct ath_common *common,

keyix = rx_stats->rs_keyix;

- if (!(keyix == ATH9K_RXKEYIX_INVALID) && !decrypt_error &&
+ if ((is_mc || !(keyix == ATH9K_RXKEYIX_INVALID)) && !decrypt_error &&
ieee80211_has_protected(fc)) {
rxs->flag |= RX_FLAG_DECRYPTED;
} else if (ieee80211_has_protected(fc)
--
1.7.2.1



2010-12-03 14:17:11

by Senthil Balasubramanian

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On Fri, Dec 03, 2010 at 01:34:44PM +0530, Ben Greear wrote:
> On 12/01/2010 11:16 PM, Mohammed Shafi wrote:
> > On Thu, Dec 2, 2010 at 11:14 AM, Ben Greear<[email protected]> wrote:
> >> On 12/01/2010 09:09 PM, Mohammed Shafi wrote:
> >>>
> >>> On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear<[email protected]>
> >>> wrote:
> >>>>
> >>>> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
> >>>>>
> >>>>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
> >>>>> this status for MIC failed frames is buggy.
> >>>>>
> >>>>> Due to this, MIC failure events for broadcast frames are not sent to
> >>>>> supplicant resulted in AP disconnecting the STA.
> >>>>>
> >>>>> Able to pass Wifi Test case 5.2.18 with this fix.
> >>>>
> >>>> Please do not apply this yet. As far as I can tell, either
> >>>> of these patches breaks multiple VIF scenarios. I'm not
> >>>> sure exactly why, but I had to revert this to get any
> >>>> of my interfaces to associate.
> >>>
> >>> Ben can you please give some more information(or just point out some
> >>> link) regarding your test case,I can try it out.
> >>> thanks,
> >>> shafi
> >>
> >> Try this script, or something like it:
> >>
> >> http://www.spinics.net/lists/linux-wireless/msg60126.html
> >>
> >> Edit 'max' to create a small number of STA interfaces or you will probably
> >> have
> >> worse issues than them just not associating!
> >>
> >> You have to enable the nohwcrypt module option.
> >>
> >> Let me know if that doesn't work and I can probably put together something
> >> more specific.
> >
> > thanks Ben.
>
> Were you able to reproduce the problem?

I can reproduce this issue with single vif itself and with s/w crypt enabled.

Can you please apply the v2 patch and try the following change on top of it in

diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
index 6c0c796..4871849 100644
--- a/drivers/net/wireless/ath/ath9k/recv.c
+++ b/drivers/net/wireless/ath/ath9k/recv.c
@@ -1069,7 +1069,7 @@ static void ath9k_rx_skb_postprocess(struct ath_common *common,

keyix = rx_stats->rs_keyix;

- if ((is_mc || !(keyix == ATH9K_RXKEYIX_INVALID)) && !decrypt_error &&
+ if (!(keyix == ATH9K_RXKEYIX_INVALID) && !decrypt_error &&
ieee80211_has_protected(fc)) {
rxs->flag |= RX_FLAG_DECRYPTED;
} else if (ieee80211_has_protected(fc)

I haven't tested s/w crypto and this check causes mac80211 not to decrypt frames so it
would have caused issues.. Can you please check this out? It wouldn't cause issues with
h/w crypt. Please let me know whether it solves your issue.. It works for me.

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

2010-12-02 05:09:22

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear <[email protected]> wrote:
> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
>>
>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
>> this status for MIC failed frames is buggy.
>>
>> Due to this, MIC failure events for broadcast frames are not sent to
>> supplicant resulted in AP disconnecting the STA.
>>
>> Able to pass Wifi Test case 5.2.18 with this fix.
>
> Please do not apply this yet. ?As far as I can tell, either
> of these patches breaks multiple VIF scenarios. ?I'm not
> sure exactly why, but I had to revert this to get any
> of my interfaces to associate.

Ben can you please give some more information(or just point out some
link) regarding your test case,I can try it out.
thanks,
shafi
>
> I'm using software encryption in case that matters.
>
> Senthil: ?Any chance you could test with a few VIFS to
> see if you see the same problem?
>
> Here is a dmesg snippet from wireless-testing with the v1 reverted
> and v2 applied:
>
> sta9: authenticate with 00:14:d1:c6:d2:54 (try 1)
> sta9: authenticated
> sta9: associate with 00:14:d1:c6:d2:54 (try 1)
> sta9: RX AssocResp from 00:14:d1:c6:d2:54 (capab=0x431 status=1 aid=0)
> sta9: 00:14:d1:c6:d2:54 denied association (code=1)
> sta9: deauthenticating from 00:14:d1:c6:d2:54 by local choice (reason=3)
> ieee80211 wiphy0: device now idle
> ieee80211 wiphy0: device no longer idle - scanning
> ieee80211 wiphy0: device now idle
> ieee80211 wiphy0: device no longer idle - working
> sta10: authenticate with 00:14:d1:c6:d2:54 (try 1)
> sta10: authenticated
> sta10: associate with 00:14:d1:c6:d2:54 (try 1)
> sta10: RX AssocResp from 00:14:d1:c6:d2:54 (capab=0x431 status=1 aid=0)
> sta10: 00:14:d1:c6:d2:54 denied association (code=1)
> sta10: deauthenticating from 00:14:d1:c6:d2:54 by local choice (reason=3)
>
> Thanks,
> Ben
>
> --
> 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
>

2010-12-02 07:16:09

by Mohammed Shafi

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On Thu, Dec 2, 2010 at 11:14 AM, Ben Greear <[email protected]> wrote:
> On 12/01/2010 09:09 PM, Mohammed Shafi wrote:
>>
>> On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear<[email protected]>
>> ?wrote:
>>>
>>> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
>>>>
>>>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
>>>> this status for MIC failed frames is buggy.
>>>>
>>>> Due to this, MIC failure events for broadcast frames are not sent to
>>>> supplicant resulted in AP disconnecting the STA.
>>>>
>>>> Able to pass Wifi Test case 5.2.18 with this fix.
>>>
>>> Please do not apply this yet. ?As far as I can tell, either
>>> of these patches breaks multiple VIF scenarios. ?I'm not
>>> sure exactly why, but I had to revert this to get any
>>> of my interfaces to associate.
>>
>> Ben can you please give some more information(or just point out some
>> link) regarding your test case,I can try it out.
>> thanks,
>> shafi
>
> Try this script, or something like it:
>
> http://www.spinics.net/lists/linux-wireless/msg60126.html
>
> Edit 'max' to create a small number of STA interfaces or you will probably
> have
> worse issues than them just not associating!
>
> You have to enable the nohwcrypt module option.
>
> Let me know if that doesn't work and I can probably put together something
> more specific.

thanks Ben.

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

2010-12-02 05:44:45

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On 12/01/2010 09:09 PM, Mohammed Shafi wrote:
> On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear<[email protected]> wrote:
>> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
>>>
>>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
>>> this status for MIC failed frames is buggy.
>>>
>>> Due to this, MIC failure events for broadcast frames are not sent to
>>> supplicant resulted in AP disconnecting the STA.
>>>
>>> Able to pass Wifi Test case 5.2.18 with this fix.
>>
>> Please do not apply this yet. As far as I can tell, either
>> of these patches breaks multiple VIF scenarios. I'm not
>> sure exactly why, but I had to revert this to get any
>> of my interfaces to associate.
>
> Ben can you please give some more information(or just point out some
> link) regarding your test case,I can try it out.
> thanks,
> shafi

Try this script, or something like it:

http://www.spinics.net/lists/linux-wireless/msg60126.html

Edit 'max' to create a small number of STA interfaces or you will probably have
worse issues than them just not associating!

You have to enable the nohwcrypt module option.

Let me know if that doesn't work and I can probably put together something
more specific.

Thanks,
Ben

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

2010-12-03 08:44:58

by Senthil Balasubramanian

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On Thu, Dec 02, 2010 at 01:28:53AM +0530, Ben Greear wrote:
> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
> > AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
> > this status for MIC failed frames is buggy.
> >
> > Due to this, MIC failure events for broadcast frames are not sent to
> > supplicant resulted in AP disconnecting the STA.
> >
> > Able to pass Wifi Test case 5.2.18 with this fix.
>
> Please do not apply this yet. As far as I can tell, either
> of these patches breaks multiple VIF scenarios. I'm not
> sure exactly why, but I had to revert this to get any
> of my interfaces to associate.
>
> I'm using software encryption in case that matters.

Are you saying that you have issues with this patch (either v1 or v2)
and only when software encryption is used ??? Please confirm.

This patch fixes a regression which was introduced in 2.6.36 kernel. so
essentially what this patch does is nothing but what was happening in
2.6.35 kernel and below. It should not create any new issues from what
I can see. Anyway let us enable s/w encryption also and verify.

>
> Senthil: Any chance you could test with a few VIFS to
> see if you see the same problem?
>
> Here is a dmesg snippet from wireless-testing with the v1 reverted
> and v2 applied:
>
> sta9: authenticate with 00:14:d1:c6:d2:54 (try 1)
> sta9: authenticated
> sta9: associate with 00:14:d1:c6:d2:54 (try 1)
> sta9: RX AssocResp from 00:14:d1:c6:d2:54 (capab=0x431 status=1 aid=0)
> sta9: 00:14:d1:c6:d2:54 denied association (code=1)
> sta9: deauthenticating from 00:14:d1:c6:d2:54 by local choice (reason=3)
> ieee80211 wiphy0: device now idle
> ieee80211 wiphy0: device no longer idle - scanning
> ieee80211 wiphy0: device now idle
> ieee80211 wiphy0: device no longer idle - working
> sta10: authenticate with 00:14:d1:c6:d2:54 (try 1)
> sta10: authenticated
> sta10: associate with 00:14:d1:c6:d2:54 (try 1)
> sta10: RX AssocResp from 00:14:d1:c6:d2:54 (capab=0x431 status=1 aid=0)
> sta10: 00:14:d1:c6:d2:54 denied association (code=1)
> sta10: deauthenticating from 00:14:d1:c6:d2:54 by local choice (reason=3)
>
> Thanks,
> Ben
>
> --
> Ben Greear <[email protected]>
> Candela Technologies Inc http://www.candelatech.com
>

2010-12-02 19:30:11

by John W. Linville

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On Wed, Dec 01, 2010 at 11:58:53AM -0800, Ben Greear wrote:
> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
> >AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
> >this status for MIC failed frames is buggy.
> >
> >Due to this, MIC failure events for broadcast frames are not sent to
> >supplicant resulted in AP disconnecting the STA.
> >
> >Able to pass Wifi Test case 5.2.18 with this fix.
>
> Please do not apply this yet. As far as I can tell, either
> of these patches breaks multiple VIF scenarios. I'm not
> sure exactly why, but I had to revert this to get any
> of my interfaces to associate.

I'm reverting v1 and ignoring v2 on the basis of this report.

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2010-12-06 11:31:19

by Senthil Balasubramanian

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On Fri, Dec 03, 2010 at 11:28:24PM +0530, Ben Greear wrote:
> On 12/03/2010 06:17 AM, Senthil Balasubramanian wrote:
> > On Fri, Dec 03, 2010 at 01:34:44PM +0530, Ben Greear wrote:
> >> On 12/01/2010 11:16 PM, Mohammed Shafi wrote:
> >>> On Thu, Dec 2, 2010 at 11:14 AM, Ben Greear<[email protected]> wrote:
> >>>> On 12/01/2010 09:09 PM, Mohammed Shafi wrote:
> >>>>>
> >>>>> On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear<[email protected]>
> >>>>> wrote:
> >>>>>>
> >>>>>> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
> >>>>>>>
> >>>>>>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
> >>>>>>> this status for MIC failed frames is buggy.
> >>>>>>>
> >>>>>>> Due to this, MIC failure events for broadcast frames are not sent to
> >>>>>>> supplicant resulted in AP disconnecting the STA.
> >>>>>>>
> >>>>>>> Able to pass Wifi Test case 5.2.18 with this fix.
> >>>>>>
> >>>>>> Please do not apply this yet. As far as I can tell, either
> >>>>>> of these patches breaks multiple VIF scenarios. I'm not
> >>>>>> sure exactly why, but I had to revert this to get any
> >>>>>> of my interfaces to associate.
> >>>>>
> >>>>> Ben can you please give some more information(or just point out some
> >>>>> link) regarding your test case,I can try it out.
> >>>>> thanks,
> >>>>> shafi
> >>>>
> >>>> Try this script, or something like it:
> >>>>
> >>>> http://www.spinics.net/lists/linux-wireless/msg60126.html
> >>>>
> >>>> Edit 'max' to create a small number of STA interfaces or you will probably
> >>>> have
> >>>> worse issues than them just not associating!
> >>>>
> >>>> You have to enable the nohwcrypt module option.
> >>>>
> >>>> Let me know if that doesn't work and I can probably put together something
> >>>> more specific.
> >>>
> >>> thanks Ben.
> >>
> >> Were you able to reproduce the problem?
> >
> > I can reproduce this issue with single vif itself and with s/w crypt enabled.
> >
> > Can you please apply the v2 patch and try the following change on top of it in
> >
> > diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> > index 6c0c796..4871849 100644
> > --- a/drivers/net/wireless/ath/ath9k/recv.c
> > +++ b/drivers/net/wireless/ath/ath9k/recv.c
> > @@ -1069,7 +1069,7 @@ static void ath9k_rx_skb_postprocess(struct ath_common *common,
> >
> > keyix = rx_stats->rs_keyix;
> >
> > - if ((is_mc || !(keyix == ATH9K_RXKEYIX_INVALID))&& !decrypt_error&&
> > + if (!(keyix == ATH9K_RXKEYIX_INVALID)&& !decrypt_error&&
> > ieee80211_has_protected(fc)) {
> > rxs->flag |= RX_FLAG_DECRYPTED;
> > } else if (ieee80211_has_protected(fc)
> >
> > I haven't tested s/w crypto and this check causes mac80211 not to decrypt frames so it
> > would have caused issues.. Can you please check this out? It wouldn't cause issues with
> > h/w crypt. Please let me know whether it solves your issue.. It works for me.
>
> I did a quick test and your -v2 patch plus this change appears to work fine.
Thanks for verifying this hunk on top of my v2.
>
> Please note that you v2 patch would need some additional changes with the
> above patch, because is_mc is no longer used in that method as far
is_mc is not required and using this was the bug as far as s/w crypto is
concerned. The else part is already handling the is_mc case.

Actually speaking the if and else is not required to run if the crypto is
not offloaded. However that is just a cleanup and not a candidate for stable.

Anyway I shall send out the updated patch without is_mc.

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

2010-12-03 08:04:51

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On 12/01/2010 11:16 PM, Mohammed Shafi wrote:
> On Thu, Dec 2, 2010 at 11:14 AM, Ben Greear<[email protected]> wrote:
>> On 12/01/2010 09:09 PM, Mohammed Shafi wrote:
>>>
>>> On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear<[email protected]>
>>> wrote:
>>>>
>>>> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
>>>>>
>>>>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
>>>>> this status for MIC failed frames is buggy.
>>>>>
>>>>> Due to this, MIC failure events for broadcast frames are not sent to
>>>>> supplicant resulted in AP disconnecting the STA.
>>>>>
>>>>> Able to pass Wifi Test case 5.2.18 with this fix.
>>>>
>>>> Please do not apply this yet. As far as I can tell, either
>>>> of these patches breaks multiple VIF scenarios. I'm not
>>>> sure exactly why, but I had to revert this to get any
>>>> of my interfaces to associate.
>>>
>>> Ben can you please give some more information(or just point out some
>>> link) regarding your test case,I can try it out.
>>> thanks,
>>> shafi
>>
>> Try this script, or something like it:
>>
>> http://www.spinics.net/lists/linux-wireless/msg60126.html
>>
>> Edit 'max' to create a small number of STA interfaces or you will probably
>> have
>> worse issues than them just not associating!
>>
>> You have to enable the nohwcrypt module option.
>>
>> Let me know if that doesn't work and I can probably put together something
>> more specific.
>
> thanks Ben.

Were you able to reproduce the problem?

Thanks,
Ben

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


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

2010-12-01 20:01:24

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
> this status for MIC failed frames is buggy.
>
> Due to this, MIC failure events for broadcast frames are not sent to
> supplicant resulted in AP disconnecting the STA.
>
> Able to pass Wifi Test case 5.2.18 with this fix.

Please do not apply this yet. As far as I can tell, either
of these patches breaks multiple VIF scenarios. I'm not
sure exactly why, but I had to revert this to get any
of my interfaces to associate.

I'm using software encryption in case that matters.

Senthil: Any chance you could test with a few VIFS to
see if you see the same problem?

Here is a dmesg snippet from wireless-testing with the v1 reverted
and v2 applied:

sta9: authenticate with 00:14:d1:c6:d2:54 (try 1)
sta9: authenticated
sta9: associate with 00:14:d1:c6:d2:54 (try 1)
sta9: RX AssocResp from 00:14:d1:c6:d2:54 (capab=0x431 status=1 aid=0)
sta9: 00:14:d1:c6:d2:54 denied association (code=1)
sta9: deauthenticating from 00:14:d1:c6:d2:54 by local choice (reason=3)
ieee80211 wiphy0: device now idle
ieee80211 wiphy0: device no longer idle - scanning
ieee80211 wiphy0: device now idle
ieee80211 wiphy0: device no longer idle - working
sta10: authenticate with 00:14:d1:c6:d2:54 (try 1)
sta10: authenticated
sta10: associate with 00:14:d1:c6:d2:54 (try 1)
sta10: RX AssocResp from 00:14:d1:c6:d2:54 (capab=0x431 status=1 aid=0)
sta10: 00:14:d1:c6:d2:54 denied association (code=1)
sta10: deauthenticating from 00:14:d1:c6:d2:54 by local choice (reason=3)

Thanks,
Ben

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


2010-12-03 17:58:28

by Ben Greear

[permalink] [raw]
Subject: Re: [PATCH v2 2/2] ath9k: Fix STA disconnect issue due to received MIC failed bcast frames

On 12/03/2010 06:17 AM, Senthil Balasubramanian wrote:
> On Fri, Dec 03, 2010 at 01:34:44PM +0530, Ben Greear wrote:
>> On 12/01/2010 11:16 PM, Mohammed Shafi wrote:
>>> On Thu, Dec 2, 2010 at 11:14 AM, Ben Greear<[email protected]> wrote:
>>>> On 12/01/2010 09:09 PM, Mohammed Shafi wrote:
>>>>>
>>>>> On Thu, Dec 2, 2010 at 1:28 AM, Ben Greear<[email protected]>
>>>>> wrote:
>>>>>>
>>>>>> On 12/01/2010 03:00 AM, Senthil Balasubramanian wrote:
>>>>>>>
>>>>>>> AR_RxKeyIdxValid will not be set for bcast/mcast frames and so relying
>>>>>>> this status for MIC failed frames is buggy.
>>>>>>>
>>>>>>> Due to this, MIC failure events for broadcast frames are not sent to
>>>>>>> supplicant resulted in AP disconnecting the STA.
>>>>>>>
>>>>>>> Able to pass Wifi Test case 5.2.18 with this fix.
>>>>>>
>>>>>> Please do not apply this yet. As far as I can tell, either
>>>>>> of these patches breaks multiple VIF scenarios. I'm not
>>>>>> sure exactly why, but I had to revert this to get any
>>>>>> of my interfaces to associate.
>>>>>
>>>>> Ben can you please give some more information(or just point out some
>>>>> link) regarding your test case,I can try it out.
>>>>> thanks,
>>>>> shafi
>>>>
>>>> Try this script, or something like it:
>>>>
>>>> http://www.spinics.net/lists/linux-wireless/msg60126.html
>>>>
>>>> Edit 'max' to create a small number of STA interfaces or you will probably
>>>> have
>>>> worse issues than them just not associating!
>>>>
>>>> You have to enable the nohwcrypt module option.
>>>>
>>>> Let me know if that doesn't work and I can probably put together something
>>>> more specific.
>>>
>>> thanks Ben.
>>
>> Were you able to reproduce the problem?
>
> I can reproduce this issue with single vif itself and with s/w crypt enabled.
>
> Can you please apply the v2 patch and try the following change on top of it in
>
> diff --git a/drivers/net/wireless/ath/ath9k/recv.c b/drivers/net/wireless/ath/ath9k/recv.c
> index 6c0c796..4871849 100644
> --- a/drivers/net/wireless/ath/ath9k/recv.c
> +++ b/drivers/net/wireless/ath/ath9k/recv.c
> @@ -1069,7 +1069,7 @@ static void ath9k_rx_skb_postprocess(struct ath_common *common,
>
> keyix = rx_stats->rs_keyix;
>
> - if ((is_mc || !(keyix == ATH9K_RXKEYIX_INVALID))&& !decrypt_error&&
> + if (!(keyix == ATH9K_RXKEYIX_INVALID)&& !decrypt_error&&
> ieee80211_has_protected(fc)) {
> rxs->flag |= RX_FLAG_DECRYPTED;
> } else if (ieee80211_has_protected(fc)
>
> I haven't tested s/w crypto and this check causes mac80211 not to decrypt frames so it
> would have caused issues.. Can you please check this out? It wouldn't cause issues with
> h/w crypt. Please let me know whether it solves your issue.. It works for me.

I did a quick test and your -v2 patch plus this change appears to work fine.

Please note that you v2 patch would need some additional changes with the
above patch, because is_mc is no longer used in that method as far
as I can tell.

Thanks,
Ben

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