netdev - Re: [PATCH net-next] ip6_tunnel: enable to change proto of fb tunnels

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: <27a87dd2-7ffe-4b4e-8001-ca0abe412b3e@6wind.com>
Date: Fri, 20 Jun 2025 10:14:26 +0200
From: Nicolas Dichtel <nicolas.dichtel@...nd.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: "David S . Miller" <davem@...emloft.net>, Paolo Abeni
 <pabeni@...hat.com>, Eric Dumazet <edumazet@...gle.com>,
 Simon Horman <horms@...nel.org>, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] ip6_tunnel: enable to change proto of fb tunnels

Le 20/06/2025 à 00:52, Jakub Kicinski a écrit :
> On Tue, 17 Jun 2025 18:01:25 +0200 Nicolas Dichtel wrote:
>> +	if (dev == ip6n->fb_tnl_dev) {
>> +		if (!data[IFLA_IPTUN_PROTO]) {
>> +			NL_SET_ERR_MSG(extack,
>> +				       "Only protocol can be changed for fallback tunnel");
>> +			return -EINVAL;
>> +		}
>> +
>> +		ip6_tnl_netlink_parms(data, &p);
>> +		ip6_tnl0_update(netdev_priv(ip6n->fb_tnl_dev), &p);
>> +		return 0;
> 
> Hm, I guess its in line with old school netlink behavior where we'd
> just toss unsupported attributes on the floor. But I wonder whether
> it'd be better to explicitly reject other attrs?

I tried to find a (simple) way to be strict but, by default 'ip link' dumps all
attributes and put them back in the message it sends.
Ie, with the command 'ip link set ip6tnl0 type ip6tnl mode any' all IFLA_IPTUN_*
attributes are set (to their current value) and only IFLA_IPTUN_PROTO has
another value.

> 
> Shouldn't be too painful with just one allowed:
> 
> 	if (!data[IFLA_IPTUN_PROTO])
> 		goto ..
> 
> 	ip6_tnl_netlink_parms(data, &p);
> 
> 	data[IFLA_IPTUN_PROTO] = NULL;
> 	if (memchr_inv(data, 0, sizeof() * ARRAY_SIZE(ip6_tnl_policy)))
> 		goto ...
> > 	ip6_tnl0_update(netdev_priv(ip6n->fb_tnl_dev), &p);
> 
> WDYT?
I already tried something similar, but it broke the 'ip link' command for the
reason explained above.
I was wondering if it's worth putting a lot of code to cover this case.
Any thoughts?

Regards,
Nicolas