mirrors/crosvm
1
0
Fork 0
mirror of https://chromium.googlesource.com/crosvm/crosvm synced 2024-11-28 01:16:50 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

Reland "crosvm: Remove CLOEXEC from fds passed into non-jailed plugin"

Browse source 
This is a reland of commit 7afcfa00e7

remove rlib as it doesn't play nicely with cros-rust.eclass' lto
BUG=none
TEST=cargo test

Original change's description:
> crosvm: Remove CLOEXEC from fds passed into non-jailed plugin
>
> Otherwise we'll lose them
>
> BUT=none
> TEST=kokoro/luci
>
> Change-Id: I6b48b802be7c985efb05a4e9ffb326c63117e677
> Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3570179
> Tested-by: kokoro <noreply+kokoro@google.com>
> Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
> Commit-Queue: Anton Romanov <romanton@google.com>

Change-Id: I74bfc2311e4892cdcf7f517d25234409176bc9dd
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/3575286
Reviewed-by: Daniel Verkamp <dverkamp@chromium.org>
Reviewed-by: Dmitry Torokhov <dtor@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Anton Romanov <romanton@google.com>
This commit is contained in:
Anton Romanov 2022-04-05 00:34:22 +00:00 committed by Chromeos LUCI
parent 3491bdfc56
commit 79b3a811ca
5 changed files with 78 additions and 30 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
base/src/unix/descriptor.rs
View file

