Skip to content

Commit 82f3522

Browse files
committed
fix(sandbox): report the real seccomp probe launcher error
The seccomp notification probes install their listener on a launcher thread and receive it over a channel. When the install fails, the launcher drops the sender and exits, so the broker only sees a closed channel and returns a generic "launcher disappeared" error. The thread is never joined, so the kernel's errno is discarded. That collapses several distinct failures into one opaque string: a notification struct size mismatch, a failed PR_SET_NO_NEW_PRIVS, and any non-EINVAL SECCOMP_SET_MODE_FILTER rejection all look identical to a pre-5.19 kernel refusing WAIT_KILLABLE_RECV, which the install path already retries without the flag. Join the launcher on the recv() failure path and propagate its error, preserving the ErrorKind so the errno survives to the qualification report. Apply this to all three probes. Signed-off-by: Russell Bryant <rbryant@redhat.com>
1 parent 021400b commit 82f3522

1 file changed

Lines changed: 97 additions & 9 deletions

File tree

‎crates/openshell-isolation-interface/src/linux/seccomp_notify.rs‎

Lines changed: 97 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -485,9 +485,9 @@ fn probe_scalar_round_trip() -> io::Result<bool> {
485485
Ok(unsafe { libc::syscall(libc::SYS_getppid) })
486486
});
487487

