Skip to content

Commit

Permalink
[HOTFIX] Resolve race condition in exit signal
Browse files Browse the repository at this point in the history
- Reinstates SIGUSR1 coordination
- Corrects signal dispatch logic
- Simplifies SignalConfig
- Reinstates SpawnDirectory functionality
  • Loading branch information
zellio committed Nov 19, 2023
1 parent e33e3b2 commit de52c2e
Show file tree
Hide file tree
Showing 14 changed files with 211 additions and 174 deletions.
6 changes: 3 additions & 3 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "bimini"
version = "0.11.0"
version = "0.11.1"
edition = "2021"

[profile.release]
Expand Down
4 changes: 2 additions & 2 deletions bimini-core/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion bimini-core/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "bimini-core"
version = "0.11.0"
version = "0.11.1"
edition = "2021"

[dependencies]
Expand Down
3 changes: 0 additions & 3 deletions bimini-core/src/nix/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,3 @@ pub use groups_iter::GroupsIter;

mod signal_config;
pub use signal_config::SignalConfig;

mod spawn_directory;
pub use spawn_directory::SpawnDirectory;
98 changes: 46 additions & 52 deletions bimini-core/src/nix/signal_config.rs
Original file line number Diff line number Diff line change
@@ -1,36 +1,40 @@
use nix::sys::signal;

use crate::error::BiminiResult;
use nix::{errno, sys::signal};

#[derive(Clone, Debug)]
#[derive(Debug, Clone)]
pub struct SignalConfig {
parent_signals: signal::SigSet,
source_signals: signal::SigSet,
mask_set: signal::SigSet,
source_set: signal::SigSet,
sigttin_action: signal::SigAction,
sigttou_action: signal::SigAction,
}

