Re: [PATCH v5 4/5] perf evsel: don't set PERF_SAMPLE_IP for unprivileged tracepoints

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] The patch erroneously drops `PERF_SAMPLE_IP` for syscall tracepoints when run by unprivileged users, breaking userspace IP collection.
- [High] Uprobes silently lose `PERF_SAMPLE_IP` sampling for unprivileged users on builds without `libtraceevent`.
- [Low] Using the `__probe_ip` and `__probe_ret_ip` fields to uniquely identify uprobes is factually incorrect and misidentifies ftrace-based kprobes.
--

--- Patch [4]: [PATCH v5 4/5] perf evsel: don't set PERF_SAMPLE_IP for unprivileged tracepoints ---
commit c869f5208b9d673450d03276805401f2ff1e6af2
Author: Anubhav Shelat <ashelat@redhat.com>

perf evsel: don't set PERF_SAMPLE_IP for unprivileged tracepoints

For tracepoint events the IP is a static kernel address.
It doesn't vary by sample and provides no useful information for
unprivileged users. Skipping setting PERF_SAMPLE_IP for unprivileged
tracepoints avoids exposing a kernel address that reveals the KASLR base
offset.

Make an exception for uprobes, which are registered as
PERF_TYPE_TRACEPOINT, because the IP is important for their
functionality and is a safe userspace address. Detect them with
__probe_ip (entry) and __probe_ret_ip (return) using evsel__field().

> diff --git a/tools/perf/util/evsel.c b/tools/perf/util/evsel.c
> index ea9fa04429f08..1d85e5503cf3d 100644
> --- a/tools/perf/util/evsel.c
> +++ b/tools/perf/util/evsel.c
> @@ -1571,7 +1571,19 @@ void evsel__config(struct evsel *evsel, const struct record_opts *opts,
>  	attr->write_backward = opts->overwrite ? 1 : 0;
>  	attr->read_format   = PERF_FORMAT_LOST;
>  
> -	evsel__set_sample_bit(evsel, IP);
> +	/*
> +	 * Don't set PERF_SAMPLE_IP for unprivileged kernel tracepoints to
> +	 * avoid exposing kernel addresses. Uprobes expose only userspace
> +	 * addresses so they're safe. Detect both entry and return uprobes.
> +	 */
> +	if (attr->type != PERF_TYPE_TRACEPOINT || perf_event_paranoid_check(1)

[Severity: High]
Does this logic mistakenly strip the PERF_SAMPLE_IP bit for syscall
tracepoints when tracing as an unprivileged user?

Syscall tracepoints carry the TRACE_EVENT_FL_CAP_ANY flag, which explicitly
permits unprivileged access. When a syscall tracepoint fires, the kernel
passes the user-space pt_regs context to perf_instruction_pointer(), so
PERF_SAMPLE_IP safely reports the dynamic user-space address of the syscall
instruction rather than a sensitive kernel address.

> +#ifdef HAVE_LIBTRACEEVENT
> +	    || evsel__field(evsel, "__probe_ip")
> +	    || evsel__field(evsel, "__probe_ret_ip")
> +#endif

[Severity: High]
Could gating these checks behind HAVE_LIBTRACEEVENT cause unprivileged
uprobes to silently lose their IP data on builds where libtraceevent is
omitted?

Without the optional library, these checks will not be evaluated, and the
uprobe exemption fails. Uprobes would then be treated like regular kernel
tracepoints and stripped of PERF_SAMPLE_IP.

[Severity: Low]
Additionally, do the __probe_ip and __probe_ret_ip fields uniquely identify
uprobes?

Ftrace-based kprobes and kretprobes contain the exact same format fields in
their definitions. While unprivileged users are blocked from opening kprobes
by the kernel core (via perf_allow_tracepoint() returning -EPERM), this
heuristic still mistakenly categorizes kprobes as userspace uprobes.

> +	    )
> +		evsel__set_sample_bit(evsel, IP);
> +
>  	evsel__set_sample_bit(evsel, TID);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260714183150.292861-2-ashelat@redhat.com?part=4