Re: ieee80211_tx_status() on injected packets

[Johannes Berg <johannes@sipsolutions.net>]

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

On Fri, 2009-07-10 at 18:32 -0400, Pavel Roskin wrote:

> /* Original radiotap header, but the length should be 0e, not 18 */
> 00 00 18 00 03 00 00 00 00 02 6c 09 a0 00

Heh.

> /* mac80211 treats this as part of the radiotap header */
> 08 03 00 00 01 0c cc 00 00 00
> /* frame control */
> 00 11
> /* duration */
> 6b 39
> /* addr1 */
> 40 19 11 04 28 00
> /* addr2 */
> 00 00 10 00 00 00
> /* addr3 - incomplete */
> 00 00 00 00
> /* sequence control - beyond the skb end */
> 
> I'm using rt73usb to inject.  ieee80211_tx_status() is scheduled by
> ieee80211_tx_status_irqsafe(), which is called in rt2x00dev.c.
> 
> If we allow to inject malformed packets, we shouldn't assume them to be
> valid 802.11 packets unless we can verify it.  And even then, maybe it's
> better to bypass ieee80211_tx_status() for injected packets, as it can
> influence statistics and rate control algorithms in unpredictable ways.

Yeah, we should verify the length. I don't think we can skip the
processing since these packets might actually be sent by hostapd which
wants the processing -- if you fuck up your connection by injecting
random junk that seems to be your own fault, but I agree we should fix
the bug.

It would probably be useful to take the code in ieee80211_tx_status up
to (but not including) the skb_orphan() call, stick it into a separate
function and call it only when at least the frame the header is valid
(i.e. skb->len >= hdrlen).

johannes

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