Thread (7 messages) 7 messages, 4 authors, 2025-08-16

Re: [REGRESSION] virtio: reject shm region if length is zero

From: "Michael S. Tsirkin" <mst@redhat.com>
Date: 2025-08-16 10:36:25
Also in: lkml, regressions

On Fri, Aug 15, 2025 at 09:19:34PM +0200, Alyssa Ross wrote:
Alyssa Ross [off-list ref] writes:
quoted
On Mon, May 12, 2025 at 07:51:53AM +0930, SamiUddinsami.md.ko@gmail.com wrote:
quoted
From: Sami Uddin <redacted>

Prevent usage of shared memory regions where the length is zero,
as such configurations are not valid and may lead to unexpected behavior.

Signed-off-by: Sami Uddin <redacted>
---
v3:
- Use idiomatic 'if (!region->len)' as suggested by reviewer
v2:
- Fixed coding style issue: added space after 'if' statement

 include/linux/virtio_config.h | 2 ++
 1 file changed, 2 insertions(+)
Hi, I'm sorry to be the bearer of bad news, but since this patch my VM
no longer works.  The system is running wayland-proxy-virtwl[1] inside
a crosvm[2] VM, using crosvm's virtio-gpu device to do cross-domain
Wayland forwarding.

Since this change, wayland-proxy-virtwl crashes with the following log
message:

	wl-proxy [WARNING]: Error handling client: Unix.Unix_error(Unix.EINVAL, "DRM_IOCTL_VIRTGPU_RESOURCE_CREATE_BLOB", "")

I'm pretty confused by what this change was supposed to do in the first
place…  Looking at how virtio_get_shm_region() is used in
virtio_gpu_init(), it's called with a pointer to zeroed memory, and then
the get_shm_region() implementation is supposed to write to the region,
without ever reading from it as far as I can tell.  Why is the initial
value of an out parameter being checked at all?  How does this prevent
using zero-length shared memory regions?

[1]: https://crosvm.dev/
[2]: https://github.com/talex5/wayland-proxy-virtwl

#regzbot introduced: 206cc44588f72b49ad4d7e21a7472ab2a72a83df
Okay, just found that it's already been reverted:
https://lore.kernel.org/all/20250808072533-mutt-send-email-mst@kernel.org/ (local)

Still, I'm confused how this was supposed to fix anything…

#regzbot fix: Revert "virtio: reject shm region if length is zero"


Are you asking why was the patch applied in the 1st place?
It seemed like an invalid behaviour to me, and I thought it's
not too late to block it so we don't need to support it
down the road.

-- 
MST
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help