linux-kernel - Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

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 for Android: free password hash cracker in your pocket

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

Message-ID: <202411271645.04C3508@keescook>
Date: Wed, 27 Nov 2024 16:53:23 -0800
From: Kees Cook <kees@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org, Alexander Viro <viro@...iv.linux.org.uk>,
	Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Nir Lichtman <nir@...htman.org>,
	Tycho Andersen <tandersen@...flix.com>,
	Vegard Nossum <vegard.nossum@...cle.com>
Subject: Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

On Tue, Nov 26, 2024 at 12:11:06PM -0800, Linus Torvalds wrote:
> And it looks like __set_task_comm() is actually buggy right now,
> because while we have a comment in linux/sched.h that says
> 
>  *   The strscpy_pad() in __set_task_comm() can ensure that the task comm is
>  *   always NUL-terminated and zero-padded.
> 
> that isn't actually true, because it looks like sized_strscpy()
> actually adds the final NUL at the end. I think that's because Andrew
> only merged a partial patch series.

Oh ew, yes, it looks like it copies the last byte, checks if it was NUL,
and then NULs it if it ran out of space. Ugh.

Before for comm we used strlen + memcpy, and so the trailing NUL was
always present. Grumble grumble. I'll look at your patch and will dig
around and see what works best. I'm on vacation currently, so there will
be some latency...

On a related note, what do you think of using execveat's "pathname"
argument as "comm" if AT_EMPTY_PATH is set? That'll give process
launchers control over comm (which is what they want), and we can keep
the dentry name fallback as proposed too?

-Kees

-- 
Kees Cook