Re: [PATCH 3/5] perf/core: add KUnit tests for AUX kernel-consumer API

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] perf: double free of perf_event in aux_test_create_event()
- [Medium] perf: memory leak of AUX pages on KUnit assertion failure
--

commit 5c8a187a6ddc0c41bba18676f8fced4749f18392
Author: Lian Wang <lianux.mm@gmail.com>

perf/core: add KUnit tests for AUX kernel-consumer API

This commit adds a KUnit test suite for the perf AUX kernel-consumer API.
It registers a self-contained dummy AUX PMU and includes 39 test cases to 
cover the setup, release, and data copying lifecycle.

> diff --git a/kernel/events/aux_kernel_test.c b/kernel/events/aux_kernel_test.c
> new file mode 100644
> --- /dev/null
> +++ b/kernel/events/aux_kernel_test.c

[ ... ]

> +static struct perf_event *aux_test_create_event(struct kunit *test)
> +{
> +	struct perf_event_attr attr = {};
> +	struct perf_event *event;
> +
> +	attr.type = dummy_aux_pmu_type;
> +	attr.size = sizeof(attr);
> +	attr.disabled = 1;
> +
> +	event = perf_event_create_kernel_counter(&attr, raw_smp_processor_id(),
> +						 NULL, NULL, NULL);
> +	if (IS_ERR(event))
> +		return event;
> +
> +	if (kunit_add_action_or_reset(test, aux_test_event_release, event)) {
> +		perf_event_release_kernel(event);

[Severity: Medium]
Will this cause a double free of the event object?

If kunit_add_action_or_reset() fails, it automatically executes the cleanup 
action, which in this case is aux_test_event_release(). That callback 
already invokes perf_event_release_kernel() on the event. Calling it a 
second time explicitly in this error path appears to trigger a double free.

> +		return ERR_PTR(-ENOMEM);
> +	}
> +
> +	return event;
> +}

[ ... ]

> +static void test_user_kernel_coexistence(struct kunit *test)
> +{
> +	struct perf_event *ev_a = aux_test_create_event(test);
> +	struct perf_event *ev_b = aux_test_create_event(test);
> +	struct perf_output_handle handle_a, handle_b;
> +	unsigned long a_head_before, b_head_before;
> +	void *addr;
> +	int ret;

[ ... ]

> +	/* Simulate userspace ownership: swap refcounts on B's rb.
> +	 * perf_mmap() would set aux_mmap_count=1 for a userspace event.
> +	 */
> +	refcount_set(&ev_b->rb->aux_mmap_count, 1);
> +	refcount_set(&ev_b->rb->aux_kernel_count, 0);
> +
> +	/* Produce on A — B's head stays 0 */
> +	addr = perf_aux_output_begin(&handle_a, ev_a);
> +	KUNIT_ASSERT_NOT_NULL(test, addr);

[Severity: Medium]
Does this leak the AUX pages if the assertion fails?

If this assertion or subsequent assertions fail while aux_kernel_count is
set to 0, the test will immediately abort and skip the manual refcount
restoration at the end of the function.

When KUnit runs the deferred cleanup action aux_test_event_release() on 
abort, perf_event_release_aux() observes that aux_kernel_count is 0 and 
returns early without tearing down the buffer, leaving the memory 
permanently leaked.

> +	perf_aux_output_end(&handle_a, 512);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004173458.837842-1-kunwu.chan@gmail.com?part=3