radiotap injection bugs & extending it

radiotap injection bugs & extending it

[Johannes Berg]

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

[John, Andy, sorry for the dupe, accidentally addressed to John instead
of the list. please reply here]

Hey,

Just noticed this:
__ieee80211_tx_prepare:

struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
[...]
tx->sta = sta_info_get(local, hdr->addr1);
[...]
__ieee80211_parse_tx_radiotap(..)

which seems a bit weird. Shouldn't we grab the sta only after removing
the radiotap stuff? :)


Also, after doing more work on mac80211 I now again have hostapd running
via monitor interfaces. However, there's a bit of a problem there.

After a few changes to mac80211, I have these TX handlers:
        ieee80211_tx_h_check_assoc,
        ieee80211_tx_h_sequence,
        ieee80211_tx_h_ps_buf,
        ieee80211_tx_h_select_key,
        ieee80211_tx_h_michael_mic_add,
        ieee80211_tx_h_fragment,
        ieee80211_tx_h_encrypt,
        ieee80211_tx_h_rate_ctrl,
        ieee80211_tx_h_misc,
        ieee80211_tx_h_load_stats,

Of these, check_assoc should be skipped unconditionally for injected
packets. sequence should be done, ps_buf I'm not sure about though I
suppose that if the STA really goes into powersave very quickly then
hostapd would need ps_buf.

Then we have select_key (and mic adding/encrypt depends on it) which
should IMHO depend on IEEE80211_RADIOTAP_F_WEP; fragment could depend on
IEEE80211_RADIOTAP_F_FRAG. rate_ctrl should depend on the presence of
the IEEE80211_RADIOTAP_RATE field, if it was present then rate_ctrl is
skipped. This addresses this TODO item:
           * TODO: auto-select when the rate field is not present!

misc does a few things:
retry should be taken from IEEE80211_RADIOTAP_DATA_RETRIES if present or
otherwise automatically assigned, cts/rts should be taken from the
radiotap TX flags IEEE80211_RADIOTAP_F_TX_CTS and
IEEE80211_RADIOTAP_F_TX_RTS or automatically determined.

This is basically it, except for short preamble setting. That is a bit
of a problem, however, because it's not a tristate in radiotap and we
want a tristate (long/short/automatic). Ideas on this item?

Comments on the whole thing? I think we shouldn't be skipping TX
handlers unconditionally.

johannes

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

Re: radiotap injection bugs & extending it

[Andy Green]

Somebody in the thread at some point said:

> struct ieee80211_hdr *hdr = (struct ieee80211_hdr *) skb->data;
> [...]
> tx->sta = sta_info_get(local, hdr->addr1);
> [...]
> __ieee80211_parse_tx_radiotap(..)
> 
> which seems a bit weird. Shouldn't we grab the sta only after removing
> the radiotap stuff? :)

Yes it's wrong, hdr is also dereferenced once more while the radiotap
header is still in place.  I made a little patch I will send in a
moment, if you already did it then ignore the patch.

> Also, after doing more work on mac80211 I now again have hostapd running
> via monitor interfaces. However, there's a bit of a problem there.

You are definitely on fire at the moment Johannes.

> After a few changes to mac80211, I have these TX handlers:
>         ieee80211_tx_h_check_assoc,
>         ieee80211_tx_h_sequence,
>         ieee80211_tx_h_ps_buf,
>         ieee80211_tx_h_select_key,
>         ieee80211_tx_h_michael_mic_add,
>         ieee80211_tx_h_fragment,
>         ieee80211_tx_h_encrypt,
>         ieee80211_tx_h_rate_ctrl,
>         ieee80211_tx_h_misc,
>         ieee80211_tx_h_load_stats,
> 
> Of these, check_assoc should be skipped unconditionally for injected
> packets. sequence should be done, ps_buf I'm not sure about though I
> suppose that if the STA really goes into powersave very quickly then
> hostapd would need ps_buf.
> 
> Then we have select_key (and mic adding/encrypt depends on it) which
> should IMHO depend on IEEE80211_RADIOTAP_F_WEP; fragment could depend on
> IEEE80211_RADIOTAP_F_FRAG. rate_ctrl should depend on the presence of
> the IEEE80211_RADIOTAP_RATE field, if it was present then rate_ctrl is
> skipped. This addresses this TODO item:
>            * TODO: auto-select when the rate field is not present!
> 
> misc does a few things:
> retry should be taken from IEEE80211_RADIOTAP_DATA_RETRIES if present or
> otherwise automatically assigned, cts/rts should be taken from the
> radiotap TX flags IEEE80211_RADIOTAP_F_TX_CTS and
> IEEE80211_RADIOTAP_F_TX_RTS or automatically determined.

