Re: [PATCH v2 2/3] drm/virtio: honor blob_alignment requirements

Dmitry Osipenko <[email protected]> writes:

> On 7/13/26 14:19, Alyssa Ross wrote:
>> Dmitry Osipenko <[email protected]> writes:
>> 
>>> On 7/1/26 14:10, Alyssa Ross wrote:
>>>> On Tue, Apr 28, 2026 at 09:44:49PM +0200, Sergio Lopez wrote:
>>>>> If VIRTIO_GPU_F_BLOB_ALIGNMENT has been negotiated, blob size must be
>>>>> aligned to blob_alignment. Validate this in verify_blob() so that
>>>>> invalid requests are rejected early.
>>>>>
>>>>> Signed-off-by: Sergio Lopez <[email protected]>
>>>>
>>>> FYI: this change breaks crosvm, which is squatting the 5 and 6 values
>>>> of VIRTIO_GPU_F_* with different meanings.  I've reported it as a
>>>> crosvm bug, so hopefully it can be taken care of there.
>>>>
>>>> https://issuetracker.google.com/issues/529852979
>>>>
>>>>> ---
>>>>>  drivers/gpu/drm/virtio/virtgpu_ioctl.c | 5 +++++
>>>>>  1 file changed, 5 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/virtio/virtgpu_ioctl.c b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>>>> index c33c057365f8..d0c4edf1eaf4 100644
>>>>> --- a/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>>>> +++ b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
>>>>> @@ -489,6 +489,11 @@ static int verify_blob(struct virtio_gpu_device *vgdev,
>>>>>  	params->size = rc_blob->size;
>>>>>  	params->blob = true;
>>>>>  	params->blob_flags = rc_blob->blob_flags;
>>>>> +
>>>>> +	if (vgdev->has_blob_alignment &&
>>>>> +	    !IS_ALIGNED(params->size, vgdev->blob_alignment))
>>>>> +		return -EINVAL;
>>>>> +
>>>>>  	return 0;
>>>>>  }
>>>>>
>>>>> --
>>>>> 2.53.0
>>>>>
>>>
>>> Thanks for the report. Indeed, crosvm will need to fix its experimental
>>> caps. CI will likely run into this problem first once it will update
>>> guest to 7.2+ kernel.
>> 
>> I sent
>> https://chromium-review.googlesource.com/c/crosvm/crosvm/+/8064741 last
>> week but still waiting to hear anything.  Do you have any insight into
>> whether it's still used / being pursued?
>
> The GUEST_HANDLE should be used by Android [1]. The FENCE_PASSING
> shouldn't be used in production, though don't know for sure.
>
> Crosvm should bump its experimental defines by +10 to prevent clash with
> upstream in future.
>
> [1]
> https://github.com/google/gfxstream/blob/main/host/vulkan/vk_decoder_global_state.cpp#L107

Thanks for the extra info. :) They've agreed to go ahead and remove both
of them, so the problem is solved.

-----BEGIN PGP SIGNATURE-----

iHUEARYKAB0WIQRV/neXydHjZma5XLJbRZGEIw/wogUCalddYAAKCRBbRZGEIw/w
orGgAQC/YXYu4X+sdCLEkVpxdPV5ypPiF82l9tUZ4Wd+kuwJqQEA3aMqqCYrmTX3
e4gNsI1wlXn3W9wPHQe7CSogvsTvPQM=
=xZq0
-----END PGP SIGNATURE-----