From b1daed6f50a3b63e4166a873fe35deff37b65c21 Mon Sep 17 00:00:00 2001 From: Matthias Schiffer Date: Wed, 27 Oct 2021 00:29:49 +0200 Subject: runner: get rid of spawn argument funnelling --- crates/runner/src/lib.rs | 29 +++++++++++++++-------------- crates/runner/src/ns.rs | 8 ++++---- crates/runner/src/tar.rs | 5 ++--- crates/runner/src/task.rs | 5 ++--- crates/runner/src/util/clone.rs | 19 +++++++++---------- 5 files changed, 32 insertions(+), 34 deletions(-) diff --git a/crates/runner/src/lib.rs b/crates/runner/src/lib.rs index 8066c58..636c112 100644 --- a/crates/runner/src/lib.rs +++ b/crates/runner/src/lib.rs @@ -11,7 +11,7 @@ use std::{ fs::File, net, os::unix::{net::UnixStream, prelude::*}, - slice, + process, slice, }; use capctl::prctl; @@ -55,7 +55,7 @@ fn handle_sigchld(ctx: &mut RunnerContext) -> Result<()> { } fn handle_request(ctx: &mut RunnerContext, request_socket: UnixStream) { - let run = |()| { + let run = || { ctx.socket.steal(); let task: Task = @@ -68,7 +68,7 @@ fn handle_request(ctx: &mut RunnerContext, request_socket: UnixStream) { drop(request_socket); }; - let pid = unsafe { clone::spawn(None, (), run) }.expect("fork()").0; + let pid = unsafe { clone::spawn(None, run) }.expect("fork()"); assert!(ctx.tasks.insert(pid)); } @@ -91,7 +91,7 @@ fn handle_socket(ctx: &mut RunnerContext) -> bool { true } -fn runner(uid: Uid, gid: Gid, socket: UnixSeqpacketConn, _lockfile: File, options: &Options) { +fn runner(uid: Uid, gid: Gid, socket: UnixSeqpacketConn, _lockfile: File, options: &Options) -> ! { ns::mount_proc(); ns::setup_userns(Uid::from_raw(0), Gid::from_raw(0), uid, gid); @@ -144,6 +144,8 @@ fn runner(uid: Uid, gid: Gid, socket: UnixSeqpacketConn, _lockfile: File, option panic!("Unexpected error status for socket file descriptor"); } } + + process::exit(0); } pub struct Runner { @@ -165,18 +167,17 @@ impl Runner { let (local, remote) = UnixSeqpacketConn::pair().expect("socketpair()"); - let (local, _remote) = clone::spawn( - Some(CloneFlags::CLONE_NEWUSER | CloneFlags::CLONE_NEWNS | CloneFlags::CLONE_NEWPID), - (local, remote), - |(local, remote)| { - drop(local); - runner(uid, gid, remote, lockfile, options); - }, + match clone::clone( + CloneFlags::CLONE_NEWUSER | CloneFlags::CLONE_NEWNS | CloneFlags::CLONE_NEWPID, ) .expect("clone()") - .1; - - Ok(Runner { socket: local }) + { + unistd::ForkResult::Parent { .. } => Ok(Runner { socket: local }), + unistd::ForkResult::Child => { + drop(local); + runner(uid, gid, remote, lockfile, options); + } + } } pub fn spawn(&self, task: &Task) -> UnixStream { diff --git a/crates/runner/src/ns.rs b/crates/runner/src/ns.rs index a075931..3a8b51f 100644 --- a/crates/runner/src/ns.rs +++ b/crates/runner/src/ns.rs @@ -27,17 +27,17 @@ pub fn setup_userns(inner_uid: Uid, inner_gid: Gid, outer_uid: Uid, outer_gid: G .expect("Failed to write /proc/self/gid_map"); } -pub unsafe fn spawn(flags: CloneFlags, arg: T, f: F) -> nix::Result<(Pid, T)> +pub unsafe fn spawn(flags: CloneFlags, f: F) -> nix::Result where - F: FnOnce(T), + F: FnOnce(), { assert!(flags.contains(CloneFlags::CLONE_NEWNS) || !flags.contains(CloneFlags::CLONE_NEWPID)); - clone::spawn(Some(flags), arg, |arg| { + clone::spawn(Some(flags), || { if flags.contains(CloneFlags::CLONE_NEWPID) { mount_proc(); } - f(arg) + f() }) } diff --git a/crates/runner/src/tar.rs b/crates/runner/src/tar.rs index 4bc343f..3fd21e7 100644 --- a/crates/runner/src/tar.rs +++ b/crates/runner/src/tar.rs @@ -55,9 +55,8 @@ pub fn pack>(archive: &mut W, source: P) -> Result<()> process::exit(127); }; - let pid = unsafe { ns::spawn(CloneFlags::CLONE_NEWNS, (), |()| exec_tar().unwrap()) } - .context("Failed to run tar")? - .0; + let pid = unsafe { ns::spawn(CloneFlags::CLONE_NEWNS, || exec_tar().unwrap()) } + .context("Failed to run tar")?; let result = io::copy(&mut piper, archive).context("Failed to write TAR archive"); diff --git a/crates/runner/src/task.rs b/crates/runner/src/task.rs index 2b0070b..dc15ec5 100644 --- a/crates/runner/src/task.rs +++ b/crates/runner/src/task.rs @@ -319,15 +319,14 @@ fn run_task(input_hash: &InputHash, task: &Task, jobserver: &mut Jobserver) -> R process::exit(127); }; - let (pid, ()) = unsafe { + let pid = unsafe { ns::spawn( CloneFlags::CLONE_NEWNS | CloneFlags::CLONE_NEWPID | CloneFlags::CLONE_NEWIPC | CloneFlags::CLONE_NEWNET | CloneFlags::CLONE_NEWUTS, - (), - |()| exec_cmd().unwrap(), + || exec_cmd().unwrap(), ) } .context("Failed to run task container")?; diff --git a/crates/runner/src/util/clone.rs b/crates/runner/src/util/clone.rs index 4835b53..0af9e4d 100644 --- a/crates/runner/src/util/clone.rs +++ b/crates/runner/src/util/clone.rs @@ -1,6 +1,9 @@ use std::{mem, process}; -use nix::{errno, sched, unistd}; +use nix::{ + errno, sched, + unistd::{self, Pid}, +}; #[repr(C)] #[derive(Debug, Default)] @@ -30,18 +33,14 @@ pub unsafe fn clone(flags: sched::CloneFlags) -> nix::Result Ok(unistd::ForkResult::Child) } else { Ok(unistd::ForkResult::Parent { - child: unistd::Pid::from_raw(pid as libc::pid_t), + child: Pid::from_raw(pid as libc::pid_t), }) } } -pub unsafe fn spawn( - flags: Option, - arg: T, - f: F, -) -> nix::Result<(unistd::Pid, T)> +pub unsafe fn spawn(flags: Option, f: F) -> nix::Result where - F: FnOnce(T), + F: FnOnce(), { let res = if let Some(flags) = flags { clone(flags) @@ -49,9 +48,9 @@ where unistd::fork() }; match res? { - unistd::ForkResult::Parent { child } => Ok((child, arg)), + unistd::ForkResult::Parent { child } => Ok(child), unistd::ForkResult::Child => { - f(arg); + f(); process::exit(0) } } -- cgit v1.2.3