linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Leo Yan <leo.yan@arm.com>
To: Alexander Viro <viro@zeniv.linux.org.uk>,
	Christian Brauner <brauner@kernel.org>, Jan Kara <jack@suse.cz>,
	Eric Biederman <ebiederm@xmission.com>,
	Kees Cook <keescook@chromium.org>,
	linux-fsdevel@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org,
	Peter Zijlstra <peterz@infradead.org>,
	Ingo Molnar <mingo@redhat.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Namhyung Kim <namhyung@kernel.org>,
	Ian Rogers <irogers@google.com>
Cc: Leo Yan <leo.yan@arm.com>, Al Grant <al.grant@arm.com>,
	James Clark <james.clark@arm.com>,
	Mark Rutland <mark.rutland@arm.com>
Subject: [PATCH] exec: Don't disable perf events for setuid root executables
Date: Fri, 22 Mar 2024 16:27:59 +0000	[thread overview]
Message-ID: <20240322162759.714141-1-leo.yan@arm.com> (raw)

Al Grant reported that the 'perf record' command terminates abnormally
after setting the setuid bit for the executable. To reproduce this
issue, an additional condition is the binary file is owned by the root
user but is running under a non-privileged user. The logs below provide
details:

    $ sudo chmod u+s perf
    $ ls -l perf
    -rwsr-xr-x 1 root root 13147600 Mar 17 14:56 perf
    $ ./perf record -e cycles -- uname
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.003 MB perf.data (7 samples) ]
    Terminated

Comparatively, the same command can succeed if the setuid bit is cleared
for the perf executable:

    $ sudo chmod u-s perf
    $ ls -l perf
    -rwxr-xr-x 1 root root 13147600 Mar 17 14:56 perf
    $ ./perf record -e cycles -- uname
    Linux
    [ perf record: Woken up 1 times to write data ]
    [ perf record: Captured and wrote 0.003 MB perf.data (13 samples) ]

After setting the setuid bit, the problem arises when begin_new_exec()
disables the perf events upon detecting that a regular user is executing
a setuid binary, which notifies the perf process. Consequently, the perf
tool in user space exits from polling and sends a SIGTERM signal to kill
child processes and itself. This explains why we observe the tool being
'Terminated'.

With the setuid bit a non-privileged user can obtain the same
permissions as the executable's owner. If the owner has the privileged
permission for accessing perf events, the kernel should keep enabling
perf events. For this reason, this patch adds a condition checking for
perfmon_capable() to not disabling perf events when the user has
privileged permission yet.

Note the begin_new_exec() function only checks permission for the
per-thread mode in a perf session. This is why we don't need to add any
extra checking for the global knob 'perf_event_paranoid', as it always
grants permission for per-thread performance monitoring for unprivileged
users (see Documentation/admin-guide/perf-security.rst).

Signed-off-by: Leo Yan <leo.yan@arm.com>
Cc: Al Grant <al.grant@arm.com>
Cc: James Clark <james.clark@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
---
 fs/exec.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/exec.c b/fs/exec.c
index ff6f26671cfc..5ded01190278 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -1401,7 +1401,8 @@ int begin_new_exec(struct linux_binprm * bprm)
 	 * wait until new credentials are committed
 	 * by commit_creds() above
 	 */
-	if (get_dumpable(me->mm) != SUID_DUMP_USER)
+	if ((get_dumpable(me->mm) != SUID_DUMP_USER) &&
+	    !perfmon_capable())
 		perf_event_exit_task(me);
 	/*
 	 * cred_guard_mutex must be held at least to this point to prevent
-- 
2.39.2



             reply	other threads:[~2024-03-22 16:28 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-22 16:27 Leo Yan [this message]
2024-03-22 17:24 ` [PATCH] exec: Don't disable perf events for setuid root executables 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=20240322162759.714141-1-leo.yan@arm.com \
    --to=leo.yan@arm.com \
    --cc=acme@kernel.org \
    --cc=al.grant@arm.com \
    --cc=brauner@kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=irogers@google.com \
    --cc=jack@suse.cz \
    --cc=james.clark@arm.com \
    --cc=keescook@chromium.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@redhat.com \
    --cc=namhyung@kernel.org \
    --cc=peterz@infradead.org \
    --cc=viro@zeniv.linux.org.uk \
    /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).