linux-kernel - Re: [PATCH AUTOSEL 6.7 021/108] r8169: improve RTL8411b phy-down fixup

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: <b7e75488-ecf9-4c83-b6ab-3f1194c63e54@lunn.ch>
Date: Wed, 17 Jan 2024 18:04:01 +0100
From: Andrew Lunn <andrew@...n.ch>
To: Mirsad Todorovac <mirsad.todorovac@....hr>
Cc: Jakub Kicinski <kuba@...nel.org>, Sasha Levin <sashal@...nel.org>,
	linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Heiner Kallweit <hkallweit1@...il.com>,
	Mirsad Todorovac <mirsad.todorovac@....unizg.hr>,
	Simon Horman <horms@...nel.org>,
	"David S . Miller" <davem@...emloft.net>, nic_swsd@...ltek.com,
	edumazet@...gle.com, pabeni@...hat.com, netdev@...r.kernel.org
Subject: Re: [PATCH AUTOSEL 6.7 021/108] r8169: improve RTL8411b phy-down
 fixup

> Hi Andrew,
> 
> Yes, I wasn't aware of the 100 lines limit, and yes, this is not a bug fix,
> but an improvement (optimisation).
> 
> I think by this I can join to consensus, this patch is not a candidate for
> backporting. :-/
> 
> However, I am concerned about the possibility of two kernel threads accessing
> the RTL NIC intermittently attempting to program the NIC over the RTL_(R|W)(8|16|32)
> instructions (which are expanded to readl/writel and assembly).

Most calls into the driver are protected by the RTNL lock. There are a
few exceptions. probe() obviously does not hold RTNL. Actually sending
packets, and interrupt handlers don't hold RTNL.

Please look at the code and see if you can see any paths which might
do parallel access without holding RTNL.

You could also do some testing. Add ASSERT_RTNL() in the code you are
worried about. If the lock is not held, you will get a stack trace.

	Andrew