Re: [PATCH v3] mm/gup: honour FOLL_PIN in NOMMU __get_user_pages_locked()

[Lance Yang]

On Sat, Sep 19, 2026 at 12:04:36PM +0200, David Hildenbrand (arm) wrote:
>From: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>
>The !CONFIG_MMU implementation of __get_user_pages_locked() takes a bare
>get_page() reference for each page regardless of foll_flags:
>	if (pages[i])
>		get_page(pages[i]);
>
>This is reached from pin_user_pages*() with FOLL_PIN set.
>unpin_user_page() is shared between MMU and NOMMU configurations and
>unconditionally calls gup_put_folio(..., FOLL_PIN), which subtracts
>GUP_PIN_COUNTING_BIAS (1024) from the folio refcount.
>
>This means that pin adds 1, and then unpin will subtract 1024.
>
>If a user maps a page (refcount 1), registers it 1023 times as an
>io_uring fixed buffer (1023 pin_user_pages calls -> refcount 1024), then
>unregisters: the first unpin_user_page subtracts 1024, refcount hits 0,
>the page is freed and returned to the buddy allocator.  The remaining
>1022 unpins write into whatever was reallocated, and the user's VMA
>still maps the freed page (NOMMU has no MMU to invalidate it).
>Reallocating the page for an io_uring pbuf_ring then lets userspace
>corrupt the new owner's data through the stale mapping.
>
>Use try_grab_folio() which adds GUP_PIN_COUNTING_BIAS for FOLL_PIN and 1
>for FOLL_GET, mirroring the CONFIG_MMU path so pin and unpin are
>symmetric. Keep supporting the traditional behavior where users specify
>a pages array but don't set FOLL_GET.
>
>While at it, don't return NULL pointers in the page array,
>as this is really not expected for GUP users; instead, just fail and return
>-EFAULT.
>
>[ david: support traditional behavior with no FOLL_GET, extend
>  description ]
>
>Cc: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
>Cc: David Hildenbrand <david@xxxxxxxxxx>
>Cc: Jason Gunthorpe <jgg@xxxxxxxx>
>Cc: John Hubbard <jhubbard@xxxxxxxxxx>
>Cc: Peter Xu <peterx@xxxxxxxxxx>
>Reported-by: Anthropic
>Fixes: 3faa52c03f44 ("mm/gup: track FOLL_PIN pages")
>Assisted-by: gkh_clanker_t1000
>Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
>Signed-off-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>
>---
>I added a best-guess fixes tag. Given that this has been around for a
>while ... and it's nommu, I think this is fine just going into the next
>merge window. In that case, I will pick this up myself. If we want this
>as a hotfix, Andrew please pick it up.
>---
> mm/gup.c | 17 ++++++++++++++---
> 1 file changed, 14 insertions(+), 3 deletions(-)
>
>diff --git a/mm/gup.c b/mm/gup.c
>index eb898ea1ee22e..1681bea8eed7e 100644
>--- a/mm/gup.c
>+++ b/mm/gup.c
>@@ -1659,6 +1659,10 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm,
> 	if (!nr_pages)
> 		return 0;
> 
>+	/* See the MMU variant: support the traditional behavior. */
>+	if (pages && !(flags & FOLL_PIN))
>+		flags |= FOLL_GET;
>+

Wait ... isn't this the wrong __get_user_pages_locked()? The MMU version
already does this a few lines below ...

Shouldn't the NOMMU version get the check instead, using foll_flags?

---8<---
diff --git a/mm/gup.c b/mm/gup.c
index 2ac04f88d244..dea61daa10fc 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -1659,10 +1659,6 @@ static __always_inline long __get_user_pages_locked(struct mm_struct *mm,
 	if (!nr_pages)
 		return 0;

-	/* See the MMU variant: support the traditional behavior. */
-	if (pages && !(flags & FOLL_PIN))
-		flags |= FOLL_GET;
-
 	/*
 	 * The internal caller expects GUP to manage the lock internally and the
 	 * lock must be released when this returns.
@@ -1997,6 +1993,10 @@ static long __get_user_pages_locked(struct mm_struct *mm, unsigned long start,
 	if (!nr_pages)
 		return 0;

+	/* See the MMU variant: support the traditional behavior. */
+	if (pages && !(foll_flags & FOLL_PIN))
+		foll_flags |= FOLL_GET;
+
 	/*
 	 * The internal caller expects GUP to manage the lock internally and the
 	 * lock must be released when this returns.
---

Cheers, Lance