lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20201119100508.483c6503@w520.home>
Date:   Thu, 19 Nov 2020 10:05:08 -0700
From:   Alex Williamson <alex.williamson@...hat.com>
To:     Jia He <justin.he@....com>
Cc:     Cornelia Huck <cohuck@...hat.com>, kvm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] vfio iommu type1: Bypass the vma permission check in
 vfio_pin_pages_remote()

On Thu, 19 Nov 2020 22:27:37 +0800
Jia He <justin.he@....com> wrote:

> The permission of vfio iommu is different and incompatible with vma
> permission. If the iotlb->perm is IOMMU_NONE (e.g. qemu side), qemu will
> simply call unmap ioctl() instead of mapping. Hence vfio_dma_map() can't
> map a dma region with NONE permission.
> 
> This corner case will be exposed in coming virtio_fs cache_size
> commit [1]
>  - mmap(NULL, size, PROT_NONE, MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
>    memory_region_init_ram_ptr()
>  - re-mmap the above area with read/write authority.
>  - vfio_dma_map() will be invoked when vfio device is hotplug added.
> 
> qemu:
> vfio_listener_region_add()
> 	vfio_dma_map(..., readonly=false)
> 		map.flags is set to VFIO_DMA_MAP_FLAG_READ|VFIO_..._WRITE
> 		ioctl(VFIO_IOMMU_MAP_DMA)
> 
> kernel:
> vfio_dma_do_map()
> 	vfio_pin_map_dma()
> 		vfio_pin_pages_remote()
> 			vaddr_get_pfn()
> 			...
> 				check_vma_flags() failed! because
> 				vm_flags hasn't VM_WRITE && gup_flags
> 				has FOLL_WRITE
> 
> It will report error in qemu log when hotplug adding(vfio) a nvme disk
> to qemu guest on an Ampere EMAG server:
> "VFIO_MAP_DMA failed: Bad address"

I don't fully understand the argument here, I think this is suggesting
that because QEMU won't call VFIO_IOMMU_MAP_DMA on a region that has
NONE permission, the kernel can ignore read/write permission by using
FOLL_FORCE.  Not only is QEMU not the only userspace driver for vfio,
but regardless of that, we can't trust the behavior of any given
userspace driver.  Bypassing the permission check with FOLL_FORCE seems
like it's placing the trust in the user, which seems like a security
issue.  Thanks,

Alex


> [1] https://gitlab.com/virtio-fs/qemu/-/blob/virtio-fs-dev/hw/virtio/vhost-user-fs.c#L502
> 
> Signed-off-by: Jia He <justin.he@....com>
> ---
>  drivers/vfio/vfio_iommu_type1.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/vfio/vfio_iommu_type1.c b/drivers/vfio/vfio_iommu_type1.c
> index 67e827638995..33faa6b7dbd4 100644
> --- a/drivers/vfio/vfio_iommu_type1.c
> +++ b/drivers/vfio/vfio_iommu_type1.c
> @@ -453,7 +453,8 @@ static int vaddr_get_pfn(struct mm_struct *mm, unsigned long vaddr,
>  		flags |= FOLL_WRITE;
>  
>  	mmap_read_lock(mm);
> -	ret = pin_user_pages_remote(mm, vaddr, 1, flags | FOLL_LONGTERM,
> +	ret = pin_user_pages_remote(mm, vaddr, 1,
> +				    flags | FOLL_LONGTERM | FOLL_FORCE,
>  				    page, NULL, NULL);
>  	if (ret == 1) {
>  		*pfn = page_to_pfn(page[0]);

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