linux-kernel - Re: [PATCH] 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] [day] [month] [year] [list]

Message-ID: <33c32912-cf13-49c9-a786-a44c0bb482a6@lunn.ch>
Date: Fri, 29 Nov 2024 16:54:59 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Potin Lai <potin.lai.pt@...il.com>
Cc: Samuel Mendoza-Jonas <sam@...dozajonas.com>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Patrick Williams <patrick@...cx.xyz>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, Cosmo Chou <cosmo.chou@...ntatw.com>,
	Potin Lai <potin.lai@...ntatw.com>
Subject: Re: [PATCH] Revert "net/ncsi: change from ndo_set_mac_address to
 dev_set_mac_address"

On Fri, Nov 29, 2024 at 05:12:56PM +0800, Potin Lai wrote:
> From: Potin Lai <potin.lai@...ntatw.com>
> 
> This reverts commit 790071347a0a1a89e618eedcd51c687ea783aeb3.
> 
> We are seeing kernel panic when enabling two NCSI interfaces at same
> time. It looks like mutex lock is being used in softirq caused the
> issue.

So a revert does make sense, you are seeing a real problem from that
commit.

However with the revert, is the code actually correct? Or is it
missing some locking? Normally dev_addr_sem is used to protect against
two calls to change the MAC address at once. Is this protection
needed? It would also be typical to hold RTNL while changing the MAC
address. So it would be nice to see an analysis of the locking, and
maybe the revert commit message says this gets you from a broken state
to a less broken state, and the real fix will be submitted soon?

	Andrew