[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250923013348.62f44bba@kmaincent-XPS-13-7390>
Date: Tue, 23 Sep 2025 01:33:48 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Wei Fang <wei.fang@....com>
Cc: Vladimir Oltean <vladimir.oltean@....com>, Claudiu Manoil
<claudiu.manoil@....com>, Clark Wang <xiaoning.wang@....com>, "Y.B. Lu"
<yangbo.lu@....com>, "richardcochran@...il.com" <richardcochran@...il.com>,
"andrew+netdev@...n.ch" <andrew+netdev@...n.ch>, "davem@...emloft.net"
<davem@...emloft.net>, "edumazet@...gle.com" <edumazet@...gle.com>,
"kuba@...nel.org" <kuba@...nel.org>, "pabeni@...hat.com"
<pabeni@...hat.com>, Frank Li <frank.li@....com>, "imx@...ts.linux.dev"
<imx@...ts.linux.dev>, "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH net-next] net: enetc: use generic interfaces to get
phc_index for ENETC v1
On Fri, 19 Sep 2025 08:48:57 +0000
Wei Fang <wei.fang@....com> wrote:
> > > It looks like we have a problem and can't call pci_get_slot(), which
> > > sleeps on down_read(&pci_bus_sem), from ethtool_ops :: get_ts_info(),
> > > which can't sleep, as of commit 4c61d809cf60 ("net: ethtool: Fix
> > > suspicious rcu_dereference usage").
> > >
> > > Köry, do you have any comments or suggestions? Patch is here:
> > >
> > https://lore.kern/
> > el.org%2Fnetdev%2F20250918074454.1742328-1-wei.fang%40nxp.com%2F&d
> > ata=05%7C02%7Cwei.fang%40nxp.com%7Cca70608eb6c9487d98e108ddf7571
> > de6%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C63893867571474
> > 2481%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRydWUsIlYiOiIwLjA
> > uMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C0%7C%7
> > C%7C&sdata=h6rMPPxArsYdoPOz95UZRU88Oz9IJ3sD6lRjqC5SHMU%3D&reser
> > ved=0
> >
> > This is annoying indeed. I don't know how this enetc drivers works but why
> > ts_info needs this pci_get_slot() call? It seems this call seems to not be
> > used in ndo_hwtstamp_get/set while ts_info which does not need any hardware
> > communication report only a list of capabilities.
> >
>
> The ENETC (MAC controller) and the PTP timer are separate devices, they
> are both PCIe devices, the PTP timer provides the PHC for ENETC to use, so
> enetc_get_ts_info() needs to get the phc_index of the PTP timer, so
> pci_get_slot() is called to get the pci_dev pointer of the PTP timer. I can
> use pci_get_domain_bus_and_slot() to instead to fix this issue.
>
> I do not know whether it is a good idea to place the get_ts_info() callback
> within an atomic lock context. I also noticed that idpf driver also has the
> same potential issue (idpf_get_ts_info()).
We can change the device providing the PTP (currently between MAC or PHY),
that's why we need to acquire the rcu lock to avoid any pointer change during
the get_ts_info callback which could cause an use after free issue.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
Powered by blists - more mailing lists