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

[Kees Cook <keescook@chromium.org>]

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