2008-04-08 01:02:42

by Alejandro Grijalba

[permalink] [raw]
Subject: [PATCH] mac80211: do not alter injected seq numbers

When injecting frames we should allow user to play with fields like
fragment or sequence numbers. This patch prevents mac80211 from modifying
those fields on injected frames.

Tested on 2.6.24.4 with aireplay-ng.

Signed-off-by: Alejandro Grijalba <[email protected]>
---
There is still a problem with some drivers (b43) that also modify seq numbers,
and i cannot find there a clean way to tell whether the frame was injected.
An alternative way would be to create a radiotap flag meaning not to modify header.

--- linux-2.6.24.4/net/mac80211/tx.c 2008-01-24 23:58:37.000000000 +0100
+++ linux-2.6.24.4-sud/net/mac80211/tx.c 2008-04-05 16:43:19.000000000 +0200
@@ -281,6 +281,9 @@ ieee80211_tx_h_sequence(struct ieee80211
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;

+ if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
+ return TXRX_CONTINUE;
+
if (ieee80211_get_hdrlen(le16_to_cpu(hdr->frame_control)) >= 24)
ieee80211_include_sequence(tx->sdata, hdr);





2008-04-09 13:42:36

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers


> This does not work, since we use injection for other types of frames. For example
> management frames from hostapd. We don't want (and can't) make hostapd keep track of
> sequence numbers.
> You'll have to contact radiotap people and add a flag for this. This would also
> solve the hardware counter problem then.

If hardware is capable of not overriding sequence numbers. I expect much
hardware to just always insert them.

