From 88da52ccd66e65f2e63a6c35c9dff55d448ef4dc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Thu, 16 May 2024 20:19:34 +0200 Subject: [PATCH 1/2] landlock: Fix d_parent walk MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WARN_ON_ONCE() in collect_domain_accesses() can be triggered when trying to link a root mount point. This cannot work in practice because this directory is mounted, but the VFS check is done after the call to security_path_link(). Do not use source directory's d_parent when the source directory is the mount point. Cc: Günther Noack Cc: Paul Moore Cc: stable@vger.kernel.org Reported-by: syzbot+bf4903dc7e12b18ebc87@syzkaller.appspotmail.com Fixes: b91c3e4ea756 ("landlock: Add support for file reparenting with LANDLOCK_ACCESS_FS_REFER") Closes: https://lore.kernel.org/r/000000000000553d3f0618198200@google.com Link: https://lore.kernel.org/r/20240516181935.1645983-2-mic@digikod.net [mic: Fix commit message] Signed-off-by: Mickaël Salaün --- security/landlock/fs.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/security/landlock/fs.c b/security/landlock/fs.c index 22d8b7c28074..7877a64cc6b8 100644 --- a/security/landlock/fs.c +++ b/security/landlock/fs.c @@ -1110,6 +1110,7 @@ static int current_check_refer_path(struct dentry *const old_dentry, bool allow_parent1, allow_parent2; access_mask_t access_request_parent1, access_request_parent2; struct path mnt_dir; + struct dentry *old_parent; layer_mask_t layer_masks_parent1[LANDLOCK_NUM_ACCESS_FS] = {}, layer_masks_parent2[LANDLOCK_NUM_ACCESS_FS] = {}; @@ -1157,9 +1158,17 @@ static int current_check_refer_path(struct dentry *const old_dentry, mnt_dir.mnt = new_dir->mnt; mnt_dir.dentry = new_dir->mnt->mnt_root; + /* + * old_dentry may be the root of the common mount point and + * !IS_ROOT(old_dentry) at the same time (e.g. with open_tree() and + * OPEN_TREE_CLONE). We do not need to call dget(old_parent) because + * we keep a reference to old_dentry. + */ + old_parent = (old_dentry == mnt_dir.dentry) ? old_dentry : + old_dentry->d_parent; + /* new_dir->dentry is equal to new_dentry->d_parent */ - allow_parent1 = collect_domain_accesses(dom, mnt_dir.dentry, - old_dentry->d_parent, + allow_parent1 = collect_domain_accesses(dom, mnt_dir.dentry, old_parent, &layer_masks_parent1); allow_parent2 = collect_domain_accesses( dom, mnt_dir.dentry, new_dir->dentry, &layer_masks_parent2); From 0055f53aac80fd938bf7cdfad7ad414ca6c0e198 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= Date: Thu, 16 May 2024 20:19:35 +0200 Subject: [PATCH 2/2] selftests/landlock: Add layout1.refer_mount_root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add tests to check error codes when linking or renaming a mount root directory. This previously triggered a kernel warning, but it is fixed with the previous commit. Cc: Günther Noack Cc: Paul Moore Link: https://lore.kernel.org/r/20240516181935.1645983-3-mic@digikod.net Signed-off-by: Mickaël Salaün --- tools/testing/selftests/landlock/fs_test.c | 45 ++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c index 6b5a9ff88c3d..7d063c652be1 100644 --- a/tools/testing/selftests/landlock/fs_test.c +++ b/tools/testing/selftests/landlock/fs_test.c @@ -35,6 +35,7 @@ * See https://sourceware.org/glibc/wiki/Synchronizing_Headers. */ #include +#include #include "common.h" @@ -47,6 +48,13 @@ int renameat2(int olddirfd, const char *oldpath, int newdirfd, } #endif +#ifndef open_tree +int open_tree(int dfd, const char *filename, unsigned int flags) +{ + return syscall(__NR_open_tree, dfd, filename, flags); +} +#endif + #ifndef RENAME_EXCHANGE #define RENAME_EXCHANGE (1 << 1) #endif @@ -2400,6 +2408,43 @@ TEST_F_FORK(layout1, refer_denied_by_default4) layer_dir_s1d1_refer); } +/* + * Tests walking through a denied root mount. + */ +TEST_F_FORK(layout1, refer_mount_root_deny) +{ + const struct landlock_ruleset_attr ruleset_attr = { + .handled_access_fs = LANDLOCK_ACCESS_FS_MAKE_DIR, + }; + int root_fd, ruleset_fd; + + /* Creates a mount object from a non-mount point. */ + set_cap(_metadata, CAP_SYS_ADMIN); + root_fd = + open_tree(AT_FDCWD, dir_s1d1, + AT_EMPTY_PATH | OPEN_TREE_CLONE | OPEN_TREE_CLOEXEC); + clear_cap(_metadata, CAP_SYS_ADMIN); + ASSERT_LE(0, root_fd); + + ruleset_fd = + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); + ASSERT_LE(0, ruleset_fd); + + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0)); + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0)); + EXPECT_EQ(0, close(ruleset_fd)); + + /* Link denied by Landlock: EACCES. */ + EXPECT_EQ(-1, linkat(root_fd, ".", root_fd, "does_not_exist", 0)); + EXPECT_EQ(EACCES, errno); + + /* renameat2() always returns EBUSY. */ + EXPECT_EQ(-1, renameat2(root_fd, ".", root_fd, "does_not_exist", 0)); + EXPECT_EQ(EBUSY, errno); + + EXPECT_EQ(0, close(root_fd)); +} + TEST_F_FORK(layout1, reparent_link) { const struct rule layer1[] = {