2017-09-18 07:45:58

by Ganapathi Bhat

[permalink] [raw]
Subject: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

Avoid calculating random MAC address in driver. Instead make
use of 'get_random_mask_addr()' function.

Signed-off-by: Ganapathi Bhat <[email protected]>
---
drivers/net/wireless/marvell/mwifiex/cfg80211.c | 11 +++++------
1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/net/wireless/marvell/mwifiex/cfg80211.c b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
index 84c1518..2b293b1 100644
--- a/drivers/net/wireless/marvell/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/marvell/mwifiex/cfg80211.c
@@ -2503,6 +2503,7 @@ static int mwifiex_set_ibss_params(struct mwifiex_private *priv,
struct ieee80211_channel *chan;
struct ieee_types_header *ie;
struct mwifiex_user_scan_cfg *user_scan_cfg;
+ u8 mac_addr[ETH_ALEN];

mwifiex_dbg(priv->adapter, CMD,
"info: received scan request on %s\n", dev->name);
@@ -2529,12 +2530,10 @@ static int mwifiex_set_ibss_params(struct mwifiex_private *priv,
priv->scan_request = request;

if (request->flags & NL80211_SCAN_FLAG_RANDOM_ADDR) {
- for (i = 0; i < ETH_ALEN; i++) {
- request->mac_addr[i] &= request->mac_addr_mask[i];
- request->mac_addr[i] |=
- get_random_int() & ~(request->mac_addr_mask[i]);
- }
- ether_addr_copy(user_scan_cfg->random_mac, request->mac_addr);
+ get_random_mask_addr(mac_addr, request->mac_addr,
+ request->mac_addr_mask);
+ ether_addr_copy(request->mac_addr, mac_addr);
+ ether_addr_copy(user_scan_cfg->random_mac, mac_addr);
}

user_scan_cfg->num_ssids = request->n_ssids;
--
1.9.1


2017-09-20 11:47:39

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

Brian Norris <[email protected]> writes:

> Hi,
>
> On Tue, Sep 19, 2017 at 05:30:06PM +0300, Kalle Valo wrote:
>> Ganapathi Bhat <[email protected]> writes:
>>
>> > Hi Kalle,
>> >>
>> >> > Avoid calculating random MAC address in driver. Instead make use of
>> >> > 'get_random_mask_addr()' function.
>> >> >
>> >> > Signed-off-by: Ganapathi Bhat <[email protected]>
>> >>
>> >> I don't see 1/2 anywhere. Did it get lost?
>> >
>> > Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C
>
> It's dependent on this patch though, which kinda should be '1/2':
>
> [PATCH] mwifiex: avoid storing random_mac in private

Thanks for pointing out, I'll make sure that I commit these in correct
order.

>> > (to correct a typo); and then tried sending it again. I think that
>> > created some problem here. Kindly let me know how to proceed.
>>
>> Ok. I'll wait for review comments and if all goes well I'll apply it in
>> few days.
>
> FWIW, this looks OK to me:
>
> Reviewed-by: Brian Norris <[email protected]>
>
> It's just a bit strange that we have to keep our own on-stack temporary
> buffer for this. Maybe this could use an in-place helper too? Or (if
> it's really legal for us to modify the cfg80211_scan_request in-place)
> why doesn't the upper-layer nl80211 code do the randomization for us?
> Many (all?) drivers I see implementing randomization have to do this
> anyway; they don't use request->mac_addr directly. (Or I suppose some
> firmware could implement the randomization on its own someday...but
> would we really trust it?)

Good questions and I was wondering the same when looking at this patch.
But I wasn't involved in the interface design so I don't really know the
background here.

I'm planning to apply this patch anyway, any improvements can be done as
a followup patch.

--
Kalle Valo

2017-09-20 12:49:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [2/2] mwifiex: use get_random_mask_addr() helper

Ganapathi Bhat <[email protected]> wrote:

> Avoid calculating random MAC address in driver. Instead make
> use of 'get_random_mask_addr()' function.
>
> Signed-off-by: Ganapathi Bhat <[email protected]>
> Reviewed-by: Brian Norris <[email protected]>

Patch applied to wireless-drivers-next.git, thanks.

e9a3846afaa4 mwifiex: use get_random_mask_addr() helper

--
https://patchwork.kernel.org/patch/9955631/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

2017-09-18 14:13:46

by Ganapathi Bhat

[permalink] [raw]
Subject: RE: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

Hi Kalle,
>
> > Avoid calculating random MAC address in driver. Instead make use of
> > 'get_random_mask_addr()' function.
> >
> > Signed-off-by: Ganapathi Bhat <[email protected]>
>
> I don't see 1/2 anywhere. Did it get lost?
Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C (to correct a typo); and then tried sending it again. I think that created some problem here. Kindly let me know how to proceed.

Thanks,
Ganapathi
>
> --
> Kalle Valo

2017-09-19 14:30:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

Ganapathi Bhat <[email protected]> writes:

> Hi Kalle,
>>
>> > Avoid calculating random MAC address in driver. Instead make use of
>> > 'get_random_mask_addr()' function.
>> >
>> > Signed-off-by: Ganapathi Bhat <[email protected]>
>>
>> I don't see 1/2 anywhere. Did it get lost?
>
> Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C
> (to correct a typo); and then tried sending it again. I think that
> created some problem here. Kindly let me know how to proceed.

Ok. I'll wait for review comments and if all goes well I'll apply it in
few days.

--
Kalle Valo

2017-09-18 13:17:12

by Kalle Valo

[permalink] [raw]
Subject: Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

Ganapathi Bhat <[email protected]> writes:

> Avoid calculating random MAC address in driver. Instead make
> use of 'get_random_mask_addr()' function.
>
> Signed-off-by: Ganapathi Bhat <[email protected]>

I don't see 1/2 anywhere. Did it get lost?

--
Kalle Valo

2017-09-19 16:43:21

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH 2/2] mwifiex: use get_random_mask_addr() helper

Hi,

On Tue, Sep 19, 2017 at 05:30:06PM +0300, Kalle Valo wrote:
> Ganapathi Bhat <[email protected]> writes:
>
> > Hi Kalle,
> >>
> >> > Avoid calculating random MAC address in driver. Instead make use of
> >> > 'get_random_mask_addr()' function.
> >> >
> >> > Signed-off-by: Ganapathi Bhat <[email protected]>
> >>
> >> I don't see 1/2 anywhere. Did it get lost?
> >
> > Actually there is no 1/2. What I did is: 'git send-email'; CTRL + C

It's dependent on this patch though, which kinda should be '1/2':

[PATCH] mwifiex: avoid storing random_mac in private

> > (to correct a typo); and then tried sending it again. I think that
> > created some problem here. Kindly let me know how to proceed.
>
> Ok. I'll wait for review comments and if all goes well I'll apply it in
> few days.

FWIW, this looks OK to me:

Reviewed-by: Brian Norris <[email protected]>

It's just a bit strange that we have to keep our own on-stack temporary
buffer for this. Maybe this could use an in-place helper too? Or (if
it's really legal for us to modify the cfg80211_scan_request in-place)
why doesn't the upper-layer nl80211 code do the randomization for us?
Many (all?) drivers I see implementing randomization have to do this
anyway; they don't use request->mac_addr directly. (Or I suppose some
firmware could implement the randomization on its own someday...but
would we really trust it?)

Brian