Re: [PATCH 1/2] objpool: keep objpool_push() correct when a push from NMI nests in it

[Andrew Morton <akpm@linux-foundation.org>]

On Mon, 28 Sep 2026 14:11:24 +0530 Shashank Mohan Jain <jain.sm@gmail.com> wrote:

> objpool_push() adds an object to the slot of the local CPU with
> interrupts disabled. It reserves an entry with a cmpxchg() on
> slot->tail, writes the entry, and publishes it with
> smp_store_release(&slot->last, tail + 1).
> 
> A push from NMI context can still interrupt it and push to the same
> slot. kretprobes may run in NMI context since commit e03b4a084ea6
> ("kprobes: Remove NMI context check"). At the time, kretprobe instances
> came from a CAS-based lockless freelist, which tolerates that. Commit
> 4bbd93455659 ("kprobes: kretprobe scalability improvement") moved
> kretprobes and rethook to objpool.
> 
> With rethook, rethook_trampoline_handler() recycles instances with
> objpool_push() after the user handler has run, when no kprobe is marked
> running anymore; rethook_flush_task() does the same. An NMI that arrives
> during such a push and runs a function probed by the same kretprobe takes
> an instance in pre_handler_kretprobe(), and when the function returns
> inside the NMI, pushes it back to the same slot. This needs a kretprobe
> on a function that runs both in NMI context and outside it, for instance
> one that perf calls from the PMU NMI handler on x86. Before v6.14, fprobe
> also used rethook and could push from NMI the same way, with one pool
> shared by all functions of an fprobe.
> 
> ...
>
> Publish the entries in order instead:

Thanks.

> --- a/include/linux/objpool.h
> +++ b/include/linux/objpool.h
> @@ -193,19 +193,40 @@ __objpool_try_add_slot(void *obj, struct objpool_head *pool, int cpu)
>  	struct objpool_slot *slot = pool->cpu_slots[cpu];
>  	uint32_t head, tail;
>  
> -	/* loading tail and head as a local snapshot, tail first */
> +	/*
> +	 * Only the local CPU pushes to its slot, with irqs disabled, but a
> +	 * push from NMI context (a kretprobe'd function returning in NMI)
> +	 * can interrupt this one at any point.
> +	 */
>  	tail = READ_ONCE(slot->tail);
> +	while (!try_cmpxchg_acquire(&slot->tail, &tail, tail + 1))
> +		;
>  
> -	do {
> -		head = READ_ONCE(slot->head);
> -		/* fault caught: something must be wrong */
> -		WARN_ON_ONCE(tail - head > pool->nr_objs);
> -	} while (!try_cmpxchg_acquire(&slot->tail, &tail, tail + 1));
> +	/*
> +	 * fault caught: something must be wrong.  Read head only after the
> +	 * reservation: a nested push and a pop on another CPU could have
> +	 * moved head past an older snapshot of tail.
> +	 */
> +	head = READ_ONCE(slot->head);
> +	WARN_ON_ONCE(tail - head > pool->nr_objs);

This code can run in NMI?

Calling WARN_ON from NMI sounds quite sketchy - the warning handler
does all sorts of stuff.  I see this is pre-existing but perhaps this
is a chance to address it.

Sashiko liked [1/2] but had a lot to say about the test module:
	https://sashiko.dev/#/patchset/20260928084125.67104-1-jain.sm@gmail.com