Skip to content

Commit 35c06aa

Browse files
committed
refactor(ffi): extract shared prepare() prologue for create/popen
sandlock_popen re-inlined almost the entire prologue of sandlock_create_with_runtime (null check, optional_name, read_argv, runtime build, with_name), risking drift: a future change to name validation or argv parsing had to be applied in two places. Extract prepare(policy, name, argv, argc, build_rt) -> Option<(Sandbox, Runtime, Vec<String>)> and have both entry points call it. Each keeps its own tail: create* drive sb.create(), popen drives sb.popen() + fd handoff. Behavior-preserving: out-fd reset and the fail-loud unknown-StdioMode rejection stay ahead of the shared prologue in popen. No ABI/header change (prepare is private). All sandlock-ffi tests green.
1 parent c043845 commit 35c06aa

1 file changed

Lines changed: 44 additions & 35 deletions

File tree

crates/sandlock-ffi/src/lib.rs

Lines changed: 44 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1021,41 +1021,61 @@ pub struct sandlock_handle_t {
10211021
runtime: tokio::runtime::Runtime,
10221022
}
10231023

1024-
/// Fork the child and install policy; the child is parked between policy
1025-
/// install and execve. Returns a live handle. Call `sandlock_start` to
1026-
/// release the child to execve.
1024+
/// Shared entry-point prologue for the create/popen family: validate the
1025+
/// pointers, parse the optional name and argv, build the requested runtime,
1026+
/// and apply the name to a cloned policy. Returns the owned
1027+
/// `(Sandbox, Runtime, args)` triple, or `None` on any invalid input or
1028+
/// runtime-build failure (callers map `None` to a null handle).
1029+
///
1030+
/// The tail stays with each caller: `sandlock_create*` drive `sb.create()`,
1031+
/// `sandlock_popen` drives `sb.popen()` + fd hand-off. Factoring the prologue
1032+
/// here keeps null/name/argv/runtime handling from drifting between them.
10271033
///
10281034
/// # Safety
10291035
/// `policy` must be a valid policy pointer. `name` may be NULL to
10301036
/// auto-generate a sandbox name, or a valid NUL-terminated string.
10311037
/// `argv` must point to `argc` C strings.
1032-
unsafe fn sandlock_create_with_runtime(
1038+
unsafe fn prepare(
10331039
policy: *const sandlock_sandbox_t,
10341040
name: *const c_char,
10351041
argv: *const *const c_char,
10361042
argc: c_uint,
10371043
build_rt: fn() -> Option<tokio::runtime::Runtime>,
1038-
) -> *mut sandlock_handle_t {
1044+
) -> Option<(Sandbox, tokio::runtime::Runtime, Vec<String>)> {
10391045
if policy.is_null() || argv.is_null() {
1040-
return ptr::null_mut();
1046+
return None;
10411047
}
10421048
let policy = &(*policy)._private;
1043-
let name = match optional_name(name) {
1044-
Ok(name) => name,
1045-
Err(_) => return ptr::null_mut(),
1046-
};
1049+
let name = optional_name(name).ok()?;
10471050
let args = read_argv(argv, argc);
1048-
let arg_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect();
1049-
1050-
let rt = match build_rt() {
1051-
Some(rt) => rt,
1052-
None => return ptr::null_mut(),
1053-
};
1054-
1055-
let mut sb = match name {
1051+
let rt = build_rt()?;
1052+
let sb = match name {
10561053
Some(ref n) => policy.clone().with_name(n.clone()),
10571054
None => policy.clone(),
10581055
};
1056+
Some((sb, rt, args))
1057+
}
1058+
1059+
/// Fork the child and install policy; the child is parked between policy
1060+
/// install and execve. Returns a live handle. Call `sandlock_start` to
1061+
/// release the child to execve.
1062+
///
1063+
/// # Safety
1064+
/// `policy` must be a valid policy pointer. `name` may be NULL to
1065+
/// auto-generate a sandbox name, or a valid NUL-terminated string.
1066+
/// `argv` must point to `argc` C strings.
1067+
unsafe fn sandlock_create_with_runtime(
1068+
policy: *const sandlock_sandbox_t,
1069+
name: *const c_char,
1070+
argv: *const *const c_char,
1071+
argc: c_uint,
1072+
build_rt: fn() -> Option<tokio::runtime::Runtime>,
1073+
) -> *mut sandlock_handle_t {
1074+
let (mut sb, rt, args) = match prepare(policy, name, argv, argc, build_rt) {
1075+
Some(t) => t,
1076+
None => return ptr::null_mut(),
1077+
};
1078+
let arg_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect();
10591079

10601080
if !matches!(block_on_runtime(&rt, sb.create(&arg_refs)), Some(Ok(()))) {
10611081
return ptr::null_mut();
@@ -1169,9 +1189,6 @@ pub unsafe extern "C" fn sandlock_popen(
11691189
*out_stderr_fd = -1;
11701190
}
11711191

1172-
if policy.is_null() || argv.is_null() {
1173-
return ptr::null_mut();
1174-
}
11751192
let (stdin, stdout, stderr) = match (
11761193
stdio_mode_from_raw(stdin_mode),
11771194
stdio_mode_from_raw(stdout_mode),
@@ -1189,22 +1206,14 @@ pub unsafe extern "C" fn sandlock_popen(
11891206
return ptr::null_mut();
11901207
}
11911208
};
1192-
let policy = &(*policy)._private;
1193-
let name = match optional_name(name) {
1194-
Ok(name) => name,
1195-
Err(_) => return ptr::null_mut(),
1196-
};
1197-
let args = read_argv(argv, argc);
1198-
let arg_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect();
1199-
1200-
let rt = match build_live_runtime() {
1201-
Some(rt) => rt,
1209+
// Shared prologue (null/name/argv/runtime) via `prepare`; popen keeps its own
1210+
// `sb.popen()` + fd-handoff tail. build_live_runtime: the multi-threaded
1211+
// runtime keeps the seccomp supervisor pumping during the caller's blocking IO.
1212+
let (mut sb, rt, args) = match prepare(policy, name, argv, argc, build_live_runtime) {
1213+
Some(t) => t,
12021214
None => return ptr::null_mut(),
12031215
};
1204-
let mut sb = match name {
1205-
Some(ref n) => policy.clone().with_name(n.clone()),
1206-
None => policy.clone(),
1207-
};
1216+
let arg_refs: Vec<&str> = args.iter().map(|s| s.as_str()).collect();
12081217

12091218
// popen returns a Process borrowing `sb`; take the owned fds and let it drop
12101219
// so `sb` can move into the handle. The process keeps running (owned by `sb`).

0 commit comments

Comments
 (0)