netdev - Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave device's upper already has a program

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: <6d38eaf5-0a13-9f85-3a5d-0ca354bc45d5@iogearbox.net>
Date: Fri, 23 Aug 2024 14:07:45 +0200
From: Daniel Borkmann <daniel@...earbox.net>
To: Jiri Pirko <jiri@...nulli.us>, Feng zhou <zhoufeng.zf@...edance.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
 pabeni@...hat.com, ast@...nel.org, hawk@...nel.org,
 john.fastabend@...il.com, bigeasy@...utronix.de, lorenzo@...nel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
 yangzhenze@...edance.com, wangdongdong.6@...edance.com,
 Toke Høiland-Jørgensen <toke@...hat.com>
Subject: Re: [PATCH bpf-next v2] net: Don't allow to attach xdp if bond slave
 device's upper already has a program

On 8/23/24 1:55 PM, Jiri Pirko wrote:
> Fri, Aug 23, 2024 at 10:42:04AM CEST, zhoufeng.zf@...edance.com wrote:
>> From: Feng Zhou <zhoufeng.zf@...edance.com>
>>
>> Cannot attach when an upper device already has a program, This
>> restriction is only for bond's slave devices or team port, and
>> should not be accidentally injured for devices like eth0 and vxlan0.
> 
> What if I attach xdp program to solo netdev and then I enslave it
> to bond/team netdev that already has xdp program attached?
> What prevents me from doing that?

In that case the enslaving of the device to bond(/team) must fail as
otherwise the latter won't be able to propagate the XDP prog downwards.

Feng, did you double check if we have net or BPF selftest coverage for
that? If not might be good to add.

>> Reviewed-by: Toke Høiland-Jørgensen <toke@...hat.com>
>> Signed-off-by: Feng Zhou <zhoufeng.zf@...edance.com>
>> ---
>> Changelog:
>> v1->v2: Addressed comments from Paolo Abeni, Jiri Pirko
>> - Use "netif_is_lag_port" relace of "netif_is_bond_slave"
>> Details in here:
>> https://lore.kernel.org/netdev/3bf84d23-a561-47ae-84a4-e99488fc762b@bytedance.com/T/
>>
>> net/core/dev.c | 10 ++++++----
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/net/core/dev.c b/net/core/dev.c
>> index f66e61407883..49144e62172e 100644
>> --- a/net/core/dev.c
>> +++ b/net/core/dev.c
>> @@ -9502,10 +9502,12 @@ static int dev_xdp_attach(struct net_device *dev, struct netlink_ext_ack *extack
>> 	}
>>
>> 	/* don't allow if an upper device already has a program */
>> -	netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>> -		if (dev_xdp_prog_count(upper) > 0) {
>> -			NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>> -			return -EEXIST;
>> +	if (netif_is_lag_port(dev)) {
>> +		netdev_for_each_upper_dev_rcu(dev, upper, iter) {
>> +			if (dev_xdp_prog_count(upper) > 0) {
>> +				NL_SET_ERR_MSG(extack, "Cannot attach when an upper device already has a program");
>> +				return -EEXIST;
>> +			}
>> 		}
>> 	}
>>
>> -- 
>> 2.30.2
>>
>