linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jann Horn <jannh@google.com>
To: enkechen@cisco.com
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H . Peter Anvin" <hpa@zytor.com>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Arnd Bergmann <arnd@arndb.de>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Khalid Aziz <khalid.aziz@oracle.com>,
	Kate Stewart <kstewart@linuxfoundation.org>,
	deller@gmx.de, Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Al Viro <viro@zeniv.linux.org.uk>,
	Andrew Morton <akpm@linux-foundation.org>,
	christian@brauner.io, Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will.deacon@arm.com>,
	Dave.Martin@arm.com, mchehab+samsung@kernel.org,
	Michal Hocko <mhocko@kernel.org>, Rik van Riel <riel@surriel.com>,
	"Kirill A . Shutemov" <kirill.shutemov@linux.intel.com>,
	guro@fb.com, Marcos Souza <marcos.souza.org@gmail.com>,
	Oleg Nesterov <oleg@redhat.com>,
	linux@dominikbrodowski.net, Cyrill Gorcunov <gorcunov@openvz.org>,
	yang.shi@linux.alibaba.com, Kees Cook <keescook@chromium.org>,
	kernel list <linux-kernel@vger.kernel.org>,
	linux-arch <linux-arch@vger.kernel.org>,
	Victor Kamensky <kamensky@cisco.com>,
	xe-linux-external@cisco.com, sstrogin@cisco.com
Subject: Re: [PATCH] kernel/signal: Signal-based pre-coredump notification
Date: Mon, 15 Oct 2018 20:54:27 +0200	[thread overview]
Message-ID: <CAG48ez2ZD=dovBApR07wi_DzYjJvEyO-Pm35eLv9AyXv1tcAFQ@mail.gmail.com> (raw)
In-Reply-To: <c5a09a04-b4f2-876b-af83-30f7e527497e@cisco.com>

On Mon, Oct 15, 2018 at 8:36 PM Enke Chen <enkechen@cisco.com> wrote:
> On 10/13/18 11:27 AM, Jann Horn wrote:
> > On Sat, Oct 13, 2018 at 2:33 AM Enke Chen <enkechen@cisco.com> wrote:
> >> For simplicity and consistency, this patch provides an implementation
> >> for signal-based fault notification prior to the coredump of a child
> >> process. A new prctl command, PR_SET_PREDUMP_SIG, is defined that can
> >> be used by an application to express its interest and to specify the
> >> signal (SIGCHLD or SIGUSR1 or SIGUSR2) for such a notification. A new
> >> signal code (si_code), CLD_PREDUMP, is also defined for SIGCHLD.
> >
> > Your suggested API looks vaguely similar to PR_SET_PDEATHSIG, but with
> > some important differences:
> >
> >  - You don't reset the signal on setuid execution.
[...]
> >
> > For both of these: Are these differences actually necessary, and if
> > so, can you provide a specific rationale? From a security perspective,
> > I would very much prefer it if this API had semantics closer to
> > PR_SET_PDEATHSIG.
>
[...]
>
> Regarding the impact of "setuid", this property "PR_SET_PREDUMP_SIG" has to
> do with the application/process whether the signal handler is set for receiving
> such a notification.  If it is set, the "uid" should not matter.

If an attacker's process first calls PR_SET_PREDUMP_SIG, then forks
off a child, then calls execve() on a setuid binary, the setuid binary
calls setuid(0), and the attacker-controlled child then crashes, the
privileged process will receive an unexpected signal that the attacker
wouldn't have been allowed to send otherwise. For similar reasons, the
parent death signal is reset when a setuid binary is executed:

void setup_new_exec(struct linux_binprm * bprm)
{
        /*
         * Once here, prepare_binrpm() will not be called any more, so
         * the final state of setuid/setgid/fscaps can be merged into the
         * secureexec flag.
         */
        bprm->secureexec |= bprm->cap_elevated;

        if (bprm->secureexec) {
                /* Make sure parent cannot signal privileged process. */
                current->pdeath_signal = 0;
[...]
        }
[...]
}

int commit_creds(struct cred *new)
{
[...]
        /* dumpability changes */
        if (!uid_eq(old->euid, new->euid) ||
            !gid_eq(old->egid, new->egid) ||
            !uid_eq(old->fsuid, new->fsuid) ||
            !gid_eq(old->fsgid, new->fsgid) ||
            !cred_cap_issubset(old, new)) {
                if (task->mm)
                        set_dumpable(task->mm, suid_dumpable);
                task->pdeath_signal = 0;
                smp_wmb();
        }
[...]
}

AppArmor and SELinux also do related changes:

static void apparmor_bprm_committing_creds(struct linux_binprm *bprm)
{
[...]
        /* bail out if unconfined or not changing profile */
        if ((new_label->proxy == label->proxy) ||
            (unconfined(new_label)))
                return;

        aa_inherit_files(bprm->cred, current->files);

        current->pdeath_signal = 0;
[...]
}

static void selinux_bprm_committing_creds(struct linux_binprm *bprm)
{
[...]
        new_tsec = bprm->cred->security;
        if (new_tsec->sid == new_tsec->osid)
                return;

        /* Close files for which the new task SID is not authorized. */
        flush_unauthorized_files(bprm->cred, current->files);

        /* Always clear parent death signal on SID transitions. */
        current->pdeath_signal = 0;
[...]
}

You should probably reset the coredump signal in the same places - or
even better, add a new helper for resetting the parent death signal,
and then add code for resetting the coredump signal in there.

  reply	other threads:[~2018-10-15 18:54 UTC|newest]

