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

[Zi Yan]

On Mon Oct 26, 2026 at 8:20 AM EDT, David Hildenbrand (Arm) wrote:
> On 10/26/26 03:04, 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>
>> ---

<snip>

>> +	/* Make sure the page is usable, like what __do_fault() does */
>> +	if (PageHWPoison(page)) {
>> +		ret = VM_FAULT_HWPOISON;
>> +		goto out;
>>  	}
>
> Is that really required? memory_failure() complains pretty hard on pages it
> doesn't know how to handle (random kernel memory).
>
> I'd assume that this here is not an ordinary pagecache page that mamoery_failure
> would know how to handle?

You are right. Will remove it.

>
>> +
>> +	/*
>> +	 * VMAs that cannot become writable map the page as a raw pfn, like
>> +	 * the device memory it replaces. The page lives as long as the file.
>> +	 */
>> +	if (!(vmf->vma->vm_flags & (VM_WRITE | VM_MAYWRITE))) {
>> +		ret = vmf_insert_pfn(vmf->vma, vmf->address, page_to_pfn(page));
>> +		goto out;
>> +	}
>> +
>> +	/*
>> +	 * This VMA is forced to always be shared so this doesn't have
>> +	 * to worry about COW.
>> +	 */
>> +	vmf->page = page;
>> +	get_page(page);
>
>
> This split handling is pretty horrible TBH.
>
> In once case we take a reference in the other not?
>
> Why not perform a PFN mapping in any case?

For writable path, a new vmf_insert_pfn_mkwrite() is needed, so it was
not fixed. I can fix that part as well.

The read-only zero page part might need more attention, since
we just prevented vmf_insert_pfn_prot() from using zero_page. Do we want
to add the support for rdma_umap_fault()?

>
> (Does GUP have to work on these things?)

These are VM_PFNMAP mappings, I guess not. But I am not a GUP expert.

-- 
Best Regards,
Yan, Zi