netdev - Re: [PATCH net-next v3 02/15] net: Implement netdev_nl_bind_queue

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: <77a3eb52-b0e0-440e-80a0-6e89322e33e9@davidwei.uk>
Date: Tue, 28 Oct 2025 14:59:05 -0700
From: David Wei <dw@...idwei.uk>
To: Jakub Kicinski <kuba@...nel.org>, Daniel Borkmann <daniel@...earbox.net>
Cc: Paolo Abeni <pabeni@...hat.com>, netdev@...r.kernel.org,
 bpf@...r.kernel.org, davem@...emloft.net, razor@...ckwall.org,
 willemb@...gle.com, sdf@...ichev.me, john.fastabend@...il.com,
 martin.lau@...nel.org, jordan@...fe.io, maciej.fijalkowski@...el.com,
 magnus.karlsson@...el.com, toke@...hat.com, yangzhenze@...edance.com,
 wangdongdong.6@...edance.com
Subject: Re: [PATCH net-next v3 02/15] net: Implement
 netdev_nl_bind_queue_doit

On 2025-10-23 19:08, Jakub Kicinski wrote:
> On Thu, 23 Oct 2025 14:48:15 +0200 Daniel Borkmann wrote:
>> On 10/23/25 12:27 PM, Paolo Abeni wrote:
>>> On 10/20/25 6:23 PM, Daniel Borkmann wrote:
>>>> +	if (!src_dev->dev.parent) {
>>>> +		err = -EOPNOTSUPP;
>>>> +		NL_SET_ERR_MSG(info->extack,
>>>> +			       "Source device is a virtual device");
>>>> +		goto err_unlock_src_dev;
>>>> +	}
>>>
>>> Is this check strictly needed? I think that if we relax it, it could be
>>> simpler to create all-virtual selftests.
>> It is needed given we need to always ensure lock ordering for the two devices,
>> that is, the order is always from the virtual to the physical device.
> 
> You do seem to be taking the lock before you check if the device was
> the type you expected tho.

I believe this is okay. Let's say we have two netdevs, A that is real
and B that is virtual. User calls netdev_nl_bind_queue_doit() twice in
two different contexts, 1 with the correct order (A as src, B as dst)
and 2 with the incorrect order (B as src, A as dst). We always try to
lock dst first, then src.

         1                 2
lock(dst == B)
                   lock(dst == A)
                   is not virtual...
                   unlock(A)
lock(src == A)


         1                 2
                   lock(dst == A)
lock(dst == B)
                   is not virtual...
                   unlock(A)
lock(src == A)

The check will prevent ABBA by never taking that final lock to complete
the cycle. Please check and lmk if I'm off, stuff like this makes my
brain hurt.