Thread overview: 70+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-13  0:33 [PATCH] kernel/signal: Signal-based pre-coredump notification Enke Chen
2018-10-13  6:40 ` Greg Kroah-Hartman
2018-10-15 18:16   ` Enke Chen
2018-10-15 18:43     ` Greg Kroah-Hartman
2018-10-15 18:49       ` Enke Chen
2018-10-15 18:58         ` Greg Kroah-Hartman
2018-10-13 10:44 ` Christian Brauner
2018-10-15 18:39   ` Enke Chen
2018-10-13 18:27 ` Jann Horn
2018-10-15 18:36   ` Enke Chen
2018-10-15 18:54     ` Jann Horn [this message]
2018-10-15 19:23       ` Enke Chen
2018-10-19 23:01       ` Enke Chen
2018-10-22 15:40         ` Jann Horn
2018-10-22 20:48           ` Enke Chen
2018-10-15 12:05 ` Oleg Nesterov
2018-10-15 18:54   ` Enke Chen
2018-10-15 19:17   ` Enke Chen
2018-10-15 19:26     ` Enke Chen
2018-10-16 14:14     ` Oleg Nesterov
2018-10-16 15:09       ` Eric W. Biederman
2018-10-17  0:39       ` Enke Chen
2018-10-15 21:21 ` Alan Cox
2018-10-15 21:31   ` Enke Chen
2018-10-15 23:28 ` Eric W. Biederman
2018-10-16  0:33   ` valdis.kletnieks
2018-10-16  0:54   ` Enke Chen
2018-10-16 15:26     ` Eric W. Biederman
2018-10-22 21:09 ` [PATCH v2] " Enke Chen
2018-10-23  9:23   ` Oleg Nesterov
2018-10-23 19:43     ` Enke Chen
2018-10-23 21:40       ` Enke Chen
2018-10-24 13:52       ` Oleg Nesterov
2018-10-24 21:56         ` Enke Chen
2018-10-24  5:39   ` [PATCH v3] " Enke Chen
2018-10-24 14:02     ` Oleg Nesterov
2018-10-24 22:02       ` Enke Chen
2018-10-25 22:56     ` [PATCH v4] " Enke Chen
2018-10-26  8:28       ` Oleg Nesterov
2018-10-26 22:23         ` Enke Chen
2018-10-29 11:18           ` Oleg Nesterov
2018-10-29 21:08             ` Enke Chen
2018-10-29 22:31             ` [PATCH v5] " Enke Chen
2018-10-30 16:46               ` Oleg Nesterov
2018-10-31  0:25                 ` Enke Chen
2018-11-22  0:37                 ` Andrew Morton
2018-11-22  1:09                   ` Enke Chen
2018-11-22  1:18                     ` Enke Chen
2018-11-22  1:33                     ` Andrew Morton
2018-11-22  4:57                       ` Enke Chen
2018-11-12 23:22               ` Enke Chen
2018-11-27 22:54               ` [PATCH v5 1/2] " Enke Chen
2018-11-28 15:19                 ` Dave Martin
2018-11-29  0:15                   ` Enke Chen
2018-11-29 11:55                     ` Dave Martin
2018-11-30  0:27                       ` Enke Chen
2018-11-30 12:03                       ` Oleg Nesterov
2018-12-05  6:47                       ` Jann Horn
2018-12-04 22:37                     ` Andrew Morton
2018-12-06 17:29                       ` Oleg Nesterov
2018-10-25 22:56     ` [PATCH] selftests/prctl: selftest for pre-coredump signal notification Enke Chen
2018-11-27 22:54       ` [PATCH v5 2/2] " Enke Chen
2018-10-24 13:29   ` [PATCH v2] kernel/signal: Signal-based pre-coredump notification Eric W. Biederman
2018-10-24 23:50     ` Enke Chen
2018-10-25 12:23       ` Eric W. Biederman
2018-10-25 20:45         ` Enke Chen
2018-10-25 21:24         ` Enke Chen
2018-10-25 21:56         ` Enke Chen
2018-10-25 13:45     ` Jann Horn
2018-10-25 20:21       ` Eric W. Biederman

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAG48ez2ZD=dovBApR07wi_DzYjJvEyO-Pm35eLv9AyXv1tcAFQ@mail.gmail.com' \
    --to=jannh@google.com \
    --cc=Dave.Martin@arm.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bp@alien8.de \
    --cc=catalin.marinas@arm.com \
    --cc=christian@brauner.io \
    --cc=deller@gmx.de \
    --cc=ebiederm@xmission.com \
    --cc=enkechen@cisco.com \
    --cc=gorcunov@openvz.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=guro@fb.com \
    --cc=hpa@zytor.com \
    --cc=kamensky@cisco.com \
    --cc=keescook@chromium.org \
    --cc=khalid.aziz@oracle.com \
    --cc=kirill.shutemov@linux.intel.com \
    --cc=kstewart@linuxfoundation.org \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@dominikbrodowski.net \
    --cc=marcos.souza.org@gmail.com \
    --cc=mchehab+samsung@kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=peterz@infradead.org \
    --cc=riel@surriel.com \
    --cc=sstrogin@cisco.com \
    --cc=tglx@linutronix.de \
    --cc=viro@zeniv.linux.org.uk \
    --cc=will.deacon@arm.com \
    --cc=x86@kernel.org \
    --cc=xe-linux-external@cisco.com \
    --cc=yang.shi@linux.alibaba.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).