netdev - Re: [PATCH net-next v2] 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: <48ae4097-8c3d-4144-83f0-6ede7d8f9e50@6wind.com>
Date: Sat, 28 Jun 2025 00:54:34 +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 v2] ip6_tunnel: enable to change proto of fb
 tunnels

Le 28/06/2025 à 00:29, Jakub Kicinski a écrit :
> On Thu, 26 Jun 2025 23:55:09 +0200 Nicolas Dichtel wrote:
>> I finally checked  all params, let's do this properly (:
> 
> Nice :)
> 
>> -static void ip6_tnl0_update(struct ip6_tnl *t, struct __ip6_tnl_parm *p)
>> +static int ip6_tnl0_update(struct ip6_tnl *t, struct __ip6_tnl_parm *p,
>> +			   bool strict)
>>  {
>> -	/* for default tnl0 device allow to change only the proto */
>> +	/* For the default ip6tnl0 device, allow changing only the protocol (the
> 
> nit: the "(the" may look better on the next line?
Ok.

> 
>> +	 * IP6_TNL_F_CAP_PER_PACKET flag is set on ip6tnl0, and all other
>> +	 * parameters are 0).
>> +	 */
>> +	if (strict &&
>> +	    (!ipv6_addr_any(&p->laddr) || !ipv6_addr_any(&p->raddr) ||
>> +	     p->flags != t->parms.flags || p->hop_limit || p->encap_limit ||
>> +	     p->flowinfo || p->link || p->fwmark || p->collect_md))
>> +		return -EINVAL;
>> +
>>  	t->parms.proto = p->proto;
>>  	netdev_state_change(t->dev);
>> +	return 0;
>>  }
>>  
>>  static void
>> @@ -1680,7 +1691,7 @@ ip6_tnl_siocdevprivate(struct net_device *dev, struct ifreq *ifr,
>>  			} else
>>  				t = netdev_priv(dev);
>>  			if (dev == ip6n->fb_tnl_dev)
>> -				ip6_tnl0_update(t, &p1);
>> +				ip6_tnl0_update(t, &p1, false);
>>  			else
>>  				ip6_tnl_update(t, &p1);
>>  		}
>> @@ -2053,8 +2064,31 @@ static int ip6_tnl_changelink(struct net_device *dev, struct nlattr *tb[],
>>  	struct ip6_tnl_net *ip6n = net_generic(net, ip6_tnl_net_id);
>>  	struct ip_tunnel_encap ipencap;
>>  
>> -	if (dev == ip6n->fb_tnl_dev)
>> -		return -EINVAL;
>> +	if (dev == ip6n->fb_tnl_dev) {
>> +		struct ip6_tnl *t = netdev_priv(ip6n->fb_tnl_dev);
> 
> the compiler complains that t is declared here but not used..
Oops

> 
>> +
>> +		if (ip_tunnel_netlink_encap_parms(data, &ipencap)) {
>> +			/* iproute2 always sets TUNNEL_ENCAP_FLAG_CSUM6, so
>> +			 * let's ignore this flag.
>> +			 */
>> +			ipencap.flags &= ~TUNNEL_ENCAP_FLAG_CSUM6;
>> +			if (memchr_inv(&ipencap, 0, sizeof(ipencap))) {
>> +				NL_SET_ERR_MSG(extack,
>> +					       "Only protocol can be changed for fallback tunnel, not encap params");
>> +				return -EINVAL;
>> +			}
>> +		}
>> +
>> +		ip6_tnl_netlink_parms(data, &p);
>> +		if (ip6_tnl0_update(netdev_priv(ip6n->fb_tnl_dev), &p,
> 
> .. you probably meant to use it here?
Yes. It was used by the v1.1 :)


Regards,
Nicolas