2007-06-20 11:18:38

by Johannes Berg

[permalink] [raw]
Subject: driver bugs with changed mac address

Hi,

I was just looking through some bcm43xx code and noticed that when you
set a different mac address for some interface, then it is stored to
wl->mac_addr but this is only ever used in
bcm43xx_write_mac_bssid_templates and not in bcm43xx_wireless_core_init.

iwlwifi on the other hand completely ignores the mac address after
printing it out.

Also in bcm43xx, when the interface is removed again wl->mac_addr isn't
set to NULL when the interface that the MAC belonged to is deleted, so
that the mac address stays programmed on the card and we continue to ACK
packets for that MAC address.

In zd1211, we start with hwaddr = dev->wiphy->perm_addr which isn't
correct either, for a pure monitor mode we want to start with a zero mac
addr to avoid acking packets. Also, zd1211rw will end up having a NULL
hwaddr when a monitor interface is added, most likely segfaulting in
zd_write_mac_addr then.

The NULL mac address will also happen to rtl818x and possibly p54 (at
least when that gets monitor mode support)

Whew. So much for our little mac address audit.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-06-20 21:55:26

by Michael Büsch

[permalink] [raw]
Subject: Re: driver bugs with changed mac address

On Wednesday 20 June 2007 12:05:38 Johannes Berg wrote:
> Hi,
>
> I was just looking through some bcm43xx code and noticed that when you
> set a different mac address for some interface, then it is stored to
> wl->mac_addr but this is only ever used in
> bcm43xx_write_mac_bssid_templates and not in bcm43xx_wireless_core_init.
>
> iwlwifi on the other hand completely ignores the mac address after
> printing it out.
>
> Also in bcm43xx, when the interface is removed again wl->mac_addr isn't
> set to NULL when the interface that the MAC belonged to is deleted, so
> that the mac address stays programmed on the card and we continue to ACK
> packets for that MAC address.
>
> In zd1211, we start with hwaddr = dev->wiphy->perm_addr which isn't
> correct either, for a pure monitor mode we want to start with a zero mac
> addr to avoid acking packets. Also, zd1211rw will end up having a NULL
> hwaddr when a monitor interface is added, most likely segfaulting in
> zd_write_mac_addr then.
>
> The NULL mac address will also happen to rtl818x and possibly p54 (at
> least when that gets monitor mode support)
>
> Whew. So much for our little mac address audit.
>
> johannes
>

Nice catch, thanks!

This should fix it on bcm43xx:
http://bu3sch.de/gitweb?p=wireless-dev.git;a=commitdiff;h=dbf39fd9f4cc4edc35ed7ab349c55137ea35aea0

--
Greetings Michael.

2007-07-26 15:39:47

by Johannes Berg

[permalink] [raw]
Subject: Re: driver bugs with changed mac address

On Wed, 2007-07-25 at 22:21 -0400, Daniel Drake wrote:

> Looking at this now, the behaviour you describe is not what happens.
> When I bring up an interface set to monitor mode, zd_mac_add_interface()
> gets called with a non-NULL mac address, the actual MAC of the device.
> So, when zd_mac_open gets called soon after, it calls zd_write_mac_addr
> with a non-NULL address, so it succeeds as normal.
>
> Should I ignore the above and just set an all-zero MAC in the hardware
> when a monitor interface is brought up? (setting all-zero will disable acks)

Oh, hm, it looks like there's a behavioural difference in mac80211 when
you have IEEE80211_HW_MONITOR_DURING_OPER vs. when you don't.

johannes


Attachments:
signature.asc (190.00 B)
This is a digitally signed message part

2007-07-26 02:23:11

by Daniel Drake

[permalink] [raw]
Subject: Re: driver bugs with changed mac address

Johannes Berg wrote:
> In zd1211, we start with hwaddr = dev->wiphy->perm_addr which isn't
> correct either, for a pure monitor mode we want to start with a zero mac
> addr to avoid acking packets. Also, zd1211rw will end up having a NULL
> hwaddr when a monitor interface is added, most likely segfaulting in
> zd_write_mac_addr then.

Sorry for the delay in getting to this.

Looking at this now, the behaviour you describe is not what happens.
When I bring up an interface set to monitor mode, zd_mac_add_interface()
gets called with a non-NULL mac address, the actual MAC of the device.
So, when zd_mac_open gets called soon after, it calls zd_write_mac_addr
with a non-NULL address, so it succeeds as normal.

Should I ignore the above and just set an all-zero MAC in the hardware
when a monitor interface is brought up? (setting all-zero will disable acks)

Thanks,
Daniel