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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87h67qoeh5.fsf@email.froward.int.ebiederm.org>
Date: Thu, 28 Nov 2024 22:23:18 -0600
From: "Eric W. Biederman" <ebiederm@...ssion.com>
To: Casey Schaufler <casey@...aufler-ca.com>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,  Kees Cook
 <kees@...nel.org>,  linux-kernel@...r.kernel.org,  Christophe JAILLET
 <christophe.jaillet@...adoo.fr>,  Nir Lichtman <nir@...htman.org>,  Tycho
 Andersen <tandersen@...flix.com>,  Vegard Nossum
 <vegard.nossum@...cle.com>, linux-security-module@...r.kernel.org, Al Viro
 <viro@...iv.linux.org.uk>
Subject: Re: [GIT PULL] execve updates for v6.13-rc1 (take 2)

Al Viro <viro@...iv.linux.org.uk> writes:

> On Thu, Nov 28, 2024 at 06:43:31PM -0800, Linus Torvalds wrote:
>> A sane setup has lots of options
>> 
>>  - just use execve() with the actual name of the executable
>> 
>>  - use hardlinks and use execveat()
>> 
>>  - use open() on a symlink and then execveat() of that file, and get
>> the actual name of the executable behind the symlink
>> 
>>  - disagree about comm[] being relevant at all, and don't use it, and
>> don't use tools that do
>> 
>> and none of those are wrong decisions.
>
> Just one thing - IMO we want to use the relative pathname when it's
> not empty.  Even in execveat().  Because some wanker *will* decide
> that newer is better and make libc use execveat() to implement execve().
> Which won't be spotted for about a year, and when it does we'll get
> seriously stuck.

For clarity the only patches I have seen so far have been
about the fexecve subset of execveat.  AKA when execveat is has
not been supplied a path.

> I agree that for fexecve() the only sane approach is to go by whatever
> that opened file refers to; I'm not sold on the _usefulness_ of
> fexecve() to start with, but if we want that thing, that's the way
> to go.

The craziness is that apparently systemd wants to implement execve in
terms of fexecve, not execveat.

...

Wanting to see what is going on I cloned the most recent version of
systemd.  I see some calls that are generally redundant.  That is
systemd opens the executable and fstat's the executable to make
certain the executable is a regular file and not a directory symlink.

Which seems harmless but pointless unless something is interesting
is going to happen with the executable_fd before it is passed to
the kernel to execute.

The only case in systemd that does something interesting with the
executable_fd (that I could see) has to do with smack.  Please see
the code below.

Casey do you have any idea why systemd would want to read the label from
the executable and apply it to the current process?  Is the systemd
smack support sensible?

As it is written this seems like the kind of logic every process that
calls execve will want to repeat.

So I am wondering isn't it easier just to get the kernel to do the right
thing and not have deeply special smack code in systemd?  Does the
kernel already do the right thing and systemd is just confused?

Right now it looks like the sane path is to sort out the systemd's
smack support and then systemd should be able to continue using
execve like any sane program.

#if ENABLE_SMACK
static int setup_smack(
                const ExecParameters *params,
                const ExecContext *context,
                int executable_fd) {
        int r;

        assert(params);
        assert(executable_fd >= 0);

        if (context->smack_process_label) {
                r = mac_smack_apply_pid(0, context->smack_process_label);
                if (r < 0)
                        return r;
        } else if (params->fallback_smack_process_label) {
                _cleanup_free_ char *exec_label = NULL;

                r = mac_smack_read_fd(executable_fd, SMACK_ATTR_EXEC, &exec_label);
                if (r < 0 && !ERRNO_IS_XATTR_ABSENT(r))
                        return r;

                r = mac_smack_apply_pid(0, exec_label ?: params->fallback_smack_process_label);
                if (r < 0)
                        return r;
        }

        return 0;
}
#endif

Eric