Received-SPF: pass (google.com: domain of linux-kernel+bounces-131765-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <77f48552-5919-4a7b-b55e-f4895f90f81d@intel.com>
Date: Thu, 4 Apr 2024 17:47:01 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH net-next v6 15/21] ip_tunnel: use a separate struct to
 store tunnel params in the kernel
To: Dan Carpenter <dan.carpenter@linaro.org>
CC: "David S. Miller" <davem@davemloft.net>, Eric Dumazet
	<edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni
	<pabeni@redhat.com>, Yury Norov <yury.norov@gmail.com>, Alexander Potapenko
	<glider@google.com>, <nex.sw.ncis.osdt.itp.upstreaming@intel.com>,
	<intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, Simon Horman <horms@kernel.org>
References: <20240327152358.2368467-1-aleksander.lobakin@intel.com>
 <20240327152358.2368467-16-aleksander.lobakin@intel.com>
 <5f63dd25-de94-4ca3-84e6-14095953db13@moroto.mountain>
From: Alexander Lobakin <aleksander.lobakin@intel.com>
Content-Language: en-US
In-Reply-To: <5f63dd25-de94-4ca3-84e6-14095953db13@moroto.mountain>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?OWlON2Rva21HUzljVjlwbVBCVkZvcGFIdVA3WU15YktpM1gwYm1nTUdsN3dS?=
 =?utf-8?B?bGVSZFpGdHA5bjBpbGRaQ1VqUmd2dWUvdzl0aERwblZHbWFnd21RWmNYd0lj?=
 =?utf-8?B?OERtVlk2L25LMFIrKzY4eGJKQjNKdDJOc2VOeDVnZWc3Q3pGRmZWaHNDQlhG?=
 =?utf-8?B?aUw1WjR5MGZFZG8xV2ExWitYaEZjK3VkRUk5TnprQ1UyVTZVQmYycnRhOWFm?=
 =?utf-8?B?eG9FQWVxMzlFa3N6aG5ndXV3YWVDa1d4emUrQlhkVWJhaXRyUVRCOHFneUg2?=
 =?utf-8?B?RFRybVIxQXBzdmVlL1YrUWs5dVliZlhiZFpGN1FQaDZvYmQ3dU8ybzVCVjB4?=
 =?utf-8?B?OTJMTEFIRGU0WG1aMVBNbFFrYlE4N3NTM0dRTzFoYTBSYzZvYXN6ejg0d1ln?=
 =?utf-8?B?YnY1WThZdkNDL0xnYWVJaTdtREw0allyTmtNV0tBME1CZWhEQU8zSTVvWm4w?=
 =?utf-8?B?TU5WYmZTTlFjYjlyT0RlVWtJbWxXOW84cHUxQUYvNDdwMVRNdWp3Z2RZam5o?=
 =?utf-8?B?Z1JLSDRhelg1SnNrSEVpM0poejJjdFUycFQ0Q1hEZk84cmdPckFNSmdNcWln?=
 =?utf-8?B?aXVmRVhTZThTSWJtd2NGdUh6MHVzNGw4RFNadmIwRjZaQWpObEl3Q0xWQXA2?=
 =?utf-8?B?SW91SFo3MlhPSzRud1pJa2psV2J2NmI2OGp2QmZKSEQ4Z0VuRmJHdXpjRlhi?=
 =?utf-8?B?bGxuYWxvU0tyMGZqeVFCc052UjVOVmlMYlFtZVViRDErdC9yZEFrN0N1c21S?=
 =?utf-8?B?WEVqTzNvVVVuZUh6SGNMNzlYVmlHcDdGbDcwU0ZQbXZCS3NvcWJqN3ZTRnhm?=
 =?utf-8?B?TWU3ZzBjTGdMWkVWRWwxeWx6amZ6YlRQTTRKVFJ1azNpcENydG4wcGlGcEtL?=
 =?utf-8?B?OVB3dXd6R3V4Q1dFODRNVis5U00zeFh1cHc5d1RqVnN4dEZNR2swR2lpTTB2?=
 =?utf-8?B?SEY4TmpZVXNlNThYcnJUTit3TW1uY1d3MmZ3dndGbnNRbTdXekdXdXlaTzNZ?=
 =?utf-8?B?NlpkeERsMmdneW9UenA1NVRvVW44aFpBQ2hOWGZvWVZZL3FXa2NaTUhlSW5J?=
 =?utf-8?B?Wk5WaGNyUmYydmhDbEc0Tk40THA5VGYvVW4zOUZ0ZlNZUEV3MDNHNVlPZm03?=
 =?utf-8?B?T1VUOUhSQ0J3SlVZeWJWTG81MmNJMUY0aXFiZ205ZDlGTm5KaUgrWHU4NlJ3?=
 =?utf-8?B?cEJtWFpZUCs5TFlPNkxYc05FdXg4R2k2TG4zditUem8wWjNQaVR0VnM0M3Q4?=
 =?utf-8?B?aks2NUdNRXFlZDd6Q1hCMWN2dWpRMUdKcjlMbVhSOGo3a3FhVlJSVXpBbjlV?=
 =?utf-8?B?bGtGUkNUR29DcWJ1RDdQZ0U2MDJ1OEZoWlpaNFhvQUQ1TDUzU0NVQmdpOXVn?=
 =?utf-8?B?K0ZjOFJvaUYxUHVQbGs5blRuNW5HMzY2ZHBORHN2Sm5NZmpmR00rVXEyT0Nh?=
 =?utf-8?B?dFhxUGFBVU1tZmlia1ZpUnhIRnRURzl6TVpMMVJoUzNxcDlFL1pDTXVQSWFp?=
 =?utf-8?B?VVBjQjk0cHJhR0R1ZXl0SHE4bUNINEZ6QzNvaGpWOFZ6Q2MxYzFPYTJaeXZY?=
 =?utf-8?B?VFRSQVlRYzJUTDMxeE0vRVB1S0pRQjhzdzhqdkdWVU8xN0NiMUswbndmZDE2?=
 =?utf-8?B?ekhVSVFnRUxGN00vRUhkUklwREo2ZnA3c3YwNVJwOTJ4RGg3YzhuQnhoZFI2?=
 =?utf-8?B?UXJmNHg5VlhkWXNnMUVQc2ZscElKZ0Q0bFR3TkRtQ1F2NDJlaStWNGtsU2hU?=
 =?utf-8?B?amhpb2JSUG9jWnVnWmZhOVhwZGs1MU1odUltUEh1OWI4UmxwZG1JeTRvUUps?=
 =?utf-8?B?SWxGbHNPOEtYdmh2bVFpVWdoMWZkM1Y2VWlZUVBGUkZtQzBtcklUZ2EvUWsv?=
 =?utf-8?B?T01YVElUdFQrV2VMekIyT0IzYnhxS2VBUHBqS3dGWDlCdUJuWUpwdC9QendS?=
 =?utf-8?B?dE45N2xKM0VkYVRxTnRERmRLRSsrT3JNOXk1ZlpneVdNdzB3dGZ3QzRSNW5l?=
 =?utf-8?B?MjgvRFQ3VTcwM21WUDBzWUdvRFNDSGV0azdzcDkyS2FDYUg1MElrYXQvNHBR?=
 =?utf-8?B?QWZGK0xpbnlSS1NWbFdzWWM1TlE0R1FsMk45anVzV1QzYnV6dXZLQmUyR1By?=
 =?utf-8?B?bm40MWhNZ3d4a2NObXkyRmQ4NkZMdEFLOVY1YlFET253UFhkNnRUenMwdmdl?=
 =?utf-8?B?VHc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b868d52b-6586-49a5-1eaf-08dc54bea81f
