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 <dverkamp@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1594158
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

1 files 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 {