| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAP-5=fXDD5ZQ0tcnyJyFzGnYxWtJmop7qu2m3ZwROYjcPSv=zg@mail.gmail.com>
Date: Fri, 25 Oct 2024 22:59:06 -0700
From: Ian Rogers <irogers@...gle.com>
To: Adrian Hunter <adrian.hunter@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>, Ingo Molnar <mingo@...hat.com>,
Arnaldo Carvalho de Melo <acme@...nel.org>, Namhyung Kim <namhyung@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Alexander Shishkin <alexander.shishkin@...ux.intel.com>, Jiri Olsa <jolsa@...nel.org>,
Kan Liang <kan.liang@...ux.intel.com>, James Clark <james.clark@....com>,
Oliver Upton <oliver.upton@...ux.dev>, Leo Yan <leo.yan@...ux.dev>,
Changbin Du <changbin.du@...wei.com>, Athira Rajeev <atrajeev@...ux.vnet.ibm.com>,
linux-perf-users@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3] perf cap: Tidy up and improve capability testing
On Wed, Oct 23, 2024 at 1:10 AM Adrian Hunter <adrian.hunter@...el.com> wrote:
>
> On 22/10/24 19:34, Ian Rogers wrote:
> > On Tue, Oct 22, 2024 at 6:56 AM Adrian Hunter <adrian.hunter@...el.com> wrote:
> >>
> >> On 7/08/24 01:06, Ian Rogers wrote:
> >>> Remove dependence on libcap. libcap is only used to query whether a
> >>> capability is supported, which is just 1 capget system call.
> >>>
> >>> If the capget system call fails, fall back on root permission
> >>> checking. Previously if libcap fails then the permission is assumed
> >>> not present which may be pessimistic/wrong.
> >>>
> >>> Add a used_root out argument to perf_cap__capable to say whether the
> >>> fall back root check was used. This allows the correct error message,
> >>> "root" vs "users with the CAP_PERFMON or CAP_SYS_ADMIN capability", to
> >>> be selected.
> >>>
> >>> Tidy uses of perf_cap__capable so that tests aren't repeated if capget
> >>> isn't supported.
> >>>
> >>> Signed-off-by: Ian Rogers <irogers@...gle.com>
> >>
> >> I find I need to revert this. E.g.
> >>
> >> $ install-perf.sh
> >> $ perf record -e intel_pt//k uname
> >> WARNING: Kernel address maps (/proc/{kallsyms,modules}) are restricted,
> >> check /proc/sys/kernel/kptr_restrict and
> >> /proc/sys/kernel/perf_event_paranoid.
> >>
> >> Samples in kernel functions may not be resolved if a suitable vmlinux
> >> file is not found in the buildid cache or in the vmlinux path.
> >>
> >> Samples in kernel modules won't be resolved at all.
> >>
> >> If some relocation was applied (e.g. kexec) symbols may be misresolved
> >> even with a suitable vmlinux or kallsyms file.
> >>
> >> Couldn't record kernel reference relocation symbol
> >> Symbol resolution may be skewed if relocation was used (e.g. kexec).
> >> Check /proc/kallsyms permission or run as root.
> >> Linux
> >> [ perf record: Woken up 1 times to write data ]
> >> [ perf record: Captured and wrote 0.061 MB perf.data ]
> >> $ git revert e25ebda78e230283bf707ae3e9655270ff40a7f9
> >> Auto-merging tools/perf/Makefile.config
> >> Auto-merging tools/perf/builtin-ftrace.c
> >> Auto-merging tools/perf/util/Build
> >> Auto-merging tools/perf/util/symbol.c
> >> [master 752707588a700] Revert "perf cap: Tidy up and improve capability
> >> testing"
> >> 7 files changed, 70 insertions(+), 73 deletions(-)
> >> $ install-perf.sh
> >> $ perf record -e intel_pt//k uname
> >> Linux
> >> [ perf record: Woken up 1 times to write data ]
> >> [ perf record: Captured and wrote 0.065 MB perf.data ]
> >
> > Sorry for this. I couldn't immediately see a code path that should
> > impact `perf record` and the reproducer works for me without the
> > revert.
>
> Sorry, I forgot to highlight that capabilities are being used
> as described here:
>
> https://www.kernel.org/doc/html/latest/admin-guide/perf-security.html
>
> i.e. # setcap "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" perf
> # setcap -v "cap_perfmon,cap_sys_ptrace,cap_syslog=ep" perf
> perf: OK
> # getcap perf
> perf = cap_sys_ptrace,cap_syslog,cap_perfmon+ep
>
> instead of changing perf_paranoid, kptr_restrict etc
Thanks Adrian. I was able to reproduce something like the problem.
Anyway, the issue appears to be include path related, I came up with:
https://lore.kernel.org/lkml/20241026055448.312247-1-irogers@google.com/
Could you take a look and test for me. Again, sorry for the inconvenience.
Ian
> > revert. Running your command under gdb looking for calls to
> > perf_cap__capable:
> >
> > 1)
> > ```
> > #0 0x00005555557f7abe in symbol__read_kptr_restrict () at util/symbol.c:2429
> > #1 0x00005555557f759f in symbol__init (env=0x0) at util/symbol.c:2523
> > #2 0x00005555555e7ca3 in cmd_record (argc=1, argv=0x7fffffffe390) at
> > builtin-record.c:4114
> > #3 0x000055555569775f in run_builtin (p=0x5555560bda50
> > <commands+288>, argc=4,
> > argv=0x7fffffffe390) at perf.c:351
> > #4 0x0000555555696e13 in handle_internal_command (argc=4,
> > argv=0x7fffffffe390) at perf.c:404
> > #5 0x000055555569762f in run_argv (argcp=0x7fffffffe1dc,
> > argv=0x7fffffffe1d0) at perf.c:448
> > #6 0x0000555555696a74 in main (argc=4, argv=0x7fffffffe390) at perf.c:562
> > ```
> > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/builtin-record.c?h=perf-tools-next#n4108
> > ```
> > /*
> > * Allow aliases to facilitate the lookup of symbols for address
> > * filters. Refer to auxtrace_parse_filters().
> > */
> > symbol_conf.allow_aliases = true;
> >
> > symbol__init(NULL);
> > ```
> > I don't think this is the cause but it seems unfortunate we're
> > enabling symbols in perf record unconditionally.
> >
> > 2)
> > ```
> > #0 perf_cap__capable (cap=21, used_root=0x7fffffffbd87) at util/cap.c:22
> > #1 0x0000555555878236 in perf_event_paranoid_check (max_level=-1) at
> > util/util.c:330
> > #2 0x00005555559574ba in intel_pt_recording_options
> > (itr=0x55555610ada0, evlist=0x5555560f36f0,
> > opts=0x5555560b4818 <record+320>) at arch/x86/util/intel-pt.c:619
> > #3 0x00005555558a46ad in auxtrace_record__options
> > (itr=0x55555610ada0, evlist=0x5555560f36f0,
> > opts=0x5555560b4818 <record+320>) at util/auxtrace.c:619
> > #4 0x00005555555e8139 in cmd_record (argc=1, argv=0x7fffffffe390) at
> > builtin-record.c:4206
> > #5 0x000055555569775f in run_builtin (p=0x5555560bda50
> > <commands+288>, argc=4,
> > argv=0x7fffffffe390) at perf.c:351
> > #6 0x0000555555696e13 in handle_internal_command (argc=4,
> > argv=0x7fffffffe390) at perf.c:404
> > #7 0x000055555569762f in run_argv (argcp=0x7fffffffe1dc,
> > argv=0x7fffffffe1d0) at perf.c:448
> > #8 0x0000555555696a74 in main (argc=4, argv=0x7fffffffe390) at perf.c:562
> > ```
> > So this looks like the cause.
> >
> > Would it be possible for you to trace (strace/gdb/perf trace) what's
> > happening in perf_cap__capable? I suspect the system call capget and
> > its interpretation are the source of the problem, but I can't eyeball
> > an issue in:
> > https://git.kernel.org/pub/scm/linux/kernel/git/perf/perf-tools-next.git/tree/tools/perf/util/cap.c?h=perf-tools-next#n53
> >
> > I'd prefer a fix to a revert but I'll leave that to others to decide.
> >
> > Thanks,
> > Ian
>
Powered by blists - more mailing lists