Re: [PATCH] NFS: nfsroot: replace strlcat() with snprintf()

[Mariia Nikitash]

Hi,



On Tue, Aug 18, 2026 at 10:42 AM Kees Cook <kees@xxxxxxxxxx> 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 <mariianikitash@xxxxxxxxxx> 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 <mariianikitash@xxxxxxxxxx>
> > > ---
> > >  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 <kees@xxxxxxxxxx>

Thanks for the review!

>
>
> --
> Kees Cook


Mariia