devices: usb: fix ownership of USB file descriptor
As part of moving from RawFd to RawDescriptor and related types
(https://crrev.com/c/2462330), the USB attach code was modified to
accept a MaybeOwnedDescriptor instead of a MaybeOwnedFd. Since
MaybeOwnedDescriptor::Owned contains a SafeDescriptor instead of a File,
and the usb_util Device::new() constructor requires a File, a conversion
is required. However, the patch mentioned above used as_raw_descriptor
and File::from_raw_descriptor to do this conversion, but this leaves
both the SafeDescriptor and the newly-created File assuming ownership of
the USB fd. When the SafeDescriptor went out of scope, the fd would be
closed, causing the fd in the File to be invalid (meaning the USB device
does not function at all).
This would show up in the crosvm logs like this:
[devices/src/usb/host_backend/utils.rs:61] fail to submit transfer IoctlFailed(2151175434, Error(25))
[devices/src/usb/xhci/xhci_transfer.rs:399] backend is already disconnected
To fix this, use into_raw_descriptor rather than as_raw_descriptor to
transfer ownership out of the SafeDescriptor without closing the fd.
BUG=b:174289633
BUG=chromium:1151144
TEST=Attach USB serial device to Crostini and verify /dev/ttyUSB0 exists
Change-Id: Ia1c5f94f69ca31ab211ab9f63f23141b4e774ef4
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2579884
Tested-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Dylan Reid <dgreid@chromium.org>
Reviewed-by: Michael Hoyle <mikehoyle@google.com>
Commit-Queue: Daniel Verkamp <dverkamp@chromium.org>
(cherry picked from commit 2f6d79efdf9210822fcc7866b4846d455ab49442)
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/2584763
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Matthew Warton <mwarton@google.com>
Commit-Queue: Vincent Palatin <vpalatin@chromium.org>
Tested-by: Vincent Palatin <vpalatin@chromium.org>
diff --git a/devices/src/usb/host_backend/host_backend_device_provider.rs b/devices/src/usb/host_backend/host_backend_device_provider.rs
index 0af8bbf..c33092a 100644
--- a/devices/src/usb/host_backend/host_backend_device_provider.rs
+++ b/devices/src/usb/host_backend/host_backend_device_provider.rs
@@ -12,7 +12,9 @@
use crate::utils::AsyncJobQueue;
use crate::utils::{EventHandler, EventLoop, FailHandle};
use base::net::UnixSeqpacket;
-use base::{error, AsRawDescriptor, FromRawDescriptor, RawDescriptor, WatchingEvents};
+use base::{
+ error, AsRawDescriptor, FromRawDescriptor, IntoRawDescriptor, RawDescriptor, WatchingEvents,
+};
use msg_socket::{MsgReceiver, MsgSender, MsgSocket};
use std::collections::HashMap;
use std::mem;
@@ -169,7 +171,7 @@
}
};
- let raw_descriptor = usb_file.as_raw_descriptor();
+ let raw_descriptor = usb_file.into_raw_descriptor();
// Safe as it is valid to have multiple variables accessing the same fd.
let device = match Device::new(unsafe { File::from_raw_descriptor(raw_descriptor) }) {
Ok(d) => d,
