Re: [PATCH v5 1/8] mm/slab, slub_kunit: register kprobe to trigger _nolock APIs
[email protected] Wed, 29 Jul 2026 08:35:30 +0000
| Newsgroups | dev.linux.lists.linux-rt-devel,org.kernel.vger.bpf,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
Thank you for your contribution! Sashiko AI review found 2 potential issue(= s) to consider: - [High] Manual cleanup of global kprobe leads to stack Use-After-Free if t= he KUnit test aborts. - [Medium] Global kprobe registration causes concurrent data races and trig= gers test allocations from unrelated system processes. -- --- Patch [1]: [PATCH v5 1/8] mm/slab, slub_kunit: register kprobe to trigg= er _nolock APIs --- commit a8a477af9c0547e09308b4a325920cd9de52b25e Author: Harry Yoo (Oracle) <[email protected]> mm/slab, slub_kunit: register kprobe to trigger _nolock APIs =20 Since kmalloc_nolock() always fails in NMI and hardirq contexts on PREEMPT_RT, slub_kunit cannot properly test _nolock() APIs. =20 Register a kprobe pre-handler to invoke kmalloc_nolock() and kfree_nolock() in the middle of the slab allocator. =20 Link: https://lore.kernel.org/linux-mm/20260427-nolock-api-fix-v2-0-a6b= [email protected] [1] Link: https://lore.kernel.org/linux-mm/6edebc2b-5f5a-4b9c-9a4c-564310ac= [email protected] [2] > diff --git a/lib/tests/slub_kunit.c b/lib/tests/slub_kunit.c > index fa6d31dbca166..8c2b9911471eb 100644 > --- a/lib/tests/slub_kunit.c > +++ b/lib/tests/slub_kunit.c [ ... ] > +#if defined(CONFIG_KPROBES) && defined(CONFIG_SMP) > +static int slab_kprobe_pre_handler(struct kprobe *p, struct pt_regs *reg= s) > +{ > + struct test_nolock_context *ctx; > + > + ctx =3D container_of(p, struct test_nolock_context, kprobe); > + test_nolock(ctx); [Severity: Medium] Will this handler execute unconditionally for any task that hits the kprobe? The kprobe is registered globally on slab_attach_kprobe_locked(). Since this function is inserted into slab_lockdep_assert_held(), which is heavily used in the slab allocator's paths, any task on any CPU performing an allocation can trigger the kprobe. This allows unrelated system processes to execute the test logic concurrent= ly. Does this code need a check to restrict execution to the test thread, such = as verifying current against the test task? Additionally, this can cause data races in test_nolock() since unrelated system processes would perform unsynchronized read-modify-write operations = on the stack-allocated ctx variables: lib/tests/slub_kunit.c:test_nolock() { ... if (objp) ctx->alloc_ok++; else ctx->alloc_fail++; ... } > + return 0; > +} > + > +static bool register_slab_kprobes(struct test_nolock_context *ctx) > +{ > + ctx->kprobe.symbol_name =3D "slab_attach_kprobe_locked"; > + ctx->kprobe.pre_handler =3D slab_kprobe_pre_handler; > + > + if (register_kprobe(&ctx->kprobe)) > + return false; > + return true; > +} [ ... ] > +static void test_kmalloc_kfree_nolock_kprobe(struct kunit *test) > +{ > + struct test_nolock_context ctx =3D { .test =3D test }; > =20 > -cleanup: > - perf_event_disable(ctx.event); > - perf_event_release_kernel(ctx.event); > + if (!register_slab_kprobes(&ctx)) > + kunit_skip(test, "Failed to register kprobe, skipping"); > =20 > - kunit_info(test, "callback_count: %d, alloc_ok: %d, alloc_fail: %d\n", > - ctx.callback_count, ctx.alloc_ok, ctx.alloc_fail); > + test_kmalloc_kfree(); > =20 > - if (alloc_fail) > - kunit_skip(test, "Allocation failed"); > + unregister_slab_kprobes(&ctx); [Severity: High] Can this manual unregistration lead to a stack use-after-free if the KUnit test aborts before completing? If the test aborts (for example, due to a KUnit timeout which is possible given the system-wide contention caused by the global kprobe), the thread could exit and its stack memory would be freed, bypassing this manual cleanup. If the global kprobe remains active and continues to fire for other tasks, would the pre-handler then dereference the ctx pointer, pointing to freed stack memory? Could kunit_add_action() be used to ensure the cleanup is always executed even if the test aborts? > KUNIT_EXPECT_EQ(test, 0, slab_errors); > } > #endif --=20 Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260729-kfree_rcu_= [email protected]?part=3D1