From 9d7a6c95f62bc335b62aaf9d50590122bd03a796 Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Mon, 5 Jul 2021 10:44:52 +0200 Subject: perf: Fix required permissions if sigtrap is requested 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") Reported-by: Dmitry Vyukov Signed-off-by: Marco Elver Signed-off-by: Peter Zijlstra (Intel) Acked-by: Dmitry Vyukov Cc: # 5.13+ Link: https://lore.kernel.org/r/20210705084453.2151729-1-elver@google.com --- kernel/events/core.c | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) (limited to 'kernel') diff --git a/kernel/events/core.c b/kernel/events/core.c index 464917096e73..c13730b7ac01 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -12158,10 +12158,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. * @@ -12171,7 +12194,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; } -- cgit From b068fc04de10fff8974f6ef32b861ad134d94ba4 Mon Sep 17 00:00:00 2001 From: Marco Elver Date: Mon, 5 Jul 2021 10:44:53 +0200 Subject: perf: Refactor permissions check into perf_check_permission() Refactor the permission check in perf_event_open() into a helper perf_check_permission(). This makes the permission check logic more readable (because we no longer have a negated disjunction). Add a comment mentioning the ptrace check also checks the uid. No functional change intended. Signed-off-by: Marco Elver Signed-off-by: Peter Zijlstra (Intel) Reviewed-by: Dmitry Vyukov Link: https://lore.kernel.org/r/20210705084453.2151729-2-elver@google.com --- kernel/events/core.c | 58 +++++++++++++++++++++++++++++----------------------- 1 file changed, 32 insertions(+), 26 deletions(-) (limited to 'kernel') diff --git a/kernel/events/core.c b/kernel/events/core.c index c13730b7ac01..1cb1f9b8392e 100644 --- a/kernel/events/core.c +++ b/kernel/events/core.c @@ -11917,6 +11917,37 @@ again: return gctx; } +static bool +perf_check_permission(struct perf_event_attr *attr, struct task_struct *task) +{ + unsigned int ptrace_mode = PTRACE_MODE_READ_REALCREDS; + bool 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. The + * ptrace check also includes checks that the current task and other + * task have matching uids, and is therefore not done here explicitly. + */ + return is_capable || ptrace_may_access(task, ptrace_mode); +} + /** * sys_perf_event_open - open a performance event, associate it to a task/cpu * @@ -12158,43 +12189,18 @@ 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. - * * We must hold exec_update_lock across this and any potential * perf_install_in_context() call for this new event to * serialize against exec() altering our credentials (and the * perf_event_exit_task() that could imply). */ err = -EACCES; - if (!is_capable && !ptrace_may_access(task, ptrace_mode)) + if (!perf_check_permission(&attr, task)) goto err_cred; } -- cgit