linux-kernel - Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201612241944.33216@pali>
Date:   Sat, 24 Dec 2016 19:44:33 +0100
From:   Pali Rohár <pali.rohar@...il.com>
To:     Pavel Machek <pavel@....cz>
Cc:     Ming Lei <ming.lei@...onical.com>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        "Greg Kroah-Hartman" <gregkh@...uxfoundation.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        David Gnedt <david.gnedt@...izone.at>,
        Michal Kazior <michal.kazior@...to.com>,
        Daniel Wagner <wagi@...om.org>,
        Tony Lindgren <tony@...mide.com>,
        Sebastian Reichel <sre@...nel.org>,
        Ivaylo Dimitrov <ivo.g.dimitrov.75@...il.com>,
        Aaro Koskinen <aaro.koskinen@....fi>,
        Grazvydas Ignotas <notasas@...il.com>,
        linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
        netdev@...r.kernel.org
Subject: Re: [PATCH 6/6] wl1251: Set generated MAC address back to NVS data

On Saturday 24 December 2016 19:17:30 Pavel Machek wrote:
> Hi!
> 
> > In case there is no valid MAC address kernel generates random one.
> > This patch propagate this generated MAC address back to NVS data
> > which will be uploaded to wl1251 chip. So HW would have same MAC
> > address as linux kernel uses.
> > 
> >  	return 0;
> >  
> >  }
> > 
> > +static int wl1251_write_nvs_mac(struct wl1251 *wl)
> > +{
> 
> The name is quite confusing, this sounds like writing into
> non-volatile storage.
> 
> > +	int i;
> > +
> > +	if (wl->nvs_len < 0x24)
> > +		return -ENODATA;
> > +
> > +	/* length is 2 and data address is 0x546c (mask is 0xfffe) */
> 
> You don't actually check for the mask.

It is quite complicated. { 0x6d, 0x54 } (= 0x546d) in data represent 
address 0x546c and content are data. You need to apply mask 0xfffe for 
0x546d and you get address where data will be written (so 0x546c).

> > +	if (wl->nvs[0x19] != 2 || wl->nvs[0x1a] != 0x6d || wl->nvs[0x1b]
> > != 0x54) +		return -EINVAL;
> 
> You have two copies of these. Does it make sense to move it to helper
> function?

I'm thinking if checks is really needed. But probably moving it to 
separate function is good idea.

-- 
Pali Rohár
pali.rohar@...il.com

Download attachment "signature.asc " of type "application/pgp-signature" (199 bytes)