[PATCH] mac80211: do not alter injected seq numbers

[PATCH] mac80211: do not alter injected seq numbers

[Alejandro Grijalba]

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 <sud@latinsud.com>
---
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);
 

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

[Michael Buesch]

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 <sud@latinsud.com>
> ---
> 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.

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 582 bytes --]


> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[JMF]

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?

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

[Stefanik Gábor]

On Thu, May 22, 2008 at 7:48 PM, JMF <tolas_feup@hotmail.com> 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 <sud@latinsud.com>
Signed-off-by: G=E1bor Stefanik <netrolller.3d@gmail.com>

---

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. :-)
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Stefanik Gábor]

On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
<johannes@sipsolutions.net> 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);

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 897 bytes --]

On Thu, 2008-05-22 at 20:52 +0200, Stefanik Gábor wrote:
> On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
> <johannes@sipsolutions.net> 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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Michael Buesch]

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
> > <johannes@sipsolutions.net> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[Michael Buesch]

On Thursday 22 May 2008 20:52:21 Stefanik G=E1bor wrote:
> On Thu, May 22, 2008 at 8:34 PM, Johannes Berg
> <johannes@sipsolutions.net> 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.
--
To unsubscribe from this list: send the line "unsubscribe linux-wireles=
s" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

[JMF]

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!

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

[Johannes Berg]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

[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

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

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

[Tomas Winkler]

On Thu, May 22, 2008 at 10:09 PM, JMF <tolas_feup@hotmail.com> 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 majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>