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

David Hildenbrand (Arm)
2026-04-24T12:16:24Z

On 4/24/26 13:31, Greg Kroah-Hartman wrote:
> On Thu, Apr 23, 2026 at 05:55:56PM +0200, David Hildenbrand (Arm) wrote:
>> On 4/23/26 16:28, Greg Kroah-Hartman wrote:
>>> 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.
>>>
>>> Cc: Andrew Morton <akpm@linux-foundation.org>
>>> Cc: David Hildenbrand <david@kernel.org>
>>> Cc: Jason Gunthorpe <jgg@ziepe.ca>
>>> Cc: John Hubbard <jhubbard@nvidia.com>
>>> Cc: Peter Xu <peterx@redhat.com>
>>> Reported-by: Anthropic
>>> Assisted-by: gkh_clanker_t1000
>>
>> Assisted-by: David :(
>>
>> (no, I'm not a tool! :) )
>
> True, sorry, I guess people can "assist", I should have added that.
>
> If Andrew's tools automatically pick this up then:
>
> Assisted-by: David Hildenbrand <david@kernel.org>

I think we usually use Suggested-by:, but really I was just joking :)

>
>>> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
>>> ---
>>> v2: - drop huge comment
>>> - rework error return value based on David's suggestion (heck,
>>> pretty much the full patch was written by him now)
>>> Link to v1: https://lore.kernel.org/r/2026042334-acutely-unadorned-e05c@gregkh
>>>
>>> mm/gup.c | 13 ++++++++++---
>>> 1 file changed, 10 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/mm/gup.c b/mm/gup.c
>>> index ad9ded39609c..2f6f95a167af 100644
>>> --- a/mm/gup.c
>>> +++ b/mm/gup.c
>>> @@ -1983,6 +1983,7 @@ static long __get_user_pages_locked(struct mm_struct *mm, unsigned long start,
>>> struct vm_area_struct *vma;
>>> bool must_unlock = false;
>>> vm_flags_t vm_flags;
>>> + int ret, err = -EFAULT;
>>> long i;
>>>
>>> if (!nr_pages)
>>> @@ -2019,8 +2020,14 @@ static long __get_user_pages_locked(struct mm_struct *mm, unsigned long start,
>>>
>>> if (pages) {
>>> pages[i] = virt_to_page((void *)start);
>>> - if (pages[i])
>>> - get_page(pages[i]);
>>> + if (!pages[i])
>>> + break;
>>
>> Best to mention that change in the patch description. I really think this is the
>> right thing to do (returning NULL in the page array is just very dubious).
>
> Ick, I see Andrew already grabbed this so I'll just leave it for now,
> thanks for the help and review!

Andrew, can you add "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."? Thanks!

--
Cheers,

David