linux-kernel - Re: [RFC] exec: add a flag for "reasonable" execveat() comm

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <87h6a1xilx.fsf@email.froward.int.ebiederm.org>
Date: Fri, 27 Sep 2024 09:43:22 -0500
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Tycho Andersen <tycho@...ho.pizza>
Cc: Aleksa Sarai <cyphar@...har.com>,  Alexander Viro
 <viro@...iv.linux.org.uk>,  Christian Brauner <brauner@...nel.org>,  Jan
 Kara <jack@...e.cz>,  Kees Cook <kees@...nel.org>,  Jeff Layton
 <jlayton@...nel.org>,  Chuck Lever <chuck.lever@...cle.com>,  Alexander
 Aring <alex.aring@...il.com>,  linux-fsdevel@...r.kernel.org,
  linux-mm@...ck.org,  linux-kernel@...r.kernel.org,  Tycho Andersen
 <tandersen@...flix.com>,  Zbigniew Jędrzejewski-Szmek
 <zbyszek@...waw.pl>
Subject: Re: [RFC] exec: add a flag for "reasonable" execveat() comm

Tycho Andersen <tycho@...ho.pizza> writes:

> On Wed, Sep 25, 2024 at 09:09:18PM -0500, Eric W. Biederman wrote:
>> Tycho Andersen <tycho@...ho.pizza> writes:
>> 
>> > Yep, I did this for the test above, and it worked fine:
>> >
>> >         if (bprm->fdpath) {
>> >                 /*
>> >                  * If fdpath was set, execveat() made up a path that will
>> >                  * probably not be useful to admins running ps or similar.
>> >                  * Let's fix it up to be something reasonable.
>> >                  */
>> >                 struct path root;
>> >                 char *path, buf[1024];
>> >
>> >                 get_fs_root(current->fs, &root);
>> >                 path = __d_path(&bprm->file->f_path, &root, buf, sizeof(buf));
>> >
>> >                 __set_task_comm(me, kbasename(path), true);
>> >         } else {
>> >                 __set_task_comm(me, kbasename(bprm->filename), true);
>> >         }
>> >
>> > obviously we don't want a stack allocated buffer, but triggering on
>> > ->fdpath != NULL seems like the right thing, so we won't need a flag
>> > either.
>> >
>> > The question is: argv[0] or __d_path()?
>> 
>> You know.  I think we can just do:
>> 
>> 	BUILD_BUG_ON(DNAME_INLINE_LEN >= TASK_COMM_LEN);
>> 	__set_task_comm(me, bprm->file->f_path.dentry->d_name.name, true);
>> 
>> Barring cache misses that should be faster and more reliable than what
>> we currently have and produce the same output in all of the cases we
>> like, and produce better output in all of the cases that are a problem
>> today.
>> 
>> Does anyone see any problem with that?
>
> Nice, this works great. We need to drop the BUILD_BUG_ON() since it is
> violated in today's tree, but I think this is safe to do anyway since
> __set_task_comm() does strscpy_pad(tsk->comm, buf, sizeof(tsk->comm)).

Doh.  I simply put the conditional in the wrong order.  That should have
been:
	BUILD_BUG_ON(TASK_COMM_LEN > DNAME_INLINE_LEN);

Sorry I was thinking of the invariant that needs to be preserved rather
than the bug that happens.

Eric