X-MS-Exchange-CrossTenant-AuthSource: DS0PR11MB8718.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Apr 2024 15:48:22.3246
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: uogoPLm4C6yKBF4ZLGXXlTHwMXV6WRZgBUIixrdiueNb5ieMyz94wpLEaYB7LYNjOmHGRUMPnN/PlGCz9ucMTgAqvZBUCFbyp4IsN7NxaMY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS7PR11MB6223
X-OriginatorOrg: intel.com

From: Dan Carpenter <dan.carpenter@linaro.org>
Date: Thu, 4 Apr 2024 17:24:23 +0300

> On Wed, Mar 27, 2024 at 04:23:52PM +0100, Alexander Lobakin wrote:
>> +bool ip_tunnel_parm_to_user(void __user *data, struct ip_tunnel_parm_kern *kp)
>> +{
>> +	struct ip_tunnel_parm p;
>> +
>> +	strscpy(p.name, kp->name);
> 
> We need to clear out p before copying to user space to avoid an
> information leak.  So this strscpy() needs to be strcpy_pad()
> 
>> +	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)));
> 
> And this memcpy() doesn't necessarily clear the whole of p.iph.

Oh, let me send a follow-up covering all this.

> 
>> +
>> +	return !copy_to_user(data, &p, sizeof(p));
>> +}
> 
> regards,
> dan carpenter

Thanks!
Olek