wayland client errors result in broken wayland forwarding and vm shutdown |
||||
Issue description
The following code can be used to reproduce the problem:
gcc -x c -o test -lwayland-client - << EOF
#include <string.h>
#include <wayland-client-protocol.h>
void global(void* data, struct wl_registry* registry, uint32_t id,
const char* interface, uint32_t version) {
if (!strcmp(interface, "wl_compositor"))
wl_registry_bind(registry, id, &wl_compositor_interface, version + 1);
}
void remove(void* data, struct wl_registry* registry, uint32_t name) {}
int main() {
struct wl_display* display = wl_display_connect(0);
struct wl_registry* registry = wl_display_get_registry(display);
struct wl_registry_listener listener = {global, remove};
wl_registry_add_listener(registry, &listener, 0);
wl_display_roundtrip(display);
while (wl_display_dispatch(display) != -1)
continue;
return 0;
}
EOF
Running that should output:
$ ./test
wl_registry@2: error 0: invalid version for global wl_compositor (1): have 3, wanted 4
Nothing wrong with that of course but connections to wayland-0 socket after that fail and VM will shutdown after about 5 minutes.
,
Feb 13 2018
,
Mar 22 2018
Some more details. After sending a request that causes the compositor (wayland server in chrome) to generate an error and close the connection. The next VIRTWL_IOCTL_RECV ioctl never returns and makes the process consume 100%.
,
Mar 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/crosvm/+/5fc80ebd71eacdba5ad23f5eadfc086edda7d827 commit 5fc80ebd71eacdba5ad23f5eadfc086edda7d827 Author: Zach Reizner <zachr@google.com> Date: Thu Mar 22 12:14:49 2018 wl: avoid inserting empty virtio queue entries The kernel driver currently short circuits the check for empty queue entries if the entry arrives empty. Ordinarily the check is run every time data is taken out of a queue entry and would recycle the entry once empty. The short circuiting is being fixed in the kernel, but this device change fixes the unnecessary empty queue entries from happening in the first place. BUG= chromium:791724 TEST=test code from the BUG Change-Id: I5b72aac843def052bfe1234dfbde236274ae02bb Reviewed-on: https://chromium-review.googlesource.com/974883 Commit-Ready: Zach Reizner <zachr@chromium.org> Tested-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Stephen Barber <smbarber@chromium.org> [modify] https://crrev.com/5fc80ebd71eacdba5ad23f5eadfc086edda7d827/devices/src/virtio/wl.rs
,
Mar 22 2018
,
Mar 23 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/kernel/+/c763b52478c1bbf10d431130aa4343ec58657e44 commit c763b52478c1bbf10d431130aa4343ec58657e44 Author: Zach Reizner <zachr@google.com> Date: Fri Mar 23 01:17:01 2018 CHROMIUM: virtwl: properly free qentries that are empty If a qentry was received from the device empty, and an ioctl recv is performed on the qentry, the vfd_qentry_free_if_empty call would be skipped over. Because the qentry is still part of the vfd, the ioctl recv would assume there is more data to read and repeat the entire proces, causing a hang. The solution in this change is to always call vfd_qentry_free_if_empty even if the entry is empty so that the entry gets removed from the vfd and the ioctl recv can return or enter a wait queue. BUG= chromium:791724 TEST=test code from BUG Change-Id: Ifb744dff6a56ac104b9f0816dee680461824daba Signed-off-by: Zach Reizner <zachr@google.com> Reviewed-on: https://chromium-review.googlesource.com/974894 Commit-Ready: Zach Reizner <zachr@chromium.org> Tested-by: Zach Reizner <zachr@chromium.org> Reviewed-by: Dmitry Torokhov <dtor@chromium.org> [modify] https://crrev.com/c763b52478c1bbf10d431130aa4343ec58657e44/drivers/virtio/virtio_wl.c |
||||
►
Sign in to add a comment |
||||
Comment 1 by reve...@chromium.org
, Dec 4 2017