[PATCH 0/2] mm/page_alloc: fixes for free_pages_nolock() on RT/UP

[PATCH 0/2] mm/page_alloc: fixes for free_pages_nolock() on RT/UP

[Brendan Jackman]

First patch was originally part of a different series but Andrew asked
for it to be separate:
https://lore.kernel.org/all/20260710170311.e22bfd21c658e8357ceddeec@linux-foundation.org/

Pre-existing bugs found by Sashiko during review of this other series:
https://lore.kernel.org/all/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com/

I have not reproduced these bugs, and I suspect there is no real-world
user that is affected by them.

Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
Brendan Jackman (2):
      mm/page_alloc: don't spin_trylock() in NMI on UP
      mm/page_alloc: don't spin_trylock() when disallowed in free_one_page()

 mm/page_alloc.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
---
base-commit: 83c85fdab41ef98198bdbd731cebc9a15a185dcc
change-id: 20260714-alloc-nolock-fixes-e30618048b99

Best regards,
--  
Brendan Jackman <jackmanb@google.com>

[PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Brendan Jackman]

As noted in can_spin_trylock(), using this is unsafe in this context.
commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
but missed the free side.

Impact: If BPF programs using these features in NMI (probably tracing)
are present on non-SMP builds this might crash the kernel and is
probably exploitable by local attackers for privilege escalation.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3%40google.com?part=18
Cc: stable@vger.kernel.org
Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 mm/page_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index af63558391345..5f9873dfccc5a 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
 		migratetype = MIGRATE_MOVABLE;
 	}
 
-	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
-		     && (in_nmi() || in_hardirq()))) {
+	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
 		add_page_to_zone_llist(zone, page, order);
 		return;
 	}

-- 
2.54.0

[PATCH 2/2] mm/page_alloc: don't spin_trylock() when disallowed in free_one_page()

[Brendan Jackman]

__free_frozen_pages() checks can_spin_trylock() before entering into the
main free_frozen_page_commit()/free_one_page() path, but before this it
can get to free_one_page() via the !pcp_allowed_order() and
MIGRATE_ISOLATE paths.

The !pcp_allowed_order() path depends on usage by callers so might not
be possible in practice. The MIGRATE_ISOLATE path probably means kernel
crashes and privilege escalation if anyone ever did memory hotplug and
BPF tracing on a PREEMPT_RT or !SMP build.

Cc: stable@vger.kernel.org
Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00%40google.com?part=2
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 mm/page_alloc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 5f9873dfccc5a..46e5ea59c71df 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -1559,7 +1559,7 @@ static void free_one_page(struct zone *zone, struct page *page,
 	unsigned long flags;
 
 	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
-		if (!spin_trylock_irqsave(&zone->lock, flags)) {
+		if (!can_spin_trylock() || !spin_trylock_irqsave(&zone->lock, flags)) {
 			add_page_to_zone_llist(zone, page, order);
 			return;
 		}

-- 
2.54.0

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Vlastimil Babka (SUSE)]

On 7/15/26 11:03, Brendan Jackman wrote:
> As noted in can_spin_trylock(), using this is unsafe in this context.
> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
> but missed the free side.
> 
> Impact: If BPF programs using these features in NMI (probably tracing)
> are present on non-SMP builds this might crash the kernel and is
> probably exploitable by local attackers for privilege escalation.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3%40google.com?part=18

Should be "Closes:"?
(Andrew can fix up locally)

> Cc: stable@vger.kernel.org
> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> Signed-off-by: Brendan Jackman <jackmanb@google.com>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/page_alloc.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index af63558391345..5f9873dfccc5a 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
> -		     && (in_nmi() || in_hardirq()))) {
> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}
> 

Re: [PATCH 2/2] mm/page_alloc: don't spin_trylock() when disallowed in free_one_page()

[Vlastimil Babka (SUSE)]

On 7/15/26 11:03, Brendan Jackman wrote:
> __free_frozen_pages() checks can_spin_trylock() before entering into the
> main free_frozen_page_commit()/free_one_page() path, but before this it
> can get to free_one_page() via the !pcp_allowed_order() and
> MIGRATE_ISOLATE paths.
> 
> The !pcp_allowed_order() path depends on usage by callers so might not
> be possible in practice. The MIGRATE_ISOLATE path probably means kernel
> crashes and privilege escalation if anyone ever did memory hotplug and
> BPF tracing on a PREEMPT_RT or !SMP build.
> 
> Cc: stable@vger.kernel.org
> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00%40google.com?part=2

Closes: also

