From: Michael Wu <[email protected]>
conf->mac_addr is not guaranteed to be set. This ensures priv->hwaddr is
always set to a valid mac address. Thanks to Johannes Berg
<[email protected]> for finding this problem.
Signed-off-by: Michael Wu <[email protected]>
---
drivers/net/wireless/rtl8187_dev.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/wireless/rtl8187_dev.c b/drivers/net/wireless/rtl8187_dev.c
index cea8589..e61c6d5 100644
--- a/drivers/net/wireless/rtl8187_dev.c
+++ b/drivers/net/wireless/rtl8187_dev.c
@@ -466,7 +466,7 @@ static int rtl8187_add_interface(struct ieee80211_hw *dev,
return -EOPNOTSUPP;
}
- priv->hwaddr = conf->mac_addr;
+ priv->hwaddr = conf->mac_addr ? conf->mac_addr : dev->wiphy->perm_addr;
return 0;
}
On Monday 16 July 2007 02:34, Johannes Berg wrote:
> Do you allow monitor mode? If so, this isn't correct unless the hardware
> is explicitly told to not do anything (most hw doesn't and we use a zero
> mac addr for that). conf->mac_addr is NULL in monitor mode for a reason:
> we don't want to have any mac address so we don't ACK packets.
>
It eliminates a NULL pointer dereference, which is a bit more important than
accidentally ACKing frames. I do not have time right now to put together a
fix better than this.
-Michael Wu
On Sun, 2007-07-15 at 17:09 -0700, Michael Wu wrote:
>
> - priv->hwaddr = conf->mac_addr;
> + priv->hwaddr = conf->mac_addr ? conf->mac_addr : dev->wiphy->perm_addr;
Do you allow monitor mode? If so, this isn't correct unless the hardware
is explicitly told to not do anything (most hw doesn't and we use a zero
mac addr for that). conf->mac_addr is NULL in monitor mode for a reason:
we don't want to have any mac address so we don't ACK packets.
johannes
On Monday 16 July 2007 11:34, Johannes Berg wrote:
> On Sun, 2007-07-15 at 17:09 -0700, Michael Wu wrote:
> >
> > - priv->hwaddr = conf->mac_addr;
> > + priv->hwaddr = conf->mac_addr ? conf->mac_addr : dev->wiphy->perm_addr;
>
> Do you allow monitor mode? If so, this isn't correct unless the hardware
> is explicitly told to not do anything (most hw doesn't and we use a zero
> mac addr for that). conf->mac_addr is NULL in monitor mode for a reason:
> we don't want to have any mac address so we don't ACK packets.
I'd suggest changing this behaviour in the stack, as this bug will
be made again for new drivers in the future.
What about pointing mac_addr to a 00:00:00:00:00 const static mac address,
instead of passing a NULL pointer?
On Tue, 2007-07-17 at 20:25 +0200, Michael Buesch wrote:
> I'd suggest changing this behaviour in the stack, as this bug will
> be made again for new drivers in the future.
> What about pointing mac_addr to a 00:00:00:00:00 const static mac address,
> instead of passing a NULL pointer?
I disagree. Having the if statement in the driver doesn't hurt and
neither does having a static zero mac address, but hardware that does
require special setup instead of a zero mac address would have to do
is_zero_etheraddr() or something to figure it out.
johannes