Skip to contentSkip to conversation

mac80211: do not alter injected seq numbers

PATCH In review Alejandro Grijalba 6 participants 13 messages Apr 8, 2008 – May 22, 2008 1 message not in the archive mac80211

lore
Share & tools
lore

Alejandro Grijalba

author
#0 lore raw

PATCH 1/1 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.

net/mac80211/tx.c +3 -0
@@ -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);
#1 lore raw
Quoted 15 lines · 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.

net/mac80211/tx.c ieee80211_tx_h_sequence(struct ieee80211 · line 289

Quoted 9 lines
net/mac80211/tx.c +3 -0
@@ -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.

Signature · 1 line
Greetings Michael.
maintainer
#2 lore raw
Quoted 6 lines

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

#3 lore raw
In reply to Michael Buesch

Michael Buesch <mb@...> writes:
(...)

Quoted 2 lines
Quoted 2 lines
net/mac80211/tx.c +2 -0
+	if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
+		return TXRX_CONTINUE;

(...)

Quoted 5 lines

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?

A message that is not in the archive (sent off-list or not yet received): [email protected]

#5 lore raw
Quoted 13 lines · JMF

Michael Buesch <mb@...> writes:
(...)

Quoted 2 lines
Quoted 2 lines
(quoted diff) +2 -0
+   if (unlikely(tx->flags & IEEE80211_TXRXD_TX_INJECTED))
+           return TXRX_CONTINUE;

(...)

Quoted 5 lines

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
Quoted 8 lines

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

---

net/mac80211/tx.c +0 -0
index f35eaea..e5e8483 100644
@@ -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 [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

maintainer
#6 lore raw
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines

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

#7 lore raw
Quoted 6 lines · Johannes Berg
Quoted 5 lines
Quoted 5 lines
Quoted 5 lines

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.

Quoted 7 lines
Quoted 2 lines

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?

---

net/mac80211/tx.c +5 -0
index 1a3b50b..1fb4f92 100644
@@ -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);
maintainer
#8 lore raw
Quoted 11 lines · Stefanik Gábor
Quoted 9 lines · Johannes Berg
Quoted 8 lines
Quoted 5 lines
Quoted 5 lines

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:

Quoted 6 lines
Quoted 4 lines

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

#9 lore raw
Quoted 5 lines · On Thursday 22 May 2008 21:03:01 Johannes Berg wrote:
Quoted 4 lines · On Thu, 2008-05-22 at 20:52 +0200, Stefanik G=E1bor wrote:
Quoted 2 lines · Johannes Berg
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

This does not work, since we use injection for other types of
frames. For example management frames from hostapd. We don't=

want

Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

(and can't) make hostapd keep track of sequence numbers.
You'll have to contact radiotap people and add a flag for thi=

s.

Quoted 11 lines
Quoted 11 lines
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
Quoted 1 line

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?

Quoted 3 lines

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

#10 lore raw
In reply to Stefanik Gábor
Quoted 4 lines · On Thursday 22 May 2008 20:52:21 Stefanik G=E1bor wrote:
Quoted 2 lines · Johannes Berg
Quoted 2 lines
Quoted 2 lines
Quoted 2 lines

This does not work, since we use injection for other types of
frames. For example management frames from hostapd. We don't w=

ant

Quoted 7 lines
Quoted 7 lines
Quoted 6 lines
Quoted 3 lines
Quoted 3 lines

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

Quoted 6 lines
Quoted 4 lines

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

#11 lore raw
In reply to Johannes Berg
Quoted 9 lines · Johannes Berg
Quoted 9 lines
Quoted 6 lines
Quoted 6 lines

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!

maintainer
#12 lore raw

[don't drop CCs, restored]

Quoted 5 lines

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.

Quoted 3 lines

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

Quoted 2 lines

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

#13 lore raw
In reply to JMF
Quoted 20 lines · JMF
Quoted 9 lines · Johannes Berg
Quoted 9 lines
Quoted 6 lines
Quoted 6 lines

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?

Quoted 5 lines

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

Quoted 6 lines

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