2017-12-18 23:17:40

by Laura Abbott

[permalink] [raw]
Subject: Coredump regression with e37fdb785a5f ("exec: Use secureexec for setting dumpability")

Hi,

Fedora got a bug report https://bugzilla.redhat.com/show_bug.cgi?id=1525974
of coredumps failing for systemd units. If systemd units segfault, no
core is produced.

commit e37fdb785a5f95ecadf43b773c97f676500ac7b8 (refs/bisect/bad)
Author: Kees Cook <[email protected]>
Date: Tue Jul 18 15:25:31 2017 -0700

exec: Use secureexec for setting dumpability

The examination of "current" to decide dumpability is wrong. This was a
check of and euid/uid (or egid/gid) mismatch in the existing process,
not the newly created one. This appears to stretch back into even the
"history.git" tree. Luckily, dumpability is later set in commit_creds().
In earlier kernel versions before creds existed, similar checks also
existed late in the exec flow, covering up the mistake as far back as I
could find.

Note that because the commit_creds() check examines differences of euid,
uid, egid, gid, and capabilities between the old and new creds, it would
look like the setup_new_exec() dumpability test could be entirely removed.
However, the secureexec test may cover a different set of tests (specific
to the LSMs) than what commit_creds() checks for. So, fix this test to
use secureexec (the removed euid tests are redundant to the commoncap
secureexec checks now).

Cc: David Howells <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Acked-by: Serge Hallyn <[email protected]>
Reviewed-by: James Morris <[email protected]>


This was still broken as of -rc3 although interestingly enough this worked
on my rawhide system. Any ideas?

Thanks,
Laura


2017-12-19 00:19:53

by Kees Cook

[permalink] [raw]
Subject: Re: Coredump regression with e37fdb785a5f ("exec: Use secureexec for setting dumpability")

On Mon, Dec 18, 2017 at 3:17 PM, Laura Abbott <[email protected]> wrote:
> Hi,
>
> Fedora got a bug report https://bugzilla.redhat.com/show_bug.cgi?id=1525974
> of coredumps failing for systemd units. If systemd units segfault, no
> core is produced.
>
> commit e37fdb785a5f95ecadf43b773c97f676500ac7b8 (refs/bisect/bad)
> Author: Kees Cook <[email protected]>
> Date: Tue Jul 18 15:25:31 2017 -0700
>
> exec: Use secureexec for setting dumpability
> The examination of "current" to decide dumpability is wrong. This
> was a
> check of and euid/uid (or egid/gid) mismatch in the existing process,
> not the newly created one. This appears to stretch back into even the
> "history.git" tree. Luckily, dumpability is later set in commit_creds().
> In earlier kernel versions before creds existed, similar checks also
> existed late in the exec flow, covering up the mistake as far back as I
> could find.
> Note that because the commit_creds() check examines differences of
> euid,
> uid, egid, gid, and capabilities between the old and new creds, it would
> look like the setup_new_exec() dumpability test could be entirely
> removed.
> However, the secureexec test may cover a different set of tests
> (specific
> to the LSMs) than what commit_creds() checks for. So, fix this test to
> use secureexec (the removed euid tests are redundant to the commoncap
> secureexec checks now).
> Cc: David Howells <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> Acked-by: Serge Hallyn <[email protected]>
> Reviewed-by: James Morris <[email protected]>
>
>
> This was still broken as of -rc3 although interestingly enough this worked
> on my rawhide system. Any ideas?

Hmmm, interesting. So dumpability can't follow secureexec -- that
seems like a bad state since secureexec represents a process with
privilege...

-Kees

--
Kees Cook
Pixel Security