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 <[email protected]> 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/[email protected]?part=2
