Skip to content

Commit

Permalink
Avoid crashes on macOS by exec(2) after daemonize
Browse files Browse the repository at this point in the history
We can encounter the following error if we continue without
re-exec(2)-ing:

    objc[11602]: +[__NSCFConstantString initialize] may have been in
    progress in another thread when fork() was called.
    We cannot safely call it or ignore it in the fork() child process.
    Crashing instead. Set a breakpoint on objc_initializeAfterForkError
    to debug.

In Mairu's case, most these crashes are triggered in
a copy_certificates_from_keychain dispatch queue thread.

This is likely relevant to security-framework crate which utilise
Security.framework for TLS functionality, and appears to be difficult to
avoid; unforeseen risks remain ahead that could cause similar issues.

We're avoiding by re-exec(2)-ing itself to prevent the issue at all.

See also: https://www.sealiesoftware.com/blog/archive/2017/6/5/Objective-C_and_fork_in_macOS_1013.html
  • Loading branch information
sorah committed Nov 1, 2024
1 parent 5d343ba commit 6b5baee
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 14 deletions.
77 changes: 73 additions & 4 deletions src/cmd/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ pub struct AgentArgs {
/// Enable logging to file ($XDG_STATE_HOME/mairu/log/*).
#[arg(long, default_value_t = false)]
pub log_to_file: bool,

#[cfg(any(target_os = "macos", target_os = "ios"))]
#[arg(long, hide = true)]
uds_fd: Option<std::os::fd::RawFd>, // See daemonize_reexec() for details
}

pub fn run(args: &AgentArgs) -> Result<(), anyhow::Error> {
Expand All @@ -16,10 +20,18 @@ pub fn run(args: &AgentArgs) -> Result<(), anyhow::Error> {
crate::config::cache_dir_mkpath()?;

if args.daemonize {
serve_on_path_daemon(path)
} else {
serve_on_path(path)
return serve_on_path_daemon(path);
}

#[cfg(any(target_os = "macos", target_os = "ios"))]
{
// See daemonize_reexec(2) for details
if let Some(uds_fd) = args.uds_fd {
return serve_on_fd(uds_fd);
}
}

serve_on_path(path)
}

fn daemonize() -> Result<(), anyhow::Error> {
Expand All @@ -37,12 +49,48 @@ fn daemonize() -> Result<(), anyhow::Error> {
);
}
daemonize::Outcome::Parent(Err(e)) => return Err(e.into()),
daemonize::Outcome::Child(Ok(_)) => {}
daemonize::Outcome::Child(Ok(_)) => {} // do nothing
daemonize::Outcome::Child(Err(e)) => return Err(e.into()),
}
Ok(())
}

#[cfg(any(target_os = "macos", target_os = "ios"))]
fn daemonize_reexec(
uds: std::os::unix::net::UnixListener,
arg0: std::path::PathBuf,
) -> Result<(), anyhow::Error> {
// We can encounter the following error if we continue without re-exec(2)-ing:
//
// objc[11602]: +[__NSCFConstantString initialize] may have been in progress in another thread when fork() was called.
// We cannot safely call it or ignore it in the fork() child process. Crashing instead. Set a breakpoint on objc_initializeAfterForkError to debug.
//
// In Mairu's case, most these crashes are triggered in a copy_certificates_from_keychain dispatch queue thread.
// This is likely relevant to security-framework crate which utilise Security.framework for TLS functionality, and
// appears to be difficult to avoid; unforeseen risks remain ahead that could cause similar issues.
//
// We're avoiding by re-exec(2)-ing itself to prevent the issue at all.
//
// See also: https://www.sealiesoftware.com/blog/archive/2017/6/5/Objective-C_and_fork_in_macOS_1013.html
use std::os::fd::IntoRawFd;
use std::os::unix::process::CommandExt;

let raw_fd = uds.into_raw_fd();
crate::os::set_cloexec(&raw_fd, false)?;

let args = std::env::args()
.skip(1)
.map(|x| x.to_owned())
.filter(|x| x.as_str() != "--daemonize")
.chain(["--uds-fd".to_string(), raw_fd.to_string()])
.collect::<Vec<_>>();

tracing::debug!(arg0 = ?arg0, args = ?args, "Re-exec(2)-ing");

let err = std::process::Command::new(arg0).args(args).exec();
panic!("exec(2) failed to re-exec itself after daemonize: {err}");
}

