devices: vhost-user: fs: keep executor fd inside jail

A previous commit moved the creation of the executor for the vhost-user
device processes before the minijail jail_and_fork() call, and the file
descriptor of the executor was not marked for preservation when entering
the jail, so running the vhost-user-fs device would result in errors
like this:

  [WARNING:cros_async/src/sys/unix/uring_executor.rs:343] Failed to
  submit NOP for waking up executor: Failed to enter io uring: 9

This is because the io_uring context file descriptor was no longer
valid, so uring system calls would return 9 (EBADF).

Add AsRawDescriptors implementations to UringExecutor and FdExecutor so
we can preserve the fd across the minijail fork.

Other vhost-user devices were not affected because they don't enter a
jail; the fs device requires this so it can do uid/gid remapping.

BUG=b:231396155
TEST=crosvm device fs --socket /tmp/vhost.sock --tag test --shared-dir /

Fixes: d368c1520a ("devices: vhost-user: fs: remove global executor variable")
Change-Id: I38cf02034bbc28de2582e2ea92bdc13b1d6055d2
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3628292
Reviewed-by: Keiichi Watanabe <keiichiw@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Lepton Wu <lepton@chromium.org>
This commit is contained in:
Daniel Verkamp 2022-05-04 14:04:34 -07:00 committed by Chromeos LUCI
parent 91c757e648
commit 9ab4f29e7a
4 changed files with 62 additions and 10 deletions

View file

@ -6,6 +6,8 @@ use std::future::Future;
use async_task::Task;
use base::{AsRawDescriptors, RawDescriptor};
use super::{
poll_source::Error as PollError, uring_executor::use_uring, FdExecutor, PollSource,
URingExecutor, UringSource,
@ -352,3 +354,12 @@ impl Executor {
}
}
}
impl AsRawDescriptors for Executor {
fn as_raw_descriptors(&self) -> Vec<RawDescriptor> {
match self {
Executor::Uring(ex) => ex.as_raw_descriptors(),
Executor::Fd(ex) => ex.as_raw_descriptors(),
}
}
}

View file

@ -23,7 +23,10 @@ use std::{
};
use async_task::Task;
use base::{add_fd_flags, warn, AsRawDescriptor, EpollContext, EpollEvents, Event, WatchingEvents};
use base::{
add_fd_flags, warn, AsRawDescriptor, AsRawDescriptors, EpollContext, EpollEvents, Event,
RawDescriptor, WatchingEvents,
};
use futures::task::noop_waker;
use pin_utils::pin_mut;
use remain::sorted;
@ -271,10 +274,17 @@ struct RawExecutor {
blocking_pool: BlockingPool,
state: AtomicI32,
notify: Event,
// Descriptor of the original event that was cloned to create notify.
// This is only needed for the AsRawDescriptors implementation.
notify_dup: RawDescriptor,
}
impl RawExecutor {
fn new(notify: Event) -> Result<Self> {
fn new(notify: &Event) -> Result<Self> {
// Save the original descriptor before cloning. This descriptor will be used when creating
// the notify task, so we need to preserve it for AsRawDescriptors.
let notify_dup = notify.as_raw_descriptor();
let notify = notify.try_clone().map_err(Error::CloneEvent)?;
Ok(RawExecutor {
queue: RunnableQueue::new(),
poll_ctx: EpollContext::new().map_err(Error::CreatingContext)?,
@ -282,6 +292,7 @@ impl RawExecutor {
blocking_pool: Default::default(),
state: AtomicI32::new(PROCESSING),
notify,
notify_dup,
})
}
@ -445,6 +456,16 @@ impl RawExecutor {
}
}
impl AsRawDescriptors for RawExecutor {
fn as_raw_descriptors(&self) -> Vec<RawDescriptor> {
vec![
self.poll_ctx.as_raw_descriptor(),
self.notify.as_raw_descriptor(),
self.notify_dup,
]
}
}
impl WeakWake for RawExecutor {
fn wake_by_ref(weak_self: &Weak<Self>) {
if let Some(arc_self) = weak_self.upgrade() {
@ -494,11 +515,7 @@ pub struct FdExecutor {
impl FdExecutor {
pub fn new() -> Result<FdExecutor> {
let notify = Event::new().map_err(Error::CreateEvent)?;
let raw = notify
.try_clone()
.map_err(Error::CloneEvent)
.and_then(RawExecutor::new)
.map(Arc::new)?;
let raw = RawExecutor::new(&notify).map(Arc::new)?;
raw.spawn(notify_task(notify, Arc::downgrade(&raw)))
.detach();
@ -553,6 +570,12 @@ impl FdExecutor {
}
}
impl AsRawDescriptors for FdExecutor {
fn as_raw_descriptors(&self) -> Vec<RawDescriptor> {
self.raw.as_raw_descriptors()
}
}
// Used to `dup` the FDs passed to the executor so there is a guarantee they aren't closed while
// waiting in TLS to be added to the main polling context.
unsafe fn dup_fd(fd: RawFd) -> Result<RawFd> {

View file

@ -73,7 +73,7 @@ use std::{
};
use async_task::Task;
use base::{warn, AsRawDescriptor, WatchingEvents};
use base::{warn, AsRawDescriptor, RawDescriptor, WatchingEvents};
use futures::task::noop_waker;
use io_uring::URingContext;
use once_cell::sync::Lazy;
@ -726,6 +726,12 @@ impl RawExecutor {
}
}
impl AsRawDescriptor for RawExecutor {
fn as_raw_descriptor(&self) -> RawDescriptor {
self.ctx.as_raw_descriptor()
}
}
impl WeakWake for RawExecutor {
fn wake_by_ref(weak_self: &Weak<Self>) {
if let Some(arc_self) = weak_self.upgrade() {
@ -850,6 +856,12 @@ impl URingExecutor {
}
}
impl AsRawDescriptor for URingExecutor {
fn as_raw_descriptor(&self) -> RawDescriptor {
self.raw.as_raw_descriptor()
}
}
// Used to dup the FDs passed to the executor so there is a guarantee they aren't closed while
// waiting in TLS to be added to the main polling context.
unsafe fn dup_fd(fd: RawFd) -> Result<RawFd> {

View file

@ -10,7 +10,10 @@ use std::sync::Arc;
use anyhow::{anyhow, bail, Context};
use argh::FromArgs;
use base::{error, get_max_open_files, warn, Event, RawDescriptor, Tube, UnlinkUnixListener};
use base::{
error, get_max_open_files, warn, AsRawDescriptors, Event, RawDescriptor, Tube,
UnlinkUnixListener,
};
use cros_async::{EventAsync, Executor};
use data_model::{DataInit, Le32};
use fuse::Server;
@ -142,10 +145,13 @@ impl FsBackend {
let mut keep_rds: Vec<RawDescriptor> = [0, 1, 2].to_vec();
keep_rds.append(&mut fs.keep_rds());
let ex = ex.clone();
keep_rds.extend(ex.as_raw_descriptors());
let server = Arc::new(Server::new(fs));
Ok(FsBackend {
ex: ex.clone(),
ex,
server,
tag: fs_tag,
avail_features,