Re: [PATCH 0/5] perf: Treat perf_event_paranoid and kptr_restrict like the kernel does it
From: Mathieu Poirier
Date: Wed Aug 28 2019 - 15:31:37 EST
On Mon, 26 Aug 2019 at 19:40, Igor Lubashev <ilubashe@xxxxxxxxxx> wrote:
> This is a follow up series to the ensure perf treats perf_event_paranoid and
> kptr_restrict in a way that is similar to the kernel's. That includes use of
> capabilities instead of euid==0, when possible, as well as adjusting the logic
> and fixing bugs.
> Prior discussion: https://lkml.kernel.org/lkml/cover.1565188228.git.ilubashe@xxxxxxxxxx
> === Testing notes ===
> I have tested on x86 with perf binary installed according to
> Documentation/admin-guide/perf-security.rst (cap_sys_admin, cap_sys_ptrace,
> cap_syslog assigned to the perf executable).
> I tested each permutation of:
> * 7 commits:
> 1. HEAD of perf/core
> 2. patch 01 on top of perf/core
> 3. patches 01-02 on top of perf/core
> 4. patches 01-03 on top of perf/core
> 5. patches 01-04 on top of perf/core
> 6. patches 01-05 on top of perf/core
> 7. HEAD of perf/cap (with known bug fixed by patch 01 of this series)
> * 2 build environments: with and without libcap-dev
> * 3 kernel.kptr_restrict values: 0, 1, 2
> * 4 kernel.perf_event_paranoid values: -1, 0, 1, 2
> * 2 users: root and non-root
> Total: 336 permutations
> Each permutation consisted of:
> perf test
> perf record -e instructions -- sleep 1
> All test runs were expected. Also, as expected, the following permutation (just
> that permutation) resulted in segmentation failure:
> commit: perf/cap
> build: no libcap-dev
> kernel.kptr_restrict: 0
> kernel.perf_event_paranoid: 2
> user: non-root
> The perf/cap commit was included in the test to ensure that we can reproduce the
> crash and hence test that the patch series fixes the crash, while retaining the
> desired behavior of perf/cap.
> === Series Contents ===
> 01: perf event: Check ref_reloc_sym before using it
> Fix the pre-existing cause of the crash above: use of ref_reloc_sym without
> a check for NULL
> 02: perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> Replace the use of euid==0 with a check for CAP_SYS_ADMIN whenever
> perf_event_paranoid level is verified.
> * This patch has been reviewed previously and is unchanged.
> * I kept Acks and Sign-offs.
> 03: perf util: kernel profiling is disallowed only when perf_event_paranoid>1
> Align perf logic regarding perf_event_paranoid to match kernel's.
> This has been reported by Arnaldo.
> 04: perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> Replace the use of uid and euid with a check for CAP_SYSLOG when
> kptr_restrict is verified (similar to kernel/kallsyms.c and lib/vsprintf.c).
> Consult perf_event_paranoid when kptr_restrict==0 (see kernel/kallsyms.c).
> * A previous version of this patch has been reviewed previously, but I
> * modified it in a non-trivial way, so I removed Acks.
> 05: perf: warn perf_event_paranoid restrict kernel symbols
> Warn that /proc/sys/kernel/perf_event_paranoid can also restrict kernel
> Igor Lubashev (5):
> perf event: Check ref_reloc_sym before using it
> perf tools: Use CAP_SYS_ADMIN with perf_event_paranoid checks
> perf util: kernel profiling is disallowed only when perf_event_paranoid > 1
> perf symbols: Use CAP_SYSLOG with kptr_restrict checks
> perf: warn that perf_event_paranoid can restrict kernel symbols
> tools/perf/arch/arm/util/cs-etm.c | 3 ++-
For the coresight part:
Reviewed-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>
> tools/perf/arch/arm64/util/arm-spe.c | 3 ++-
> tools/perf/arch/x86/util/intel-bts.c | 3 ++-
> tools/perf/arch/x86/util/intel-pt.c | 2 +-
> tools/perf/builtin-record.c | 2 +-
> tools/perf/builtin-top.c | 2 +-
> tools/perf/builtin-trace.c | 2 +-
> tools/perf/util/event.c | 7 ++++---
> tools/perf/util/evsel.c | 2 +-
> tools/perf/util/symbol.c | 15 ++++++++++++---
> 10 files changed, 27 insertions(+), 14 deletions(-)
For the set:
Tested-by: Mathieu Poirier <mathieu.poirier@xxxxxxxxxx>