fn protect_process() {
nix::sys::stat::umask(nix::sys::stat::Mode::from_bits(0o077).unwrap());

Expand Down Expand Up @@ -71,11 +119,32 @@ pub async fn serve_on_path(path: std::path::PathBuf) -> Result<(), anyhow::Error
serve(uds).await
}

#[tokio::main]
pub async fn serve_on_fd(raw_fd: std::os::fd::RawFd) -> Result<(), anyhow::Error> {
use std::os::fd::FromRawFd;
tracing::info!(raw_fd = ?raw_fd, "Server starting using given fd");
crate::os::set_cloexec(&raw_fd, true)?;
// SAFETY: we validate liveness roughly by resetting O_CLOXEC
let uds = unsafe { std::os::unix::net::UnixListener::from_raw_fd(raw_fd) };
uds.set_nonblocking(true)?;
serve(tokio::net::UnixListener::from_std(uds)?).await
}

#[cfg_attr(any(target_os = "macos", target_os = "ios"), allow(unreachable_code))]
pub fn serve_on_path_daemon(path: std::path::PathBuf) -> Result<(), anyhow::Error> {
tracing::debug!(path = ?path, "Server starting as a daemon");
let uds = try_bind_or_check_liveness_std(&path)?;
tracing::info!(path = ?path, "Agent starting as a daemon");

// See daemonize_reexec() for details
#[cfg(any(target_os = "macos", target_os = "ios"))]
let arg0 = process_path::get_executable_path().expect("Can't get executable path");

daemonize()?;

#[cfg(any(target_os = "macos", target_os = "ios"))]
return daemonize_reexec(uds, arg0);

serve_on_path_daemon2(uds)
}

Expand Down
12 changes: 2 additions & 10 deletions src/cmd/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,16 +98,8 @@ cfg_if::cfg_if! {
use nix::fcntl::{fcntl, FdFlag, F_GETFD, F_SETFD};

let (i, o) = nix::unistd::pipe()?;
{
let mut fdopts = FdFlag::from_bits(fcntl(i.as_raw_fd(), F_GETFD)?).unwrap();
fdopts.set(FdFlag::FD_CLOEXEC, true);
fcntl(i.as_raw_fd(), F_SETFD(fdopts))?;
}
{
let mut fdopts = FdFlag::from_bits(fcntl(o.as_raw_fd(), F_GETFD)?).unwrap();
fdopts.set(FdFlag::FD_CLOEXEC, true);
fcntl(o.as_raw_fd(), F_SETFD(fdopts))?;
}
crate::os::set_cloexec(&i, true)?;
crate::os::set_cloexec(&o, true)?;
Ok((i,o))

}
Expand Down
3 changes: 3 additions & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub enum Error {
#[error(transparent)]
IoError(#[from] std::io::Error),

#[error(transparent)]
NixErrnoError(#[from] nix::errno::Errno),

#[error(transparent)]
UrlParseError(#[from] url::ParseError),

Expand Down
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ pub use error::{Error, Result};

pub mod ext_axum;
pub mod ext_oauth2;
pub mod os;
pub mod ppid;
pub mod singleflight;
pub mod terminal;
Expand Down
10 changes: 10 additions & 0 deletions src/os.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
pub(crate) fn set_cloexec<T>(fd: &T, value: bool) -> crate::Result<()>
where
T: std::os::fd::AsRawFd,
{
use nix::fcntl::{fcntl, FdFlag, F_GETFD, F_SETFD};
let mut fdopts = FdFlag::from_bits(fcntl(fd.as_raw_fd(), F_GETFD)?).unwrap();
fdopts.set(FdFlag::FD_CLOEXEC, value);
fcntl(fd.as_raw_fd(), F_SETFD(fdopts))?;
Ok(())
}

0 comments on commit 6b5baee

Please sign in to comment.