netdev - Re: [PATCH iwl-next v3 1/6] ip_tunnel: use a separate struct to store tunnel params in the kernel

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] [day] [month] [year] [list]

Date: Mon, 24 Jul 2023 16:21:52 +0200
From: Alexander Lobakin <aleksander.lobakin@...el.com>
To: Andy Shevchenko <andy@...nel.org>
CC: Marcin Szycik <marcin.szycik@...ux.intel.com>,
	<intel-wired-lan@...ts.osuosl.org>, <netdev@...r.kernel.org>,
	<wojciech.drewek@...el.com>, <michal.swiatkowski@...ux.intel.com>,
	<davem@...emloft.net>, <kuba@...nel.org>, <jiri@...nulli.us>,
	<pabeni@...hat.com>, <jesse.brandeburg@...el.com>,
	<simon.horman@...igine.com>, <idosch@...dia.com>
Subject: Re: [PATCH iwl-next v3 1/6] ip_tunnel: use a separate struct to store
 tunnel params in the kernel

From: Andy Shevchenko <andy@...nel.org>
Date: Fri, 21 Jul 2023 17:21:51 +0300

> On Fri, Jul 21, 2023 at 09:15:27AM +0200, Marcin Szycik wrote:
>> From: Alexander Lobakin <aleksander.lobakin@...el.com>
>>
>> Unlike IPv6 tunnels which use purely-kernel __ip6_tnl_parm structure
>> to store params inside the kernel, IPv4 tunnel code uses the same
>> ip_tunnel_parm which is being used to talk with the userspace.
>> This makes it difficult to alter or add any fields or use a
>> different format for whatever data.
>> Define struct ip_tunnel_parm_kern, a 1:1 copy of ip_tunnel_parm for
>> now, and use it throughout the code. The two places where the latter
>> is used to interact with the userspace, now do a conversion from one
>> type to another, with manual field-by-field assignments.
>> Must be done at once, since ip_tunnel::parms is being used in most
>> of those places.
> 
> ...
> 
>> +	strscpy(kp.name, p.name, sizeof(kp.name));
>> +	kp.link = p.link;
>> +	kp.i_flags = p.i_flags;
>> +	kp.o_flags = p.o_flags;
>> +	kp.i_key = p.i_key;
>> +	kp.o_key = p.o_key;
>> +	memcpy(&kp.iph, &p.iph, min(sizeof(kp.iph), sizeof(p.iph)));
>> +
>> +	err = dev->netdev_ops->ndo_tunnel_ctl(dev, &kp, cmd);
>> +	if (err)
>> +		return err;
>> +
>> +	strscpy(p.name, kp.name, sizeof(p.name));
>> +	p.link = kp.link;
>> +	p.i_flags = kp.i_flags;
>> +	p.o_flags = kp.o_flags;
>> +	p.i_key = kp.i_key;
>> +	p.o_key = kp.o_key;
>> +	memcpy(&p.iph, &kp.iph, min(sizeof(p.iph), sizeof(kp.iph)));
> 
>> +		strscpy(kp.name, p.name, sizeof(kp.name));
>> +		kp.link = p.link;
>> +		kp.i_flags = p.i_flags;
>> +		kp.o_flags = p.o_flags;
>> +		kp.i_key = p.i_key;
>> +		kp.o_key = p.o_key;
>> +		memcpy(&kp.iph, &p.iph, min(sizeof(p.iph), sizeof(kp.iph)));
> 
> Seems to me these two deserves separate helpers to avoid such a duplication(s).
> 

Sounds reasonable, thanks!

Olek