Re: [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked() from strlcpy() to strscpy()

From: Kees Cook
Date: Thu Nov 30 2023 - 16:02:37 EST


On Thu, Nov 30, 2023 at 09:38:11PM +0100, Christophe JAILLET wrote:
> Le 30/11/2023 à 21:12, Kees Cook a écrit :
> [...]
> > diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c
> > index 8c0e5442597e..183f353b3852 100644
> > --- a/fs/kernfs/dir.c
> > +++ b/fs/kernfs/dir.c
> [...]
> > @@ -158,18 +159,22 @@ static int kernfs_path_from_node_locked(struct kernfs_node *kn_to,
> > buf[0] = '\0';
> > - for (i = 0; i < depth_from; i++)
> > - len += strlcpy(buf + len, parent_str,
> > - len < buflen ? buflen - len : 0);
> > + for (i = 0; i < depth_from; i++) {
> > + copied = strscpy(buf + len, parent_str, buflen - len);
> > + if (copied < 0)
> > + return copied;
> > + len += copied;
> > + }
> > /* Calculate how many bytes we need for the rest */
> > for (i = depth_to - 1; i >= 0; i--) {
> > for (kn = kn_to, j = 0; j < i; j++)
> > kn = kn->parent;
> > - len += strlcpy(buf + len, "/",
> > - len < buflen ? buflen - len : 0);
> > - len += strlcpy(buf + len, kn->name,
> > - len < buflen ? buflen - len : 0);
> > +
> > + copied = scnprintf(buf + len, buflen - len, "/%s", kn->name);
> > + if (copied < 0)
>
> Can scnprintf() return <0 ?

Ah, yeah, it's can't at all[1]. I forgot! :) Honestly, that function
should return size_t, not int...

I will send a v3 with this adjusted, but I'll wait for more review...

Thanks!

-Kees

[1] https://docs.kernel.org/core-api/kernel-api.html#c.scnprintf

--
Kees Cook