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

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 <[email protected]>

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/[email protected]?part=1