New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 791724 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Chrome
Pri: 2
Type: Bug



Sign in to add a comment

wayland client errors result in broken wayland forwarding and vm shutdown

Project Member Reported by reve...@chromium.org, Dec 4 2017

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.
 
Description: Show this description
Components: OS>Systems>Containers
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%.
Project Member

Comment 4 by bugdroid1@chromium.org, 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

Comment 5 by za...@chromium.org, Mar 22 2018

Status: Verified (was: Assigned)
Project Member

Comment 6 by bugdroid1@chromium.org, Mar 23 2018

Labels: merge-merged-chromeos-4.14
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