Re: [PATCH] tracing/filters: Check perf permissions before resolving .function

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] tracing: perf string filters on pointer fields disclose kernel memory
--

commit 169e523d3d15d13ccfdf41ba6a65f6c9fb12fff8
Author: Kyle Zeng <kylebot@openai.com>
tracing/filters: Check perf permissions before resolving .function

This commit adds a permission check to the .function filter suffix in
perf counting filters to prevent exposing kernel symbol ranges. It closes
an oracle that could allow KASLR bypass via unprivileged tracing.

> diff --git a/kernel/trace/trace_events_filter.c b/kernel/trace/trace_events_filter.c
> index 2b46ca5360458..0f8c2054f4a72 100644
> --- a/kernel/trace/trace_events_filter.c
> +++ b/kernel/trace/trace_events_filter.c
[ ... ]
> @@ -1683,6 +1689,14 @@ static int parse_pred(const char *str, void *data,
>  
>  	/* See if the field is a kernel function name */
>  	if ((len = str_has_prefix(str + i, ".function"))) {
> +		/* Even counting filters can disclose kernel addresses. */
> +		if (pdata->is_perf) {
> +			ret = perf_allow_tracepoint();
> +			if (ret) {
> +				parse_error(pe, ret, pos + i);
> +				return ret;
> +			}
> +		}
>  		function = true;
>  		i += len;
>  	}

[Severity: High]
This successfully prevents KASLR bypass via the .function suffix, but does
it leave a similar memory disclosure oracle open for standard string matches
on pointer fields?

Later in parse_pred(), if the field is a pointer and the user omits the
.ustring suffix, the predicate is assigned FILTER_PRED_FN_PCHAR without
checking perf_allow_tracepoint():

kernel/trace/trace_events_filter.c:parse_pred() {
    ...
		if (ustring)
			pred->fn_num = FILTER_PRED_FN_PCHAR_USER;
		else
			pred->fn_num = FILTER_PRED_FN_PCHAR;
    ...
}

Could an unprivileged user create a counting perf event without
PERF_SAMPLE_RAW on a syscall tracepoint like sys_enter_openat (which is
allowed because syscall tracepoints have the TRACE_EVENT_FL_CAP_ANY flag)?

If they set a string wildcard filter (e.g., ~ "secret*") on a pointer field
like filename, wouldn't the filter engine read from the user-controlled
kernel_addr when evaluating FILTER_PRED_FN_PCHAR?

kernel/trace/trace_events_filter.c:test_string() {
    ...
	/* For safety, do not trust the string pointer */
	if (strncpy_from_kernel_nofault(kstr, str, USTRING_BUF_SIZE) < 0)
		return NULL;
    ...
}

If the kernel memory successfully matches the wildcard, the perf counter
increments, which could allow an attacker to extract arbitrary kernel memory
byte-by-byte.

Should perf_allow_tracepoint() also be checked before assigning
FILTER_PRED_FN_PCHAR to prevent this disclosure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261006225112.53503-1-kylebot@openai.com?part=1