Hi all,
It seems the problem is a typo in net/mac80211/tx.c that causes
injected frames from hostapd not to be associated with the correct ap
interface before going into ieee80211_tx_h_sequence(). This tiny patch
solves my problems (and looks reasonable to me in any case):
diff -urN compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
compat-wireless-2009-10-21/net/mac80211/tx.c
--- compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
2009-10-23 17:20:52.000000000 +0200
+++ compat-wireless-2009-10-21/net/mac80211/tx.c 2009-10-23
17:21:28.000000000 +0200
@@ -1445,7 +1445,7 @@
if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
continue;
if (compare_ether_addr(tmp_sdata->dev->dev_addr,
- hdr->addr2)) {
+ hdr->addr2) == 0) {
dev_hold(tmp_sdata->dev);
dev_put(sdata->dev);
sdata = tmp_sdata;
/Bj?rn
2009/10/23 Bj?rn Smedman <[email protected]>
>
> Hi all,
> Thanks for your help. I will also keep my tcpdumps to myself for now.
> Is anybody working on a fix? I suspect it's a misunderstanding between hostapd and mac80211 about who should set the sequence number on injected frames, correct? Could you describe the outlines of the appropriate patch so I could experiment and verify that this is my problem?
> /Bj?rn
> On Fri, Oct 23, 2009 at 11:15 AM, Joerg Pommnitz <[email protected]> wrote:
>>
>> Since Jouni has confirmed my suspicion about the invalid sequence number I will refrain from spamming the list with tcpdumps.
>> Will, thanks for bringing me into the loop of this discussion!
>>
>> --
>> Regards
>> Joerg
>>
>>
>>
>
>
>
> --
> Venatech AB
> Ideon Innovation
> Ole R?mers v?g 12
> SE-22370 LUND
> Sweden
>
> +46 (0) 46 286 86 20
> [email protected]
> http://www.venatech.se
On Fri, Oct 23, 2009 at 05:27:27PM +0200, Björn Smedman wrote:
> It seems the problem is a typo in net/mac80211/tx.c that causes
> injected frames from hostapd not to be associated with the correct ap
> interface before going into ieee80211_tx_h_sequence(). This tiny patch
> solves my problems (and looks reasonable to me in any case):
Thanks! Would you be able to make the same patch against the
wireless-testing.git repository and add a Signed-off-by line to meet the
kernel submission requirements?
> diff -urN compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
> compat-wireless-2009-10-21/net/mac80211/tx.c
> @@ -1445,7 +1445,7 @@
> if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
> continue;
> if (compare_ether_addr(tmp_sdata->dev->dev_addr,
> - hdr->addr2)) {
> + hdr->addr2) == 0) {
> dev_hold(tmp_sdata->dev);
> dev_put(sdata->dev);
> sdata = tmp_sdata;
This does indeed look like a typo. Though, I'm not sure how this would
have caused a regression between compat-wireless-2009-06-02 and
compat-wireless-2.6.32-rc1. The incorrect compare_ether_addr() use seems
to be there in the original commit that added this code
(25d834e16294c8dfd923dae6bdb8a055391a99a5 from September 12, 2008)..
Johannes: Any idea why the sequence number allocation for injected
frames from hostapd would have changed between 2009-06-02 and now? This
bug seems to be too old to explain that ;-).
--
Jouni Malinen PGP id EFC895FA
I'll test tomorrow. Something is bothering me here: I have a current wireless-testing kernel
with ath5k and ath9 driver. I have traces that show that Win Vista successfully associates
with ath5k running in AP mode but fails with ath9k. How is this difference possible if the bug
is in the common code? Might there be another offsetting bug in ath5k?
--
Regards
Joerg
----- Urspr?ngliche Mail ----
Von: Jouni Malinen <[email protected]>
An: Bj?rn Smedman <[email protected]>
CC: Joerg Pommnitz <[email protected]>; Will Dyson <[email protected]>; Johannes Berg <[email protected]>; [email protected]; [email protected]
Gesendet: Freitag, den 23. Oktober 2009, 18:30:01 Uhr
Betreff: Re: [ath9k-devel] mac80211/ath9k/hostapd: Some clients unable to associate with AP
On Fri, Oct 23, 2009 at 05:27:27PM +0200, Bj?rn Smedman wrote:
> It seems the problem is a typo in net/mac80211/tx.c that causes
> injected frames from hostapd not to be associated with the correct ap
> interface before going into ieee80211_tx_h_sequence(). This tiny patch
> solves my problems (and looks reasonable to me in any case):
Thanks! Would you be able to make the same patch against the
wireless-testing.git repository and add a Signed-off-by line to meet the
kernel submission requirements?
> diff -urN compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
> compat-wireless-2009-10-21/net/mac80211/tx.c
> @@ -1445,7 +1445,7 @@
> if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
> continue;
> if (compare_ether_addr(tmp_sdata->dev->dev_addr,
> - hdr->addr2)) {
> + hdr->addr2) == 0) {
> dev_hold(tmp_sdata->dev);
> dev_put(sdata->dev);
> sdata = tmp_sdata;
This does indeed look like a typo. Though, I'm not sure how this would
have caused a regression between compat-wireless-2009-06-02 and
compat-wireless-2.6.32-rc1. The incorrect compare_ether_addr() use seems
to be there in the original commit that added this code
(25d834e16294c8dfd923dae6bdb8a055391a99a5 from September 12, 2008)..
Johannes: Any idea why the sequence number allocation for injected
frames from hostapd would have changed between 2009-06-02 and now? This
bug seems to be too old to explain that ;-).
--
Jouni Malinen PGP id EFC895FA
2009/10/23 Jouni Malinen <[email protected]>:
> On Fri, Oct 23, 2009 at 05:27:27PM +0200, Bj?rn Smedman wrote:
>
>> It seems the problem is a typo in net/mac80211/tx.c that causes
>> injected frames from hostapd not to be associated with the correct ap
>> interface before going into ieee80211_tx_h_sequence(). This tiny patch
>> solves my problems (and looks reasonable to me in any case):
>
> Thanks! Would you be able to make the same patch against the
> wireless-testing.git repository and add a Signed-off-by line to meet the
> kernel submission requirements?
It's time i learn. :) I'll give it a shot tomorrow.
>
>> diff -urN compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
>> compat-wireless-2009-10-21/net/mac80211/tx.c
>> @@ -1445,7 +1445,7 @@
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? continue;
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? if (compare_ether_addr(tmp_sdata->dev->dev_addr,
>> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?hdr->addr2)) {
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?hdr->addr2) == 0) {
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_hold(tmp_sdata->dev);
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? dev_put(sdata->dev);
>> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? sdata = tmp_sdata;
>
> This does indeed look like a typo. Though, I'm not sure how this would
> have caused a regression between compat-wireless-2009-06-02 and
> compat-wireless-2.6.32-rc1. The incorrect compare_ether_addr() use seems
> to be there in the original commit that added this code
> (25d834e16294c8dfd923dae6bdb8a055391a99a5 from September 12, 2008)..
Interesting puzzle. :) It looks like there was a complementary bug
(the pointer hdr was set to point len_rthdr * sizeof(struct
ieee80211_hdr) bytes into the skbuff) in that commit:
...
+ len_rthdr = ieee80211_get_radiotap_len(skb->data);
+ hdr = (struct ieee80211_hdr *)skb->data + len_rthdr;
...
So the frame source address used to be compared with random data which
was likely to result in inequality, causing the first ap interface to
be "found" and the code to work as expected. I guess the pointer bug
was fixed somewhere between 2006-06-02 and now "causing" the sequence
number problem.
/Bj?rn
On Fri, 2009-10-23 at 09:30 -0700, Jouni Malinen wrote:
> > diff -urN compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
> > compat-wireless-2009-10-21/net/mac80211/tx.c
> > @@ -1445,7 +1445,7 @@
> > if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
> > continue;
> > if (compare_ether_addr(tmp_sdata->dev->dev_addr,
> > - hdr->addr2)) {
> > + hdr->addr2) == 0) {
> > dev_hold(tmp_sdata->dev);
> > dev_put(sdata->dev);
> > sdata = tmp_sdata;
>
> This does indeed look like a typo. Though, I'm not sure how this would
> have caused a regression between compat-wireless-2009-06-02 and
> compat-wireless-2.6.32-rc1. The incorrect compare_ether_addr() use seems
> to be there in the original commit that added this code
> (25d834e16294c8dfd923dae6bdb8a055391a99a5 from September 12, 2008)..
>
> Johannes: Any idea why the sequence number allocation for injected
> frames from hostapd would have changed between 2009-06-02 and now? This
> bug seems to be too old to explain that ;-).
Indeed, I have no idea though.
johannes
On Sat, 2009-10-24 at 05:12 +0200, Björn Smedman wrote:
> >> diff -urN compat-wireless-2009-10-21-before_seqnum_fix/net/mac80211/tx.c
> >> compat-wireless-2009-10-21/net/mac80211/tx.c
> >> @@ -1445,7 +1445,7 @@
> >> if (tmp_sdata->vif.type != NL80211_IFTYPE_AP)
> >> continue;
> >> if (compare_ether_addr(tmp_sdata->dev->dev_addr,
> >> - hdr->addr2)) {
> >> + hdr->addr2) == 0) {
> >> dev_hold(tmp_sdata->dev);
> >> dev_put(sdata->dev);
> >> sdata = tmp_sdata;
> >
> > This does indeed look like a typo. Though, I'm not sure how this would
> > have caused a regression between compat-wireless-2009-06-02 and
> > compat-wireless-2.6.32-rc1. The incorrect compare_ether_addr() use seems
> > to be there in the original commit that added this code
> > (25d834e16294c8dfd923dae6bdb8a055391a99a5 from September 12, 2008)..
>
> Interesting puzzle. :) It looks like there was a complementary bug
> (the pointer hdr was set to point len_rthdr * sizeof(struct
> ieee80211_hdr) bytes into the skbuff) in that commit:
> ...
> + len_rthdr = ieee80211_get_radiotap_len(skb->data);
> + hdr = (struct ieee80211_hdr *)skb->data + len_rthdr;
> ...
> So the frame source address used to be compared with random data which
> was likely to result in inequality, causing the first ap interface to
> be "found" and the code to work as expected. I guess the pointer bug
> was fixed somewhere between 2006-06-02 and now "causing" the sequence
> number problem.
Heh, indeed, interesting. I remember somebody fixing that, but was
unaware of the second bug (obviously).
johannes