Re: [PATCH] RDMA/uverbs: Don't return the shared zero page from rdma_umap_fault()

[Zi Yan]

On Mon Oct 26, 2026 at 7:49 AM EDT, Jason Gunthorpe wrote:
> On Sun, Oct 25, 2026 at 10:04:42PM -0400, Zi Yan wrote:
>> After disassociation, rdma_umap_fault() returns the shared zero page from
>> ->fault for VMAs without VM_WRITE and VM_MAYWRITE. Such VMAs come from
>> rdma_user_mmap_io() in drivers that clear VM_MAYWRITE and they are shared
>> VM_PFNMAP. A page returned from ->fault is mapped as a normal page and goes
>> through rmap code. Two issues:
>> 
>> 1. shared zero page causes VM_WARN_ONs in rmap code,
>> 
>> 2. normal pages in shared VM_PFNMAP VMAs cause accounting leak on arch
>>    without pte_special(), namely the page refcount, mapcount, and vm stats
>>    are never dropped.
>> 
>> Map ufile->disassociate_page for VMAs without VM_WRITE and VM_MAYWRITE,
>> with vmf_insert_pfn(). The page lives until ib_uverbs_release_file(), which
>> cannot run while a VMA holds a reference to the file.
>> 
>> Writable VMAs keep returning disassociate_page in vmf->page, thus its
>> accounting leak on arch without pte_special() needs a separate fix.
>> 
>> Userspace can see two differences after the patch. A mapping without
>> VM_WRITE and VM_MAYWRITE now reads what was written through writable
>> mappings of the same uverbs file instead of zeroes; after disassociation
>> both are dummy data. The first fault now allocates the page and fails with
>> VM_FAULT_SIGBUS if the allocation fails, as a write fault already does.
>> 
>> Fixes: 67f269b37f9b ("RDMA/ucontext: Fix regression with disassociate")
>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>> ---
>> The issue was discovered during PG_folio development when making sure only
>> rmappable folios can enter rmap code path.
>> 
>> Alternative: map the zero page with vmf_insert_pfn_prot()
>> ===
>> 
>> Read-only mappings could keep reading zeroes if rdma_umap_fault()
>> installed the zero page itself as a special PTE:
>> 
>> return vmf_insert_pfn_prot(vmf->vma, vmf->address, zero_pfn(vmf->address));
>> 
>> It does not work today. Commit 7b466529307be ("mm: disallow raw PFN
>> mappings of huge/shared zeropage") makes vmf_insert_pfn_prot() return
>> VM_FAULT_SIGBUS for the zero pfn. vmf_insert_mixed() does not support
>> VM_PFNMAP, since vm_mixed_zeropage_allowed() warns on VM_PFNMAP.
>> 
>> The first version of that patch [1] did not change vmf_insert_pfn_prot().
>> Due to no in-kernel user at that time and complication it can cause,
>> vmf_insert_pfn_prot() rejects zero pages now.
>> 
>> Fix to writable VMA path
>> ===
>> A new vmf_insert_pfn_mkwrite() is needed, since vmf_insert_pfn() has a
>> narrow failing scenario. When a process mapping the disassociated page has
>> soft-dirty tracking enabled, vm_page_prot becomes read-only;
>> vmf_insert_pfn() then uses a read-only PTE instead of a writable one;
>> fixup_user_fault() is used to write to the page (e.g., KVM with the uverbs
>> mapping as guest memory) and fails, because it does not take the second
>> fault to upgrade the read-only PTE to a writable one.
>> 
>> LLM was used for investigation and code development.
>
> Is that why all of the above was so hard to read?

LLM drafted the original long and full of details commit message and
cover letter, I trimmed down as much as I can but kept the necessary details.
Let me know which part is hard to read.

I provided the details because:

1. the original code returns a page on VM_PFNMAP VMAs instead of a
   proper vm_insert*()
2. vmf_insert_pfn_prot() is the proper fix but it has been changed to
   block zero_page, so I would like to know it is worth the effort to
   add complication for this single user.
3. the writable path needs a separate fix, which also requires
   additional MM fixes, a new vmf_insert_pfn_mkwrite() function.

Probably I should have sent it as a RFC instead.

>
> The zero page was used because things like the clock page really need
> to be zero. Reflecting back random things from the disassociate page
> is not going to work out.

Got it. I did not realize that each ufile has only one disassociate page and
the read-only page and writable page can be used in a single ufile.

>
> I don't think rdma supports any architectures without the special bit
> so I'm not sure this is a real issue in practice.

Sounds good.

>
> I guess using vmf_insert_pfn_prot() consistently is the right answer
> since these VMAs are not supposed to be mixed maps.

It requires nontrivial MM changes[1] plus rdma_umap_fault() needs to
handle when mm_forbids_zeropage() is true. Basically, it needs to get a
zero page itself when that happens.

Maybe it is better to add a dedicated zero_disassociate_page for it, to
avoid the fallback handling.

In terms of allowing vmf_insert_pfn_prot() to accept zero_page, I would
like to get inputs from David and Kiril. Or the rule can be no zero_page
for PFNMAP and device needs to manage its own zero page and do the
vm_insert*().


[1] https://lore.kernel.org/all/c0979ad4-9fec-49f1-8442-d90061b79504@xxxxxxxxxx/

-- 
Best Regards,
Yan, Zi