On Sat, Aug 16, 2025 at 01:05:21PM +0200, Alyssa Ross wrote: > "Michael S. Tsirkin" <mst@xxxxxxxxxx> writes: > > > On Fri, Aug 15, 2025 at 09:19:34PM +0200, Alyssa Ross wrote: > >> Alyssa Ross <hi@xxxxxxxxx> writes: > >> > >> > On Mon, May 12, 2025 at 07:51:53AM +0930, SamiUddinsami.md.ko@xxxxxxxxx wrote: > >> >> From: Sami Uddin <sami.md.ko@xxxxxxxxx> > >> >> > >> >> 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 <sami.md.ko@xxxxxxxxx> > >> >> --- > >> >> 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@xxxxxxxxxx/ > >> > >> 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. > > So you just weren't aware during the review that it's an output > parameter rather than an input? Should the parameter maybe be renamed > or something to make that more obvious? Sounds like a good idea. -- MST
