From 5fda848d2080f3327f473ef2cb3f42fab6ba42b7 Mon Sep 17 00:00:00 2001 From: Super User Date: Wed, 17 Apr 2024 11:29:43 +0800 Subject: [PATCH] linux: readonlyPaths should inherit flags from parent mount --- src/libcrun/linux.c | 27 +++++++++++++++++++++------ tests/test_mounts.py | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/libcrun/linux.c b/src/libcrun/linux.c index e8cc0b9..a3c08d2 100644 --- a/src/libcrun/linux.c +++ b/src/libcrun/linux.c @@ -1060,13 +1060,15 @@ has_mount_for (libcrun_container_t *container, const char *destination) } static int -do_masked_or_readonly_path (libcrun_container_t *container, const char *rel_path, bool readonly, +do_masked_or_readonly_path (libcrun_container_t *container, const char *rel_path, bool readonly, bool keep_flags, libcrun_error_t *err) { + unsigned long mount_flags = 0; size_t rootfs_len = get_private_data (container)->rootfs_len; const char *rootfs = get_private_data (container)->rootfs; int rootfsfd = get_private_data (container)->rootfsfd; cleanup_close int pathfd = -1; + struct statfs sfs; int ret; mode_t mode; @@ -1088,8 +1090,21 @@ do_masked_or_readonly_path (libcrun_container_t *container, const char *rel_path proc_fd_path_t source_buffer; get_proc_self_fd_path (source_buffer, pathfd); - - ret = do_mount (container, source_buffer, pathfd, rel_path, NULL, MS_BIND | MS_PRIVATE | MS_RDONLY | MS_REC, NULL, + mount_flags = MS_BIND | MS_PRIVATE | MS_RDONLY | MS_REC; + if (keep_flags) + { + ret = statfs (source_buffer, &sfs); + if (UNLIKELY (ret < 0)) + return crun_make_error (err, errno, "statfs `%s`", source_buffer); + mount_flags = mount_flags | sfs.f_flags; + + // Parent might contain `MS_REMOUNT` but the new readonly path is not + // actually mounted. Specifically in the case of `/proc` this will end + // up with EINVAL therefore remove `MS_REMOUNT` if its getting + // inherited from the parent. + mount_flags = mount_flags & ~MS_REMOUNT; + } + ret = do_mount (container, source_buffer, pathfd, rel_path, NULL, mount_flags, NULL, LABEL_NONE, err); if (UNLIKELY (ret < 0)) return ret; @@ -1178,7 +1193,7 @@ do_mount (libcrun_container_t *container, const char *source, int targetfd, if (UNLIKELY (ret < 0)) return crun_make_error (err, errno, "bind mount /sys from the host"); - return do_masked_or_readonly_path (container, "/sys/fs/cgroup", false, err); + return do_masked_or_readonly_path (container, "/sys/fs/cgroup", false, false, err); } mountfd = get_bind_mount (-1, "/sys", true, true, err); @@ -1819,13 +1834,13 @@ do_masked_and_readonly_paths (libcrun_container_t *container, libcrun_error_t *e for (i = 0; i < def->linux->masked_paths_len; i++) { - ret = do_masked_or_readonly_path (container, def->linux->masked_paths[i], false, err); + ret = do_masked_or_readonly_path (container, def->linux->masked_paths[i], false, false, err); if (UNLIKELY (ret < 0)) return ret; } for (i = 0; i < def->linux->readonly_paths_len; i++) { - ret = do_masked_or_readonly_path (container, def->linux->readonly_paths[i], true, err); + ret = do_masked_or_readonly_path (container, def->linux->readonly_paths[i], true, true, err); if (UNLIKELY (ret < 0)) return ret; } diff --git a/tests/test_mounts.py b/tests/test_mounts.py index 57c16d2..31f0c0e 100755 --- a/tests/test_mounts.py +++ b/tests/test_mounts.py @@ -115,6 +115,43 @@ def test_mount_symlink_not_existing(): return 0 return -1 +def test_mount_readonly_should_inherit_options_from_parent(): + conf = base_config() + conf['process']['args'] = ['/init', 'cat', '/proc/self/mountinfo'] + add_all_namespaces(conf) + mount_opt = {"destination": "/test", "type": "bind", "source": "/tmp", "options": ["rbind", "nosuid","noexec","nodev"]} + conf['mounts'].append(mount_opt) + mount_opt = {"destination": "/test/world", "type": "bind", "source": "/etc", "options": ["rbind", "nosuid","noexec","nodev"]} + conf['mounts'].append(mount_opt) + + # Move test/world to a readonly path + conf['linux']['readonlyPaths'] = ["/test/world"] + out, _ = run_and_get_output(conf, hide_stderr=True) + + # final mount info must contain /test/world which is converted to readonly + # but also inherits the flags from its parent + if "/test/world ro,nosuid,nodev,noexec,relatime" in out: + return 0 + return -1 + +def test_proc_readonly_should_inherit_options_from_parent(): + conf = base_config() + conf['process']['args'] = ['/init', 'cat', '/proc/self/mountinfo'] + add_all_namespaces(conf) + for mount in conf['mounts']: + if mount['destination'] == "/proc": + mount['options'] = ["nosuid", "noexec","nodev"] + + # Move test/world to a readonly path + conf['linux']['readonlyPaths'] = ["/proc/bus"] + out, _ = run_and_get_output(conf, hide_stderr=True) + + # final mount info must contain /proc/bus which is converted to readonly + # but also inherits the flags from /proc + if "/proc/bus ro,nosuid,nodev,noexec,relatime" in out: + return 0 + return -1 + def test_mount_path_with_multiple_slashes(): conf = base_config() conf['process']['args'] = ['/init', 'cat', '/proc/self/mountinfo'] @@ -407,6 +444,8 @@ all_tests = { "mount-userns-bind-mount" : test_userns_bind_mount, "mount-idmapped-mounts" : test_idmapped_mounts, "mount-idmapped-mounts-symlink" : test_userns_bind_mount_symlink, + "mount-linux-readonly-should-inherit-flags": test_mount_readonly_should_inherit_options_from_parent, + "proc-linux-readonly-should-inherit-flags": test_proc_readonly_should_inherit_options_from_parent, "mount-ro-cgroup": test_ro_cgroup, } -- 2.43.0