> Signed-off-by: Brendan Jackman <jackmanb@google.com>

Reviewed-by: Vlastimil Babka (SUSE) <vbabka@kernel.org>

> ---
>  mm/page_alloc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 5f9873dfccc5a..46e5ea59c71df 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -1559,7 +1559,7 @@ static void free_one_page(struct zone *zone, struct page *page,
>  	unsigned long flags;
>  
>  	if (unlikely(fpi_flags & FPI_TRYLOCK)) {
> -		if (!spin_trylock_irqsave(&zone->lock, flags)) {
> +		if (!can_spin_trylock() || !spin_trylock_irqsave(&zone->lock, flags)) {
>  			add_page_to_zone_llist(zone, page, order);
>  			return;
>  		}
> 

Re: [PATCH 0/2] mm/page_alloc: fixes for free_pages_nolock() on RT/UP

[Andrew Morton]

On Wed, 15 Jul 2026 09:03:57 +0000 Brendan Jackman <jackmanb@google.com> wrote:

> First patch was originally part of a different series but Andrew asked
> for it to be separate:
> https://lore.kernel.org/all/20260710170311.e22bfd21c658e8357ceddeec@linux-foundation.org/
> 
> Pre-existing bugs found by Sashiko during review of this other series:
> https://lore.kernel.org/all/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com/
> 
> I have not reproduced these bugs, and I suspect there is no real-world
> user that is affected by them.

Right, thanks.  I'll stage these for the next merge window - no need to
rush these fixes into mainline.

And I staged them ahead of your "mm/page_alloc: rename FPI_TRYLOCK ->
FPI_NOLOCK", to lessen backporting hassles.

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 921 bytes --]



On 7/15/26 6:03 PM, Brendan Jackman wrote:
> As noted in can_spin_trylock(), using this is unsafe in this context.
> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
> but missed the free side.
> 
> Impact: If BPF programs using these features in NMI (probably tracing)
> are present on non-SMP builds this might crash the kernel and is
> probably exploitable by local attackers for privilege escalation.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3%40google.com?part=18
> Cc: stable@vger.kernel.org
> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---

Looks good to me, so:
Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 2/2] mm/page_alloc: don't spin_trylock() when disallowed in free_one_page()

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 985 bytes --]



On 7/15/26 6:03 PM, Brendan Jackman wrote:
> __free_frozen_pages() checks can_spin_trylock() before entering into the
> main free_frozen_page_commit()/free_one_page() path, but before this it
> can get to free_one_page() via the !pcp_allowed_order() and
> MIGRATE_ISOLATE paths.
> 
> The !pcp_allowed_order() path depends on usage by callers so might not
> be possible in practice. The MIGRATE_ISOLATE path probably means kernel
> crashes and privilege escalation if anyone ever did memory hotplug and
> BPF tracing on a PREEMPT_RT or !SMP build.
> 
> Cc: stable@vger.kernel.org
> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00%40google.com?part=2
> Signed-off-by: Brendan Jackman <jackmanb@google.com>
> ---

Looks good to me, so:
Reviewed-by: Harry Yoo (Oracle) <harry@kernel.org>

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 1406 bytes --]



On 7/14/26 6:52 PM, Brendan Jackman wrote:
> On Mon Jul 13, 2026 at 4:15 PM UTC, Harry Yoo wrote:
>> On 7/13/26 11:31 PM, Brendan Jackman wrote:
>>> On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
>>>> On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:
>>>>> As noted in can_spin_trylock(), using this is unsafe in this context.
>>>>> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
>>>>> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
>>>>> but missed the free side.
>>
>> Ouch, do we allow alloc_pages() -> free_pages_nolock()?
>> Didn't notice.
> 
> We don't explicitly disallow that but I'd say it's "forbidden by
> default"...
> 
> But I think that's unrelated? It doesn't mean you can't use
> free_pages_nolock() from NMI, right? (Would be weird to alloc from
> another context and then free in NMI, but I don't think it's "forbidden
> by default" in the way that using unmatched APIs is).

If you can't free pages that are not allocated via alloc_pages_nolock(),
and if alloc_pages_nolock() always fails, you can't really use
free_pages_nolock().

But yeah BPF seems to do that and also the comment says:

/*
 * Can be called while holding raw_spin_lock or from IRQ and NMI for any
 * page type (not only those that came from alloc_pages_nolock)
 */

-- 
Cheers,
Harry / Hyeonggon


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Brendan Jackman]

