diff --git a/docs/jailer.md b/docs/jailer.md index aae3edf9a09..59a783b8e25 100644 --- a/docs/jailer.md +++ b/docs/jailer.md @@ -271,10 +271,6 @@ Note: default value for `` is `/run/firecracker.socket`. binary file in a new PID namespace, in order to become a pseudo-init process. Alternatively, the user can spawn the jailer in a new PID namespace via a combination of `clone()` with the `CLONE_NEWPID` flag and `exec()`. -- When running with `--daemonize`, the jailer will fail to start if it's a - process group leader, because `setsid()` returns an error in this case. - Spawning the jailer via `clone()` and `exec()` also ensures it cannot be a - process group leader. - We run the jailer as the `root` user; it actually requires a more restricted set of capabilities, but that's to be determined as features stabilize. - The jailer can only log messages to stdout/err for now, which is why the diff --git a/src/jailer/src/env.rs b/src/jailer/src/env.rs index 9b91bfef119..44482002bab 100644 --- a/src/jailer/src/env.rs +++ b/src/jailer/src/env.rs @@ -8,7 +8,7 @@ use std::os::unix::fs::PermissionsExt; use std::os::unix::io::AsRawFd; use std::os::unix::process::CommandExt; use std::path::{Component, Path, PathBuf}; -use std::process::{Command, Stdio}; +use std::process::{exit, Command, Stdio}; use std::{fmt, io}; use utils::arg_parser::Error::MissingValue; @@ -674,12 +674,45 @@ impl Env { // Daemonize before exec, if so required (when the dev_null variable != None). if let Some(dev_null) = dev_null { - // Call setsid(). + // We follow the double fork method to daemonize the jailer referring to + // https://0xjet.github.io/3OHA/2022/04/11/post.html + // setsid() will fail if the calling process is a process group leader. + // By calling fork(), we guarantee that the newly created process inherits + // the PGID from its parent and, therefore, is not a process group leader. + // SAFETY: Safe because it's a library function. + let child_pid = unsafe { libc::fork() }; + if child_pid < 0 { + return Err(JailerError::Daemonize(io::Error::last_os_error())); + } + + if child_pid != 0 { + // parent exiting + exit(0); + } + + // Call setsid() in child // SAFETY: Safe because it's a library function. SyscallReturnCode(unsafe { libc::setsid() }) .into_empty_result() .map_err(JailerError::SetSid)?; + // Daemons should not have controlling terminals. + // If a daemon has a controlling terminal, it can receive signals + // from it that might cause it to halt or exit unexpectedly. + // The second fork() ensures that grandchild is not a session, + // leader and thus cannot reacquire a controlling terminal. + // SAFETY: Safe because it's a library function. + let grandchild_pid = unsafe { libc::fork() }; + if grandchild_pid < 0 { + return Err(JailerError::Daemonize(io::Error::last_os_error())); + } + + if grandchild_pid != 0 { + // child exiting + exit(0); + } + + // grandchild is the daemon // Replace the stdio file descriptors with the /dev/null fd. dup2(dev_null.as_raw_fd(), STDIN_FILENO)?; dup2(dev_null.as_raw_fd(), STDOUT_FILENO)?; diff --git a/src/jailer/src/main.rs b/src/jailer/src/main.rs index 59be688c735..d9c969b2bcc 100644 --- a/src/jailer/src/main.rs +++ b/src/jailer/src/main.rs @@ -64,6 +64,8 @@ pub enum JailerError { CreateDir(PathBuf, io::Error), #[error("Encountered interior \\0 while parsing a string")] CStringParsing(NulError), + #[error("Failed to daemonize: {0}")] + Daemonize(io::Error), #[error("Failed to open directory {0}: {1}")] DirOpen(String, String), #[error("Failed to duplicate fd: {0}")]