netdev - Re: [net-next,v5,11/16] netkit: Add netkit notifier to check for unregistering devices

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: <20260113035358.405484-1-kuba@kernel.org>
Date: Mon, 12 Jan 2026 19:53:58 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: daniel@...earbox.net
Cc: Jakub Kicinski <kuba@...nel.org>,
	netdev@...r.kernel.org,
	bpf@...r.kernel.org,
	jordan@...fe.io,
	maciej.fijalkowski@...el.com,
	willemb@...gle.com,
	yangzhenze@...edance.com,
	razor@...ckwall.org,
	dw@...idwei.uk,
	pabeni@...hat.com,
	sdf@...ichev.me,
	wangdongdong.6@...edance.com,
	john.fastabend@...il.com,
	martin.lau@...nel.org,
	magnus.karlsson@...el.com,
	toke@...hat.com,
	davem@...emloft.net
Subject: Re: [net-next,v5,11/16] netkit: Add netkit notifier to check for unregistering devices

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least pausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
netkit: Add netkit notifier to check for unregistering devices

This commit adds a netdevice notifier to watch for NETDEV_UNREGISTER
events and unregister netkit devices that have leased queues from a
physical device being unregistered.

> diff --git a/drivers/net/netkit.c b/drivers/net/netkit.c
> index 9d9d74a7e589..d4426c30e0fd 100644
> --- a/drivers/net/netkit.c
> +++ b/drivers/net/netkit.c
> @@ -1037,6 +1037,49 @@ static int netkit_change_link(struct net_device *dev, struct nlattr *tb[],
>  	return 0;
>  }
>
> +static void netkit_check_lease_unregister(struct net_device *dev)
> +{
> +	LIST_HEAD(list_kill);
> +	u32 q_idx;
> +
> +	if (READ_ONCE(dev->reg_state) != NETREG_UNREGISTERING ||
> +	    !dev->dev.parent)
> +		return;
> +
> +	for (q_idx = 0; q_idx < dev->real_num_rx_queues; q_idx++) {
> +		struct net_device *tmp = dev;
> +		u32 tmp_q_idx = q_idx;
> +
> +		netdev_lock_ops(dev);
> +		if (netif_rx_queue_lease_get_owner(&tmp, &tmp_q_idx)) {
> +			if (tmp->netdev_ops != &netkit_netdev_ops)
> +				continue;
                                ^^^^^^^^

Can this continue leak the lock acquired by netdev_lock_ops(dev) above? The
netdev_unlock_ops(dev) call is outside this if block, so continuing here
would skip the unlock and potentially deadlock on the next loop iteration
when netdev_lock_ops(dev) is called again.

> +			/* A single phys device can have multiple queues leased
> +			 * to one netkit device. We can only queue that netkit
> +			 * device once to the list_kill. Queues of that phys
> +			 * device can be leased with different individual netkit
> +			 * devices, hence we batch via list_kill.
> +			 */
> +			if (unregister_netdevice_queued(tmp))
> +				continue;
                                ^^^^^^^^

Same issue here - this continue also skips netdev_unlock_ops(dev).

> +			netkit_del_link(tmp, &list_kill);
> +		}
> +		netdev_unlock_ops(dev);
> +	}

[ ... ]