On Tue Jul 14, 2026 at 9:52 AM UTC, Brendan Jackman wrote:
> On Mon Jul 13, 2026 at 4:15 PM UTC, Harry Yoo wrote:
>>
>>
>> On 7/13/26 11:31 PM, Brendan Jackman wrote:
>>> On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
>>>> On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:
>>>>
>>>>> As noted in can_spin_trylock(), using this is unsafe in this context.
>>>>> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
>>>>> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
>>>>> but missed the free side.
>>
>> Ouch, do we allow alloc_pages() -> free_pages_nolock()?
>> Didn't notice.
>
> We don't explicitly disallow that but I'd say it's "forbidden by
> default"...

Oh, the BPF arena code does it.

>>>> Also, Sashiko might have found yet more pre-existing issues:
>>>> 	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com
>>> 
>>> There are 2 cases here:
>>> 
>>> 1: !pcp_allowed_order() -> This is forbidden by alloc_order_allowed(),
>>>    quite a bad miss from Sashiko IMO.
>>
>> Hmm but alloc_order_allowed() would return true for !pcp_allowed_order()
>> when spinning is allowed, then pages can be freed via
>> free_pages_nolock()?

So yeah Sashiko was right after all.

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Brendan Jackman]

On Mon Jul 13, 2026 at 4:15 PM UTC, Harry Yoo wrote:
>
>
> On 7/13/26 11:31 PM, Brendan Jackman wrote:
>> On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
>>> On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:
>>>
>>>> As noted in can_spin_trylock(), using this is unsafe in this context.
>>>> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
>>>> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
>>>> but missed the free side.
>
> Ouch, do we allow alloc_pages() -> free_pages_nolock()?
> Didn't notice.

We don't explicitly disallow that but I'd say it's "forbidden by
default"...

But I think that's unrelated? It doesn't mean you can't use
free_pages_nolock() from NMI, right? (Would be weird to alloc from
another context and then free in NMI, but I don't think it's "forbidden
by default" in the way that using unmatched APIs is).

>>>> Reported-by: sashiko-bot@kernel.org
>>>> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
>>>> Cc: stable@vger.kernel.org
>>>> Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
>>>
>>> Is this correct?  I'm not seeing anything in that commit which could
>>> have caused this?
>> 
>> Oh yeah I guess it should be:
>> 
>> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
>> 
>> This is confusing coz we have:
>> 
>> A: commit d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
>> B: commit 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
>> ...
>> X: commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from alloc_frozen_pages_nolock() in NMI on UP") 
>> 
>> X is marked as Fixing A, but it was an incomplete fix. I just copy
>> pasted the Fixes tag. But actually I'm now changing the free path that
>> was only introduced in B. 
>> 
>>>> --- a/mm/page_alloc.c
>>>> +++ b/mm/page_alloc.c
>>>> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>>>>  		migratetype = MIGRATE_MOVABLE;
>>>>  	}
>>>>  
>>>> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
>>>> -		     && (in_nmi() || in_hardirq()))) {
>>>> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>>>>  		add_page_to_zone_llist(zone, page, order);
>>>>  		return;
>>>>  	}
>>>
>>> It would be nice to include a description of the userspace impact.  I'm
>>> suspecting that's "none known", but some speculation on what might
>>> happen to someone is appropriate.
>> 
>> Ack. I think if you trigger this bug by accident it will probably crash
>> your machine in extremely confusing ways. If you can trigger it
>> deliberately from unpriv (depends on the rest of the host setup, e.g.
>> what tracing is being used) you can probably use it to get root/ring0.
>> Can mention this in the commit message.
>
> Just noting, it is quite niche because it requires UP, and tracing
> something that can be called in NMI on UP is even nicher.
>
>>> Also, Sashiko might have found yet more pre-existing issues:
>>> 	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com
>> 
>> There are 2 cases here:
>> 
>> 1: !pcp_allowed_order() -> This is forbidden by alloc_order_allowed(),
>>    quite a bad miss from Sashiko IMO.
>
> Hmm but alloc_order_allowed() would return true for !pcp_allowed_order()
> when spinning is allowed, then pages can be freed via
> free_pages_nolock()?

As noted above I think that's forbidden. Now I think about it, I'll
include a patch to document this.

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Harry Yoo]