@ -47,6 +47,29 @@ fn clone_fd(fd: &dyn AsRawFd) -> Result<RawFd> {
}
}
/// Clears CLOEXEC flag on descriptor
pub fn clear_descriptor_cloexec<A: AsRawDescriptor>(fd_owner: &A) -> Result<()> {
clear_fd_cloexec(&fd_owner.as_raw_descriptor())
}
/// Clears CLOEXEC flag on fd
fn clear_fd_cloexec<A: AsRawFd>(fd_owner: &A) -> Result<()> {
let fd = fd_owner.as_raw_fd();
// Safe because fd is read only.
let flags = unsafe { libc::fcntl(fd, libc::F_GETFD) };
if flags == -1 {
return errno_result();
}
let masked_flags = flags & !libc::FD_CLOEXEC;
// Safe because this has no side effect(s) on the current process.
if masked_flags != flags && unsafe { libc::fcntl(fd, libc::F_SETFD, masked_flags) } == -1 {
errno_result()
} else {
Ok(())
}
}
const KCMP_FILE: u32 = 0;
impl PartialEq for SafeDescriptor {

11
crosvm_plugin/src/lib.rs
View file

@ -63,6 +63,11 @@ const CROSVM_VCPU_EVENT_KIND_PAUSED: u32 = 2;
const CROSVM_VCPU_EVENT_KIND_HYPERV_HCALL: u32 = 3;
const CROSVM_VCPU_EVENT_KIND_HYPERV_SYNIC: u32 = 4;
pub const CROSVM_GPU_SERVER_FD_ENV: &str = "CROSVM_GPU_SERVER_FD";
pub const CROSVM_SOCKET_ENV: &str = "CROSVM_SOCKET";
#[cfg(feature = "stats")]
pub const CROSVM_STATS_ENV: &str = "CROSVM_STATS";
#[repr(C)]
#[derive(Copy, Clone)]
pub struct crosvm_net_config {
@ -236,7 +241,7 @@ fn record(_a: Stat) -> u32 {
#[cfg(feature = "stats")]
fn printstats() {
// Unsafe due to racy access - OK for stats
if std::env::var("CROSVM_STATS").is_ok() {
if std::env::var(CROSVM_STATS_ENV).is_ok() {
unsafe {
stats::STATS.print();
}
@ -1365,7 +1370,7 @@ fn to_crosvm_rc<T>(r: result::Result<T, c_int>) -> c_int {
#[no_mangle]
pub unsafe extern "C" fn crosvm_get_render_server_fd() -> c_int {
let fd = match env::var("CROSVM_GPU_SERVER_FD") {
let fd = match env::var(CROSVM_GPU_SERVER_FD_ENV) {
Ok(v) => v,
_ => return -EINVAL,
};
@ -1379,7 +1384,7 @@ pub unsafe extern "C" fn crosvm_get_render_server_fd() -> c_int {
#[no_mangle]
pub unsafe extern "C" fn crosvm_connect(out: *mut *mut crosvm) -> c_int {
let _u = record(Stat::Connect);
let socket_name = match env::var("CROSVM_SOCKET") {
let socket_name = match env::var(CROSVM_SOCKET_ENV) {
Ok(v) => v,
_ => return -ENOTCONN,
};

24
src/plugin/mod.rs
View file

@ -9,7 +9,6 @@ use std::fs::File;
use std::io;
use std::io::Read;
use std::os::unix::net::UnixDatagram;
use std::os::unix::prelude::RawFd;
use std::path::Path;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Barrier};
@ -22,16 +21,16 @@ use libc::{
MS_RDONLY, O_NONBLOCK, SIGCHLD, SOCK_SEQPACKET,
};
use anyhow::{anyhow, bail, Context, Result};
use protobuf::ProtobufError;
use remain::sorted;
use thiserror::Error;
use anyhow::{anyhow, bail, Context, Result};
use base::{
add_fd_flags, block_signal, clear_signal, drop_capabilities, enable_core_scheduling, error,
getegid, geteuid, info, pipe, register_rt_signal_handler, validate_raw_descriptor, warn,
AsRawDescriptor, Error as SysError, Event, FromRawDescriptor, Killable, MmapError, PollToken,
Result as SysResult, SignalFd, WaitContext, SIGRTMIN,
AsRawDescriptor, Descriptor, Error as SysError, Event, FromRawDescriptor, Killable, MmapError,
PollToken, RawDescriptor, Result as SysResult, SignalFd, WaitContext, SIGRTMIN,
};
use kvm::{Cap, Datamatch, IoeventAddress, Kvm, Vcpu, VcpuExit, Vm};
use minijail::{self, Minijail};
@ -44,6 +43,7 @@ use crate::{Config, Executable};
const MAX_DATAGRAM_SIZE: usize = 4096;
const MAX_VCPU_DATAGRAM_SIZE: usize = 0x40000;
const CROSVM_GPU_SERVER_FD_ENV: &str = "CROSVM_GPU_SERVER_FD";
/// An error that occurs when communicating with the plugin process.
#[sorted]
@ -500,7 +500,7 @@ pub fn run_config(cfg: Config) -> Result<()> {
.context("error marking stderr nonblocking")?;
#[allow(unused_mut)]
let mut fds: Vec<(String, RawFd)> = Vec::default();
let mut env_fds: Vec<(String, Descriptor)> = Vec::default();
let _default_render_server_params = crate::platform::GpuRenderServerParameters {
path: std::path::PathBuf::from("/usr/libexec/virgl_render_server"),
@ -524,7 +524,10 @@ pub fn run_config(cfg: Config) -> Result<()> {
let (_render_server_jail, _render_server_fd) =
if let Some(parameters) = &gpu_render_server_parameters {
let (jail, fd) = crate::platform::gpu::start_gpu_render_server(&cfg, parameters)?;
fds.push((String::from("CROSVM_GPU_SERVER_FD"), fd.as_raw_descriptor()));
env_fds.push((
CROSVM_GPU_SERVER_FD_ENV.to_string(),
Descriptor(fd.as_raw_descriptor()),
));
(
Some(crate::platform::jail_helpers::ScopedMinijail(jail)),
Some(fd),
@ -643,7 +646,14 @@ pub fn run_config(cfg: Config) -> Result<()> {
.context("failed to create kvm irqchip")?;
vm.create_pit().context("failed to create kvm PIT")?;
let mut plugin = Process::new(vcpu_count, plugin_path, &plugin_args, jail, stderr_wr, fds)?;
let mut plugin = Process::new(
vcpu_count,
plugin_path,
&plugin_args,
jail,
stderr_wr,
env_fds,
)?;
// Now that the jail for the plugin has been created and we had a chance to adjust gids there,
// we can drop all our capabilities in case we had any.
drop_capabilities().context("failed to drop process capabilities")?;

47
src/plugin/process.rs
View file

@ -8,7 +8,6 @@ use std::fs::File;
use std::io::{IoSlice, IoSliceMut, Write};
use std::mem::transmute;
use std::os::unix::net::UnixDatagram;
use std::os::unix::prelude::RawFd;
use std::path::Path;
use std::process::Command;
use std::result;
@ -25,9 +24,8 @@ use libc::{
use protobuf::Message;
use base::{
error, AsRawDescriptor, Error as SysError, Event, IntoRawDescriptor, Killable,
MemoryMappingBuilder, RawDescriptor, Result as SysResult, ScmSocket, SharedMemory,
SharedMemoryUnix, SIGRTMIN,
error, AsRawDescriptor, Descriptor, Error as SysError, Event, IntoRawDescriptor, Killable,
MemoryMappingBuilder, Result as SysResult, ScmSocket, SharedMemory, SharedMemoryUnix, SIGRTMIN,
};
use kvm::{dirty_log_bitmap_size, Datamatch, IoeventAddress, IrqRoute, IrqSource, PicId, Vm};
use kvm_sys::{kvm_clock_data, kvm_ioapic_state, kvm_pic_state, kvm_pit_state2};
@ -53,6 +51,8 @@ unsafe impl DataInit for VmPitState {}
struct VmClockState(kvm_clock_data);
unsafe impl DataInit for VmClockState {}
const CROSVM_SOCKET_ENV: &str = "CROSVM_SOCKET";
fn get_vm_state(vm: &Vm, state_set: MainRequest_StateSet) -> SysResult<Vec<u8>> {
Ok(match state_set {
MainRequest_StateSet::PIC0 => VmPicState(vm.get_pic_state(PicId::Primary)?)
@ -158,7 +158,7 @@ impl Process {
args: &[&str],
jail: Option<Minijail>,
stderr: File,
env_fds: Vec<(String, RawFd)>,
env_fds: Vec<(String, Descriptor)>,
) -> Result<Process> {
let (request_socket, child_socket) =
new_seqpacket_pair().context("error creating main request socket")?;
@ -174,17 +174,17 @@ impl Process {
let plugin_pid = match jail {
Some(jail) => {
set_var(
"CROSVM_SOCKET",
CROSVM_SOCKET_ENV,
child_socket.as_raw_descriptor().to_string(),
);
env_fds
.iter()
.for_each(|(k, fd)| set_var(k, fd.to_string()));
.for_each(|(k, fd)| set_var(k, fd.as_raw_descriptor().to_string()));
jail.run_remap(
cmd,
&env_fds
.into_iter()
.map(|(_, fd)| (fd, fd))
.map(|(_, fd)| (fd.as_raw_descriptor(), fd.as_raw_descriptor()))
.chain(
[
(stderr.as_raw_descriptor(), STDERR_FILENO),
@ -200,17 +200,26 @@ impl Process {
)
.context("failed to run plugin jail")?
}
None => Command::new(cmd)
.args(args)
.envs(env_fds.into_iter().map(|(k, fd)| (k, fd.to_string())))
.env(
"CROSVM_SOCKET",
child_socket.as_raw_descriptor().to_string(),
)
.stderr(stderr)
.spawn()
.context("failed to spawn plugin")?
.id() as pid_t,
None => {
for (_, fd) in env_fds.iter() {
base::clear_descriptor_cloexec(fd)?;
}
Command::new(cmd)
.args(args)
.envs(
env_fds
.into_iter()
.map(|(k, fd)| (k, { fd.as_raw_descriptor().to_string() })),
)
.env(
CROSVM_SOCKET_ENV,
child_socket.as_raw_descriptor().to_string(),
)
.stderr(stderr)
.spawn()
.context("failed to spawn plugin")?
.id() as pid_t
}
};
Ok(Process {

3
tests/plugin_net_config.c
View file

@ -49,7 +49,8 @@ int main(int argc, char** argv) {
unsigned int features;
if (ioctl(net_config.tap_fd, TUNGETFEATURES, &features) < 0) {
fprintf(stderr,
"failed to read tap features: %s\n",
"failed to read tap(fd: %d) features: %s\n",
net_config.tap_fd,
strerror(errno));
return 1;
}