Re: [PATCH net-next v21 3/5] net: Add the possibility to support a selected hwtstamp in netdevice

[Kory Maincent]

On Fri, 17 Jan 2025 13:57:41 +0200
Claudiu Beznea <claudiu.beznea@xxxxxxxxx> wrote:

> Hi, Kory,
> 
> On 12.12.2024 19:06, Kory Maincent wrote:
> > Introduce the description of a hwtstamp provider, mainly defined with a
> > the hwtstamp source and the phydev pointer.
> > 
> > Add a hwtstamp provider description within the netdev structure to
> > allow saving the hwtstamp we want to use. This prepares for future
> > support of an ethtool netlink command to select the desired hwtstamp
> > provider. By default, the old API that does not support hwtstamp
> > selectability is used, meaning the hwtstamp provider pointer is unset.
> > 
> > Signed-off-by: Kory Maincent <kory.maincent@xxxxxxxxxxx>  
> 
> I'm getting this error when doing suspend/resume on the Renesas RZ/G3S
> Smarc Module + RZ SMARC Carrier II board:
> 
> [   39.032969] =============================
> [   39.032983] WARNING: suspicious RCU usage
> [   39.033000] 6.13.0-rc7-next-20250116-arm64-renesas-00002-g35245dfdc62c
> #7 Not tainted
> [   39.033019] -----------------------------
> [   39.033033] drivers/net/phy/phy_device.c:2004 suspicious
> rcu_dereference_protected() usage!

Thanks for the report.
Oh so it seems there are cases where phy_detach is not called under RTNL lock!

This should solve the issue:
-               hwprov = rtnl_dereference(dev->hwprov);
+               rcu_read_lock()
+               hwprov = rcu_dereference(dev->hwprov);
                /* Disable timestamp if it is the one selected */
                if (hwprov && hwprov->phydev == phydev) {
                        rcu_assign_pointer(dev->hwprov, NULL);
                        kfree_rcu(hwprov, rcu_head);
                }
+               rcu_read_unlock();

I will send a patch soon.

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