Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use the double fork method to daemonize jailer #4259

Merged
merged 1 commit into from
Nov 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions docs/jailer.md
Original file line number Diff line number Diff line change
Expand Up @@ -271,10 +271,6 @@ Note: default value for `<api-sock>` 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
Expand Down
37 changes: 35 additions & 2 deletions src/jailer/src/env.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
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;
Expand Down Expand Up @@ -674,12 +674,45 @@

// 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 {

Check warning on line 688 in src/jailer/src/env.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/env.rs#L683-L688

Added lines #L683 - L688 were not covered by tests
// parent exiting
exit(0);
}

// Call setsid() in child

Check warning on line 693 in src/jailer/src/env.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/env.rs#L690-L693

Added lines #L690 - L693 were not covered by tests
// 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 {

Check warning on line 710 in src/jailer/src/env.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/env.rs#L705-L710

Added lines #L705 - L710 were not covered by tests
// child exiting
exit(0);
}

// grandchild is the daemon

Check warning on line 715 in src/jailer/src/env.rs

View check run for this annotation

Codecov / codecov/patch

src/jailer/src/env.rs#L712-L715

Added lines #L712 - L715 were not covered by tests
// 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)?;
Expand Down
2 changes: 2 additions & 0 deletions src/jailer/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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}")]
Expand Down