impl SignalConfig {
pub fn parent_signals(&self) -> &signal::SigSet {
&self.parent_signals
}

pub fn source_signals(&self) -> &signal::SigSet {
&self.source_signals
}

pub fn sigttin_action(&self) -> &signal::SigAction {
&self.sigttin_action
}

pub fn sigttou_action(&self) -> &signal::SigAction {
&self.sigttou_action
impl Default for SignalConfig {
fn default() -> Self {
Self::new(&[
// Un-blockable signals
signal::SIGKILL,
signal::SIGSTOP,
// Program signals
signal::SIGABRT,
signal::SIGBUS,
#[cfg(not(target_os = "linux"))]
signal::SIGEMT,
signal::SIGFPE,
signal::SIGILL,
signal::SIGIOT,
signal::SIGSEGV,
signal::SIGSYS,
signal::SIGTRAP,
])
}
}

pub fn new(protected_signals: Vec<signal::Signal>) -> Self {
let mut parent_signals = signal::SigSet::all();
for signal in protected_signals {
parent_signals.remove(signal)
impl SignalConfig {
pub fn new(protected_signals: &[signal::Signal]) -> Self {
let mut mask = signal::SigSet::all();
for sig in protected_signals {
mask.remove(*sig);
}

let ignore_action = signal::SigAction::new(
Expand All @@ -39,59 +43,49 @@ impl SignalConfig {
signal::SigSet::empty(),
);

SignalConfig {
parent_signals,
source_signals: signal::SigSet::empty(),
Self {
mask_set: mask,
source_set: signal::SigSet::empty(),
sigttin_action: ignore_action,
sigttou_action: ignore_action,
}
}

pub fn mask(&mut self) -> BiminiResult<()> {
signal::sigprocmask(
pub fn mask(&mut self) -> BiminiResult<&mut Self> {
signal::pthread_sigmask(
signal::SigmaskHow::SIG_SETMASK,
Some(&self.parent_signals),
Some(&mut self.source_signals),
Some(&self.mask_set),
Some(&mut self.source_set),
)?;

unsafe {
self.sigttin_action = signal::sigaction(signal::SIGTTIN, &self.sigttin_action)?;
self.sigttou_action = signal::sigaction(signal::SIGTTOU, &self.sigttou_action)?;
}

Ok(())
Ok(self)
}

pub fn unmask(&mut self) -> BiminiResult<()> {
signal::sigprocmask(
pub fn unmask(&mut self) -> BiminiResult<&mut Self> {
signal::pthread_sigmask(
signal::SigmaskHow::SIG_SETMASK,
Some(&self.source_signals),
Some(&self.source_set),
None,
)?;

self.source_signals = signal::SigSet::empty();

unsafe {
signal::sigaction(signal::SIGTTIN, &self.sigttin_action)?;
signal::sigaction(signal::SIGTTOU, &self.sigttou_action)?;
}

Ok(())
Ok(self)
}
}

impl Default for SignalConfig {
fn default() -> Self {
SignalConfig::new(vec![
signal::SIGFPE,
signal::SIGILL,
signal::SIGSEGV,
signal::SIGBUS,
signal::SIGABRT,
signal::SIGTRAP,
signal::SIGSYS,
signal::SIGTTIN,
signal::SIGTTOU,
])
pub fn mask_wait(&self) -> Result<signal::Signal, errno::Errno> {
self.mask_set.wait()
}

pub fn source_wait(&self) -> Result<signal::Signal, errno::Errno> {
self.source_set.wait()
}
}
46 changes: 0 additions & 46 deletions bimini-core/src/nix/spawn_directory.rs

This file was deleted.

4 changes: 2 additions & 2 deletions bimini-core/src/nix/user_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use crate::{error::BiminiResult, nix::ToEnv};

#[derive(Clone, Debug)]
pub struct UserSpec {
user: Option<unistd::User>,
group: Option<unistd::Group>,
pub user: Option<unistd::User>,
pub group: Option<unistd::Group>,
}

impl UserSpec {
Expand Down
29 changes: 19 additions & 10 deletions bimini-core/src/proc/child.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
use crate::{
aws::AwsClient,
error::{BiminiError, BiminiResult},
nix::{SignalConfig, SpawnDirectory, ToEnv, UserSpec},
nix::{SignalConfig, ToEnv, UserSpec},
proc::SpawnDirectory,
vault::VaultClient,
};
use derive_builder::Builder;
use nix::{errno, unistd};
use std::{collections::HashMap, env, os::unix::process::CommandExt, process};
use std::{collections::HashMap, env, os::unix::process::CommandExt, path::PathBuf, process};

#[derive(Builder)]
#[builder(build_fn(error = "BiminiError"), pattern = "owned")]
Expand All @@ -17,7 +18,7 @@ pub struct Child<'a> {
user_spec: Option<UserSpec>,

#[builder(default)]
spawn_directory: Option<SpawnDirectory>,
spawn_directory: Option<PathBuf>,

#[builder(default)]
aws_client: Option<AwsClient>,
Expand Down Expand Up @@ -76,15 +77,23 @@ impl<'a> Child<'a> {
proc.envs(user_spec.to_env());
}

if let Some(spawn_directory) = &self.spawn_directory {
tracing::debug!("Spawn directory provided, changing proc root");
spawn_directory.chdir()?;
proc.envs(spawn_directory.to_env());
}

tracing::trace!("Changing proc working directory");
let mut spawn_directory = SpawnDirectory::new(
self.spawn_directory.clone(),
self.user_spec
.as_ref()
.and_then(|user_spec| user_spec.user.clone()),
);
spawn_directory.chdir();
proc.envs(spawn_directory.to_env());

tracing::trace!("Creating a new proc group for child");
self.isolate()?;

tracing::trace!("Clearing inherited sigmask");
self.signal_config.unmask()?;

tracing::trace!("Execing child proc");
let error = proc.exec();

if let Some(errno) = error.raw_os_error() {
Expand All @@ -101,7 +110,7 @@ impl<'a> ToEnv for Child<'a> {
(String::from("BIMINI"), String::from("true")),
(
String::from("BIMINI_VERSION"),
String::from(env!("CARGO_PKG_VERSION")),
String::from(crate::PKG_VERSION),
),
(
String::from("BIMINI_CHILD_PID"),
Expand Down
Loading

0 comments on commit de52c2e

Please sign in to comment.