[PATCH] rtl8187: ensure priv->hwaddr is always valid

[PATCH] rtl8187: ensure priv->hwaddr is always valid

[Michael Wu]

From: Michael Wu <flamingice@sourmilk.net>

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
<johannes@sipsolutions.net> for finding this problem.

Signed-off-by: Michael Wu <flamingice@sourmilk.net>
---

 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;
 }

Re: [PATCH] rtl8187: ensure priv->hwaddr is always valid

[Johannes Berg]

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

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

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

Re: [PATCH] rtl8187: ensure priv->hwaddr is always valid

[Michael Buesch]

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?

Re: [PATCH] rtl8187: ensure priv->hwaddr is always valid

[Michael Wu]

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

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

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: [PATCH] rtl8187: ensure priv->hwaddr is always valid

[Johannes Berg]

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

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

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