Re: [PATCH v5 18/18] mm: factor out can_spin_trylock()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Unsafe trylock from NMI on UP systems due to missed deduplication in `free_unref_page_commit()`
--

commit b53c6ec11ab7f82add433847a77e54a590090942
Author: Brendan Jackman <jackmanb@google.com>

mm: factor out can_spin_trylock()

Deduplicate checks for whether the current context is safe for
spin_trylock().

Link: https://lore.kernel.org/all/397859cb-b127-4cc6-9c71-044afc99bf0c@kernel.org/

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index c3b246e67ed14..a63733dac659e 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c

[ ... ]

>  static inline bool alloc_nolock_allowed(void)
>  {

[ ... ]

> -	if (IS_ENABLED(CONFIG_PREEMPT_RT) && (in_nmi() || in_hardirq()))
> -		return false;
> -
> -	/* On UP, spin_trylock() always succeeds even when it is locked */
> -	if (!IS_ENABLED(CONFIG_SMP) && in_nmi())
> +	if (!can_spin_trylock())
>  		return false;

[Severity: High]
This is a pre-existing issue, but could this deduplication also be applied to
free_unref_page_commit() in mm/page_alloc.c?

It looks like free_unref_page_commit() still uses an open-coded check:

	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
		     && (in_nmi() || in_hardirq()))) {
		add_page_to_zone_llist(zone, page, order);
		return;
	}

Since the open-coded check in free_unref_page_commit() doesn't check for UP
systems, would an asynchronous page free executed from an NMI context on a
uniprocessor system (e.g., via a BPF memory allocator) incorrectly proceed past
this check?

On UP systems, spin_trylock_irqsave() only disables interrupts and
unconditionally returns true, providing no mutual exclusion against the
interrupted context. Could this allow both the NMI and the interrupted process
to concurrently modify the zone list structures and corrupt memory?

Applying !can_spin_trylock() here seems like it would fulfill the intent of
this patch and resolve the UP + NMI case.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com?part=18