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

Hi,

On Tue, Aug 18, 2026 at 1:55 AM David Laight
<[email protected]> 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;

Can we run into underflow issues if `len == 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.

Maybe this isn't reachable as @destlen may always be larger than what
`strlen()` can give us but that increment looks suspicious.

>
> 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
>
> >       return 0;
> >  }
>

Justin