Message-ID: <202411211011.C2E3ABEAB@keescook>
Date: Thu, 21 Nov 2024 10:50:20 -0800
From: Kees Cook <kees@...nel.org>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Zbigniew Jędrzejewski-Szmek <zbyszek@...waw.pl>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	linux-kernel@...r.kernel.org,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Christophe JAILLET <christophe.jaillet@...adoo.fr>,
	Dan Carpenter <dan.carpenter@...aro.org>,
	Nir Lichtman <nir@...htman.org>,
	syzbot+03e1af5c332f7e0eb84b@...kaller.appspotmail.com,
	Tycho Andersen <tandersen@...flix.com>,
	Vegard Nossum <vegard.nossum@...cle.com>
Subject: Re: [GIT PULL] execve updates for v6.13-rc1

On Thu, Nov 21, 2024 at 10:02:03AM -0800, Linus Torvalds wrote:
> On Thu, 21 Nov 2024 at 10:00, Zbigniew Jędrzejewski-Szmek
> <zbyszek@...waw.pl> wrote:
> >
> > Identical — as far as the callee is concerned.
> > Basically, we'd like to switch the execve() that we use in systemd
> > to start everything with fexecve(), but this should be invisible to
> > both the programs that are started and users who call ps/pgrep/….
> 
> I'm not discussing this. If you cannot understand the difference
> between comm[] and argv[0], this discussion is entirely pointless.
> 
> I'd suggest you just not use fexecve().

I think you're talking past each other. Here's my thought process:

To Linus's point, comm[] is "garbage" in that a process can change it to
anything it wants (i.e. PR_SET_NAME). I think everyone understands this,
but that's not what what I see as the issue.

What commp[] does have, however, is a "default" (starting) value set by
execve(), which is that of the basename of the "pathname" argument of
the syscall (yes, not argv[0], but see below).

Most process list analysis tools (i.e. "ps") use /proc/*/comm for the
short name view of a process. Most process launchers (i.e. shells,
systemd), tend to use basename(pathname) as argv[0] when running
programs. Again, I think everyone understands these things too, but I
think it's worth calling out that while comm[] and argv[0] are obviously
different things, in most cases they start out with identical values
after execve().

The problem is fexecve(), where the pathname is lost since it was,
obviously, only passed to open(). Right now, we re-use bprm->fdpath since
it was already there for scripts, but that unhelpfully just shoves the
fd number into comm[], making "ps" output unreadable. Nobody likes
seeing basename(fdpath) in comm[] today.

I don't think it's unreasonable to want to retain the basename(pathname)
starting value of comm[] when switching from execve() to fexecve().

Using the f_path name is certainly better than basename(fdpath), but it
doesn't really give userspace what they'd like nor expect. Since comm[]
is mutable anyway, why not just copy argv[0] for this case, as that
would give userspace exactly what it was expecting and does no harm?
i.e. yes, comm[] is "garbage", but let's replicate in fexecve() as close
to the default behavior we have from execve() as we can. Why expose
f_path, which doesn't appear in comm[] nor cmdline currently?

The only flip side I can see is that "ps" etc, should just never use comm
at all, and instead use argv[0] from cmdline. That would get the same
behavior as described here, but it is much more expensive in that is takes
the mm lock and has to walk userspace memory. But then we don't need
to change anything on the kernel side and just leave basename(fdpath)
alone. I would note, of course, that cmdline can also be changed by the
process (PR_SET_MM_ARG_START), too, so it is also "garbage". Just more
expensive garbage than comm[].

I still think the original proposal is the most helpful to userspace.

-Kees

-- 
Kees Cook

Powered by blists - more mailing lists