lkml.org 
[lkml]   [2021]   [Jul]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [PATCH v3 1/2] perf: Fix required permissions if sigtrap is requested
    On Mon, Jul 5, 2021 at 10:45 AM Marco Elver <elver@google.com> wrote:
    >
    > If perf_event_open() is called with another task as target and
    > perf_event_attr::sigtrap is set, and the target task's user does not
    > match the calling user, also require the CAP_KILL capability or
    > PTRACE_MODE_ATTACH permissions.
    >
    > Otherwise, with the CAP_PERFMON capability alone it would be possible
    > for a user to send SIGTRAP signals via perf events to another user's
    > tasks. This could potentially result in those tasks being terminated if
    > they cannot handle SIGTRAP signals.
    >
    > Note: The check complements the existing capability check, but is not
    > supposed to supersede the ptrace_may_access() check. At a high level we
    > now have:
    >
    > capable of CAP_PERFMON and (CAP_KILL if sigtrap)
    > OR
    > ptrace_may_access(...) // also checks for same thread-group and uid
    >
    > Fixes: 97ba62b27867 ("perf: Add support for SIGTRAP on perf events")
    > Cc: <stable@vger.kernel.org> # 5.13+
    > Reported-by: Dmitry Vyukov <dvyukov@google.com>
    > Signed-off-by: Marco Elver <elver@google.com>

    Acked-by: Dmitry Vyukov <dvyukov@google.com>

    > ---
    > v3:
    > * Upgrade ptrace mode check to ATTACH if attr.sigtrap, otherwise it's
    > possible to change the target task (send signal) even if only read
    > ptrace permissions were granted (reported by Eric W. Biederman).
    >
    > v2: https://lkml.kernel.org/r/20210701083842.580466-1-elver@google.com
    > * Drop kill_capable() and just check CAP_KILL (reported by Ondrej Mosnacek).
    > * Use ns_capable(__task_cred(task)->user_ns, CAP_KILL) to check for
    > capability in target task's ns (reported by Ondrej Mosnacek).
    >
    > v1: https://lkml.kernel.org/r/20210630093709.3612997-1-elver@google.com
    > ---
    > kernel/events/core.c | 25 ++++++++++++++++++++++++-
    > 1 file changed, 24 insertions(+), 1 deletion(-)
    >
    > diff --git a/kernel/events/core.c b/kernel/events/core.c
    > index fe88d6eea3c2..f79ee82e644a 100644
    > --- a/kernel/events/core.c
    > +++ b/kernel/events/core.c
    > @@ -12152,10 +12152,33 @@ SYSCALL_DEFINE5(perf_event_open,
    > }
    >
    > if (task) {
    > + unsigned int ptrace_mode = PTRACE_MODE_READ_REALCREDS;
    > + bool is_capable;
    > +
    > err = down_read_interruptible(&task->signal->exec_update_lock);
    > if (err)
    > goto err_file;
    >
    > + is_capable = perfmon_capable();
    > + if (attr.sigtrap) {
    > + /*
    > + * perf_event_attr::sigtrap sends signals to the other
    > + * task. Require the current task to also have
    > + * CAP_KILL.
    > + */
    > + rcu_read_lock();
    > + is_capable &= ns_capable(__task_cred(task)->user_ns, CAP_KILL);
    > + rcu_read_unlock();
    > +
    > + /*
    > + * If the required capabilities aren't available, checks
    > + * for ptrace permissions: upgrade to ATTACH, since
    > + * sending signals can effectively change the target
    > + * task.
    > + */
    > + ptrace_mode = PTRACE_MODE_ATTACH_REALCREDS;
    > + }
    > +
    > /*
    > * Preserve ptrace permission check for backwards compatibility.
    > *
    > @@ -12165,7 +12188,7 @@ SYSCALL_DEFINE5(perf_event_open,
    > * perf_event_exit_task() that could imply).
    > */
    > err = -EACCES;
    > - if (!perfmon_capable() && !ptrace_may_access(task, PTRACE_MODE_READ_REALCREDS))
    > + if (!is_capable && !ptrace_may_access(task, ptrace_mode))
    > goto err_cred;
    > }
    >
    > --
    > 2.32.0.93.g670b81a890-goog
    >

    \
     
     \ /
      Last update: 2021-07-06 08:17    [W:3.761 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site