* [PATCH 1/2] capabilities: Check PR_SET_SECUREBITS with ns_capable()
2026-11-10 20:28 [PATCH 0/2] Fix LSM mediation of PR_SET_SECUREBITS Mickaël Salaün
@ 2026-11-10 20:28 ` Mickaël Salaün
2026-11-10 20:28 ` [PATCH 2/2] selftests/landlock: Test PR_SET_SECUREBITS mediation Mickaël Salaün
1 sibling, 0 replies; 3+ messages in thread
From: Mickaël Salaün @ 2026-11-10 20:28 UTC (permalink / raw)
To: James Morris, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Ada Smith, Christian Brauner,
Günther Noack, John Johansen, KP Singh, Matt Bobrowski,
Stephen Smalley, apparmor, bpf, kernel-team, linux-kernel,
linux-security-module, selinux, stable
Per-process securebits were introduced with a direct cap_capable() check
for CAP_SETPCAP. The capable LSM hook and SELinux capability policy
already existed, and SELinux forwarded task_prctl operations to
commoncap. The direct call therefore bypassed SELinux policy from the
start. It now also bypasses AppArmor, Landlock, and BPF LSM programs
attached to the capable hook.
A task with CAP_SETPCAP in its effective set can consequently change
privileged securebits despite an LSM denial. This can alter how its
capabilities evolve across UID and execution transitions without the
expected LSM denial or audit record.
Use ns_capable(), as cap_prctl_drop() already does for PR_CAPBSET_DROP.
Call it only for a no-op, which requires privilege for legacy reasons,
or when a privileged bit changes. Changes limited to
SECURE_ALL_UNPRIVILEGED and their lock bits do not need the capability,
so they must not invoke an LSM check or set PF_SUPERPRIV. Successful
privileged checks now set PF_SUPERPRIV, as other ns_capable() callers
do. Without other LSMs, the same requests remain allowed as before.
The check now reaches every capable hook before commoncap commits the
new credentials. A later task_prctl hook cannot provide equivalent
mediation because commoncap runs first and commits the change itself.
Cc: Christian Brauner <brauner@kernel.org>
Cc: Günther Noack <gnoack@google.com>
Cc: James Morris <jmorris@namei.org>
Cc: John Johansen <john.johansen@canonical.com>
Cc: KP Singh <kpsingh@kernel.org>
Cc: Matt Bobrowski <matt@bobrowski.net>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Stephen Smalley <stephen.smalley.work@gmail.com>
Cc: stable@vger.kernel.org
Reported-by: Ada Smith <adasmithgt@gmail.com>
Fixes: 3898b1b4ebff ("capabilities: implement per-process securebits")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/commoncap.c | 42 ++++++++++++++++++++----------------------
1 file changed, 20 insertions(+), 22 deletions(-)
diff --git a/security/commoncap.c b/security/commoncap.c
index d47ab3022343..1947128396e2 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -1283,6 +1283,23 @@ static int cap_prctl_drop(unsigned long cap)
return commit_creds(new);
}
+/*
+ * Doing anything with securebits requires privilege (go read about the
+ * "sendmail capabilities bug"), except for unprivileged bits. Indeed, the
+ * SECURE_ALL_UNPRIVILEGED bits are not restrictions enforced by the kernel but
+ * by user space on itself. For legacy reasons, a non-change requires
+ * privilege too. Only checking CAP_SETPCAP when it is needed keeps LSMs from
+ * seeing (and logging) a capability check for unprivileged changes.
+ */
+static bool securebits_need_setpcap(unsigned long old, unsigned long new)
+{
+ const unsigned long unpriv_and_locks = SECURE_ALL_UNPRIVILEGED |
+ SECURE_ALL_UNPRIVILEGED << 1;
+ const unsigned long changed = old ^ new;
+
+ return !changed || (changed & ~unpriv_and_locks);
+}
+
/**
* cap_task_prctl - Implement process control functions for this security module
* @option: The process control function requested
@@ -1346,28 +1363,9 @@ int cap_task_prctl(int option, unsigned long arg2, unsigned long arg3,
/* cannot change a locked bit */
return -EPERM;
- /*
- * Doing anything requires privilege (go read about the
- * "sendmail capabilities bug"), except for unprivileged bits.
- * Indeed, the SECURE_ALL_UNPRIVILEGED bits are not
- * restrictions enforced by the kernel but by user space on
- * itself.
- */
- if (cap_capable(current_cred(), current_cred()->user_ns,
- CAP_SETPCAP, CAP_OPT_NONE) != 0) {
- const unsigned long unpriv_and_locks =
- SECURE_ALL_UNPRIVILEGED |
- SECURE_ALL_UNPRIVILEGED << 1;
- const unsigned long changed = old->securebits ^ arg2;
-
- /* For legacy reason, denies non-change. */
- if (!changed)
- return -EPERM;
-
- /* Denies privileged changes. */
- if (changed & ~unpriv_and_locks)
- return -EPERM;
- }
+ if (securebits_need_setpcap(old->securebits, arg2) &&
+ !ns_capable(current_user_ns(), CAP_SETPCAP))
+ return -EPERM;
new = prepare_creds();
if (!new)
--
2.56.0
^ permalink raw reply [flat|nested] 3+ messages in thread* [PATCH 2/2] selftests/landlock: Test PR_SET_SECUREBITS mediation
2026-11-10 20:28 [PATCH 0/2] Fix LSM mediation of PR_SET_SECUREBITS Mickaël Salaün
2026-11-10 20:28 ` [PATCH 1/2] capabilities: Check PR_SET_SECUREBITS with ns_capable() Mickaël Salaün
@ 2026-11-10 20:28 ` Mickaël Salaün
1 sibling, 0 replies; 3+ messages in thread
From: Mickaël Salaün @ 2026-11-10 20:28 UTC (permalink / raw)
To: James Morris, Paul Moore, Serge Hallyn
Cc: Mickaël Salaün, Ada Smith, Christian Brauner,
Günther Noack, John Johansen, KP Singh, Matt Bobrowski,
Stephen Smalley, apparmor, bpf, kernel-team, linux-kernel,
linux-security-module, selinux
The preceding commoncap fix routes the CAP_SETPCAP check for privileged
PR_SET_SECUREBITS requests through the ordinary capable hooks. Add a
focused Landlock regression test for this path.
Keep CAP_SETPCAP effective to isolate Landlock mediation and run the
audit case in a child domain. A privileged change returns EPERM and
emits an exact CAP_SETPCAP record, then an unprivileged-only change
succeeds. After the child exits, require the domain deallocation record
to report exactly one denial; this proves that the second operation did
not create a hidden denial without relying on an asynchronous no-record
timeout.
The existing check-exec securebits fixture already covers commoncap's
privileged, unprivileged, locked, and user-namespace semantics. Without
the preceding fix, this Landlock audit test fails.
Cc: Christian Brauner <brauner@kernel.org>
Cc: Günther Noack <gnoack@google.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: Serge Hallyn <serge@hallyn.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
tools/testing/selftests/landlock/cap_test.c | 69 +++++++++++++++++++++
1 file changed, 69 insertions(+)
diff --git a/tools/testing/selftests/landlock/cap_test.c b/tools/testing/selftests/landlock/cap_test.c
index c79bb42c6282..741567c5ab41 100644
--- a/tools/testing/selftests/landlock/cap_test.c
+++ b/tools/testing/selftests/landlock/cap_test.c
@@ -1275,6 +1275,75 @@ TEST_F(cap_audit, denied)
EXPECT_EQ(0, records.domain);
}
+/*
+ * A privileged denial allocates the domain ID. The following unprivileged-only
+ * change must not create a hidden Landlock denial, so the domain's deallocation
+ * record must report exactly one denial.
+ */
+TEST_F(cap_audit, securebits)
+{
+ const unsigned long unpriv = SECBIT_EXEC_RESTRICT_FILE |
+ SECBIT_EXEC_RESTRICT_FILE_LOCKED;
+ __u64 *domain_id, deallocated_dom = 0;
+ unsigned long bits;
+ int err, ruleset_fd, status;
+ pid_t child;
+
+ domain_id = mmap(NULL, sizeof(*domain_id), PROT_READ | PROT_WRITE,
+ MAP_SHARED | MAP_ANONYMOUS, -1, 0);
+ ASSERT_NE(MAP_FAILED, domain_id);
+ *domain_id = 0;
+
+ ruleset_fd = create_cap_ruleset();
+ ASSERT_LE(0, ruleset_fd);
+ /* Allow the child to replace the audit filter before exiting. */
+ ASSERT_EQ(0, add_cap_rule(ruleset_fd, CAP_AUDIT_CONTROL));
+
+ child = fork();
+ ASSERT_LE(0, child);
+ if (child == 0) {
+ enforce_ruleset(_metadata, ruleset_fd);
+ EXPECT_EQ(0, close(ruleset_fd));
+
+ set_cap(_metadata, CAP_SETPCAP);
+ bits = prctl(PR_GET_SECUREBITS);
+ EXPECT_EQ(-1, prctl(PR_SET_SECUREBITS,
+ bits | SECBIT_NO_CAP_AMBIENT_RAISE));
+ EXPECT_EQ(EPERM, errno);
+
+ EXPECT_EQ(0, matches_log_cap(self->audit_fd, CAP_SETPCAP));
+ EXPECT_EQ(0, matches_log_domain_allocated(self->audit_fd,
+ getpid(), domain_id));
+
+ EXPECT_EQ(0, prctl(PR_SET_SECUREBITS, bits | unpriv));
+ clear_cap(_metadata, CAP_SETPCAP);
+
+ /* Receive the asynchronous domain deallocation record. */
+ set_cap(_metadata, CAP_AUDIT_CONTROL);
+ EXPECT_EQ(0, audit_filter_drop(self->audit_fd, AUDIT_ADD_RULE));
+ EXPECT_EQ(0,
+ audit_filter_exe(self->audit_fd, &self->audit_filter,
+ AUDIT_DEL_RULE));
+ clear_cap(_metadata, CAP_AUDIT_CONTROL);
+
+ _exit(_metadata->exit_code);
+ return;
+ }
+
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ if (WIFSIGNALED(status) || !WIFEXITED(status) ||
+ WEXITSTATUS(status) != EXIT_SUCCESS)
+ _metadata->exit_code = KSFT_FAIL;
+
+ EXPECT_NE(0, *domain_id);
+ err = matches_log_domain_deallocated(self->audit_fd, 1, *domain_id,
+ &deallocated_dom);
+ EXPECT_EQ(0, err);
+ EXPECT_EQ(*domain_id, deallocated_dom);
+ EXPECT_EQ(0, munmap(domain_id, sizeof(*domain_id)));
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
/*
* Denials in the capset and mmap_addr paths are audited like capable hook
* denials. The vm_enough_memory hook only withholds the admin reserve, with
--
2.56.0
^ permalink raw reply [flat|nested] 3+ messages in thread