[PATCH v4 1/5] landlock: Check landlock_restrict_self(2)'s flags before privileges

Justin Suess <[email protected]>
Newsgroups org.kernel.vger.linux-security-module,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
sys_landlock_restrict_self() currently checks the no_new_privs /
CAP_SYS_ADMIN requirement before validating the flags argument.  An
unprivileged caller without no_new_privs thus gets EPERM even when the
passed flags are invalid, hiding the EINVAL error.

Move the no_new_privs / CAP_SYS_ADMIN check just after the flags check
so that malformed calls consistently error out with EINVAL whatever the
caller's privileges, the same way seccomp(2) validates its flags before
checking no_new_privs.

Update the restrict_self_checks_ordering test accordingly.

Cc: Mickaël Salaün <[email protected]>
Signed-off-by: Justin Suess <[email protected]>
---

Notes:
    v4:
        - New patch, following Mickaël's review of the main patch: check
          the flags argument before the no_new_privs / CAP_SYS_ADMIN
          requirement, in the same order as seccomp(2).

 security/landlock/syscalls.c                 | 8 ++++----
 tools/testing/selftests/landlock/base_test.c | 6 +++++-
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 36b02892c62f..e3ef7b980c82 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -535,6 +535,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
 	if (!is_initialized())
 		return -EOPNOTSUPP;
 
+	if ((flags | LANDLOCK_MASK_RESTRICT_SELF) !=
+	    LANDLOCK_MASK_RESTRICT_SELF)
+		return -EINVAL;
+
 	/*
 	 * Similar checks as for seccomp(2), except that an -EPERM may be
 	 * returned.
@@ -543,10 +547,6 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
 	    !ns_capable_noaudit(current_user_ns(), CAP_SYS_ADMIN))
 		return -EPERM;
 
-	if ((flags | LANDLOCK_MASK_RESTRICT_SELF) !=
-	    LANDLOCK_MASK_RESTRICT_SELF)
-		return -EINVAL;
-
 	/* Translates "off" flag to boolean. */
 	log_same_exec = !(flags & LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF);
 	/* Translates "on" flag to boolean. */
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index cbd3c1669951..f3c126d5c003 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -255,8 +255,12 @@ TEST(restrict_self_checks_ordering)
 
 	/* Checks unprivileged enforcement without no_new_privs. */
 	drop_caps(_metadata);
+	/*
+	 * The flags validity is checked before the no_new_privs /
+	 * CAP_SYS_ADMIN requirement.
+	 */
 	ASSERT_EQ(-1, landlock_restrict_self(-1, -1));
-	ASSERT_EQ(EPERM, errno);
+	ASSERT_EQ(EINVAL, errno);
 	ASSERT_EQ(-1, landlock_restrict_self(-1, 0));
 	ASSERT_EQ(EPERM, errno);
 	ASSERT_EQ(-1, landlock_restrict_self(ruleset_fd, 0));
-- 
2.55.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.