netdev - Re: [PATCH net-next 1/3] iavf: add check for current MAC address in set

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]

Date: Wed, 7 Jun 2023 22:22:19 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Jakub Kicinski <kuba@...nel.org>
CC: Piotr Gardocki <piotrx.gardocki@...el.com>, Tony Nguyen
	<anthony.l.nguyen@...el.com>, <davem@...emloft.net>, <pabeni@...hat.com>,
	<edumazet@...gle.com>, <netdev@...r.kernel.org>, Michal Swiatkowski
	<michal.swiatkowski@...ux.intel.com>, Rafal Romanowski
	<rafal.romanowski@...el.com>, <aleksander.lobakin@...el.com>
Subject: Re: [PATCH net-next 1/3] iavf: add check for current MAC address in
 set_mac callback

On Wed, Jun 07, 2023 at 09:38:10AM -0700, Jakub Kicinski wrote:
> On Wed, 7 Jun 2023 12:29:36 +0200 Piotr Gardocki wrote:
> > I need a piece of advice though:
> > 1) Should I fix it in this patch set, or treat it as a separate thread?
> 
> Separate is probably better, you can post such a change directly 
> to netdev, without going via the Intel tree.

That's what we like:D

> 
> > 2) I suppose the change is required only in dev_set_mac_address function, but
> > am I right assuming we should do it before call to dev_pre_changeaddr_notify
> > and return from function early? What about call to add_device_randomness?
> 
> I'd add the check right after the netif_device_present() check and not
> worry about notifier or randomness. The address isn't changing so
> nothing to notify about and no real randomness to be gained.

I find this as positive side effect - why would i want to notify anyone
that my addr has changed if it was not in fact changed? This is happening
in the current approach where you exit with success code from
ndo_set_mac_address().