From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 2483A199385 for ; Thu, 9 Jan 2025 18:14:14 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.178 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736446457; cv=none; b=YYWk35c7XNozeV8Xc1uqi5ZvxwxwBNks0amp7SuXXXei5eS+uIcs+d0nLdkCF54C6R52qRPAZ0v1UDKE+Ugd5KP5AdEQ3r8phAxOlGUA8nNnRgZbgeMyURsi3EWsckRLzD605tQnbbg8FDvBLXXEqELK+uNfl8pUxF86s9hNINs= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736446457; c=relaxed/simple; bh=/K6ncGzRMDwpYLLo2rh5hjIpHtdzOSE3W5pb9ATXG+4=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=PtSUV+QqqB4jbwzKznrue4lQ5q/32ESs9urXpt1H2RJsu+dxwDNThKTCuR7/CwkOv2xeaz/7skaec+9wkon7v1Pn05wjxi39NSO+BaZQqMmZVlivS+9JLoz/NyCyBn2hMgdKLTv6FuKHrQxuRWqzsbJTe+eqpBjtK1WPsZ8l+Jg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=dlO5voT+; arc=none smtp.client-ip=209.85.214.178 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="dlO5voT+" Received: by mail-pl1-f178.google.com with SMTP id d9443c01a7336-2163affd184so5345ad.1 for ; Thu, 09 Jan 2025 10:14:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1736446454; x=1737051254; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=bINg/gnprvUTwQzCA8pY7wFm7enPM0xkckdTziiODlQ=; b=dlO5voT+1g3eXQlv2lWaJ44tP7nGiraUbCUlm1M8NYxQczcr9yb+nPrnQZKjhE8di7 AuvEXC+3ri8q+pkZyoF1CQglHznUF4siThXoagGHhDY/Gpm03eS/OYDE+3T8oTX9StBj rliD/8cyhQwenXCP3aPXAjhaq2TgW6ytD506LYOpzGL9mllP2MDeLgZCDZ+y/eUuFyyw kcLzBoMV+0SQUBH3tWT5pJv2EldbsU/uZp0fD3ez8iRvPYH6druDlQf1Q3qR6p6ptrP4 BWzjWmUG6ijRxM36qaDRxUpleGP2lpYKKQHpXggkGOEDKS2PW6swCw43L37+hFriNQZp DHcg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1736446454; x=1737051254; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=bINg/gnprvUTwQzCA8pY7wFm7enPM0xkckdTziiODlQ=; b=fcQUAMki5pMeBrWOxuv+9nPigPn5dBXTi30+bQIwJ7BuhnnSqPTTkzDauvDX12/NwC 4qf5xR4i5/jyWs+hbTkftMSSa+H0412B9biTYGIcLbiodpoUSDeQAWI9hXjSH9Bwh0xm 8M2IVfFDmidbQlmWm0oF/QmqeBq7Xek75q4D191NnUUWiciHQM7y6+bLxfuwmfCjDUBZ yl2lbwchN4zKuYHKy6HNk2PeKQpToYUhtVSZy3wySuKML3Dypp/v7TLgFzNe4wUR6gCL P4WEJL5ujDzvPoZLciFg/30w3Cb5yXM19ASrB+NqgMjQHqkwAJxYKUSE/xQh13wyLl/y Lu9Q== X-Forwarded-Encrypted: i=1; AJvYcCVlJKhSZmoOKD3YzAbirsCLtyFX80DaCpG6knTW0F9H+IO4vcTu0t6b1m22pqWm5IlyPN7ycA9LMylRrlQ=@vger.kernel.org X-Gm-Message-State: AOJu0YzZPQ6j9DBIDuK4CEJwBe7CdmZChln03N9CuTprVCaFOjA+dnJj kDL/rI5nngbbwEH2xrD/QC5lEt3Z++1XJT6S9LhFAusBI3qbyNkS6pPP+g/RrQ== X-Gm-Gg: ASbGncsPeqkozVqlhZqgNgkw+ZCuxA1dUBHn6bL7WCtw9U1wFS75kO7JRxYLipYPTy8 8Pisguw2+05Achmwu0Qlb8dsktSpH4mgbsXpd/NyYRzsYXDVqBWmKuoOw5/Tl3zGHDvSJunNOFZ kNL4+Eyun9l5MctYV2vWv6j15c2g6/P2xzQc6vE743zSBQuFsMpggmg30mm/M3S4I0Mo39XanO8 Ia49BAwonsS8UKRN6O051P41bUhTOwkQzkB/Cq9M5oB+49fsFqctrJ1Ag== X-Google-Smtp-Source: AGHT+IEzlkvSLAwKZQsIRypnWj8w1B6QkECfu1OuKbwjVtb8GtlSzWtAVLCnKsIjaRBjZzQ+WA0LsA== X-Received: by 2002:a17:902:f7c5:b0:21a:87e8:3897 with SMTP id d9443c01a7336-21a8ed272f9mr3002245ad.4.1736446454149; Thu, 09 Jan 2025 10:14:14 -0800 (PST) Received: from google.com ([2620:15c:2d:3:e84d:972b:9ee4:3ad7]) by smtp.gmail.com with ESMTPSA id d2e1a72fcca58-72d4054893esm92029b3a.20.2025.01.09.10.14.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 09 Jan 2025 10:14:13 -0800 (PST) Date: Thu, 9 Jan 2025 10:14:09 -0800 From: Isaac Manjarres To: Lorenzo Stoakes Cc: Andrew Morton , kaleshsingh@google.com, jstultz@google.com, aliceryhl@google.com, surenb@google.com, kernel-team@android.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 2/2] mm/memfd: Use strncpy_from_user() to read memfd name Message-ID: References: <20250107184804.4074147-1-isaacmanjarres@google.com> <20250107184804.4074147-3-isaacmanjarres@google.com> <4291d9f0-4483-40e5-a54b-d006eb52c8cb@lucifer.local> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Thu, Jan 09, 2025 at 11:31:59AM +0000, Lorenzo Stoakes wrote: > On Wed, Jan 08, 2025 at 06:15:36PM -0800, Isaac Manjarres wrote: > > On Wed, Jan 08, 2025 at 06:58:00PM +0000, Lorenzo Stoakes wrote: > > > On Tue, Jan 07, 2025 at 10:48:02AM -0800, Isaac J. Manjarres wrote: > > > > goto err_name; > > > > - } > > > > - > > > > - /* terminating-zero may have changed after strnlen_user() returned */ > > > > - if (name[len + MFD_NAME_PREFIX_LEN - 1]) { > > > > - error = -EFAULT; > > > > + } else if (len > MFD_NAME_MAX_LEN) { > > > > + error = -EINVAL; > > > > > > I don't think this can ever happen? It just truncates, looking at the code > > > for strncpy_from_user(). > > > > > > > I double checked, and this case is possible. The maximum we allow to > > strncpy_from_user() to read is MFD_NAME_MAX_LEN + 1 via the count > > argument, so that includes the NULL terminator in the userspace buffer. > > > > > strncpy_from_user() then returns the length of the string without the > > NULL terminator. The check is for just MFD_NAME_MAX_LEN, so this is > > meant to catch the case where the string, not including the NULL > > terminator, is greater than MFD_NAME_MAX_LEN, which is invalid, as > > well as the case where the string becomes malformed/corrupted mid-copy. > > Actually you're right :) apologies, I misread the strncpy_from_user() > implementation. > > So I think you should be good here - have you tested this scenario in > practice just to confirm? > > Cheers! No worries! Yes, I tested this out and confirmed that we do return -EINVAL in this case before and after this change, so it should be fine. Thanks for the review :)! I'll be sending out v3 of this series shortly. --Isaac