Re: [PATCH v2] drm/virtio: do not enforce blob_alignment in cross-domain contexts

[Dmitry Osipenko <dmitry.osipenko@collabora.com>]

On 10/8/26 23:59, Val Packett wrote:
> 
> On 10/8/26 4:33 AM, Dmitry Osipenko wrote:
>> On 10/6/26 18:42, Val Packett wrote:
>>> VIRTIO_GPU_F_BLOB_ALIGNMENT is designed for GPU rendering context types,
>>> however VIRTIO_GPU_CAPSET_CROSS_DOMAIN is not one of those. Resources
>>> created under cross-domain contexts are arbitrary shared system memory
>>> files, which includes unusual things like Wayland keymaps received
>>> from the host compositor. Do not enforce alignment requirements there.
>>>
>>> Fixes: 47248e0d8264 ("drm/virtio: honor blob_alignment requirements")
>>> Signed-off-by: Val Packett <val@invisiblethingslab.com>
>>> ---
>>>
>>> v2: also check has_virgl_3d to prevent dereferencing NULL vfpriv
>>> v1: https://lore.kernel.org/all/20261006004044.2242154-1-
>>> val@invisiblethingslab.com/
>>>
>>> ~val
>>>
>>> ---
>>>   drivers/gpu/drm/virtio/virtgpu_ioctl.c | 4 +++-
>>>   1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/
>>> drm/virtio/virtgpu_ioctl.c
>>> index 3ddd5481d6bc..4d5ae062834d 100644
>>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>> @@ -551,7 +551,9 @@ static int verify_blob(struct virtio_gpu_device
>>> *vgdev,
>>>       params->blob_flags = rc_blob->blob_flags;
>>>       params->blob_hints = rc_blob->blob_hints;
>>>   -    if (vgdev->has_blob_alignment &&
>>> +    if (vgdev->has_blob_alignment && vgdev->has_virgl_3d &&
>>> +        (vfpriv->context_init &
>>> VIRTIO_GPU_CONTEXT_INIT_CAPSET_ID_MASK) !=
>>> +        VIRTIO_GPU_CAPSET_CROSS_DOMAIN &&
>>>           !IS_ALIGNED(params->size, vgdev->blob_alignment))
>>>           return -EINVAL;
>>>   
>> What userspace has this problem and why it re-purposes virtio-gpu to act
>> as udmabuf for arbitrary data that never reaches host?
> 
> Sorry, what do you mean by that?
> 
> This is not related to data that "never reaches host" at all! The error
> I referred to in the patch message literally says "received *from* the
> host"…??
> 
> Guest userspace is any of the cross-domain proxies:
> 
> * https://codeberg.org/drakulix/wl-cross-domain-proxy
> * https://github.com/talex5/wayland-proxy-virtwl
> * https://github.com/google/sommelier-rs (or old C++ version)
> 
> VIRTIO_GPU_CAPSET_CROSS_DOMAIN "repurposes virtio-gpu" to act as a
> cross-VM UNIX socket transport that supports fd passing. By referring to
> virtio-gpu resources in the send command the guest can make the device
> send the resources' corresponding host fds to the host Wayland
> compositor. It can refer to resources from other contexts, in order to
> present drm/venus/etc rendered buffers on Wayland surfaces. (i.e. the
> host would send the actual dma-buf from virglrenderer) But the resources
> created on the cross-domain context itself are system memory buffers,
> generally used for software rendering and miscellaneous Wayland "stuff"
> like keymaps which are sent *from* the host, and exposed to Wayland
> clients in the guest. Socket receive handling in rutabaga-gfx stores
> incoming fds and returns their IDs to the guest, then the guest does a
> CREATE_BLOB with matching blob_id, which creates a virtio-gpu resource
> from the incoming buffer, so the guest can just MAP_BLOB it. These are
> the blobs that are not aligned to anything because they're just
> arbitrary memfds on the host.

Blob is a chunk of memory intended to be shared between host and guest.

The shared mappable buffer size must be aligned to a page size
granularity that satisfies both host and guest requirement.

If I'm now reading correctly, you're saying that here blob size is
arbitrary payload data size, even not the the aligned size of memory
blob that contains payload. And userspace code implicitly relies on
kernel to round-up payload size to a guest's page size.

We now have blob_alignment and verifying the unaligned params->size.
Given the userspace dependency on the kernel's implicit round-up, we
should be checking blob_alignment of the rounded-up params->size:

+	if (*guest_blob)
+		params->size = roundup(params->size, PAGE_SIZE);

	if (vgdev->has_blob_alignment &&
	    !IS_ALIGNED(params->size, vgdev->blob_alignment))
		return -EINVAL;

Does it work for you?

-- 
Best regards,
Dmitry