Skip to content

Commit 834922c

Browse files
russellbrussellb
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 unchanged, so both the ErrorKind and raw_os_error survive to the qualification report. Apply this to all three probes. Signed-off-by: Russell Bryant <rbryant@redhat.com>
1 parent 021400b commit 834922c

1 file changed

Lines changed: 100 additions & 9 deletions

File tree

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

Lines changed: 100 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,25 @@ 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. Return that error
741+
/// unchanged rather than reformatting it, so `raw_os_error` still reports the
742+
/// kernel's errno to callers that branch on it.
743+
fn launcher_failure<T>(launcher: thread::JoinHandle<io::Result<T>>, role: &str) -> io::Error {
744+
match launcher.join() {
745+
Ok(Err(error)) => error,
746+
Ok(Ok(_)) => io::Error::other(format!(
747+
"{role} launcher exited before sending its listener"
748+
)),
749+
Err(_) => io::Error::other(format!("{role} launcher panicked")),
750+
}
751+
}
752+
734753
fn receive_probe_notification(listener: &NotificationListener) -> io::Result<Notification> {
735754
let mut descriptor = libc::pollfd {
736755
fd: listener.as_raw_fd(),
@@ -968,6 +987,7 @@ mod tests {
968987
use super::*;
969988

970989
const NONDUMPABLE_PROBE_CHILD: &str = "OPENSHELL_NONDUMPABLE_PROBE_CHILD";
990+
const REJECTED_INSTALL_PROBE_CHILD: &str = "OPENSHELL_REJECTED_INSTALL_PROBE_CHILD";
971991

972992
#[test]
973993
fn filter_rejects_empty_syscall_set() {
@@ -1020,6 +1040,77 @@ mod tests {
10201040
assert!(status.success(), "nondumpable probe child failed: {status}");
10211041
}
10221042

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

0 commit comments

Comments
 (0)