From f2346c1b5f30bb3ddf72ade48034f72ea48d2d6e Mon Sep 17 00:00:00 2001 From: Daniel Verkamp Date: Fri, 3 May 2019 09:40:48 -0700 Subject: usb: fix deadlock in try_detach removal path The UsbHub::try_detach function, which is only called from the libusb hotplug callback when devices are removed, calls into port.get_backend_device(), which acquires the backend_device lock, and then calls port.detach(), which also tries to acquire the backend_device lock, while still holding onto the backend_device. This clearly leads to a deadlock. Add an extra block in try_detach to keep the backend_device lock scoped so that it is dropped before calling port.detach(). BUG=chromium:958117 TEST=Hot remove usb device from Crostini; verify it is gone in lsusb Change-Id: Ibbbf68623390e3d25576f544b815c2a5607934fc Signed-off-by: Daniel Verkamp Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1594158 Tested-by: kokoro Reviewed-by: Dylan Reid Reviewed-by: Zach Reizner --- devices/src/usb/xhci/usb_hub.rs | 30 +++++++++++++++++++----------- 1 file changed, 19 insertions(+), 11 deletions(-) diff --git a/devices/src/usb/xhci/usb_hub.rs b/devices/src/usb/xhci/usb_hub.rs index ae4f2f8..28c7f19 100644 --- a/devices/src/usb/xhci/usb_hub.rs +++ b/devices/src/usb/xhci/usb_hub.rs @@ -208,20 +208,28 @@ impl UsbHub { /// Try to detach device of bus, addr, vid, pid pub fn try_detach(&self, bus: u8, addr: u8, vid: u16, pid: u16) -> Result<()> { for port in &self.ports { - let backend_device = port.get_backend_device(); + // This block exists so that we only hold the backend device + // lock while checking the address. It needs to be dropped before + // calling port.detach(), because that acquires the backend + // device lock again. + { + let backend_device = port.get_backend_device(); - let d = match backend_device.as_ref() { - None => continue, - Some(d) => d, - }; + let d = match backend_device.as_ref() { + None => continue, + Some(d) => d, + }; - if d.host_bus() == bus - && d.host_address() == addr - && d.get_vid() == vid - && d.get_pid() == pid - { - return port.detach(); + if d.host_bus() != bus + || d.host_address() != addr + || d.get_vid() != vid + || d.get_pid() != pid + { + continue; + } } + + return port.detach(); } Err(Error::NoSuchDevice { -- cgit 1.4.1