netdev - Re: [PATCH net-next v10 08/14] net: hold netdev instance lock during sysfs operations

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: <8e5bf1dffe7c5ae2191e9082dcd0f72469b4fc0b.camel@nvidia.com>
Date: Mon, 24 Mar 2025 17:06:09 +0000
From: Cosmin Ratiu <cratiu@...dia.com>
To: "stfomichev@...il.com" <stfomichev@...il.com>
CC: "pabeni@...hat.com" <pabeni@...hat.com>, "kuba@...nel.org"
	<kuba@...nel.org>, "edumazet@...gle.com" <edumazet@...gle.com>,
	"netdev@...r.kernel.org" <netdev@...r.kernel.org>, "saeed@...nel.org"
	<saeed@...nel.org>, "sdf@...ichev.me" <sdf@...ichev.me>,
	"davem@...emloft.net" <davem@...emloft.net>
Subject: Re: [PATCH net-next v10 08/14] net: hold netdev instance lock during
 sysfs operations

On Mon, 2025-03-24 at 09:06 -0700, Stanislav Fomichev wrote:
> On 03/24, Cosmin Ratiu wrote:
> > Call Trace:
> > dump_stack_lvl+0x62/0x90
> > print_deadlock_bug+0x274/0x3b0
> > __lock_acquire+0x1229/0x2470
> > lock_acquire+0xb7/0x2b0
> > __mutex_lock+0xa6/0xd20
> > dev_disable_lro+0x20/0x80
> > inetdev_init+0x12f/0x1f0
> > inetdev_event+0x48b/0x870
> > notifier_call_chain+0x38/0xf0
> > netif_change_net_namespace+0x72e/0x9f0
> > do_setlink.isra.0+0xd5/0x1220
> > rtnl_newlink+0x7ea/0xb50
> > rtnetlink_rcv_msg+0x459/0x5e0
> > netlink_rcv_skb+0x54/0x100
> > netlink_unicast+0x193/0x270
> > netlink_sendmsg+0x204/0x450
> 
> I think something like the patch below should fix it? inetdev_init is
> called for blackhole (sw device, we don't care about ops lock) and
> from
> REGISTER/UNREGISTER notifiers. We hold the lock during REGISTER,
> and will soon hold the lock during UNREGISTER:
> https://lore.kernel.org/netdev/20250312223507.805719-9-kuba@kernel.org/
> 
> (might also need to EXPORT_SYM netif_disable_lro)
> 
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 754f60fb6e25..77e5705ac799 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -281,7 +281,7 @@ static struct in_device *inetdev_init(struct
> net_device *dev)
>  	if (!in_dev->arp_parms)
>  		goto out_kfree;
>  	if (IPV4_DEVCONF(in_dev->cnf, FORWARDING))
> -		dev_disable_lro(dev);
> +		netif_disable_lro(dev);
>  	/* Reference in_dev->dev */
>  	netdev_hold(dev, &in_dev->dev_tracker, GFP_KERNEL);
>  	/* Account for reference dev->ip_ptr (below) */

Unfortunately, this seems to result, on another code path, in:
WARNING: CPU: 10 PID: 1479 at ./include/net/netdev_lock.h:54
__netdev_update_features+0x65f/0xca0
__warn+0x81/0x180
__netdev_update_features+0x65f/0xca0
report_bug+0x156/0x180
handle_bug+0x4f/0x90
exc_invalid_op+0x13/0x60
asm_exc_invalid_op+0x16/0x20
__netdev_update_features+0x65f/0xca0
netif_disable_lro+0x30/0x1d0
inetdev_init+0x12f/0x1f0
inetdev_event+0x48b/0x870
notifier_call_chain+0x38/0xf0
register_netdevice+0x741/0x8b0
register_netdev+0x1f/0x40
mlx5e_probe+0x4e3/0x8e0 [mlx5_core]
auxiliary_bus_probe+0x3f/0x90
really_probe+0xc3/0x3a0
__driver_probe_device+0x80/0x150
driver_probe_device+0x1f/0x90
__device_attach_driver+0x7d/0x100
bus_for_each_drv+0x80/0xd0
__device_attach+0xb4/0x1c0
bus_probe_device+0x91/0xa0
device_add+0x657/0x870

I see register_netdevice briefly acquires the netdev lock in two
separate blocks and has a __netdev_update_features call in one of the
blocks, but the lock is not held for
call_netdevice_notifiers(NETDEV_REGISTER, dev).

Cosmin.