From cf0d298223af6f30fc1e49a4c2367ae1829fd8d2 Mon Sep 17 00:00:00 2001 From: Zach Reizner Date: Tue, 21 May 2019 17:19:27 -0700 Subject: [PATCH] wl: limit vfd recv size to leave room for VFDs The IN_BUFFER_LEN variable limits the amount of data that will be read into a single page sized descriptor. The old value for it left room for the 16 byte header but reserved no space for VFDs. This happens to work fine if the size of the read data and VFDs did not exceed the buffer size, but, in rare circumstance, the maximum amount of data would be read along with a FD getting received, spilling the descriptor and causing it to fail to write to it. The guest driver does not handle this gracefully and usually panics due to corruption. The new value reserves room for the max number of VFDs so that the descriptors will not end up with too much data. BUG=chromium:951576 TEST=while true; do (/opt/google/cros-containers/bin/wayland_demo&); pkill -f /opt/google/cros-containers/lib/ld-linux-x86-64.so.2; done Change-Id: Ic0c1c10f81a91b5e5cd076e3ded8d3cc0564b614 Reviewed-on: https://chromium-review.googlesource.com/1623558 Commit-Ready: Zach Reizner Tested-by: Zach Reizner Tested-by: kokoro Legacy-Commit-Queue: Commit Bot Reviewed-by: Daniel Verkamp Reviewed-by: Dylan Reid --- devices/src/virtio/wl.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/devices/src/virtio/wl.rs b/devices/src/virtio/wl.rs index 1e99bfaa18..83fcdcacb6 100644 --- a/devices/src/virtio/wl.rs +++ b/devices/src/virtio/wl.rs @@ -110,7 +110,10 @@ const QUEUE_SIZES: &[u16] = &[QUEUE_SIZE, QUEUE_SIZE]; const NEXT_VFD_ID_BASE: u32 = 0x40000000; const VFD_ID_HOST_MASK: u32 = NEXT_VFD_ID_BASE; -const IN_BUFFER_LEN: usize = 4080; +// Each in-vq buffer is one page, so we need to leave space for the control header and the maximum +// number of allocs. +const IN_BUFFER_LEN: usize = + 0x1000 - size_of::() - VIRTWL_SEND_MAX_ALLOCS * size_of::(); #[cfg(feature = "wl-dmabuf")] const VIRTIO_WL_VFD_DMABUF_SYNC_VALID_FLAG_MASK: u32 = 0x7;