netdev - Re: [PATCH net-next] net: enetc: use generic interfaces to get phc

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: <20250919103232.6d668441@kmaincent-XPS-13-7390>
Date: Fri, 19 Sep 2025 10:32:32 +0200
From: Kory Maincent <kory.maincent@...tlin.com>
To: Vladimir Oltean <vladimir.oltean@....com>
Cc: Wei Fang <wei.fang@....com>, claudiu.manoil@....com,
 xiaoning.wang@....com, yangbo.lu@....com, richardcochran@...il.com,
 andrew+netdev@...n.ch, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, pabeni@...hat.com, Frank.Li@....com, imx@...ts.linux.dev,
 netdev@...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 Thu, 18 Sep 2025 15:48:23 +0300
Vladimir Oltean <vladimir.oltean@....com> wrote:

> On Thu, Sep 18, 2025 at 03:44:54PM +0800, Wei Fang wrote:
> > @@ -954,17 +957,9 @@ static int enetc_get_ts_info(struct net_device *ndev,
> >  	if (!enetc_ptp_clock_is_enabled(si))
> >  		goto timestamp_tx_sw;
> >  
> > -	if (is_enetc_rev1(si)) {
> > -		phc_idx = symbol_get(enetc_phc_index);
> > -		if (phc_idx) {
> > -			info->phc_index = *phc_idx;  
> 
> phc_idx remains unused in enetc_get_ts_info() after this change, and it
> produces a build warning.
> 
> > -			symbol_put(enetc_phc_index);
> > -		}
> > -	} else {
> > -		info->phc_index = enetc4_get_phc_index(si);
> > -		if (info->phc_index < 0)
> > -			goto timestamp_tx_sw;
> > -	}
> > +	info->phc_index = enetc_get_phc_index(si);
> > +	if (info->phc_index < 0)
> > +		goto timestamp_tx_sw;
> >  
> >  	enetc_get_ts_generic_info(ndev, info);
> >    
> 
> Also, testing reveals:
> 
> root@fii:~# ethtool -T eno2
> [   43.374227] BUG: sleeping function called from invalid context at
> kernel/locking/rwsem.c:1536 [   43.383268] in_atomic(): 0, irqs_disabled():
> 0, non_block: 0, pid: 460, name: ethtool [   43.392076] preempt_count: 0,
> expected: 0 [   43.396454] RCU nest depth: 1, expected: 0
> [   43.400908] 3 locks held by ethtool/460:
> [   43.405206]  #0: ffffcb976c5fb608 (cb_lock){++++}-{4:4}, at:
> genl_rcv+0x30/0x60 [   43.412886]  #1: ffffcb976c5e9f88
> (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock+0x28/0x40 [   43.420931]  #2:
> ffffcb976c0b32d0 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x4/0x48 [
>   43.429785] CPU: 1 UID: 0 PID: 460 Comm: ethtool Not tainted 6.17.0-rc5+
> #2920 PREEMPT [   43.429796] Call trace: [   43.429799]  show_stack+0x24/0x38
> (C) [   43.429814]  dump_stack_lvl+0x40/0xa0
> [   43.429822]  dump_stack+0x18/0x24
> [   43.429828]  __might_resched+0x200/0x218
> [   43.429837]  __might_sleep+0x54/0x90
> [   43.429844]  down_read+0x3c/0x1f0
> [   43.429852]  pci_get_slot+0x30/0x88
> [   43.429860]  enetc_get_ts_info+0x108/0x1a0
> [   43.429867]  __ethtool_get_ts_info+0x140/0x218
> [   43.429875]  tsinfo_prepare_data+0x9c/0xc8
> [   43.429881]  ethnl_default_doit+0x1cc/0x410
> [   43.429888]  genl_rcv_msg+0x2d8/0x358
> [   43.429896]  netlink_rcv_skb+0x124/0x148
> [   43.429903]  genl_rcv+0x40/0x60
> [   43.429910]  netlink_unicast+0x198/0x358
> [   43.429916]  netlink_sendmsg+0x22c/0x348
> [   43.429923]  __sys_sendto+0x138/0x1d8
> [   43.429928]  __arm64_sys_sendto+0x34/0x50
> [   43.429933]  invoke_syscall+0x4c/0x110
> [   43.429940]  el0_svc_common+0xb8/0xf0
> [   43.429946]  do_el0_svc+0x28/0x40
> [   43.429953]  el0_svc+0x4c/0xe0
> [   43.429960]  el0t_64_sync_handler+0x78/0x130
> [   43.429967]  el0t_64_sync+0x198/0x1a0
> [   43.429974]
> [   43.537263] =============================
> [   43.541282] [ BUG: Invalid wait context ]
> [   43.545301] 6.17.0-rc5+ #2920 Tainted: G        W
> [   43.550891] -----------------------------
> [   43.554909] ethtool/460 is trying to lock:
> [   43.559016] ffffcb976c26ab80 (pci_bus_sem){++++}-{4:4}, at:
> pci_get_slot+0x30/0x88 [   43.566628] other info that might help us debug
> this: [   43.571694] context-{5:5}
> [   43.574317] 3 locks held by ethtool/460:
> [   43.578251]  #0: ffffcb976c5fb608 (cb_lock){++++}-{4:4}, at:
> genl_rcv+0x30/0x60 [   43.585603]  #1: ffffcb976c5e9f88
> (rtnl_mutex){+.+.}-{4:4}, at: rtnl_lock+0x28/0x40 [   43.593301]  #2:
> ffffcb976c0b32d0 (rcu_read_lock){....}-{1:3}, at: rcu_lock_acquire+0x4/0x48 [
>   43.601786] stack backtrace: [   43.604672] CPU: 1 UID: 0 PID: 460 Comm:
> ethtool Tainted: G        W           6.17.0-rc5+ #2920 PREEMPT [
> 43.604679] Tainted: [W]=WARN [   43.604683] Call trace:
> [   43.604685]  show_stack+0x24/0x38 (C)
> [   43.604692]  dump_stack_lvl+0x40/0xa0
> [   43.604699]  dump_stack+0x18/0x24
> [   43.604706]  __lock_acquire+0xab4/0x31f8
> [   43.604713]  lock_acquire+0x11c/0x278
> [   43.604720]  down_read+0x6c/0x1f0
> [   43.604726]  pci_get_slot+0x30/0x88
> [   43.604732]  enetc_get_ts_info+0x108/0x1a0
> [   43.604738]  __ethtool_get_ts_info+0x140/0x218
> [   43.604745]  tsinfo_prepare_data+0x9c/0xc8
> [   43.604750]  ethnl_default_doit+0x1cc/0x410
> [   43.604757]  genl_rcv_msg+0x2d8/0x358
> [   43.604765]  netlink_rcv_skb+0x124/0x148
> [   43.604771]  genl_rcv+0x40/0x60
> [   43.604778]  netlink_unicast+0x198/0x358
> [   43.604784]  netlink_sendmsg+0x22c/0x348
> [   43.604790]  __sys_sendto+0x138/0x1d8
> [   43.604795]  __arm64_sys_sendto+0x34/0x50
> [   43.604799]  invoke_syscall+0x4c/0x110
> [   43.604806]  el0_svc_common+0xb8/0xf0
> [   43.604812]  do_el0_svc+0x28/0x40
> [   43.604818]  el0_svc+0x4c/0xe0
> [   43.604825]  el0t_64_sync_handler+0x78/0x130
> [   43.604832]  el0t_64_sync+0x198/0x1a0
> Time stamping parameters for eno2:
> Capabilities:
>         hardware-transmit
>         software-transmit
>         hardware-receive
>         software-receive
>         software-system-clock
>         hardware-raw-clock
> PTP Hardware Clock: 0
> Hardware Transmit Timestamp Modes:
>         off
>         on
>         onestep-sync
> Hardware Receive Filter Modes:
>         none
>         all
> 
> 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.kernel.org/netdev/20250918074454.1742328-1-wei.fang@nxp.com/

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.

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