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

[Justin Suess]

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 <mic@xxxxxxxxxxx>
Signed-off-by: Justin Suess <utilityemal77@xxxxxxxxx>
---

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