Skip to content

Commit 82438f0

Browse files
russellbjohntmyers
authored andcommitted
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> Signed-off-by: John Myers <9696606+johntmyers@users.noreply.github.com>
1 parent 82977e2 commit 82438f0

1 file changed

Lines changed: 99 additions & 9 deletions

File tree

‎crates/openshell-sandbox/src/linux/seccomp_notify.rs‎

Lines changed: 99 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -381,9 +381,9 @@ fn probe_scalar_round_trip() -> io::Result<()> {
381381
Ok(unsafe { libc::syscall(libc::SYS_getppid) })
382382
});
383383

384-
let listener = receiver
385-
.recv()
386-
.map_err(|_| io::Error::other("notification launcher disappeared"))?;
384+
let Ok(listener) = receiver.recv() else {
385+
return Err(launcher_failure(launcher, "notification"));
386+
};
387387
let notification = match receive_probe_notification(&listener) {
388388
Ok(notification) => notification,
389389
Err(error) => {
@@ -440,9 +440,9 @@ fn probe_addfd_send() -> io::Result<()> {
440440
Ok(())
441441
});
442442

443-
let listener = receiver
444-
.recv()
445-
.map_err(|_| io::Error::other("ADDFD launcher disappeared"))?;
443+
let Ok(listener) = receiver.recv() else {
444+
return Err(launcher_failure(launcher, "ADDFD"));
445+
};
446446
let notification = match receive_probe_notification(&listener) {
447447
Ok(notification) => notification,
448448
Err(error) => {
@@ -542,9 +542,9 @@ fn probe_connected_sendto_fast_path() -> io::Result<()> {
542542
Ok(())
543543
});
544544

545-
let listener = receiver
546-
.recv()
547-
.map_err(|_| io::Error::other("sendto launcher disappeared"))?;
545+
let Ok(listener) = receiver.recv() else {
546+
return Err(launcher_failure(launcher, "sendto"));
547+
};
548548
let destination = match receive_probe_notification(&listener) {
549549
Ok(notification) => notification,
550550
Err(error) => {
@@ -586,6 +586,24 @@ fn probe_connected_sendto_fast_path() -> io::Result<()> {
586586
Ok(())
587587
}
588588

589+
/// Recover the real error from a probe launcher that exited before handing its
590+
/// listener to the broker.
591+
///
592+
/// The launcher owns the only `install_listener` result, so a failed install
593+
/// just closes the handover channel. Join the thread to report the kernel's
594+
/// refusal, including the errno that identifies a restrictive host. Return
595+
/// that error unchanged rather than reformatting it, so `raw_os_error` still
596+
/// reports the kernel's errno to callers that branch on it.
597+
fn launcher_failure<T>(launcher: thread::JoinHandle<io::Result<T>>, role: &str) -> io::Error {
598+
match launcher.join() {
599+
Ok(Err(error)) => error,
600+
Ok(Ok(_)) => io::Error::other(format!(
601+
"{role} launcher exited before sending its listener"
602+
)),
603+
Err(_) => io::Error::other(format!("{role} launcher panicked")),
604+
}
605+
}
606+
589607
fn receive_probe_notification(listener: &NotificationListener) -> io::Result<Notification> {
590608
use std::os::fd::AsFd as _;
591609

@@ -838,6 +856,7 @@ mod tests {
838856
use super::*;
839857

840858
const NONDUMPABLE_PROBE_CHILD: &str = "OPENSHELL_NONDUMPABLE_PROBE_CHILD";
859+
const REJECTED_INSTALL_PROBE_CHILD: &str = "OPENSHELL_REJECTED_INSTALL_PROBE_CHILD";
841860

842861
#[test]
843862
fn filter_rejects_empty_syscall_set() {
@@ -890,6 +909,77 @@ mod tests {
890909
assert!(status.success(), "nondumpable probe child failed: {status}");
891910
}
892911

912+
#[test]
913+
fn notification_probe_reports_rejected_listener_install() {
914+
if std::env::var_os(REJECTED_INSTALL_PROBE_CHILD).is_some() {
915+
// Model a host that refuses the listener install with EPERM.
916+
// The install happens on the launcher thread, so the probe must
917+
// join that thread to report the kernel's refusal instead of only
918+
// observing a closed channel.
919+
install_set_mode_filter_eperm_filter().expect("install SET_MODE_FILTER EPERM filter");
920+
let error = probe_notification_api().expect_err("rejected install must fail the probe");
921+
assert_eq!(error.kind(), io::ErrorKind::PermissionDenied);
922+
assert_eq!(error.raw_os_error(), Some(libc::EPERM));
923+
assert!(
924+
error.to_string().contains("os error 1"),
925+
"probe error must carry the install errno, got: {error}"
926+
);
927+
return;
928+
}
929+
930+
let executable = std::env::current_exe().expect("resolve test executable");
931+
let status = Command::new(executable)
932+
.arg("--exact")
933+
.arg("linux::seccomp_notify::tests::notification_probe_reports_rejected_listener_install")
934+
.arg("--nocapture")
935+
.env(REJECTED_INSTALL_PROBE_CHILD, "1")
936+
.status()
937+
.expect("run disposable rejected-install probe process");
938+
assert!(
939+
status.success(),
940+
"rejected-install probe child failed: {status}"
941+
);
942+
}
943+
944+
/// Refuse `seccomp(SECCOMP_SET_MODE_FILTER, ...)` with `EPERM` while the
945+
/// notification size query keeps working, so `install_listener` fails the
946+
/// way a restrictive host does.
947+
fn install_set_mode_filter_eperm_filter() -> io::Result<()> {
948+
const SECCOMP_RET_ERRNO: u32 = 0x0005_0000;
949+
let seccomp_number = u32::try_from(libc::SYS_seccomp)
950+
.map_err(|_| io::Error::other("syscall number does not fit u32"))?;
951+
let mut instructions = [
952+
stmt(BPF_LD_W_ABS, SECCOMP_DATA_NR_OFFSET),
953+
jump(BPF_JMP_JEQ_K, seccomp_number, 0, 3),
954+
stmt(BPF_LD_W_ABS, argument_word_offset(0, 0)),
955+
jump(BPF_JMP_JEQ_K, SECCOMP_SET_MODE_FILTER, 0, 1),
956+
stmt(BPF_RET_K, SECCOMP_RET_ERRNO | libc::EPERM.unsigned_abs()),
957+
stmt(BPF_RET_K, SECCOMP_RET_ALLOW),
958+
];
959+
let length = u16::try_from(instructions.len())
960+
.map_err(|_| io::Error::other("test seccomp filter is too large"))?;
961+
let mut program = libc::sock_fprog {
962+
len: length,
963+
filter: instructions.as_mut_ptr(),
964+
};
965+
set_no_new_privileges()?;
966+
// SAFETY: program references the complete live test filter. No flags
967+
// are required because the disposable process has one calling thread.
968+
let result = unsafe {
969+
libc::syscall(
970+
libc::SYS_seccomp,
971+
SECCOMP_SET_MODE_FILTER,
972+
0,
973+
std::ptr::addr_of_mut!(program),
974+
)
975+
};
976+
if result < 0 {
977+
Err(io::Error::last_os_error())
978+
} else {
979+
Ok(())
980+
}
981+
}
982+
893983
fn install_process_vm_enosys_filter() -> io::Result<()> {
894984
const SECCOMP_RET_ERRNO: u32 = 0x0005_0000;
895985
let syscall_number = |number: libc::c_long| {

0 commit comments

Comments
 (0)