[PATCH] vduse: reject non-coherent iotlb entries in the coherent fault path

[PATCH] vduse: reject non-coherent iotlb entries in the coherent fault path

[Yu Zhang]

vduse_domain_get_coherent_page() turns an iotlb entry into a struct page
with pfn_to_page((map->addr + iova - map->start) >> PAGE_SHIFT) and takes
a reference on it, with no check that map->addr is a physical address.

That assumption only holds for entries created by the domain's own
coherent allocator (vduse_domain_alloc_coherent(), which stores
virt_to_phys()). VDUSE registers its vdpa device with use_va=true, so for
a use_va device vhost_vdpa_va_map() passes a userspace virtual address as
the map address, and vduse_domain_set_map() copies those entries verbatim
into the same domain->iotlb. Both kinds of entry share one tree, and the
mmap fault handler only splits them by iova < bounce_size, so a use_va
entry placed above bounce_size lands in the coherent path.

A userspace daemon can exploit this: bind the device to virtio_vdpa so
the coherent allocator mints an entry whose backing file is the domain's
anon inode, obtain the domain fd with VDUSE_IOTLB_GET_FD2, rebind to
vhost_vdpa without deleting the vdpa device (vdpa_dev_del() only calls
_vdpa_unregister_device(), so the domain and the fd survive), install a
VHOST_IOTLB_UPDATE over the same iova with an arbitrary uaddr, then fault
the domain mapping. The kernel computes pfn = uaddr >> PAGE_SHIFT from
the userspace value and hands the daemon a struct page for a pfn of its
choosing -- an out-of-bounds vmemmap access for a wild value:

  BUG: unable to handle page fault for address: ffffea8000000008
  RIP: vduse_domain_mmap_fault+0x100/0x330

or, for a value that resolves to a valid pfn, a read/write window onto an
arbitrary physical page (confirmed: a write through the domain mapping
lands on a page selected purely by the fabricated pfn).

Coherent entries are always registered with domain->file as their backing
file, while entries installed through a vhost IOTLB message carry the
mapped vma's file (vhost_vdpa_va_map() stores get_file(vma->vm_file)).
Reject any entry whose file is not domain->file before calling
pfn_to_page(); the fault then returns VM_FAULT_SIGBUS. Legitimate coherent
mappings are unaffected.

Fixes: 8c773d53fb7b ("vduse: Implement an MMU-based software IOTLB")
Signed-off-by: Yu Zhang <yuz08559@gmail.com>
---
 drivers/vdpa/vdpa_user/iova_domain.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/drivers/vdpa/vdpa_user/iova_domain.c b/drivers/vdpa/vdpa_user/iova_domain.c
index 4dc76c0..847f010 100644
--- a/drivers/vdpa/vdpa_user/iova_domain.c
+++ b/drivers/vdpa/vdpa_user/iova_domain.c
@@ -221,6 +221,7 @@ vduse_domain_get_coherent_page(struct vduse_iova_domain *domain, u64 iova)
 	u64 start = iova & PAGE_MASK;
 	u64 last = start + PAGE_SIZE - 1;
 	struct vhost_iotlb_map *map;
+	struct vdpa_map_file *map_file;
 	struct page *page = NULL;
 
 	spin_lock(&domain->iotlb_lock);
@@ -228,6 +229,17 @@ vduse_domain_get_coherent_page(struct vduse_iova_domain *domain, u64 iova)
 	if (!map)
 		goto out;
 
+	/*
+	 * Only coherent allocations made by this domain are backed by a real
+	 * struct page here: their map->addr is a physical address and their
+	 * backing file is the domain's own anon inode. Entries installed via a
+	 * vhost IOTLB message on a use_va device instead carry a userspace
+	 * virtual address in map->addr, and must never be fed to pfn_to_page().
+	 */
+	map_file = (struct vdpa_map_file *)map->opaque;
+	if (map_file->file != domain->file)
+		goto out;
+
 	page = pfn_to_page((map->addr + iova - map->start) >> PAGE_SHIFT);
 	get_page(page);
 out:
-- 
2.43.0

Re: [PATCH] vduse: reject non-coherent iotlb entries in the coherent fault path

[sashiko-bot]

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

Pre-existing issues:
- [Critical] This problem wasn't introduced by this patch, but the patch fails to completely mitigate the pre-existing arbitrary physical memory read/write vulnerability because an attacker can map the VDUSE domain file descriptor and use it to trivially bypass the new backing file check.
--

--- Patch [1]: [PATCH] vduse: reject non-coherent iotlb entries in the coherent fault path ---
commit f4c50be37a9ca120f63ebae4dfab39a075d198c4
Author: Yu Zhang <yuz08559@gmail.com>

vduse: reject non-coherent iotlb entries in the coherent fault path

This commit attempts to fix a vulnerability where non-coherent IOTLB entries
are treated as physical addresses in the coherent fault path. It introduces a
check to ensure the backing file of the IOTLB entry matches the domain's file,
assuming only valid coherent mappings use this file.

> diff --git a/drivers/vdpa/vdpa_user/iova_domain.c b/drivers/vdpa/vdpa_user/iova_domain.c
> index b6c958224b7ce..aee0ae1c408e0 100644
> --- a/drivers/vdpa/vdpa_user/iova_domain.c
> +++ b/drivers/vdpa/vdpa_user/iova_domain.c

[ ... ]

> @@ -234,6 +235,17 @@ vduse_domain_get_coherent_page(struct vduse_iova_domain *domain, u64 iova)
>  	if (!map)
>  		goto out;
>  
> +	/*
> +	 * Only coherent allocations made by this domain are backed by a real
> +	 * struct page here: their map->addr is a physical address and their
> +	 * backing file is the domain's own anon inode. Entries installed via a
> +	 * vhost IOTLB message on a use_va device instead carry a userspace
> +	 * virtual address in map->addr, and must never be fed to pfn_to_page().
> +	 */
> +	map_file = (struct vdpa_map_file *)map->opaque;
> +	if (map_file->file != domain->file)
> +		goto out;

[Severity: Critical]
This is a pre-existing issue, but does this check completely mitigate the
vulnerability?

An attacker can obtain the domain->file descriptor via the VDUSE_IOTLB_GET_FD2
ioctl and mmap() it into their address space. If they use this new virtual
address in a VHOST_IOTLB_UPDATE message, vhost_vdpa_va_map() will assign this
same domain->file to the mapping's vm_file.

When the attacker triggers a page fault on this IOVA in
vduse_domain_get_coherent_page(), map_file->file will match domain->file,
successfully passing this check. The kernel will then pass the attacker's
arbitrary virtual address to pfn_to_page(), allowing them to regain arbitrary
physical memory read/write access.

> +
>  	page = pfn_to_page((map->addr + iova - map->start) >> PAGE_SHIFT);
>  	get_page(page);
>  out:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807022657.11318-1-yuz08559@gmail.com?part=1