netdev - Re: [PATCH net v2] Revert "net/ncsi: change from ndo_set_mac_address to dev_set_mac

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: <Z37eu/758pzGSGzO@home.paul.comp>
Date: Wed, 8 Jan 2025 23:23:23 +0300
From: Paul Fertser <fercerpav@...il.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
        pabeni@...hat.com, andrew+netdev@...n.ch, horms@...nel.org,
        Potin Lai <potin.lai@...ntatw.com>, Potin Lai <potin.lai.pt@...il.com>,
        sam@...dozajonas.com, fr0st61te@...il.com
Subject: Re: [PATCH net v2] Revert "net/ncsi: change from ndo_set_mac_address
 to dev_set_mac_address"

Hello,

On Wed, Jan 08, 2025 at 11:23:46AM -0800, Jakub Kicinski wrote:
> Looks like we're not making any progress on this one, so let's
> go with the revert for 6.13.

But this does break userspace, the commit was there for a reason.

Potin Lai, have you tried deferring this to a work queue instead of
reverting to the code which has always been wrong?

> Original posting by Potin Lai:
>   https://lore.kernel.org/20241129-potin-revert-ncsi-set-mac-addr-v1-1-94ea2cb596af@gmail.com

Unfortunately I wasn't on CC and missed that. Can we fix this proper
way please, do we have few more days?

If I get it right dev_set_mac_address() just isn't meant to be ever
called from a softirq (and NCSI here is special in getting a MAC
address update from a "network" frame so that happens in
net_rx_action() context). So postponing the actual processing of this
reply looks like the way to go, right?

> -	rtnl_lock();
> -	ret = dev_set_mac_address(ndev, &saddr, NULL);
> -	rtnl_unlock();
> +	ret = ops->ndo_set_mac_address(ndev, &saddr);
>
>  	if (ret < 0)
>  		netdev_warn(ndev, "NCSI: 'Writing mac address to device failed\n");

I expect if this is scheduled on events/x with queue_work() then we'll
get what we need.

-- 
Be free, use free (http://www.gnu.org/philosophy/free-sw.html) software!
mailto:fercerpav@...il.com