Re: [PATCH v7 3/4] drm/virtio: implement userptr support for zero-copy memory access

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] mm: silent data loss due to missing dirty tracking on unpin
- [Critical] virtgpu: privilege escalation via read-write PRIME export of read-only userptrs
- [High] virtgpu: missing DMA sync for userptr objects causes stale reads
--

commit 419406d5baa93528ec030b7b95cc7d5ada190751
Author: Honglei Huang <honghuan@amd.com>

drm/virtio: implement userptr support for zero-copy memory access

This commit implements userptr support for virtio-gpu by adding a new
virtio_gpu_object_userptr type. It pins user pages via pin_user_pages_fast()
and maps them via the DMA API when required.

> diff --git a/drivers/gpu/drm/virtio/virtgpu_userptr.c b/drivers/gpu/drm/virtio/virtgpu_userptr.c
> new file mode 100644
> index 0000000000000..009755489774c
> --- /dev/null
> +++ b/drivers/gpu/drm/virtio/virtgpu_userptr.c
> @@ -0,0 +1,335 @@
[ ... ]
> +static const struct drm_gem_object_funcs virtio_gpu_userptr_funcs = {
> +	.open = virtio_gpu_gem_object_open,
> +	.close = virtio_gpu_gem_object_close,
> +	.free = virtio_gpu_userptr_free,
> +	.export = virtgpu_gem_prime_export,
> +	.get_sg_table = virtio_gpu_userptr_get_sg_table,
> +};

[Severity: Critical]
Is it safe to expose these objects to the PRIME export subsystem without
verifying their read-only status?

If a userptr object is created with VIRTGPU_BLOB_FLAG_USERPTR_RDONLY to pin
read-only memory, userspace could potentially export it via PRIME with
O_RDWR permissions and import it into another device that performs DMA writes.
Could this allow hardware to bypass CPU page table protections and overwrite
read-only system memory or files?

[ ... ]
> +static void
> +virtio_gpu_userptr_put_pages(struct virtio_gpu_object_userptr *userptr)
> +{
[ ... ]
> +	if (userptr->pages) {
> +		unpin_user_pages(userptr->pages, userptr->npages);
> +		kvfree(userptr->pages);
> +		userptr->pages = NULL;
> +	}

[Severity: Critical]
Are we missing dirty tracking when releasing these pages back to the OS?

If a userptr object is created without the read-only flag, pinned with
FOLL_WRITE, written to by the device, and then destroyed, we call
unpin_user_pages() here without explicitly marking the pages as dirty. If the
pages are file-backed or swapped, could the kernel memory management subsystem
silently discard the data written by the host or device?

[ ... ]
> +int virtio_gpu_userptr_create(struct virtio_gpu_device *vgdev,
> +			      struct drm_file *file,
> +			      struct virtio_gpu_object_params *params,
> +			      struct virtio_gpu_object **bo_ptr)
> +{
[ ... ]
> +	/*
> +	 * Match shmem blobs: only DMA-map when the virtio DMA API is in
> +	 * use. Mapping unconditionally can create SWIOTLB bounce buffers
> +	 * that get copied back over guest pages on unmap even though the
> +	 * host was given sg_phys() addresses.
> +	 */
> +	if (virtio_gpu_use_dma_api(vgdev->vdev)) {
> +		enum dma_data_direction dir =
> +			(userptr->flags & VIRTGPU_BLOB_FLAG_USERPTR_RDONLY) ?
> +			DMA_TO_DEVICE : DMA_BIDIRECTIONAL;
> +
> +		ret = dma_map_sgtable(drm_dev_dma_dev(vgdev->ddev), sgt,
> +				      dir, 0);

[Severity: High]
How does this DMA-mapped memory get synchronized to avoid stale reads?

When mapping with DMA_BIDIRECTIONAL and the guest modifies the userptr memory,
the host might read stale data from bounce buffers unless a DMA sync occurs. 
However, in virtio_gpu_cmd_transfer_to_host_3d() and 2d transfers, the sync
check looks like this:

	if (virtio_gpu_is_shmem(bo) && use_dma_api)
		dma_sync_sgtable_for_device(vgdev->vdev->dev.parent,
					    bo->base.sgt, DMA_TO_DEVICE);

Since virtio_gpu_is_shmem() evaluates to false for userptr objects, they
appear to be excluded from this synchronization. Will this skip cause the
host to read stale data during transfers?

> +		if (ret)
> +			goto err_cleanup;
> +
> +		userptr->dma_dir = dir;
> +		userptr->dma_mapped = true;
> +	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918021154.1432154-1-honghuan@amd.com?part=3