[-- Attachment #1.1: Type: text/plain, Size: 3090 bytes --]



On 7/13/26 11:31 PM, Brendan Jackman wrote:
> On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
>> On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:
>>
>>> As noted in can_spin_trylock(), using this is unsafe in this context.
>>> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
>>> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
>>> but missed the free side.

Ouch, do we allow alloc_pages() -> free_pages_nolock()?
Didn't notice.

>>> Reported-by: sashiko-bot@kernel.org
>>> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
>>> Cc: stable@vger.kernel.org
>>> Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
>>
>> Is this correct?  I'm not seeing anything in that commit which could
>> have caused this?
> 
> Oh yeah I guess it should be:
> 
> Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> 
> This is confusing coz we have:
> 
> A: commit d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
> B: commit 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
> ...
> X: commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from alloc_frozen_pages_nolock() in NMI on UP") 
> 
> X is marked as Fixing A, but it was an incomplete fix. I just copy
> pasted the Fixes tag. But actually I'm now changing the free path that
> was only introduced in B. 
> 
>>> --- a/mm/page_alloc.c
>>> +++ b/mm/page_alloc.c
>>> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>>>  		migratetype = MIGRATE_MOVABLE;
>>>  	}
>>>  
>>> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
>>> -		     && (in_nmi() || in_hardirq()))) {
>>> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>>>  		add_page_to_zone_llist(zone, page, order);
>>>  		return;
>>>  	}
>>
>> It would be nice to include a description of the userspace impact.  I'm
>> suspecting that's "none known", but some speculation on what might
>> happen to someone is appropriate.
> 
> Ack. I think if you trigger this bug by accident it will probably crash
> your machine in extremely confusing ways. If you can trigger it
> deliberately from unpriv (depends on the rest of the host setup, e.g.
> what tracing is being used) you can probably use it to get root/ring0.
> Can mention this in the commit message.

Just noting, it is quite niche because it requires UP, and tracing
something that can be called in NMI on UP is even nicher.

>> Also, Sashiko might have found yet more pre-existing issues:
>> 	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com
> 
> There are 2 cases here:
> 
> 1: !pcp_allowed_order() -> This is forbidden by alloc_order_allowed(),
>    quite a bad miss from Sashiko IMO.

Hmm but alloc_order_allowed() would return true for !pcp_allowed_order()
when spinning is allowed, then pages can be freed via
free_pages_nolock()?

-- 
Cheers,
Harry / Hyeonggon

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Brendan Jackman]

On Sat Jul 11, 2026 at 12:03 AM UTC, Andrew Morton wrote:
> On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:
>
>> As noted in can_spin_trylock(), using this is unsafe in this context.
>> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
>> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
>> but missed the free side.
>> 
>> Reported-by: sashiko-bot@kernel.org
>> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
>> Cc: stable@vger.kernel.org
>> Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
>
> Is this correct?  I'm not seeing anything in that commit which could
> have caused this?

Oh yeah I guess it should be:

Fixes: 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")

This is confusing coz we have:

A: commit d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
B: commit 8c57b687e8331 ("mm, bpf: Introduce free_pages_nolock()")
...
X: commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from alloc_frozen_pages_nolock() in NMI on UP") 

X is marked as Fixing A, but it was an incomplete fix. I just copy
pasted the Fixes tag. But actually I'm now changing the free path that
was only introduced in B. 

>> --- a/mm/page_alloc.c
>> +++ b/mm/page_alloc.c
>> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>>  		migratetype = MIGRATE_MOVABLE;
>>  	}
>>  
>> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
>> -		     && (in_nmi() || in_hardirq()))) {
>> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>>  		add_page_to_zone_llist(zone, page, order);
>>  		return;
>>  	}
>
> It would be nice to include a description of the userspace impact.  I'm
> suspecting that's "none known", but some speculation on what might
> happen to someone is appropriate.

Ack. I think if you trigger this bug by accident it will probably crash
your machine in extremely confusing ways. If you can trigger it
deliberately from unpriv (depends on the rest of the host setup, e.g.
what tracing is being used) you can probably use it to get root/ring0.
Can mention this in the commit message.

> Also, please let's not combine a cc:stable bugfix with a minor macro
> renaming.  They're very different things and will take quite different
> paths into mainline and -stable kernels.

Ack. I assume by "combine" you mean put them in the same series - let
me know if I misunderstood that. Will separate them for v2.

> Also, Sashiko might have found yet more pre-existing issues:
> 	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com

There are 2 cases here:

1: !pcp_allowed_order() -> This is forbidden by alloc_order_allowed(),
   quite a bad miss from Sashiko IMO. Usually I expect AI to do better
   at spotting this kind of thing, makes me wonder if I'm the dumb one
   here.

2. pageblock is isolated -> Yeah this looks broken to me. 

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Andrew Morton]

On Fri, 10 Jul 2026 10:42:20 +0000 Brendan Jackman <jackmanb@google.com> wrote:

> As noted in can_spin_trylock(), using this is unsafe in this context.
> commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
> alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
> but missed the free side.
> 
> Reported-by: sashiko-bot@kernel.org
> Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
> Cc: stable@vger.kernel.org
> Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")

Is this correct?  I'm not seeing anything in that commit which could
have caused this?

> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
> -		     && (in_nmi() || in_hardirq()))) {
> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}

It would be nice to include a description of the userspace impact.  I'm
suspecting that's "none known", but some speculation on what might
happen to someone is appropriate.

Also, please let's not combine a cc:stable bugfix with a minor macro
renaming.  They're very different things and will take quite different
paths into mainline and -stable kernels.

Also, Sashiko might have found yet more pre-existing issues:
	https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com

Re: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[sashiko-bot]

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

Pre-existing issues:
- [High] The `!can_spin_trylock()` safety check in `__free_frozen_pages()` is bypassed for allocations with order > `pcp_allowed_order()` and for isolated pages, leaving them vulnerable to unsafe lock acquisition and potential memory corruption on UP systems or deadlocks on PREEMPT_RT.
--

--- Patch [1]: [PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP ---
commit af22d8399ea9f942d20dfb24bc4e88e16f5fe2e1
Author: Brendan Jackman <jackmanb@google.com>

mm/page_alloc: don't spin_trylock() in NMI on UP
    
This commit updates the NMI and hardirq safety check in __free_frozen_pages()
to use can_spin_trylock(). It addresses an issue on UP systems where
spin_trylock() is unsafe in NMI context, which was missed on the free path.

Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com

> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 9c97a86da2b9f..5fe1c11f919d7 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
>  		migratetype = MIGRATE_MOVABLE;
>  	}
>  
> -	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
> -		     && (in_nmi() || in_hardirq()))) {
> +	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {

[Severity: High]
This isn't a bug introduced by this patch, but since the goal is to prevent
unsafe lock acquisition, does this check need to be moved earlier in
__free_frozen_pages() in mm/page_alloc.c?

There are early returns earlier in __free_frozen_pages() that bypass this
safety check completely. For example, for large orders:

    if (!pcp_allowed_order(order)) {
        __free_pages_ok(page, order, fpi_flags);
        return;
    }

And for isolated pages:

        if (unlikely(is_migrate_isolate(migratetype))) {
            free_one_page(zone, page, pfn, order, fpi_flags);
            return;
        }

Both of these paths eventually reach free_one_page() (around line 1561), which
unconditionally calls spin_trylock_irqsave() if FPI_TRYLOCK is set:

    if (unlikely(fpi_flags & FPI_TRYLOCK)) {
        if (!spin_trylock_irqsave(&zone->lock, flags)) {
            add_page_to_zone_llist(zone, page, order);
            return;
        }
    }

Could this lead to memory corruption on UP systems or deadlocks on PREEMPT_RT
if a high-order or isolated page is freed in NMI context via
free_frozen_pages_nolock() with FPI_TRYLOCK?

>  		add_page_to_zone_llist(zone, page, order);
>  		return;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-spin-trylock-followup-v1-0-affb5fe5ed00@google.com?part=1

[PATCH 1/2] mm/page_alloc: don't spin_trylock() in NMI on UP

[Brendan Jackman]

As noted in can_spin_trylock(), using this is unsafe in this context.
commit 620b46ed6ae17 ("mm/page_alloc: return NULL early from
alloc_frozen_pages_nolock() in NMI on UP") fixed this on the alloc side
but missed the free side.

Reported-by: sashiko-bot@kernel.org
Link: https://sashiko.dev/#/patchset/20260703-alloc-trylock-v5-0-c87b714e19d3@google.com
Cc: stable@vger.kernel.org
Fixes: d7242af86434 ("mm: Introduce alloc_frozen_pages_nolock()")
Signed-off-by: Brendan Jackman <jackmanb@google.com>
---
 mm/page_alloc.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 9c97a86da2b9f..5fe1c11f919d7 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2979,8 +2979,7 @@ static void __free_frozen_pages(struct page *page, unsigned int order,
 		migratetype = MIGRATE_MOVABLE;
 	}
 
-	if (unlikely((fpi_flags & FPI_TRYLOCK) && IS_ENABLED(CONFIG_PREEMPT_RT)
-		     && (in_nmi() || in_hardirq()))) {
+	if (unlikely((fpi_flags & FPI_TRYLOCK) && !can_spin_trylock())) {
 		add_page_to_zone_llist(zone, page, order);
 		return;
 	}

-- 
2.54.0