From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f47.google.com (mail-wr1-f47.google.com [209.85.221.47]) (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 633384399F7 for ; Wed, 19 Aug 2026 10:22:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.47 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787134979; cv=none; b=jOlskzMOsiEqVPZ8y5fYrlO6pTVvaf8OV0Kxosj02BLU7GQyKSXBA9q47LFZEelMZcignoCymUvOySwkD6/FkDjENY0vMYyEJr8dyruNAb1rsPghCp5MPqSIJ4EoBlhGZ0aU74JIL0MKHZ0dF06f8JDcQGfZoeYoliHBs3ZykH8= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1787134979; c=relaxed/simple; bh=rW/e1CxWcba8RMrjEjk+asvAa8OSuX3JiGrYAp/zjkI=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=NyqYGejz0ffjpEhZAXJhzPN8xXE4MoCYVS/oGj2poY0SHJO98Wk8BS+VAcf156/I1I60K5J9PaExOwT0XOmLys/FulgfuRhdSaWG48fgieeR6PdlLI9aUU5LWw0c6vla9I8f1bVn2Hzi0z9bqtCD5v2HlGFwpvkxTTnRiRZgn/o= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=eeqKIxDU; arc=none smtp.client-ip=209.85.221.47 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="eeqKIxDU" Received: by mail-wr1-f47.google.com with SMTP id ffacd0b85a97d-47fe2d179e2so504957f8f.1 for ; Wed, 19 Aug 2026 03:22:58 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1787134977; x=1787739777; darn=vger.kernel.org; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:from:to:cc:subject :date:message-id:reply-to:content-type; bh=F2O1QeWpjEXp0I6ZiHe6QrGIN+8dZKSNTYFq4Xm0R9g=; b=eeqKIxDUvL5ihSUwqNwVM6Y8DVGi37Ciihh5Avf6tagYZ429UIAdFKgttI++upjPn2 ryRyoIHAVSNJptUtC4cED/akCPqJrBw+YNyVTOvVXVNP59phoeATCyFwbTxBeyxzkCAN 9nkvCa/e8Jv9RMUbV8hNmxTHXomyARsc3eB6Epw7m41rnk2zaFVCpXgGBvMYoiTeNDPF XLTIVZZxSVLlip2YaO0N/WAWL/Ey16JrjF5kGBONQRLlph0mPDNJlf5BqkY8JdoJ7JWl jDjfaCrZhXc8ABzP56pcJh8WPHKaNPfeXl7GUdkfBlb32XRfj1MZ6xgQ0qk1gvjCVhFB 7ODw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1787134977; x=1787739777; h=content-transfer-encoding:content-type:mime-version:references :in-reply-to:message-id:subject:cc:to:from:date:x-gm-gg :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to :content-type; bh=F2O1QeWpjEXp0I6ZiHe6QrGIN+8dZKSNTYFq4Xm0R9g=; b=XbV3r1/R5z3J6+SGV8ilX/xTgFvYgpJFoH8qn92R9XbznyLUO+sXmxZNmnRqdtIlD5 1VFN8x+q71ZfciQhvQxWIONFBn0Yj0oNLQcQB+aFt5pG6AmVAAXOgpuQviVQlogCzpI3 rAY0AzLe7Lx4xNiWu9WvTfdEAtCjepAYYMXbk0OFaTLZZV8mIcHsb62xUxznHOEoJGOd DIwsmYg9P/rrQC6hGCDzEbtte7GjY5+af0IE4x0zuS9ePheN+Dv0DyGOleQlCMQKsscE Ci411j09BUCSnUoci09wltvHgZq4HoUZEmnD0emZ3LLz6vXqca60Cl8awnvFb2OLB+NC Oj4g== X-Forwarded-Encrypted: i=1; AHgh+RoY5Td6rsp4a2j+H42Eb9C2WmIFiFGiJ9vEAtBKY0hUbPE4bOjps/QpjezhkoO57TMmI7BYn20oGCs8uXg=@vger.kernel.org X-Gm-Message-State: AFuF++ncPBogqxRvOOv1cjZv+/RxFAzPb7QdDuJOfiBkKECe/hI5kzvZ qTkVDdu5105ioGGFdZaY1emlqE/uS9QbZmgIQ1/nJdmrPdkIc3MHpFnS X-Gm-Gg: AR+sD13sSO/5tsGbRcyJpw5W5FR+jrIEjYTZUcl7uxBa6AKx3lH9ZWZd0O8Jrd3w7gB Ge43PCo7bV93qwDnLLZr2DXBEfprZaqP3XkGsFePcnHmwLo1Xp0DgfP69eyGu9u02ms+lHDIoF9 fCMD/oOt0wCmzXtmmh/c4DCoEciIHgsRm4gQo6X3j9OCgcsdLZG8BViTB1DJZ51eIzax67W0iPh gnivzvROPgSLbXm7ZtO9gAILsyiEFKTZ57dXTMnzB/NLbFiT0HFL6dm7k5sX2Xw50GYdm4u+VLZ FlZ7joljHucvlt+lDqFGJR0nQ4y27J0oNIcux0a/HEMuEULtyFCzInJQuJC0ffrqR1JKLPhk4SU J5QLYZiPfKdbP5xSh5dF1be2jXCl6/Cvs2th2iHcPTin32l6/AEGS0+iu6B1qqQfM0sYItnOQd5 GbO2kn80v5fjSxQgMwkx9Vq3rq5wavGqDyQ8TSjwnPnF2vulWleKkpWNqZLaEt57abbyF92LwGp znqaX8b2D4r314rtZF1UDd+8JUie6t+2wUz X-Received: by 2002:a05:6000:401e:b0:482:a610:7a27 with SMTP id ffacd0b85a97d-482b1ff2808mr5345072f8f.20.1787134976552; Wed, 19 Aug 2026 03:22:56 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-482b14c338dsm4051137f8f.25.2026.08.19.03.22.55 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 19 Aug 2026 03:22:55 -0700 (PDT) Date: Wed, 19 Aug 2026 11:22:54 +0100 From: David Laight To: Justin Stitt Cc: Mariia Nikitash , trondmy@kernel.org, anna@kernel.org, keescook@google.com, linux-hardening@vger.kernel.org, linux-nfs@vger.kernel.org, linux-kernel@vger.kernel.org, nikitash.mariiaw@gmail.com Subject: Re: [PATCH] NFS: nfsroot: replace strlcat() with snprintf() Message-ID: <20260819112254.3e7d2b9c@pumpkin> In-Reply-To: References: <27956255dde39f58d73a7b51cb53cbe3d7804f54.1786411026.git.mariianikitash@google.com> <20260818095548.27e0edb2@pumpkin> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) 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=UTF-8 Content-Transfer-Encoding: quoted-printable On Tue, 18 Aug 2026 14:05:41 -0700 Justin Stitt wrote: > Hi, >=20 > On Tue, Aug 18, 2026 at 1:55=E2=80=AFAM David Laight > wrote: > > > > On Fri, 14 Aug 2026 00:28:45 -0700 > > Mariia Nikitash wrote: > > =20 > > > In preparation for removing the deprecated strlcat() API[1], replace = its > > > uses in root_nfs_cat() with snprintf(). > > > > > > Build the separator and source string in a single call using the > > > remaining space in the destination buffer. snprintf() returns the len= gth > > > it would have written excluding the terminating NUL, so comparing the > > > return value against the remaining buffer space preserves the existing > > > truncation check. > > > > > > Link: https://github.com/KSPP/linux/issues/370 [1] > > > Signed-off-by: Mariia Nikitash > > > --- > > > fs/nfs/nfsroot.c | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/fs/nfs/nfsroot.c b/fs/nfs/nfsroot.c > > > index 432612d22437..e951fe731679 100644 > > > --- a/fs/nfs/nfsroot.c > > > +++ b/fs/nfs/nfsroot.c > > > @@ -173,12 +173,12 @@ static int __init root_nfs_cat(char *dest, cons= t char *src, > > > const size_t destlen) > > > { > > > size_t len =3D strlen(dest); > > > + size_t remaining =3D destlen - len; > > > + const char *sep =3D ""; > > > > > > if (len && dest[len - 1] !=3D ',') > > > - if (strlcat(dest, ",", destlen) >=3D destlen) > > > - return -1; > > > - > > > - if (strlcat(dest, src, destlen) >=3D destlen) > > > + sep =3D ","; > > > + if (snprintf(dest + len, remaining, "%s%s", sep, src) >=3D rema= ining) > > > return -1; =20 > > > > I think I'd have gone for: > > size_t len =3D strlen(dest); > > if (len && dest[len - 1] !=3D ',' && ++len < destlen) > > dest[len - 1] =3D ','; > > if (strscpy(dest + len, src, destlen - len) < 0) > > return -1; =20 >=20 > Can we run into underflow issues if `len =3D=3D destlen` during the > increment. imagine @len is 10 and @destlen is also 10. We end up > incrementing len and our strscpy receives `10 - 11` wrapping to > SIZE_MAX. len can only be less than destlen, so strscpy() can only see a zero length - which is fine, This whole code could be rewritten to avoid the strlen(). The function is only used twice to append data to a single static string. Tweaking the initialisation would allow sizeof() be used to set the initial length. The initialisation also means that the ',' is always needed. The second call appends data from an snprintf() into a temporary buffer. It wouldn't be hard to directly add the data to the actual buffer. David >=20 > Maybe this isn't reachable as @destlen may always be larger than what > `strlen()` can give us but that increment looks suspicious. >=20 > > > > Although it would be better as an 'add_option()' function. > > I suspect the it used to be just strcat(). > > (similarly for root_nfs_copy() which is a pointless wrapper on strscpy(= )). > > > > Much more worth while would be fixing the sprintf() for NFS_ROOT. > > > > David > > =20 > > > return 0; > > > } =20 > > =20 >=20 > Justin