linux-hardening - Re: [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <202311301300.6BAB981@keescook>
Date: Thu, 30 Nov 2023 13:02:32 -0800
From: Kees Cook <keescook@...omium.org>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Tejun Heo <tj@...nel.org>, Zefan Li <lizefan.x@...edance.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Waiman Long <longman@...hat.com>, cgroups@...r.kernel.org,
	Azeem Shaikh <azeemshaikh38@...il.com>,
	linux-kernel@...r.kernel.org, bpf@...r.kernel.org,
	linux-hardening@...r.kernel.org
Subject: Re: [PATCH v2 3/3] kernfs: Convert kernfs_path_from_node_locked()
 from strlcpy() to strscpy()

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