The patch below was submitted to be applied to the 5.18-stable tree.
I fail to see how this patch meets the stable kernel rules as found at Documentation/process/stable-kernel-rules.rst.
I could be totally wrong, and if so, please respond to stable@vger.kernel.org and let me know why this patch should be applied. Otherwise, it is now dropped from my patch queues, never to be seen again.
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From eba39ca4b155c54adf471a69e91799cc1727873f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Sala=C3=BCn?= mic@digikod.net Date: Fri, 6 May 2022 18:08:19 +0200 Subject: [PATCH] landlock: Change landlock_restrict_self(2) check ordering MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit
According to the Landlock goal to be a security feature available to unprivileges processes, it makes more sense to first check for no_new_privs before checking anything else (i.e. syscall arguments).
Merge inval_fd_enforce and unpriv_enforce_without_no_new_privs tests into the new restrict_self_checks_ordering. This is similar to the previous commit checking other syscalls.
Link: https://lore.kernel.org/r/20220506160820.524344-10-mic@digikod.net Cc: stable@vger.kernel.org Signed-off-by: Mickaël Salaün mic@digikod.net
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c index a7396220c9d4..507d43827afe 100644 --- a/security/landlock/syscalls.c +++ b/security/landlock/syscalls.c @@ -405,10 +405,6 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, if (!landlock_initialized) return -EOPNOTSUPP;
- /* No flag for now. */ - if (flags) - return -EINVAL; - /* * Similar checks as for seccomp(2), except that an -EPERM may be * returned. @@ -417,6 +413,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32, !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN)) return -EPERM;
+ /* No flag for now. */ + if (flags) + return -EINVAL; + /* Gets and checks the ruleset. */ ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ); if (IS_ERR(ruleset)) diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c index 18b779471dcb..21fb33581419 100644 --- a/tools/testing/selftests/landlock/base_test.c +++ b/tools/testing/selftests/landlock/base_test.c @@ -168,22 +168,49 @@ TEST(add_rule_checks_ordering) ASSERT_EQ(0, close(ruleset_fd)); }
-TEST(inval_fd_enforce) +/* Tests ordering of syscall argument and permission checks. */ +TEST(restrict_self_checks_ordering) { + const struct landlock_ruleset_attr ruleset_attr = { + .handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE, + }; + struct landlock_path_beneath_attr path_beneath_attr = { + .allowed_access = LANDLOCK_ACCESS_FS_EXECUTE, + .parent_fd = -1, + }; + const int ruleset_fd = + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0); + + ASSERT_LE(0, ruleset_fd); + path_beneath_attr.parent_fd = + open("/tmp", O_PATH | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC); + ASSERT_LE(0, path_beneath_attr.parent_fd); + ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, + &path_beneath_attr, 0)); + ASSERT_EQ(0, close(path_beneath_attr.parent_fd)); + + /* Checks unprivileged enforcement without no_new_privs. */ + drop_caps(_metadata); + ASSERT_EQ(-1, landlock_restrict_self(-1, -1)); + ASSERT_EQ(EPERM, errno); + ASSERT_EQ(-1, landlock_restrict_self(-1, 0)); + ASSERT_EQ(EPERM, errno); + ASSERT_EQ(-1, landlock_restrict_self(ruleset_fd, 0)); + ASSERT_EQ(EPERM, errno); + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
+ /* Checks invalid flags. */ + ASSERT_EQ(-1, landlock_restrict_self(-1, -1)); + ASSERT_EQ(EINVAL, errno); + + /* Checks invalid ruleset FD. */ ASSERT_EQ(-1, landlock_restrict_self(-1, 0)); ASSERT_EQ(EBADF, errno); -} - -TEST(unpriv_enforce_without_no_new_privs) -{ - int err;
- drop_caps(_metadata); - err = landlock_restrict_self(-1, 0); - ASSERT_EQ(EPERM, errno); - ASSERT_EQ(err, -1); + /* Checks valid call. */ + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0)); + ASSERT_EQ(0, close(ruleset_fd)); }
TEST(ruleset_fd_io)