Sounds great to me.  I guess you are up for doing this?

> This is basically it, except for short preamble setting. That is a bit
> of a problem, however, because it's not a tristate in radiotap and we
> want a tristate (long/short/automatic). Ideas on this item?

No idea about it here.

-Andy

Re: radiotap injection bugs & extending it

[Johannes Berg]

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

On Sat, 2007-09-01 at 10:00 +0100, Andy Green wrote:

> Yes it's wrong, hdr is also dereferenced once more while the radiotap
> header is still in place.  I made a little patch I will send in a
> moment, if you already did it then ignore the patch.

No, sent the mail just before going to sleep :) I'll review your patch
and adopt it. Guess I get to rebase my ~70ish patches to after it
then ;)

> > Also, after doing more work on mac80211 I now again have hostapd running
> > via monitor interfaces. However, there's a bit of a problem there.
> 
> You are definitely on fire at the moment Johannes.

Heh, yeah, got a bit of time on my hands right now.

> > Of these, check_assoc should be skipped unconditionally for injected
> > packets. sequence should be done, ps_buf I'm not sure about though I
> > suppose that if the STA really goes into powersave very quickly then
> > hostapd would need ps_buf.
> > 
> > Then we have select_key (and mic adding/encrypt depends on it) which
> > should IMHO depend on IEEE80211_RADIOTAP_F_WEP; fragment could depend on
> > IEEE80211_RADIOTAP_F_FRAG. rate_ctrl should depend on the presence of
> > the IEEE80211_RADIOTAP_RATE field, if it was present then rate_ctrl is
> > skipped. This addresses this TODO item:
> >            * TODO: auto-select when the rate field is not present!
> > 
> > misc does a few things:
> > retry should be taken from IEEE80211_RADIOTAP_DATA_RETRIES if present or
> > otherwise automatically assigned, cts/rts should be taken from the
> > radiotap TX flags IEEE80211_RADIOTAP_F_TX_CTS and
> > IEEE80211_RADIOTAP_F_TX_RTS or automatically determined.
> 
> Sounds great to me.  I guess you are up for doing this?

Yeah, I can do that. Not sure about PS mode though. I suppose your use
case won't suffer if I simply do it unconditionally since you're always
sending multicast frames anyhow.

> > This is basically it, except for short preamble setting. That is a bit
> > of a problem, however, because it's not a tristate in radiotap and we
> > want a tristate (long/short/automatic). Ideas on this item?
> 
> No idea about it here.

I could propose adding a new radiotap TX flag, I suppose...

johannes

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

Re: radiotap injection bugs & extending it

[Andy Green]

Somebody in the thread at some point said:
> On Sat, 2007-09-01 at 10:00 +0100, Andy Green wrote:
> 
>> Yes it's wrong, hdr is also dereferenced once more while the radiotap
>> header is still in place.  I made a little patch I will send in a
>> moment, if you already did it then ignore the patch.
> 
> No, sent the mail just before going to sleep :) I'll review your patch
> and adopt it. Guess I get to rebase my ~70ish patches to after it
> then ;)

Hopefully it's just fuzz...

>>> Also, after doing more work on mac80211 I now again have hostapd running
>>> via monitor interfaces. However, there's a bit of a problem there.
>> You are definitely on fire at the moment Johannes.
> 
> Heh, yeah, got a bit of time on my hands right now.

Opposite for me, having to do paying work at the moment.

