Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid

[Pali Rohár]

On Saturday 24 December 2016 19:08:54 Pavel Machek wrote:
> On Sat 2016-12-24 17:52:59, Pali Rohár wrote:
> > Before this patch driver generated random MAC address every time
> > when was doing initialization. And after that random MAC address
> > could be overwritten with fixed one if provided.
> 
> Before this patch, driver generated random MAC address every time it
> was initialized. After that random MAC address could be overwritten
> with fixed one, if provided.
> 
> > This patch changes order. First it tries to read fixed MAC address
> > and if it fails then driver generates random MAC address.
> 
> I don't quite get where the advantage is supposed to be. Is it that
> "use_eeprom" is set, but reading fails?

Random bytes are read from kernel only if random MAC address is needed. 
And in wl->mac_addr is always either invalid address or permanenent mac 
address which will be used. Without patch in wl->mac_addr can be random 
temporary address for some time...

> The only case where this helps is if wl1251_read_eeprom_mac()
> succeeds but reads invalid address.
> 
> > Signed-off-by: Pali Rohár 
> 
> Acked-by: Pavel Machek 

-- 
Pali Rohár
pali.ro...@gmail.com


signature.asc
Description: This is a digitally signed message part.

Re: [PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid

[Pavel Machek]

On Sat 2016-12-24 17:52:59, Pali Rohár wrote:

> Before this patch driver generated random MAC address every time when was
> doing initialization. And after that random MAC address could be
> overwritten with fixed one if provided.

Before this patch, driver generated random MAC address every time it
was initialized. After that random MAC address could be overwritten
with fixed one, if provided.

> This patch changes order. First it tries to read fixed MAC address and if
> it fails then driver generates random MAC address.

I don't quite get where the advantage is supposed to be. Is it that
"use_eeprom" is set, but reading fails?

The only case where this helps is if wl1251_read_eeprom_mac() succeeds
but reads invalid address.

> Signed-off-by: Pali Rohár 

Acked-by: Pavel Machek 

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) 
http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


signature.asc
Description: Digital signature

[PATCH 4/6] wl1251: Generate random MAC address only if driver does not have valid

[Pali Rohár]

Before this patch driver generated random MAC address every time when was
doing initialization. And after that random MAC address could be
overwritten with fixed one if provided.

This patch changes order. First it tries to read fixed MAC address and if
it fails then driver generates random MAC address.

Signed-off-by: Pali Rohár 
---
 drivers/net/wireless/ti/wl1251/main.c |   27 ++-
 1 file changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ti/wl1251/main.c 
b/drivers/net/wireless/ti/wl1251/main.c
index 8971b64..c3fa0b6 100644
--- a/drivers/net/wireless/ti/wl1251/main.c
+++ b/drivers/net/wireless/ti/wl1251/main.c
@@ -1582,7 +1582,24 @@ int wl1251_init_ieee80211(struct wl1251 *wl)
wl->hw->queues = 4;
 
if (wl->use_eeprom)
-   wl1251_read_eeprom_mac(wl);
+   ret = wl1251_read_eeprom_mac(wl);
+   else
+   ret = -EINVAL;
+
+   if (ret == 0 && !is_valid_ether_addr(wl->mac_addr))
+   ret = -EINVAL;
+
+   if (ret < 0) {
+   /*
+* In case our MAC address is not correctly set,
+* we use a random but Nokia MAC.
+*/
+   static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
+   memcpy(wl->mac_addr, nokia_oui, 3);
+   get_random_bytes(wl->mac_addr + 3, 3);
+   wl1251_warning("MAC address in eeprom or nvs data is not 
valid");
+   wl1251_warning("Setting random MAC address: %pM", wl->mac_addr);
+   }
 
ret = wl1251_register_hw(wl);
if (ret)
@@ -1623,7 +1640,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
struct ieee80211_hw *hw;
struct wl1251 *wl;
int i;
-   static const u8 nokia_oui[3] = {0x00, 0x1f, 0xdf};
 
hw = ieee80211_alloc_hw(sizeof(*wl), _ops);
if (!hw) {
@@ -1674,13 +1690,6 @@ struct ieee80211_hw *wl1251_alloc_hw(void)
INIT_WORK(>irq_work, wl1251_irq_work);
INIT_WORK(>tx_work, wl1251_tx_work);
 
-   /*
-* In case our MAC address is not correctly set,
-* we use a random but Nokia MAC.
-*/
-   memcpy(wl->mac_addr, nokia_oui, 3);
-   get_random_bytes(wl->mac_addr + 3, 3);
-
wl->state = WL1251_STATE_OFF;
mutex_init(>mutex);
 
-- 
1.7.9.5