Re: [PATCH 2/2] lib/tests: add KUnit test for nested objpool pushes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] lib/tests: UAF and memory leaks on KUnit assertion failure or timeout
- [High] lib/tests: module unload UAF due to unsafe kthread completion
- [Medium] lib/tests: missing memory barrier causes stale reads of test statistics
--

commit 6bf48aebedba8949a86524af4e186a561332ca36
Author: Shashank Mohan Jain <jain.sm@gmail.com>

    lib/tests: add KUnit test for nested objpool pushes
    
    objpool_push() only pushes to the slot of the local CPU, but a push from
    NMI context (a kretprobe'd function returning in NMI) can interrupt it
    and push to the same slot.

diff --git a/lib/tests/objpool_kunit.c b/lib/tests/objpool_kunit.c
new file mode 100644
index 0000000000000..d86ef00dd0887
--- /dev/null
+++ b/lib/tests/objpool_kunit.c
@@ -0,0 +1,329 @@
[ ... ]
> +static int op_task_fn(void *data)
> +{
[ ... ]
> +	while (ctx->remote_pop && !READ_ONCE(ctx->popper_done)) {

[Severity: Medium]
Will this polling cause stale reads of test statistics?

The flag is polled using READ_ONCE without an acquire barrier.

> +		/* keep one reserve object for the teardown */
> +		if (ctx->nreserve > 1) {
> +			obj = ctx->reserve[--ctx->nreserve];
> +			atomic_set(&obj->held, 0);
> +			local_irq_save(flags);
> +			__objpool_try_add_slot(obj, &ctx->pool, ctx->cpu);
> +			local_irq_restore(flags);
> +		}
> +		schedule_timeout_uninterruptible(1);
> +	}
> +
> +	complete(&ctx->done);
> +	return 0;
> +}

[Severity: High]
Can this create a module unload race condition?

The kthread signals completion before fully exiting. The thread can be preempted
before returning, and because the main test thread returns immediately after
wait_for_completion(), module unload could proceed before this kthread has
fully exited module text. If the module is removed, the kthread will resume
execution in unmapped memory.

[ ... ]
> +static int op_popper_fn(void *data)
> +{
[ ... ]
> +	while (!READ_ONCE(ctx->stop)) {
> +		obj = objpool_pop(&ctx->pool);
> +		if (obj)
> +			op_popped(ctx, obj);
> +		cond_resched();
> +	}
> +	WRITE_ONCE(ctx->popper_done, true);

[Severity: Medium]
Does this need a release barrier?

The completion flag is published using WRITE_ONCE without a release barrier,
allowing prior statistic updates to be reordered. On weakly ordered
architectures, op_task_fn might observe the completion state before the
statistic updates (like ctx->pops and ctx->doubles) are globally visible.

> +	return 0;
> +}

[ ... ]
> +static void op_run(struct kunit *test, bool remote_pop)
> +{
[ ... ]
> +	/* take every object out of the pool; hand them to the stocks */
> +	while (n < OP_NR_OBJS && (obj = objpool_pop(&ctx->pool)))
> +		ctx->objs[n++] = obj;
> +	KUNIT_ASSERT_EQ(test, n, OP_NR_OBJS);

[Severity: High]
Does an assertion failure here leak memory?

If this assertion fails, the test aborts and skips objpool_fini(), leaking
the pool internals.

[ ... ]
> +	task = kthread_create(op_task_fn, ctx, "objpool_kunit_push");
> +	KUNIT_ASSERT_FALSE(test, IS_ERR(task));
> +	kthread_bind(task, cpu);
> +	if (remote_pop) {
> +		popper = kthread_create(op_popper_fn, ctx, "objpool_kunit_pop");
> +		KUNIT_ASSERT_FALSE(test, IS_ERR(popper));

[Severity: High]
Would an assertion failure here leak the kthread and the objpool?

If the KUNIT_ASSERT_FALSE macro triggers a test abort, the already-created
'task' thread is never stopped or freed, and the objpool is leaked.

> +		kthread_bind(popper, pcpu);
> +		wake_up_process(popper);
> +	}
> +	wake_up_process(task);
> +	wait_for_completion(&ctx->done);

[Severity: High]
Is there a use-after-free risk if the test times out here?

If the test times out while waiting for completion, KUnit will free ctx without
stopping the kthreads. The background kthreads will continue running, access
the freed ctx memory, and trigger a use-after-free.

Additionally, because the main thread returns immediately after this completion,
it does not wait for the kthreads to fully exit. As mentioned above, this
exposes the test to a module unload crash where the kthreads resume execution
in unmapped memory.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260928084125.67104-1-jain.sm@gmail.com?part=2