488-
let (listener, wait_killable) = receiver
489-
.recv()
490-
.map_err(|_| io::Error::other("notification launcher disappeared"))?;
488+
let Ok((listener, wait_killable)) = receiver.recv() else {
489+
return Err(launcher_failure(launcher, "notification"));
490+
};
491491
let notification = match receive_probe_notification(&listener) {
492492
Ok(notification) => notification,
493493
Err(error) => {
@@ -556,9 +556,9 @@ fn probe_addfd_send() -> io::Result<()> {
556556
Ok(())
557557
});
558558

559-
let listener = receiver
560-
.recv()
561-
.map_err(|_| io::Error::other("ADDFD launcher disappeared"))?;
559+
let Ok(listener) = receiver.recv() else {
560+
return Err(launcher_failure(launcher, "ADDFD"));
561+
};
562562
let notification = match receive_probe_notification(&listener) {
563563
Ok(notification) => notification,
564564
Err(error) => {
@@ -687,9 +687,9 @@ fn probe_connected_sendto_fast_path() -> io::Result<()> {
687687
Ok(())
688688
});
689689

690-
let listener = receiver
691-
.recv()
692-
.map_err(|_| io::Error::other("sendto launcher disappeared"))?;
690+
let Ok(listener) = receiver.recv() else {
691+
return Err(launcher_failure(launcher, "sendto"));
692+
};
693693
let destination = match receive_probe_notification(&listener) {
694694
Ok(notification) => notification,
695695
Err(error) => {
@@ -731,6 +731,23 @@ fn probe_connected_sendto_fast_path() -> io::Result<()> {
731731
Ok(())
732732
}
733733

734+
/// Recover the real error from a probe launcher that exited before handing its
735+
/// listener to the broker.
736+
///
737+
/// The launcher owns the only `install_listener` result, so a failed install
738+
/// just closes the handover channel. Join the thread to report the kernel's
739+
/// refusal — the errno distinguishes a pre-5.19 `WAIT_KILLABLE_RECV` rejection
740+
/// from a restrictive host that the fallback cannot retry.
741+
fn launcher_failure<T>(launcher: thread::JoinHandle<io::Result<T>>, role: &str) -> io::Error {
742+
match launcher.join() {
743+
Ok(Err(error)) => io::Error::new(error.kind(), format!("{role} launcher failed: {error}")),
744+
Ok(Ok(_)) => io::Error::other(format!(
745+
"{role} launcher exited before sending its listener"
746+
)),
747+
Err(_) => io::Error::other(format!("{role} launcher panicked")),
748+
}
749+
}
750+
734751
fn receive_probe_notification(listener: &NotificationListener) -> io::Result<Notification> {
735752
let mut descriptor = libc::pollfd {
736753
fd: listener.as_raw_fd(),
@@ -968,6 +985,7 @@ mod tests {
968985
use super::*;
969986

970987
const NONDUMPABLE_PROBE_CHILD: &str = "OPENSHELL_NONDUMPABLE_PROBE_CHILD";
988+
const REJECTED_INSTALL_PROBE_CHILD: &str = "OPENSHELL_REJECTED_INSTALL_PROBE_CHILD";
971989

972990
#[test]
973991
fn filter_rejects_empty_syscall_set() {
@@ -1020,6 +1038,76 @@ mod tests {
10201038
assert!(status.success(), "nondumpable probe child failed: {status}");
10211039
}
10221040

1041+
#[test]
1042+
fn notification_probe_reports_rejected_listener_install() {
1043+
if std::env::var_os(REJECTED_INSTALL_PROBE_CHILD).is_some() {
1044+
// Model a host that refuses the listener install with an errno the
1045+
// `WAIT_KILLABLE_RECV` fallback cannot retry. The install happens on
1046+
// the launcher thread, so the probe must join that thread to report
1047+
// the kernel's refusal instead of only observing a closed channel.
1048+
install_set_mode_filter_eperm_filter().expect("install SET_MODE_FILTER EPERM filter");
1049+
let error = probe_notification_api().expect_err("rejected install must fail the probe");
1050+
assert_eq!(error.kind(), io::ErrorKind::PermissionDenied);
1051+
assert!(
1052+
error.to_string().contains("os error 1"),
1053+
"probe error must carry the install errno, got: {error}"
1054+
);
1055+
return;
1056+
}
1057+
1058+
let executable = std::env::current_exe().expect("resolve test executable");
1059+
let status = Command::new(executable)
1060+
.arg("--exact")
1061+
.arg("linux::seccomp_notify::tests::notification_probe_reports_rejected_listener_install")
1062+
.arg("--nocapture")
1063+
.env(REJECTED_INSTALL_PROBE_CHILD, "1")
1064+
.status()
1065+
.expect("run disposable rejected-install probe process");
1066+
assert!(
1067+
status.success(),
1068+
"rejected-install probe child failed: {status}"
1069+
);
1070+
}
1071+
1072+
/// Refuse `seccomp(SECCOMP_SET_MODE_FILTER, ...)` with `EPERM` while the
1073+
/// notification size query keeps working, so `install_listener` fails the
1074+
/// way a restrictive host does rather than the way a pre-5.19 kernel does.
1075+
fn install_set_mode_filter_eperm_filter() -> io::Result<()> {
1076+
const SECCOMP_RET_ERRNO: u32 = 0x0005_0000;
1077+
let seccomp_number = u32::try_from(libc::SYS_seccomp)
1078+
.map_err(|_| io::Error::other("syscall number does not fit u32"))?;
1079+
let mut instructions = [
1080+
stmt(BPF_LD_W_ABS, SECCOMP_DATA_NR_OFFSET),
1081+
jump(BPF_JMP_JEQ_K, seccomp_number, 0, 3),
1082+
stmt(BPF_LD_W_ABS, argument_word_offset(0, 0)),
1083+
jump(BPF_JMP_JEQ_K, SECCOMP_SET_MODE_FILTER, 0, 1),
1084+
stmt(BPF_RET_K, SECCOMP_RET_ERRNO | libc::EPERM.unsigned_abs()),
1085+
stmt(BPF_RET_K, SECCOMP_RET_ALLOW),
1086+
];
1087+
let length = u16::try_from(instructions.len())
1088+
.map_err(|_| io::Error::other("test seccomp filter is too large"))?;
1089+
let mut program = libc::sock_fprog {
1090+
len: length,
1091+
filter: instructions.as_mut_ptr(),
1092+
};
1093+
set_no_new_privileges()?;
1094+
// SAFETY: program references the complete live test filter. No flags
1095+
// are required because the disposable process has one calling thread.
1096+
let result = unsafe {
1097+
libc::syscall(
1098+
libc::SYS_seccomp,
1099+
SECCOMP_SET_MODE_FILTER,
1100+
0,
1101+
std::ptr::addr_of_mut!(program),
1102+
)
1103+
};
1104+
if result < 0 {
1105+
Err(io::Error::last_os_error())
1106+
} else {
1107+
Ok(())
1108+
}
1109+
}
1110+
10231111
fn install_process_vm_enosys_filter() -> io::Result<()> {
10241112
const SECCOMP_RET_ERRNO: u32 = 0x0005_0000;
10251113
let syscall_number = |number: libc::c_long| {

0 commit comments

Comments
 (0)