From bf8d5afc3e9330e1e61918fdbaf4f322e55a8807 Mon Sep 17 00:00:00 2001 From: Robert McConnell Date: Fri, 23 Oct 2026 13:54:38 +0000 Subject: [PATCH] mnt: apply readonly recursively to new-api bind mounts --- Makefile | 10 +- mnt_newapi.cc | 34 +++++-- tests/newapi_recursive_readonly.sh | 153 +++++++++++++++++++++++++++++ 3 files changed, 188 insertions(+), 9 deletions(-) create mode 100755 tests/newapi_recursive_readonly.sh diff --git a/Makefile b/Makefile index 68c308b..d1b33bf 100644 --- a/Makefile +++ b/Makefile @@ -159,8 +159,14 @@ test-cmdline: $(BIN) $(call run_test, ./nsjail --rlimit_cpu 1.5 --help > /dev/null, 255) $(call run_test, ./nsjail --time_limit 0x10 --cgroup_mem_swap_max=-1 --user 1000:1000:1 --help > /dev/null, 0) -.PHONY: test -test: $(BIN) $(TEST_BINS) test-cmdline +.PHONY: test test-newapi-recursive-readonly +# Requires Linux >= 6.3 and mount privileges. Exit 77 is a capability skip. +test-newapi-recursive-readonly: $(BIN) + @./tests/newapi_recursive_readonly.sh ./nsjail; ret=$$?; \ + if [ "$$ret" -eq 77 ]; then echo "SKIP: new mount API integration test"; \ + elif [ "$$ret" -ne 0 ]; then exit "$$ret"; fi + +test: $(BIN) $(TEST_BINS) test-cmdline test-newapi-recursive-readonly $(call run_test, ./tests/nstun_buffer_budget_test, 0) $(call run_test, ./tests/nstun_policy_test, 0) $(call run_test, ./tests/nstun_ip_test, 0) diff --git a/mnt_newapi.cc b/mnt_newapi.cc index 6f142ef..1c33ccb 100644 --- a/mnt_newapi.cc +++ b/mnt_newapi.cc @@ -492,15 +492,35 @@ static bool doBindMountAt(mount_t* mpt, int parent_fd, const char* basename) { } /* - * Apply non-RO flags now; RO applied later via remount. - * When the bind is recursive (MS_REC), open_tree above cloned the whole - * subtree (AT_RECURSIVE), so the nosuid/nodev/noexec attributes must be - * applied recursively as well -- otherwise nested submounts of the source - * keep their original suid/dev/exec-permitting attributes inside the jail. - * This mirrors the recursive read-only pass done later on the root. + * Restrict cloned submounts now, but leave the top bind writable during + * staging if its source was writable. Later mounts may need to create + * targets beneath it. remountPt() makes the top read-only after staging, + * while the original nested mounts remain read-only. */ - if (!applyMountFlags(mnt_fd, mpt->flags & ~MS_RDONLY, true, (mpt->flags & MS_REC) != 0)) { + const bool recursive_ro = (mpt->flags & (MS_REC | MS_RDONLY)) == (MS_REC | MS_RDONLY); + struct statvfs original_vfs = {}; + if (recursive_ro && fstatvfs(mnt_fd, &original_vfs) == -1) { + PLOG_W("fstatvfs(bind mount fd) for '%s'", basename); + close(mnt_fd); + return false; + } + const uintptr_t early_flags = recursive_ro ? mpt->flags : (mpt->flags & ~MS_RDONLY); + if (!applyMountFlags(mnt_fd, early_flags, true, (mpt->flags & MS_REC) != 0)) { LOG_W("Failed to apply mount flags to '%s'", basename); + if (recursive_ro) { + close(mnt_fd); + return false; + } + } + if (recursive_ro && !(original_vfs.f_flag & ST_RDONLY)) { + struct mount_attr writable_top = {}; + writable_top.attr_clr = MOUNT_ATTR_RDONLY; + if (util::syscall(__NR_mount_setattr, (uintptr_t)mnt_fd, (uintptr_t)"", + (uintptr_t)AT_EMPTY_PATH, (uintptr_t)&writable_top, sizeof(writable_top)) < 0) { + PLOG_W("mount_setattr(clear top-level readonly) for '%s'", basename); + close(mnt_fd); + return false; + } } if (util::syscall(__NR_move_mount, (uintptr_t)mnt_fd, (uintptr_t)"", (uintptr_t)parent_fd, diff --git a/tests/newapi_recursive_readonly.sh b/tests/newapi_recursive_readonly.sh new file mode 100755 index 0000000..a2c88df --- /dev/null +++ b/tests/newapi_recursive_readonly.sh @@ -0,0 +1,153 @@ +#!/bin/sh +# Integration regression for the new mount API's recursive read-only bind mounts. +# Requires Linux >= 6.3, root/CAP_SYS_ADMIN, and a usable mount namespace. +# Run directly or through: make test-newapi-recursive-readonly +set -eu + +if [ "${NSJAIL_NEWAPI_MOUNT_NS:-}" != 1 ]; then + if [ "$(uname -s)" != Linux ] || [ "$(id -u)" -ne 0 ] || + ! unshare --mount --propagation private true 2>/dev/null; then + echo "SKIP: requires Linux, root/CAP_SYS_ADMIN and mount namespaces" + exit 77 + fi + kernel_major=$(uname -r | cut -d. -f1) + kernel_minor=$(uname -r | cut -d. -f2) + if [ "$kernel_major" -lt 6 ] || + { [ "$kernel_major" -eq 6 ] && [ "$kernel_minor" -lt 3 ]; }; then + echo "SKIP: requires Linux >= 6.3" + exit 77 + fi + exec unshare --mount --propagation private env NSJAIL_NEWAPI_MOUNT_NS=1 "$0" "$@" +fi + +nsjail_bin=${1:-./nsjail} +src=$(mktemp -d "${TMPDIR:-/tmp}/nsjail-newapi-ro.XXXXXXXX") +child=$(mktemp -d "${TMPDIR:-/tmp}/nsjail-newapi-child.XXXXXXXX") +cleanup() { + umount "$child/nested" 2>/dev/null || : + umount "$src/nested" 2>/dev/null || : + rm -rf "$child" "$src" +} +trap cleanup EXIT HUP INT TERM + +mkdir "$src/nested" +printf 'ordinary\n' > "$src/ordinary-file" +mount -t tmpfs -o size=4m tmpfs "$src/nested" +printf 'nested\n' > "$src/nested/nested-file" +mountpoint -q "$src/nested" + +# --rw makes is_root_rw=true; --experimental_mnt=new forces the new API. +# Disable unrelated namespaces so the test isolates mount behavior. +for mode in ro rw; do + if [ "$mode" = ro ]; then + bind_arg=-R + else + bind_arg=-B + fi + + "$nsjail_bin" --experimental_mnt=new -q -Mo --rw \ + -R /usr:/usr -s /usr/lib:/lib -s /usr/lib64:/lib64 \ + --disable_clone_newuser --disable_clone_newnet \ + --disable_clone_newcgroup \ + "$bind_arg" "$src:/tmp/nsjail-newapi-check" -- \ + /usr/bin/sh -c ' + set -eu + dest=/tmp/nsjail-newapi-check + mode=$1 + test -f "$dest/ordinary-file" + test -f "$dest/nested/nested-file" + touch /tmp/nsjail-newapi-root-writable + rm /tmp/nsjail-newapi-root-writable + + for target in "$dest" "$dest/nested"; do + opts=$(findmnt -n -o VFS-OPTIONS -T "$target") + echo "$mode $target $opts" + case ",$opts," in + *,ro,*) state=ro ;; + *,rw,*) state=rw ;; + *) echo "FAIL: no ro/rw VFS flag for $target" >&2; exit 1 ;; + esac + if [ "$state" != "$mode" ]; then + echo "FAIL: $target is $state, expected $mode" >&2 + exit 1 + fi + if touch "$target/nsjail-369-probe" 2>/tmp/nsjail-newapi-probe.err; then + if [ "$mode" = ro ]; then + echo "FAIL: write succeeded on read-only $target" >&2 + exit 1 + fi + rm "$target/nsjail-369-probe" + elif [ "$mode" = rw ]; then + echo "FAIL: write rejected on writable $target" >&2 + exit 1 + fi + done + ' sh "$mode" +done + +# A later writable recursive bind must be stageable beneath a read-only bind. +# Its independently cloned nested mounts must remain writable (not inherited +# from the earlier read-only parent). The destination intentionally does not +# exist in the source before nsjail constructs the mount tree. +mkdir "$child/nested" +mount -t tmpfs -o size=4m tmpfs "$child/nested" +printf 'child\n' > "$child/nested/nested-file" +test ! -e "$src/newchild" +"$nsjail_bin" --experimental_mnt=new -q -Mo --rw \ + -R /usr:/usr -s /usr/lib:/lib -s /usr/lib64:/lib64 \ + --disable_clone_newuser --disable_clone_newnet \ + --disable_clone_newcgroup \ + -R "$src:/tmp/nsjail-newapi-check" \ + -B "$child:/tmp/nsjail-newapi-check/newchild" -- \ + /usr/bin/sh -c ' + set -eu + parent=/tmp/nsjail-newapi-check + child="$parent/newchild" + test -f "$child/nested/nested-file" + for entry in "ro:$parent" "ro:$parent/nested" \ + "rw:$child" "rw:$child/nested"; do + expected=${entry%%:*} + target=${entry#*:} + opts=$(findmnt -n -o VFS-OPTIONS -T "$target") + echo "override $expected $target $opts" + case ",$opts," in + *,ro,*) actual=ro ;; + *,rw,*) actual=rw ;; + *) echo "FAIL: no mount VFS state on $target" >&2; exit 1 ;; + esac + if [ "$actual" != "$expected" ]; then + echo "FAIL: $target is $actual, expected $expected" >&2 + exit 1 + fi + if touch "$target/nsjail-369-override" 2>/tmp/nsjail-newapi-probe.err; then + if [ "$expected" = ro ]; then + echo "FAIL: write succeeded on read-only $target" >&2 + exit 1 + fi + rm "$target/nsjail-369-override" + elif [ "$expected" = rw ]; then + echo "FAIL: write failed on writable $target" >&2 + exit 1 + fi + done + ' + +# File bind with no nested submounts must still be read-only. +"$nsjail_bin" --experimental_mnt=new -q -Mo --rw \ + -R /usr:/usr -s /usr/lib:/lib -s /usr/lib64:/lib64 \ + --disable_clone_newuser --disable_clone_newnet \ + --disable_clone_newcgroup \ + -R "$src/ordinary-file:/tmp/nsjail-newapi-file" -- \ + /usr/bin/sh -c ' + set -eu + test -f /tmp/nsjail-newapi-file + opts=$(findmnt -n -o VFS-OPTIONS -T /tmp/nsjail-newapi-file) + echo "ro-file $opts" + case ",$opts," in *,ro,*) : ;; *) exit 1 ;; esac + if touch /tmp/nsjail-newapi-file 2>/tmp/nsjail-newapi-probe.err; then + echo "FAIL: read-only file bind was writable" >&2 + exit 1 + fi + ' + +echo "PASS: recursive ro/rw, later writable nested child, and file-bind ro"