> Yeah, I can do that. Not sure about PS mode though. I suppose your use
> case won't suffer if I simply do it unconditionally since you're always
> sending multicast frames anyhow.

Well because our frames aren't targeted at any real MAC I'm not sure the
AP will buffer them for devices in PS or not.  Maybe the client having
an interface up in hardware promisc should be a sign for it to not go
into PS mode anyway?

For sure things will be much better and more consistent if we can inject
down unassociated interfaces reliably, so this is a nice move.

>>> This is basically it, except for short preamble setting. That is a bit
>>> of a problem, however, because it's not a tristate in radiotap and we
>>> want a tristate (long/short/automatic). Ideas on this item?
>> No idea about it here.
> 
> I could propose adding a new radiotap TX flag, I suppose...

I don't know enough about it to have a proper idea, sorry.

-Andy

Re: radiotap injection bugs & extending it

[Johannes Berg]

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

On Sat, 2007-09-01 at 10:21 +0100, Andy Green wrote:

> > No, sent the mail just before going to sleep :) I'll review your patch
> > and adopt it. Guess I get to rebase my ~70ish patches to after it
> > then ;)
> 
> Hopefully it's just fuzz...

In fact, it's commutative, not even as much as line offsets. :)

> Well because our frames aren't targeted at any real MAC I'm not sure the
> AP will buffer them for devices in PS or not.  Maybe the client having
> an interface up in hardware promisc should be a sign for it to not go
> into PS mode anyway?

Well, for one, the PS code is only relevant if we're ourselves operating
in IBSS or AP mode. If we're operating in AP/IBSS mode we'll also
automatically buffer your injected multicast frames for after the DTIM
beacon. I think we should just buffer injected unicast frames as well,
it won't hurt your use case (multicast destination) and makes things
more consistent.

> For sure things will be much better and more consistent if we can inject
> down unassociated interfaces reliably, so this is a nice move.

Well, no, that's unrelated, and it's a bug in iwlwifi that it craps out
over that ;)

johannes

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

Re: radiotap injection bugs & extending it

[Andy Green]

Somebody in the thread at some point said:

> Well, for one, the PS code is only relevant if we're ourselves operating
> in IBSS or AP mode. If we're operating in AP/IBSS mode we'll also
> automatically buffer your injected multicast frames for after the DTIM
> beacon. I think we should just buffer injected unicast frames as well,
> it won't hurt your use case (multicast destination) and makes things
> more consistent.

Fine by me.

>> For sure things will be much better and more consistent if we can inject
>> down unassociated interfaces reliably, so this is a nice move.
> 
> Well, no, that's unrelated, and it's a bug in iwlwifi that it craps out
> over that ;)

There are other problems hiding somewhere, perhaps iwl3945, as well.
Having a secondary network interface like mon0 in Monitor mode is still
not quite the same as having the primary network interface in Monitor mode.

Externally generated Packetspammer broadcasts are not visible down a
monitor mode secondary network interface with current wireless-dev on
iwl3945 in the case where it is associated to a network and a secondary
network interface is in Monitor mode.

Deassociate and stick the wlan0 interface in Monitor mode though
(without changing or deleting the mon0 interface) and you suddenly see
the packetspammer broadcasts from the other box just fine.

-Andy

Re: radiotap injection bugs & extending it

[Johannes Berg]

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

On Sat, 2007-09-01 at 10:47 +0100, Andy Green wrote:

> > Well, for one, the PS code is only relevant if we're ourselves operating
> > in IBSS or AP mode. If we're operating in AP/IBSS mode we'll also
> > automatically buffer your injected multicast frames for after the DTIM
> > beacon. I think we should just buffer injected unicast frames as well,
> > it won't hurt your use case (multicast destination) and makes things
> > more consistent.
> 
> Fine by me.

Good. I'll look at it later or next week.

> There are other problems hiding somewhere, perhaps iwl3945, as well.
> Having a secondary network interface like mon0 in Monitor mode is still
> not quite the same as having the primary network interface in Monitor mode.

Definitely iwlwifi. Might be caused by going into power management mode
though, not much you can do about it then. But that reminds me of
something I need to check.

johannes

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