From 693fda41b1c3a45345c80bcce9090ea405c798c3 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Mon, 1 Jun 2026 15:48:10 -0700 Subject: [PATCH] chroot: mediate path-based xattr syscalls under fs_mount Signed-off-by: Cong Wang --- crates/sandlock-core/src/chroot/dispatch.rs | 193 +++++++++++++++++++ crates/sandlock-core/src/context.rs | 12 ++ crates/sandlock-core/src/seccomp/dispatch.rs | 11 ++ python/tests/test_fs_mount.py | 67 ++++++- tests/rootfs-helper.c | 47 +++++ 5 files changed, 329 insertions(+), 1 deletion(-) diff --git a/crates/sandlock-core/src/chroot/dispatch.rs b/crates/sandlock-core/src/chroot/dispatch.rs index 6592dbe..8af85bd 100644 --- a/crates/sandlock-core/src/chroot/dispatch.rs +++ b/crates/sandlock-core/src/chroot/dispatch.rs @@ -1263,6 +1263,199 @@ pub(crate) async fn handle_chroot_readlink( write_target(display.to_string_lossy().as_bytes()) } +// ============================================================ +// xattr handler +// ============================================================ + +/// The four path-based xattr operations. Each maps onto a pair of libc +/// syscalls (follow / no-follow) that differ only in their `l` prefix. +#[derive(Clone, Copy)] +enum XattrOp { + /// `getxattr(path, name, value, size)` — copy a value out to the child. + Get, + /// `setxattr(path, name, value, size, flags)` — copy a value in. + Set, + /// `listxattr(path, list, size)` — copy the name list out. + List, + /// `removexattr(path, name)`. + Remove, +} + +/// Classify a syscall as a path-based xattr op plus whether it follows the +/// final symlink. Returns `None` for anything else. +fn classify_xattr(nr: i64) -> Option<(XattrOp, bool)> { + Some(match nr { + libc::SYS_getxattr => (XattrOp::Get, true), + libc::SYS_lgetxattr => (XattrOp::Get, false), + libc::SYS_setxattr => (XattrOp::Set, true), + libc::SYS_lsetxattr => (XattrOp::Set, false), + libc::SYS_listxattr => (XattrOp::List, true), + libc::SYS_llistxattr => (XattrOp::List, false), + libc::SYS_removexattr => (XattrOp::Remove, true), + libc::SYS_lremovexattr => (XattrOp::Remove, false), + _ => return None, + }) +} + +/// Kernel ceiling for an xattr value (`XATTR_SIZE_MAX`) and name list +/// (`XATTR_LIST_MAX`). The supervisor never needs a larger buffer, so +/// clamping the child's requested size here both bounds our allocation and +/// can never cause a spurious `ERANGE` (a real result never exceeds it). +const XATTR_MAX: usize = 65536; + +/// Shared read path for `getxattr`/`listxattr`: run the syscall on the +/// rewritten host path into a supervisor buffer, then copy the result back +/// into the child's buffer. `name` is `Some` for getxattr, `None` for +/// listxattr. `buf_idx`/`size_idx` are the child arg positions of the output +/// buffer pointer and its capacity. +fn xattr_read( + notif: &SeccompNotif, + notif_fd: RawFd, + c_path: &CString, + name: Option<&CString>, + follow: bool, + buf_idx: usize, + size_idx: usize, +) -> NotifAction { + let buf_addr = notif.data.args[buf_idx]; + let size = (notif.data.args[size_idx] as usize).min(XATTR_MAX); + let mut buf = vec![0u8; size]; + // size == 0 is a probe for the needed length — pass NULL so the kernel + // just reports the size without touching a buffer. + let buf_ptr = if size == 0 { + std::ptr::null_mut() + } else { + buf.as_mut_ptr() as *mut libc::c_void + }; + // getxattr takes a name (4 args), listxattr does not (3 args). + let ret = unsafe { + match name { + Some(n) if follow => { + libc::syscall(libc::SYS_getxattr, c_path.as_ptr(), n.as_ptr(), buf_ptr, size) + } + Some(n) => { + libc::syscall(libc::SYS_lgetxattr, c_path.as_ptr(), n.as_ptr(), buf_ptr, size) + } + None if follow => libc::syscall(libc::SYS_listxattr, c_path.as_ptr(), buf_ptr, size), + None => libc::syscall(libc::SYS_llistxattr, c_path.as_ptr(), buf_ptr, size), + } + }; + if ret < 0 { + return NotifAction::Errno(last_errno(libc::ENODATA)); + } + // size == 0 returns the needed length without writing anything back. + if size > 0 && ret as usize > 0 { + let written = write_child_mem(notif_fd, notif.id, notif.pid, buf_addr, &buf[..ret as usize]); + if written.is_err() { + return NotifAction::Errno(libc::EFAULT); + } + } + NotifAction::ReturnValue(ret) +} + +/// Mediate the path-based xattr syscalls. Without this, a `getxattr` on a +/// path under an `fs_mount`/chroot resolves against the empty real mount +/// point and returns `ENOENT`, even though `statx` on the same path is +/// rewritten correctly (issue #84). +pub(crate) async fn handle_chroot_xattr( + notif: &SeccompNotif, + _chroot_state: &Arc>, + cow_state: &Arc>, + notif_fd: RawFd, + ctx: &ChrootCtx<'_>, +) -> NotifAction { + let (op, follow) = match classify_xattr(notif.data.nr as i64) { + Some(x) => x, + None => return NotifAction::Continue, + }; + + // The path is always arg 0; xattr syscalls have no dirfd, so relative + // paths resolve against the child's cwd (AT_FDCWD). + let path = match read_path(notif, notif.data.args[0], notif_fd) { + Some(p) if !p.is_empty() => p, + _ => return NotifAction::Continue, + }; + let (host_path, vp) = + match resolve_chroot_path_existing(notif, libc::AT_FDCWD as i64, &path, ctx) { + Some(r) => r, + None => return NotifAction::Errno(libc::ENOENT), + }; + + let writing = matches!(op, XattrOp::Set | XattrOp::Remove); + let allowed = if writing { ctx.can_write(&vp) } else { ctx.can_read(&vp) }; + if !allowed { + return NotifAction::Errno(libc::EACCES); + } + + let real_path = match cow_resolve(cow_state, &host_path).await { + Ok(p) => p, + Err(a) => return a, + }; + let c_path = match path_cstr(&real_path, libc::ENOENT) { + Ok(c) => c, + Err(a) => return a, + }; + + // Read the attribute name (arg 1) for the ops that carry one. + let read_name = || -> Result { + let n = read_path(notif, notif.data.args[1], notif_fd) + .ok_or(NotifAction::Errno(libc::EFAULT))?; + CString::new(n).map_err(|_| NotifAction::Errno(libc::EINVAL)) + }; + + match op { + XattrOp::Get => { + let name = match read_name() { + Ok(n) => n, + Err(a) => return a, + }; + xattr_read(notif, notif_fd, &c_path, Some(&name), follow, 2, 3) + } + XattrOp::List => xattr_read(notif, notif_fd, &c_path, None, follow, 1, 2), + XattrOp::Set => { + let name = match read_name() { + Ok(n) => n, + Err(a) => return a, + }; + let size = notif.data.args[3] as usize; + let flags = notif.data.args[4] as i32; + let value = match read_child_mem(notif_fd, notif.id, notif.pid, notif.data.args[2], size) { + Ok(v) => v, + Err(_) => return NotifAction::Errno(libc::EFAULT), + }; + let nr = if follow { libc::SYS_setxattr } else { libc::SYS_lsetxattr }; + let ret = unsafe { + libc::syscall( + nr, + c_path.as_ptr(), + name.as_ptr(), + value.as_ptr() as *const libc::c_void, + size, + flags, + ) + }; + if ret < 0 { + NotifAction::Errno(last_errno(libc::EIO)) + } else { + NotifAction::ReturnValue(0) + } + } + XattrOp::Remove => { + let name = match read_name() { + Ok(n) => n, + Err(a) => return a, + }; + let nr = if follow { libc::SYS_removexattr } else { libc::SYS_lremovexattr }; + let ret = unsafe { libc::syscall(nr, c_path.as_ptr(), name.as_ptr()) }; + if ret < 0 { + NotifAction::Errno(last_errno(libc::EIO)) + } else { + NotifAction::ReturnValue(0) + } + } + } +} + // ============================================================ // getdents handler // ============================================================ diff --git a/crates/sandlock-core/src/context.rs b/crates/sandlock-core/src/context.rs index fff11e2..2fa442b 100644 --- a/crates/sandlock-core/src/context.rs +++ b/crates/sandlock-core/src/context.rs @@ -298,6 +298,18 @@ fn chroot_path_syscalls() -> Vec { libc::SYS_getcwd, libc::SYS_statfs, libc::SYS_utimensat, + // xattr family (path-based): must be mediated so that paths under an + // fs_mount/chroot resolve to the real backing file rather than the + // empty mount point (issue #84). The fd-based f*xattr variants need + // no mediation — their fd already points at the resolved file. + libc::SYS_getxattr, + libc::SYS_lgetxattr, + libc::SYS_setxattr, + libc::SYS_lsetxattr, + libc::SYS_listxattr, + libc::SYS_llistxattr, + libc::SYS_removexattr, + libc::SYS_lremovexattr, ]; v.extend( [ diff --git a/crates/sandlock-core/src/seccomp/dispatch.rs b/crates/sandlock-core/src/seccomp/dispatch.rs index 4f3b6ba..c1ab630 100644 --- a/crates/sandlock-core/src/seccomp/dispatch.rs +++ b/crates/sandlock-core/src/seccomp/dispatch.rs @@ -900,6 +900,17 @@ fn register_chroot_handlers( crate::chroot::dispatch::handle_chroot_statfs)); table.register(libc::SYS_utimensat as i64, chroot_handler!(policy, crate::chroot::dispatch::handle_chroot_utimensat)); + + // xattr family (path-based) — get/set/list/remove and their l* variants + for &nr in &[ + libc::SYS_getxattr, libc::SYS_lgetxattr, + libc::SYS_setxattr, libc::SYS_lsetxattr, + libc::SYS_listxattr, libc::SYS_llistxattr, + libc::SYS_removexattr, libc::SYS_lremovexattr, + ] { + table.register(nr, chroot_handler!(policy, + crate::chroot::dispatch::handle_chroot_xattr)); + } } // ============================================================ diff --git a/python/tests/test_fs_mount.py b/python/tests/test_fs_mount.py index 8fe4d7c..c43ffdc 100644 --- a/python/tests/test_fs_mount.py +++ b/python/tests/test_fs_mount.py @@ -32,7 +32,8 @@ def rootfs(tmp_path): for name in ("sh", "cat", "echo", "ls", "pwd", "readlink", "stat", "mkdir", "rmdir", "chmod", "ln", "rm", "mv", "true", - "false", "write", "access"): + "false", "write", "access", "getxattr", "setxattr", + "listxattr"): link = tmp_path / "usr" / "bin" / name if not link.exists(): os.symlink("rootfs-helper", link) @@ -98,6 +99,70 @@ def test_fs_mount_ls_directory(self, rootfs, tmp_path): assert b"aaa.txt" in result.stdout assert b"bbb.txt" in result.stdout + def test_fs_mount_getxattr(self, rootfs, tmp_path): + """getxattr on a mounted file must resolve to the mount source (issue #84). + + The *xattr syscalls were not mediated, so a path-based getxattr + resolved against the empty real mount point and returned ENOENT + even though statx on the same path succeeded. + """ + work_dir = tmp_path / "hostwork" + work_dir.mkdir() + target = work_dir / "f.txt" + target.write_text("hi") + try: + os.setxattr(target, "user.greeting", b"hello") + except OSError: + pytest.skip("filesystem does not support user xattrs") + + policy = _mount_policy(rootfs, work_dir) + result = policy.run(["getxattr", "/work/f.txt", "user.greeting"]) + assert result.success, f"failed: {result.stderr.decode(errors='replace')}" + assert b"OK hello" in result.stdout + + def test_fs_mount_listxattr(self, rootfs, tmp_path): + """listxattr on a mounted file must resolve to the mount source (issue #84).""" + work_dir = tmp_path / "hostwork" + work_dir.mkdir() + target = work_dir / "f.txt" + target.write_text("hi") + try: + os.setxattr(target, "user.alpha", b"1") + except OSError: + pytest.skip("filesystem does not support user xattrs") + + policy = _mount_policy(rootfs, work_dir) + result = policy.run(["listxattr", "/work/f.txt"]) + assert result.success, f"failed: {result.stderr.decode(errors='replace')}" + assert b"user.alpha" in result.stdout + + def test_fs_mount_setxattr(self, rootfs, tmp_path): + """setxattr on a mounted file must write through to the mount source (issue #84).""" + work_dir = tmp_path / "hostwork" + work_dir.mkdir() + target = work_dir / "f.txt" + target.write_text("hi") + # Probe host xattr support up front so an unsupported fs skips cleanly. + try: + os.setxattr(target, "user.probe", b"x") + os.removexattr(target, "user.probe") + except OSError: + pytest.skip("filesystem does not support user xattrs") + + # /work must be writable for setxattr; _mount_policy maps it + # readable-only, so build a writable policy here. + policy = Sandbox( + chroot=str(rootfs), + fs_mount={"/work": str(work_dir)}, + fs_readable=list(_FS_READABLE), + fs_writable=["/work"], + clean_env=True, + env={"PATH": "/bin:/usr/bin"}, + ) + result = policy.run(["setxattr", "/work/f.txt", "user.color", "blue"]) + assert result.success, f"failed: {result.stderr.decode(errors='replace')}" + assert os.getxattr(target, "user.color") == b"blue" + def test_fs_mount_cwd(self, rootfs, tmp_path): """Set cwd=/work, verify cat with relative path works.""" work_dir = tmp_path / "hostwork" diff --git a/tests/rootfs-helper.c b/tests/rootfs-helper.c index 68e5135..c04acf4 100644 --- a/tests/rootfs-helper.c +++ b/tests/rootfs-helper.c @@ -19,6 +19,7 @@ #include #include #include +#include #include /* ── echo ───────────────────────────────────────────────────── */ @@ -222,6 +223,49 @@ static int cmd_access(int argc, char **argv) { return 0; } +/* ── getxattr (non-standard: print an extended attribute value) ── */ +static int cmd_getxattr(int argc, char **argv) { + if (argc < 2) { fprintf(stderr, "getxattr: usage: getxattr \n"); return 1; } + char buf[4096]; + ssize_t n = getxattr(argv[0], argv[1], buf, sizeof(buf)); + if (n < 0) { + printf("ERR %d\n", errno); + return 1; + } + fputs("OK ", stdout); + fflush(stdout); + write(STDOUT_FILENO, buf, n); + putchar('\n'); + return 0; +} + +/* ── setxattr (non-standard: set an extended attribute) ──────── */ +static int cmd_setxattr(int argc, char **argv) { + if (argc < 3) { fprintf(stderr, "setxattr: usage: setxattr \n"); return 1; } + if (setxattr(argv[0], argv[1], argv[2], strlen(argv[2]), 0) < 0) { + printf("ERR %d\n", errno); + return 1; + } + printf("OK\n"); + return 0; +} + +/* ── listxattr (non-standard: print attribute names, NUL -> ',') ─ */ +static int cmd_listxattr(int argc, char **argv) { + if (argc < 1) { fprintf(stderr, "listxattr: usage: listxattr \n"); return 1; } + char buf[4096]; + ssize_t n = listxattr(argv[0], buf, sizeof(buf)); + if (n < 0) { + printf("ERR %d\n", errno); + return 1; + } + fputs("OK ", stdout); + for (ssize_t i = 0; i < n; i++) + putchar(buf[i] ? buf[i] : ','); + putchar('\n'); + return 0; +} + /* ── legacy syscall wrappers (for testing chroot handler) ──── */ #if defined(SYS_stat) && defined(SYS_lstat) && defined(SYS_open) && \ @@ -524,6 +568,9 @@ static int dispatch(const char *cmd, int argc, char **argv) { return 1; } if (strcmp(cmd, "access") == 0) return cmd_access(argc, argv); + if (strcmp(cmd, "getxattr") == 0) return cmd_getxattr(argc, argv); + if (strcmp(cmd, "setxattr") == 0) return cmd_setxattr(argc, argv); + if (strcmp(cmd, "listxattr") == 0) return cmd_listxattr(argc, argv); if (strcmp(cmd, "fstat-fd") == 0) return cmd_fstat_fd(argc, argv); if (strcmp(cmd, "true") == 0) return 0; if (strcmp(cmd, "false") == 0) return 1;