Hi,
On Tue, Aug 18, 2026 at 10:42 AM Kees Cook <[email protected]> wrote: > > On Tue, Aug 18, 2026 at 09:55:48AM +0100, David Laight wrote: > > On Fri, 14 Aug 2026 00:28:45 -0700 > > Mariia Nikitash <[email protected]> wrote: > > > > > 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 length > > > 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 <[email protected]> > > > --- > > > 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, const > > > char *src, > > > const size_t destlen) > > > { > > > size_t len = strlen(dest); > > > + size_t remaining = destlen - len; > > > + const char *sep = ""; > > > > > > if (len && dest[len - 1] != ',') > > > - if (strlcat(dest, ",", destlen) >= destlen) > > > - return -1; > > > - > > > - if (strlcat(dest, src, destlen) >= destlen) > > > + sep = ","; > > > + if (snprintf(dest + len, remaining, "%s%s", sep, src) >= remaining) > > > return -1; > > > > I think I'd have gone for: > > size_t len = strlen(dest); > > if (len && dest[len - 1] != ',' && ++len < destlen) > > dest[len - 1] = ','; > > if (strscpy(dest + len, src, destlen - len) < 0) > > return -1; > > This is valid, but I just feel like %s%s is more readable for what it > does. You've open-coded the first %s, and the ++len's interaction > between the logic and the argument to strscpy is subtle. Since this > isn't fast-path, let's use what Mariia has proposed. I had also considered similar approach to what David suggested, I am open to any path forward based on what maintainers want. > > > > > Although it would be better as an 'add_option()' function. > > If this were done in more places, I'd agree, but as-is it's pretty > limited. > > Reviewed-by: Kees Cook <[email protected]> Thanks for the review! > > > -- > Kees Cook Mariia
