2010-03-02 02:52:00

by Bruno Randolf

[permalink] [raw]
Subject: [PATCH v2] ath5k: fix injection in monitor mode

injected frames have to use AR5K_PKT_TYPE_NORMAL, otherwise the hardware thinks
it can mess with the contents of the frame - e.g. update the TSF of an injected
beacon. injected frames should be sent as they are provided.

Signed-off-by: Bruno Randolf <[email protected]>
---

drivers/net/wireless/ath/ath5k/base.c | 11 ++++++++++-
1 files changed, 10 insertions(+), 1 deletions(-)

diff --git a/drivers/net/wireless/ath/ath5k/base.c b/drivers/net/wireless/ath/ath5k/base.c
index a078f1e..ced13fd 100644
--- a/drivers/net/wireless/ath/ath5k/base.c
+++ b/drivers/net/wireless/ath/ath5k/base.c
@@ -1285,6 +1285,7 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
u16 cts_rate = 0;
u16 duration = 0;
u8 rc_flags;
+ enum ath5k_pkt_type pkt_type;

flags = AR5K_TXDESC_INTREQ | AR5K_TXDESC_CLRDMASK;

@@ -1322,9 +1323,17 @@ ath5k_txbuf_setup(struct ath5k_softc *sc, struct ath5k_buf *bf,
duration = le16_to_cpu(ieee80211_ctstoself_duration(sc->hw,
sc->vif, pktlen, info));
}
+
+ /* we don't want the hardware to mess with injected frames in monitor
+ * mode (e.g. update TSF in beacons) */
+ if (info->flags & IEEE80211_TX_CTL_INJECTED)
+ pkt_type = AR5K_PKT_TYPE_NORMAL;
+ else
+ pkt_type = get_hw_packet_type(skb);
+
ret = ah->ah_setup_tx_desc(ah, ds, pktlen,
ieee80211_get_hdrlen_from_skb(skb),
- get_hw_packet_type(skb),
+ pkt_type,
(sc->power_level * 2),
hw_rate,
info->control.rates[0].count, keyidx, ah->ah_tx_ant, flags,



2010-03-03 01:11:11

by Bruno Randolf

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: fix injection in monitor mode

On Tuesday 02 March 2010 18:42:38 Jouni Malinen wrote:
> On Tue, Mar 02, 2010 at 11:51:58AM +0900, Bruno Randolf wrote:
> > injected frames have to use AR5K_PKT_TYPE_NORMAL, otherwise the hardware
> > thinks it can mess with the contents of the frame - e.g. update the TSF
> > of an injected beacon. injected frames should be sent as they are
> > provided.
>
> Why would we never want the hardware to update fields in injected
> frames? Unless I missed something, this seems to break hostapd use cases
> where the hardware is indeed expected to update the timestamp in Probe
> Response frames and set up seq# and duration etc. as appropriate for the
> frames (the latter may be driver/mac80211 work; the timestamp needs to
> come from hardware/firmware).

ok, i see.

> If we want to have an option to prevent hardware from touching the frame
> payload, that really should be an option (a radiotap and TX control
> flags, etc.), not default functionality for monitor interface. The use
> case for this seems to be some kind of testing purpose and that should
> not really break more common functionality.

yes, we use it for testing IBSS mode (merging, TSF updates) by injecting
custom beacons. i guess other packet injectors would also assume that their
packets go out untouched.

so what would be a way to support that properly?
what about a monitor mode flag?

bruno

2010-03-03 08:19:04

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: fix injection in monitor mode

On Wed, Mar 03, 2010 at 10:10:49AM +0900, Bruno Randolf wrote:
> On Tuesday 02 March 2010 18:42:38 Jouni Malinen wrote:
> > If we want to have an option to prevent hardware from touching the frame
> > payload, that really should be an option (a radiotap and TX control
> > flags, etc.), not default functionality for monitor interface.

> yes, we use it for testing IBSS mode (merging, TSF updates) by injecting
> custom beacons. i guess other packet injectors would also assume that their
> packets go out untouched.

Like I said, not all packet injectors do and hostapd certainly depends
on the injected packet being updated (both for contents and for
selecting a suitable TX rate and also to encrypt the frame when a key is
set for this).

> so what would be a way to support that properly?
> what about a monitor mode flag?

My preference is shown in the quoted text above, i.e., a new radiotap
(per packet, not per monitor interface) flag and then internally in
mac80211--driver a new TX control flag to indicate that the frame is not
to be updated.

--
Jouni Malinen PGP id EFC895FA

2010-03-02 10:02:49

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: fix injection in monitor mode

On Tue, Mar 2, 2010 at 10:42 AM, Jouni Malinen <[email protected]> wrote:
> On Tue, Mar 02, 2010 at 11:51:58AM +0900, Bruno Randolf wrote:
>> injected frames have to use AR5K_PKT_TYPE_NORMAL, otherwise the hardware thinks
>> it can mess with the contents of the frame - e.g. update the TSF of an injected
>> beacon. injected frames should be sent as they are provided.
>
> Why would we never want the hardware to update fields in injected
> frames? Unless I missed something, this seems to break hostapd use cases
> where the hardware is indeed expected to update the timestamp in Probe
> Response frames and set up seq# and duration etc. as appropriate for the
> frames (the latter may be driver/mac80211 work; the timestamp needs to
> come from hardware/firmware).
>
> If we want to have an option to prevent hardware from touching the frame
> payload, that really should be an option (a radiotap and TX control
> flags, etc.), not default functionality for monitor interface. The use
> case for this seems to be some kind of testing purpose and that should
> not really break more common functionality.
>
>
>> + ? ? /* we don't want the hardware to mess with injected frames in monitor
>> + ? ? ?* mode (e.g. update TSF in beacons) */
>> + ? ? if (info->flags & IEEE80211_TX_CTL_INJECTED)
>> + ? ? ? ? ? ? pkt_type = AR5K_PKT_TYPE_NORMAL;
>
> This is the part I object to. IEEE80211_TX_CTL_INJECTED should not be
> used for this.
>

Umm... what should be used, then? A check for monitor mode? That's
even more wrong...

> --
> Jouni Malinen ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?PGP id EFC895FA
>



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

2010-03-03 08:42:37

by Gábor Stefanik

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: fix injection in monitor mode

On Wed, Mar 3, 2010 at 9:18 AM, Jouni Malinen <[email protected]> wrote:
> On Wed, Mar 03, 2010 at 10:10:49AM +0900, Bruno Randolf wrote:
>> On Tuesday 02 March 2010 18:42:38 Jouni Malinen wrote:
>> > If we want to have an option to prevent hardware from touching the frame
>> > payload, that really should be an option (a radiotap and TX control
>> > flags, etc.), not default functionality for monitor interface.
>
>> yes, we use it for testing IBSS mode (merging, TSF updates) by injecting
>> custom beacons. i guess other packet injectors would also assume that their
>> packets go out untouched.
>
> Like I said, not all packet injectors do and hostapd certainly depends
> on the injected packet being updated (both for contents and for
> selecting a suitable TX rate and also to encrypt the frame when a key is
> set for this).
>
>> so what would be a way to support that properly?
>> what about a monitor mode flag?
>
> My preference is shown in the quoted text above, i.e., a new radiotap
> (per packet, not per monitor interface) flag and then internally in
> mac80211--driver a new TX control flag to indicate that the frame is not
> to be updated.
>
> --
> Jouni Malinen ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?PGP id EFC895FA
>

Shouldn't making this depend on COOK_FRAMES not being set be enough? A
while ago, the consensus was that injectors that don't set COOK_FRAMES
(should) expect packets to be transmitted unmodified.

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

2010-03-02 09:42:59

by Jouni Malinen

[permalink] [raw]
Subject: Re: [PATCH v2] ath5k: fix injection in monitor mode

On Tue, Mar 02, 2010 at 11:51:58AM +0900, Bruno Randolf wrote:
> injected frames have to use AR5K_PKT_TYPE_NORMAL, otherwise the hardware thinks
> it can mess with the contents of the frame - e.g. update the TSF of an injected
> beacon. injected frames should be sent as they are provided.

Why would we never want the hardware to update fields in injected
frames? Unless I missed something, this seems to break hostapd use cases
where the hardware is indeed expected to update the timestamp in Probe
Response frames and set up seq# and duration etc. as appropriate for the
frames (the latter may be driver/mac80211 work; the timestamp needs to
come from hardware/firmware).

If we want to have an option to prevent hardware from touching the frame
payload, that really should be an option (a radiotap and TX control
flags, etc.), not default functionality for monitor interface. The use
case for this seems to be some kind of testing purpose and that should
not really break more common functionality.


> + /* we don't want the hardware to mess with injected frames in monitor
> + * mode (e.g. update TSF in beacons) */
> + if (info->flags & IEEE80211_TX_CTL_INJECTED)
> + pkt_type = AR5K_PKT_TYPE_NORMAL;

This is the part I object to. IEEE80211_TX_CTL_INJECTED should not be
used for this.

--
Jouni Malinen PGP id EFC895FA