Re: [PATCH net-next v3] net: phy: Fix suspicious rcu_dereference usage

[Kory Maincent <kory.maincent@bootlin.com>]

On Thu, 23 Jan 2025 13:25:57 +0200
Claudiu Beznea <claudiu.beznea@tuxon.dev> wrote:

> >> ravb_ptp_stop() modifies a couple of device registers and calls
> >> ptp_clock_unregister(). I don't see anything to suggest that this
> >> requires the rtnl lock to be held, unless I am missing something.  
> > 
> > What happens if two ptp_clock_unregister() with the same ptp_clock pointer
> > are called simultaneously? From ravb_suspend and ravb_set_ringparam for
> > example. It may cause some errors.  
> 
> Can this happen? I see ethtool_ops::set_ringparam() references only in
> ethtool or ioctl files:
> 
> net/ethtool/ioctl.c:2066
> net/ethtool/ioctl.c:2081
> net/ethtool/rings.c:212
> net/ethtool/rings.c:304
> 
> At the time the suspend/resume APIs are called the user space threads are
> frozen.

Maybe, I don't know the suspend path, and what the state of user space threads
at that time. This was an example but Wake on Lan setup could also have some
issue. IMHO I think it is more precautions to have it under rtnl lock.

Regards,
-- 
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com