All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Harry Yoo (Oracle)" <harry@kernel.org>
Cc: linux-rt-devel@lists.linux.dev, linux-kernel@vger.kernel.org,
	bpf@vger.kernel.org
Subject: Re: [PATCH slab/for-next v4 1/8] mm/slab, slub_kunit: register kprobe to trigger _nolock APIs
Date: Mon, 20 Jul 2026 12:56:08 +0000	[thread overview]
Message-ID: <20260720125609.030E31F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260720-kfree_rcu_nolock-v4-1-964e03c41a4e@kernel.org>

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The globally registered kprobe pre-handler executes system-wide on all CPUs, causing non-test tasks to inadvertently execute KUnit test logic and concurrently mutate the test thread's stack-allocated context.
- [Medium] The refactoring to `kmalloc_obj(*objects[j], gfp)` incorrectly evaluates the allocation size to 1 byte instead of the intended 64 bytes.
--

--- Patch [1]: [PATCH slab/for-next v4 1/8] mm/slab, slub_kunit: register kprobe to trigger _nolock APIs ---
commit 632627083aa5c71fb7be8b45f24099b08c94d6a7
Author: Harry Yoo (Oracle) <harry@kernel.org>

mm/slab, slub_kunit: register kprobe to trigger _nolock APIs

This commit registers a kprobe pre-handler in slub_kunit to invoke and
test kmalloc_nolock() and kfree_nolock() from within the slab allocator.
It adds a wrapper for lockdep_assert_held() to attach the kprobe, and
refactors the test allocation loop into a shared helper function.

Link: https://lore.kernel.org/linux-mm/20260427-nolock-api-fix-v2-0-a6b83a92d9a4@kernel.org [1]
Link: https://lore.kernel.org/linux-mm/6edebc2b-5f5a-4b9c-9a4c-564310acee1b@kernel.org  [2]

> diff --git a/lib/tests/slub_kunit.c b/lib/tests/slub_kunit.c
> index fa6d31dbca166..a391467c1aa6e 100644
> --- a/lib/tests/slub_kunit.c
> +++ b/lib/tests/slub_kunit.c

[ ... ]

> @@ -302,26 +303,40 @@ struct test_nolock_context {
>  	int callback_count;
>  	int alloc_ok;
>  	int alloc_fail;
> +#ifdef CONFIG_PERF_EVENTS
>  	struct perf_event *event;
> +#endif
> +#if defined(CONFIG_KPROBES) && defined(CONFIG_SMP)
> +	struct kprobe kprobe;
> +#endif
>  };
>  
> -static struct perf_event_attr hw_attr = {
> -	.type = PERF_TYPE_HARDWARE,
> -	.config = PERF_COUNT_HW_CPU_CYCLES,
> -	.size = sizeof(struct perf_event_attr),
> -	.pinned = 1,
> -	.disabled = 1,
> -	.freq = 1,
> -	.sample_freq = 100000,
> -};
> +static void test_kmalloc_kfree(void)
> +{
> +	int i, j;
> +
> +	for (i = 0; i < NR_ITERATIONS; i++) {
> +		for (j = 0; j < NR_OBJECTS; j++) {
> +			gfp_t gfp = (i % 2) ? GFP_KERNEL : GFP_KERNEL_ACCOUNT;
>  
> -static void overflow_handler_test_kmalloc_kfree_nolock(struct perf_event *event,
> -						       struct perf_sample_data *data,
> -						       struct pt_regs *regs)
> +			objects[j] = kmalloc_obj(*objects[j], gfp);

[Severity: Medium]
Does this refactoring unintentionally reduce the allocation size to 1 byte?

Since objects is declared as an array of void *, the dereferenced type
of *objects[j] is void. The kmalloc_obj() macro evaluates
sizeof(typeof(VAR_OR_TYPE)), which resolves to sizeof(void). Under GCC,
sizeof(void) evaluates to 1.

Does this silently reduce the test coverage by targeting kmalloc-8 instead
of the original 64-byte allocations targeted in the previous code?

> +			if (!objects[j]) {
> +				j--;
> +				while (j >= 0)
> +					kfree(objects[j--]);
> +				return;
> +			}
> +		}
> +

[ ... ]

> +#if defined(CONFIG_KPROBES) && defined(CONFIG_SMP)
> +static int slab_kprobe_pre_handler(struct kprobe *p, struct pt_regs *regs)
> +{
> +	struct test_nolock_context *ctx;
> +
> +	ctx = container_of(p, struct test_nolock_context, kprobe);
> +	test_nolock(ctx);
> +	return 0;
> +}

[Severity: High]
Does this kprobe pre-handler execute for all CPUs system-wide?

Since this function is invoked via the kprobe attached to
slab_attach_kprobe_locked(), and there is no check ensuring
current == test_task, it appears any non-test task executing SLUB
slowpaths (like add_partial or remove_full) will also trigger this
handler.

Could this allow unrelated tasks to inadvertently execute test_nolock()
from arbitrary contexts and concurrently mutate the test thread's
stack-allocated ctx, causing a data race on ctx->alloc_ok and
ctx->callback_count?

> +
> +static bool register_slab_kprobes(struct test_nolock_context *ctx)
> +{
> +	ctx->kprobe.symbol_name = "slab_attach_kprobe_locked";
> +	ctx->kprobe.pre_handler = slab_kprobe_pre_handler;
> +
> +	if (register_kprobe(&ctx->kprobe))
> +		return false;
> +	return true;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-kfree_rcu_nolock-v4-0-964e03c41a4e@kernel.org?part=1

  reply	other threads:[~2026-07-20 12:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-20 12:44 [PATCH slab/for-next v4 0/8] mm/slab: introduce kfree_rcu_nolock() and improve slub_kunit coverage Harry Yoo (Oracle)
2026-07-20 12:44 ` [PATCH slab/for-next v4 1/8] mm/slab, slub_kunit: register kprobe to trigger _nolock APIs Harry Yoo (Oracle)
2026-07-20 12:56   ` sashiko-bot [this message]
2026-07-20 12:44 ` [PATCH slab/for-next v4 2/8] mm/slab: handle the !allow_spin case in kfree_rcu_sheaf() Harry Yoo (Oracle)
2026-07-20 13:11   ` sashiko-bot
2026-07-20 12:44 ` [PATCH slab/for-next v4 3/8] mm/slab: use call_rcu() in unknown context if irqs are enabled Harry Yoo (Oracle)
2026-07-20 13:01   ` sashiko-bot
2026-07-20 12:44 ` [PATCH slab/for-next v4 4/8] mm/slab: extend deferred free mechanism to handle rcu sheaves Harry Yoo (Oracle)
2026-07-20 13:03   ` sashiko-bot
2026-07-20 12:44 ` [PATCH slab/for-next v4 5/8] mm/slab: allow kfree_rcu_sheaf() on PREEMPT_RT Harry Yoo (Oracle)
2026-07-20 12:56   ` sashiko-bot
2026-07-20 12:44 ` [PATCH slab/for-next v4 6/8] mm/slab: introduce struct kvfree_rcu_head for kvfree_rcu batching Harry Yoo (Oracle)
2026-07-20 13:09   ` sashiko-bot
2026-07-20 12:44 ` [PATCH slab/for-next v4 7/8] mm/slab: introduce kfree_rcu_nolock() Harry Yoo (Oracle)
2026-07-20 13:07   ` sashiko-bot
2026-07-20 12:44 ` [PATCH slab/for-next v4 8/8] slub_kunit: extend the test for kfree_rcu_nolock() Harry Yoo (Oracle)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260720125609.030E31F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=harry@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-rt-devel@lists.linux.dev \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.