Wasn't the original goal to control fragmentation rather than sequence
numbers? That could possibly be done with fewer problems.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-04-08 08:26:05

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Tuesday 08 April 2008 02:11:21 Alejandro Grijalba wrote:
> When injecting frames we should allow user to play with fields like
> fragment or sequence numbers. This patch prevents mac80211 from modifying
> those fields on injected frames.
>
> Tested on 2.6.24.4 with aireplay-ng.
>
> Signed-off-by: Alejandro Grijalba <[email protected]>
> ---
> There is still a problem with some drivers (b43) that also modify seq numbers,
> and i cannot find there a clean way to tell whether the frame was injected.
> An alternative way would be to create a radiotap flag meaning not to modify header.
>
> --- linux-2.6.24.4/net/mac80211/tx.c 2008-01-24 23:58:37.000000000 +0100
> +++ linux-2.6.24.4-sud/net/mac80211/tx.c 2008-04-05 16:43:19.000000000 +0200
> @@ -281,6 +281,9 @@ ieee80211_tx_h_sequence(struct ieee80211
> {
> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;
>
> + if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
> + return TXRX_CONTINUE;
> +
> if (ieee80211_get_hdrlen(le16_to_cpu(hdr->frame_control)) >= 24)
> ieee80211_include_sequence(tx->sdata, hdr);
>

This does not work, since we use injection for other types of frames. For example
management frames from hostapd. We don't want (and can't) make hostapd keep track of
sequence numbers.
You'll have to contact radiotap people and add a flag for this. This would also
solve the hardware counter problem then.

--
Greetings Michael.

2008-05-22 17:55:04

by JMF

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

Michael Buesch <mb@...> writes:
(...)
> > + if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
> > + return TXRX_CONTINUE;
(...)
> This does not work, since we use injection for other types of
> frames. For example management frames from hostapd. We don't want
> (and can't) make hostapd keep track of sequence numbers.
> You'll have to contact radiotap people and add a flag for this.
> This would also solve the hardware counter problem then.

The last time I saw tx.c, this should now be something like
info->flags & IEEE80211_TX_CTL_INJECTED
But as far as I can see the usage is the same.
And, unless I'm wrong, IEEE80211_TX_CTL_INJECTED is only used
with Monitor mode interfaces.
Does hostapd use monitor mode interfaces? Because if it doesn't,
and if I saw things correcly, info->flags won't be set to
IEEE80211_TX_CTL_INJECTED, right?

Any comments on this?


2008-05-22 18:35:48

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

> >> This does not work, since we use injection for other types of
> >> frames. For example management frames from hostapd. We don't want
> >> (and can't) make hostapd keep track of sequence numbers.
> >> You'll have to contact radiotap people and add a flag for this.
> >> This would also solve the hardware counter problem then.

> The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
> current wireless-testing:

Reorder the two blocks and you have your answer. NO.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-05-22 19:04:04

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Thu, 2008-05-22 at 20:52 +0200, Stefanik Gábor wrote:
> On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
> <[email protected]> wrote:
> >> >> This does not work, since we use injection for other types of
> >> >> frames. For example management frames from hostapd. We don't want
> >> >> (and can't) make hostapd keep track of sequence numbers.
> >> >> You'll have to contact radiotap people and add a flag for this.
> >> >> This would also solve the hardware counter problem then.
> >
> >> The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
> >> current wireless-testing:
> >
> > Reorder the two blocks and you have your answer. NO.
> >
> > johannes
> >
>
> Hmm, shouldn't this version fix the hostap and other non-monitor-mode cases?

No, because hostapd uses monitor interfaces. Give up, it doesn't work
without an extra radiotap flag.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-05-22 19:16:37

by Johannes Berg

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

[don't drop CCs, restored]

> Are you really sure the flag is IEEE80211_TX_INJECTED?
> For the current (today, 22 May) wireless-testing, it seems it's been
> renamed to IEEE80211_TX_CTL_INJECTED. Also, not tx->flags anymore, but
> info->flags instead.
> Can you confirm that?

Yeah both of those are correct, due to a recent patch of mine.

> I was going to say something else, but I saw another post by Johannes... Strange
> that hostapd uses injection in monitor mode interfaces, I thought it would be
> only AP mode interfaces.

No, it uses monitor for a bunch of things, including getting access to
management frames (before a station has associated.)

> And it is also strange that iwlwifi developers wanted to disallow injection in
> monitor mode, with hostapd apparently needing it!

No, not strange either, for one iwlwifi doesn't seem to support AP yet,
and secondly the monitor interface would be secondary so iwlwifi
wouldn't even notice it.

johannes


Attachments:
signature.asc (828.00 B)
This is a digitally signed message part

2008-05-22 19:09:47

by JMF

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

Johannes Berg <johannes@...> writes:

>
> > >> This does not work, since we use injection for other types of
> > >> frames. For example management frames from hostapd. We don't want
> > >> (and can't) make hostapd keep track of sequence numbers.
> > >> You'll have to contact radiotap people and add a flag for this.
> > >> This would also solve the hardware counter problem then.
>
> > The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
> > current wireless-testing:


Are you really sure the flag is IEEE80211_TX_INJECTED?
For the current (today, 22 May) wireless-testing, it seems it's been
renamed to IEEE80211_TX_CTL_INJECTED. Also, not tx->flags anymore, but
info->flags instead.
Can you confirm that?


I was going to say something else, but I saw another post by Johannes... Strange
that hostapd uses injection in monitor mode interfaces, I thought it would be
only AP mode interfaces.
And it is also strange that iwlwifi developers wanted to disallow injection in
monitor mode, with hostapd apparently needing it!


2008-05-22 18:52:24

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
<[email protected]> wrote:
>> >> This does not work, since we use injection for other types of
>> >> frames. For example management frames from hostapd. We don't want
>> >> (and can't) make hostapd keep track of sequence numbers.
>> >> You'll have to contact radiotap people and add a flag for this.
>> >> This would also solve the hardware counter problem then.
>
>> The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
>> current wireless-testing:
>
> Reorder the two blocks and you have your answer. NO.
>
> johannes
>

Hmm, shouldn't this version fix the hostap and other non-monitor-mode cases?

---

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index 1a3b50b..1fb4f92 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -292,6 +292,11 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *tx)
{
struct ieee80211_hdr *hdr = (struct ieee80211_hdr *)tx->skb->data;

+ /* Monitor mode injectors handle sequence numbers themselves */
+ if (unlikely(tx->flags & IEEE80211_TX_INJECTED) &&
+ tx->sdata->vif.type == IEEE80211_IF_TYPE_MNTR)
+ return TX_CONTINUE;
+
if (ieee80211_get_hdrlen(le16_to_cpu(hdr->frame_control)) >= 24)
ieee80211_include_sequence(tx->sdata, hdr);

2008-05-22 19:08:29

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Thursday 22 May 2008 21:03:01 Johannes Berg wrote:
> On Thu, 2008-05-22 at 20:52 +0200, Stefanik G=E1bor wrote:
> > On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
> > <[email protected]> wrote:
> > >> >> This does not work, since we use injection for other types of
> > >> >> frames. For example management frames from hostapd. We don't=
want
> > >> >> (and can't) make hostapd keep track of sequence numbers.
> > >> >> You'll have to contact radiotap people and add a flag for thi=
s.
> > >> >> This would also solve the hardware counter problem then.
> > >
> > >> The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
> > >> current wireless-testing:
> > >
> > > Reorder the two blocks and you have your answer. NO.
> > >
> > > johannes
> > >
> >=20
> > Hmm, shouldn't this version fix the hostap and other non-monitor-mo=
de cases?
>=20
> No, because hostapd uses monitor interfaces. Give up, it doesn't work
> without an extra radiotap flag.

Well, don't give up, but please contact the radiotap people for adding =
such
a flag to the header. That's really the only way to go.
There are more flags to be added, like we-are-not-interested-in-acks.

--=20
Greetings Michael.

2008-05-22 19:04:08

by Michael Büsch

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Thursday 22 May 2008 20:52:21 Stefanik G=E1bor wrote:
> On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
> <[email protected]> wrote:
> >> >> This does not work, since we use injection for other types of
> >> >> frames. For example management frames from hostapd. We don't w=
ant
> >> >> (and can't) make hostapd keep track of sequence numbers.
> >> >> You'll have to contact radiotap people and add a flag for this.
> >> >> This would also solve the hardware counter problem then.
> >
> >> The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
> >> current wireless-testing:
> >
> > Reorder the two blocks and you have your answer. NO.
> >
> > johannes
> >
>=20
> Hmm, shouldn't this version fix the hostap and other non-monitor-mode=
cases?

No.
Hostapd is _not_ a non-monitor-case.

--=20
Greetings Michael.

2008-05-22 19:19:35

by Tomas Winkler

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Thu, May 22, 2008 at 10:09 PM, JMF <[email protected]> wrote:
> Johannes Berg <johannes@...> writes:
>
>>
>> > >> This does not work, since we use injection for other types of
>> > >> frames. For example management frames from hostapd. We don't want
>> > >> (and can't) make hostapd keep track of sequence numbers.
>> > >> You'll have to contact radiotap people and add a flag for this.
>> > >> This would also solve the hardware counter problem then.
>>
>> > The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
>> > current wireless-testing:
>
>
> Are you really sure the flag is IEEE80211_TX_INJECTED?
> For the current (today, 22 May) wireless-testing, it seems it's been
> renamed to IEEE80211_TX_CTL_INJECTED. Also, not tx->flags anymore, but
> info->flags instead.
> Can you confirm that?
>
>
> I was going to say something else, but I saw another post by Johannes... Strange
> that hostapd uses injection in monitor mode interfaces, I thought it would be
> only AP mode interfaces.
> And it is also strange that iwlwifi developers wanted to disallow injection in
> monitor mode, with hostapd apparently needing it!

On behalf of iwlwifi, this is legacy behavior that wasn't treated for
a,b,c reasons.
There used to be management interface that was used by hostapd and
monitor interface was used for sniffing. So with wanting or not
wanting it is actually all around.

Tomas

>
> --
> 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
>

2008-05-22 18:21:20

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH] mac80211: do not alter injected seq numbers

On Thu, May 22, 2008 at 7:48 PM, JMF <[email protected]> wrote:
> Michael Buesch <mb@...> writes:
> (...)
>> > + if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
>> > + return TXRX_CONTINUE;
> (...)
>> This does not work, since we use injection for other types of
>> frames. For example management frames from hostapd. We don't want
>> (and can't) make hostapd keep track of sequence numbers.
>> You'll have to contact radiotap people and add a flag for this.
>> This would also solve the hardware counter problem then.
>
> The last time I saw tx.c, this should now be something like
> info->flags & IEEE80211_TX_CTL_INJECTED
> But as far as I can see the usage is the same.
> And, unless I'm wrong, IEEE80211_TX_CTL_INJECTED is only used
> with Monitor mode interfaces.
> Does hostapd use monitor mode interfaces? Because if it doesn't,
> and if I saw things correcly, info->flags won't be set to
> IEEE80211_TX_CTL_INJECTED, right?
>
> Any comments on this?

The flag is IEEE80211_TX_INJECTED. Here is a new patch, for the
current wireless-testing:

Signed-off-by: Alejandro Grijalba Mart=EDnez <[email protected]>
Signed-off-by: G=E1bor Stefanik <[email protected]>

---

diff --git a/net/mac80211/tx.c b/net/mac80211/tx.c
index f35eaea..e5e8483 100644
--- a/net/mac80211/tx.c
+++ b/net/mac80211/tx.c
@@ -292,6 +292,9 @@ ieee80211_tx_h_sequence(struct ieee80211_tx_data *t=
x)
{
struct ieee80211_hdr *hdr =3D (struct ieee80211_hdr *)tx->skb->d=
ata;

+ if (unlikely(tx->flags & IEEE80211_TX_INJECTED))
+ return TX_CONTINUE;
+
if (ieee80211_get_hdrlen(le16_to_cpu(hdr->frame_control)) >=3D 2=
4)
ieee80211_include_sequence(tx->sdata, hdr);



--=20
Vista: [V]iruses, [I]ntruders, [S]pyware, [T]rojans and [A]dware. :-)