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: <0d85faff-50d7-4ac5-8e82-fe406c87e3ad@iogearbox.net>
Date: Fri, 24 Oct 2025 21:15:57 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Stanislav Fomichev <stfomichev@...il.com>
Cc: netdev@...r.kernel.org, bpf@...r.kernel.org, kuba@...nel.org,
 davem@...emloft.net, razor@...ckwall.org, pabeni@...hat.com,
 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, dw@...idwei.uk, 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 10/24/25 8:20 PM, Stanislav Fomichev wrote:
[...]
>> +	/* Locking order is always from the virtual to the physical device
>> +	 * since this is also the same order when applications open the
>> +	 * memory provider later on.
>> +	 */
>> +	dst_dev = netdev_get_by_index_lock(genl_info_net(info), dst_ifidx);
>> +	if (!dst_dev) {
>> +		err = -ENODEV;
>> +		goto err_genlmsg_free;
>> +	}
> 
> ...
> 
>> +	src_dev = netdev_get_by_index_lock(genl_info_net(info), src_ifidx);
>> +	if (!src_dev) {
>> +		err = -ENODEV;
>> +		goto err_unlock_dst_dev;
>> +	}
> 
> But isn't the above susceptible to ABBA exploitation from the userspace?
> I can try to concurrently do two requests, the second one being with
> dst_dev and src_dev swapped. Or do we assume that we exit earlier for
> the swapped case based on some other condition?

Hm, in all of the locking that was reworked, we only ever let the case succeed
where it locks both devices when the dst_dev is a virtual device, and the
src_dev is a phys device. If this is not given and the dst_dev is a phys device,
we error out via err_unlock_dst_dev and unlock the dst_dev again but never
proceed further to attempt to lock src_dev (as mentioned in the comment) -
basically what you mentioned the swapped case cannot lock both devs.

Thanks,
Daniel