* [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork()
@ 2026-04-04 8:49 Mickaël Salaün
2026-04-04 8:49 ` [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1 Mickaël Salaün
2026-04-07 7:30 ` [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Günther Noack
0 siblings, 2 replies; 7+ messages in thread
From: Mickaël Salaün @ 2026-04-04 8:49 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, stable
hook_cred_transfer() only copies the Landlock security blob when the
source credential has a domain. This is inconsistent with
landlock_restrict_self() which can set log_subdomains_off on a
credential without creating a domain (via the ruleset_fd=-1 path): the
field is committed but not preserved across fork() because the child's
prepare_creds() calls hook_cred_transfer() which skips the copy when
domain is NULL.
This breaks the documented use case where a process mutes subdomain logs
before forking sandboxed children: the children lose the muting and
their domains produce unexpected audit records.
Fix this by unconditionally copying the Landlock credential blob.
landlock_get_ruleset(NULL) is already a safe no-op.
Cc: Günther Noack <gnoack@google.com>
Cc: stable@vger.kernel.org
Fixes: ead9079f7569 ("landlock: Add LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
security/landlock/cred.c | 6 +-
tools/testing/selftests/landlock/audit_test.c | 88 +++++++++++++++++++
2 files changed, 90 insertions(+), 4 deletions(-)
diff --git a/security/landlock/cred.c b/security/landlock/cred.c
index 0cb3edde4d18..cc419de75cd6 100644
--- a/security/landlock/cred.c
+++ b/security/landlock/cred.c
@@ -22,10 +22,8 @@ static void hook_cred_transfer(struct cred *const new,
const struct landlock_cred_security *const old_llcred =
landlock_cred(old);
- if (old_llcred->domain) {
- landlock_get_ruleset(old_llcred->domain);
- *landlock_cred(new) = *old_llcred;
- }
+ landlock_get_ruleset(old_llcred->domain);
+ *landlock_cred(new) = *old_llcred;
}
static int hook_cred_prepare(struct cred *const new,
diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
index 46d02d49835a..20099b8667e7 100644
--- a/tools/testing/selftests/landlock/audit_test.c
+++ b/tools/testing/selftests/landlock/audit_test.c
@@ -279,6 +279,94 @@ TEST_F(audit, thread)
&audit_tv_default, sizeof(audit_tv_default)));
}
+/*
+ * Verifies that log_subdomains_off set via the ruleset_fd=-1 path (without
+ * creating a domain) is inherited by children across fork(). This exercises
+ * the hook_cred_transfer() fix: the Landlock credential blob must be copied
+ * even when the source credential has no domain.
+ *
+ * Phase 1 (baseline): a child without muting creates a domain and triggers a
+ * denial that IS logged.
+ *
+ * Phase 2 (after muting): the parent mutes subdomain logs, forks another child
+ * who creates a domain and triggers a denial that is NOT logged.
+ */
+TEST_F(audit, log_subdomains_off_fork)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .scoped = LANDLOCK_SCOPE_SIGNAL,
+ };
+ struct audit_records records;
+ int ruleset_fd, status;
+ pid_t child;
+
+ 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));
+
+ /*
+ * Phase 1: forks a child that creates a domain and triggers a denial
+ * before any muting. This proves the audit path works.
+ */
+ child = fork();
+ ASSERT_LE(0, child);
+ if (child == 0) {
+ ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
+ ASSERT_EQ(-1, kill(getppid(), 0));
+ ASSERT_EQ(EPERM, errno);
+ _exit(0);
+ return;
+ }
+
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ ASSERT_EQ(true, WIFEXITED(status));
+ ASSERT_EQ(0, WEXITSTATUS(status));
+
+ /* The denial must be logged (baseline). */
+ EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd, getpid(),
+ NULL));
+
+ /* Drains any remaining records (e.g. domain allocation). */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+
+ /*
+ * Mutes subdomain logs without creating a domain. The parent's
+ * credential has domain=NULL and log_subdomains_off=1.
+ */
+ ASSERT_EQ(0, landlock_restrict_self(
+ -1, LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF));
+
+ /*
+ * Phase 2: forks a child that creates a domain and triggers a denial.
+ * Because log_subdomains_off was inherited via fork(), the child's
+ * domain has log_status=LANDLOCK_LOG_DISABLED.
+ */
+ child = fork();
+ ASSERT_LE(0, child);
+ if (child == 0) {
+ ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
+ ASSERT_EQ(-1, kill(getppid(), 0));
+ ASSERT_EQ(EPERM, errno);
+ _exit(0);
+ return;
+ }
+
+ ASSERT_EQ(child, waitpid(child, &status, 0));
+ ASSERT_EQ(true, WIFEXITED(status));
+ ASSERT_EQ(0, WEXITSTATUS(status));
+
+ /* No denial record should appear. */
+ EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
+ getpid(), NULL));
+
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ EXPECT_EQ(0, close(ruleset_fd));
+}
+
FIXTURE(audit_flags)
{
struct audit_filter audit_filter;
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1
2026-04-04 8:49 [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Mickaël Salaün
@ 2026-04-04 8:49 ` Mickaël Salaün
2026-04-07 8:25 ` Günther Noack
2026-04-07 7:30 ` [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Günther Noack
1 sibling, 1 reply; 7+ messages in thread
From: Mickaël Salaün @ 2026-04-04 8:49 UTC (permalink / raw)
To: Günther Noack
Cc: Mickaël Salaün, linux-security-module, stable
LANDLOCK_RESTRICT_SELF_TSYNC does not allow
LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with ruleset_fd=-1, preventing
a multithreaded process from atomically propagating subdomain log muting
to all threads without creating a domain layer. Relax the fd=-1
condition to accept TSYNC alongside LOG_SUBDOMAINS_OFF, and update the
documentation accordingly.
Add flag validation tests for all TSYNC combinations with ruleset_fd=-1,
and audit tests verifying both transition directions: muting via TSYNC
(logged to not logged) and override via TSYNC (not logged to logged).
Cc: Günther Noack <gnoack@google.com>
Cc: stable@vger.kernel.org
Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
include/uapi/linux/landlock.h | 4 +-
security/landlock/syscalls.c | 14 +-
tools/testing/selftests/landlock/audit_test.c | 233 ++++++++++++++++++
tools/testing/selftests/landlock/tsync_test.c | 74 ++++++
4 files changed, 319 insertions(+), 6 deletions(-)
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index f88fa1f68b77..d37603efc273 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -116,7 +116,9 @@ struct landlock_ruleset_attr {
* ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``, this flag only affects
* future nested domains, not the one being created. It can also be used
* with a @ruleset_fd value of -1 to mute subdomain logs without creating a
- * domain.
+ * domain. When combined with %LANDLOCK_RESTRICT_SELF_TSYNC and a
+ * @ruleset_fd value of -1, this configuration is propagated to all threads
+ * of the current process.
*
* The following flag supports policy enforcement in multithreaded processes:
*
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 0d66a68677b7..a0bb664e0d31 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -512,10 +512,13 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
/*
* It is allowed to set LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with
- * -1 as ruleset_fd, but no other flag must be set.
+ * -1 as ruleset_fd, optionally combined with
+ * LANDLOCK_RESTRICT_SELF_TSYNC to propagate this configuration to all
+ * threads. No other flag must be set.
*/
if (!(ruleset_fd == -1 &&
- flags == LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
+ (flags & ~LANDLOCK_RESTRICT_SELF_TSYNC) ==
+ LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
/* Gets and checks the ruleset. */
ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);
if (IS_ERR(ruleset))
@@ -537,9 +540,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
/*
* The only case when a ruleset may not be set is if
- * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set and ruleset_fd is -1.
- * We could optimize this case by not calling commit_creds() if this flag
- * was already set, but it is not worth the complexity.
+ * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set (optionally with
+ * LANDLOCK_RESTRICT_SELF_TSYNC) and ruleset_fd is -1. We could
+ * optimize this case by not calling commit_creds() if this flag was
+ * already set, but it is not worth the complexity.
*/
if (ruleset) {
/*
diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
index 20099b8667e7..a193d8a97560 100644
--- a/tools/testing/selftests/landlock/audit_test.c
+++ b/tools/testing/selftests/landlock/audit_test.c
@@ -162,6 +162,7 @@ TEST_F(audit, layers)
struct thread_data {
pid_t parent_pid;
int ruleset_fd, pipe_child, pipe_parent;
+ bool mute_subdomains;
};
static void *thread_audit_test(void *arg)
@@ -367,6 +368,238 @@ TEST_F(audit, log_subdomains_off_fork)
EXPECT_EQ(0, close(ruleset_fd));
}
+/*
+ * Thread function: runs two rounds of (create domain, trigger denial, signal
+ * back), waiting for the main thread before each round. When mute_subdomains
+ * is set, phase 1 also mutes subdomain logs via the fd=-1 path before creating
+ * the domain. The ruleset_fd is kept open across both rounds so each
+ * restrict_self call stacks a new domain layer.
+ */
+static void *thread_sandbox_deny_twice(void *arg)
+{
+ const struct thread_data *data = (struct thread_data *)arg;
+ uintptr_t err = 0;
+ char buffer;
+
+ /* Phase 1: optionally mutes, creates a domain, and triggers a denial. */
+ if (read(data->pipe_parent, &buffer, 1) != 1) {
+ err = 1;
+ goto out;
+ }
+
+ if (data->mute_subdomains &&
+ landlock_restrict_self(-1,
+ LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
+ err = 2;
+ goto out;
+ }
+
+ if (landlock_restrict_self(data->ruleset_fd, 0)) {
+ err = 3;
+ goto out;
+ }
+
+ if (kill(data->parent_pid, 0) != -1 || errno != EPERM) {
+ err = 4;
+ goto out;
+ }
+
+ if (write(data->pipe_child, ".", 1) != 1) {
+ err = 5;
+ goto out;
+ }
+
+ /* Phase 2: stacks another domain and triggers a denial. */
+ if (read(data->pipe_parent, &buffer, 1) != 1) {
+ err = 6;
+ goto out;
+ }
+
+ if (landlock_restrict_self(data->ruleset_fd, 0)) {
+ err = 7;
+ goto out;
+ }
+
+ if (kill(data->parent_pid, 0) != -1 || errno != EPERM) {
+ err = 8;
+ goto out;
+ }
+
+ if (write(data->pipe_child, ".", 1) != 1) {
+ err = 9;
+ goto out;
+ }
+
+out:
+ close(data->ruleset_fd);
+ close(data->pipe_child);
+ close(data->pipe_parent);
+ return (void *)err;
+}
+
+/*
+ * Verifies that LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with
+ * LANDLOCK_RESTRICT_SELF_TSYNC and ruleset_fd=-1 propagates log_subdomains_off
+ * to a sibling thread, suppressing audit logging on domains it subsequently
+ * creates.
+ *
+ * Phase 1 (before TSYNC) acts as an inline baseline: the sibling creates a
+ * domain and triggers a denial that IS logged.
+ *
+ * Phase 2 (after TSYNC) verifies suppression: the sibling stacks another domain
+ * and triggers a denial that is NOT logged.
+ */
+TEST_F(audit, log_subdomains_off_tsync)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .scoped = LANDLOCK_SCOPE_SIGNAL,
+ };
+ struct audit_records records;
+ struct thread_data child_data;
+ int pipe_child[2], pipe_parent[2];
+ char buffer;
+ pthread_t thread;
+ void *thread_ret;
+
+ child_data.parent_pid = getppid();
+ ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+ child_data.pipe_child = pipe_child[1];
+ ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+ child_data.pipe_parent = pipe_parent[0];
+ child_data.ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, child_data.ruleset_fd);
+
+ ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
+
+ /* Creates the sibling thread. */
+ ASSERT_EQ(0, pthread_create(&thread, NULL, thread_sandbox_deny_twice,
+ &child_data));
+
+ /*
+ * Phase 1: the sibling creates a domain and triggers a denial before
+ * any log muting. This proves the audit path works.
+ */
+ ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+ ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
+
+ /* The denial must be logged. */
+ EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd,
+ child_data.parent_pid, NULL));
+
+ /* Drains any remaining records (e.g. domain allocation). */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+
+ /*
+ * Mutes subdomain logs and propagates to the sibling thread via TSYNC,
+ * without creating a domain.
+ */
+ ASSERT_EQ(0, landlock_restrict_self(
+ -1, LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
+ LANDLOCK_RESTRICT_SELF_TSYNC));
+
+ /*
+ * Phase 2: the sibling stacks another domain and triggers a denial.
+ * Because log_subdomains_off was propagated via TSYNC, the new domain
+ * has log_status=LANDLOCK_LOG_DISABLED.
+ */
+ ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+ ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
+
+ /* No denial record should appear. */
+ EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
+ child_data.parent_pid, NULL));
+
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ EXPECT_EQ(0, close(pipe_child[0]));
+ EXPECT_EQ(0, close(pipe_parent[1]));
+ ASSERT_EQ(0, pthread_join(thread, &thread_ret));
+ EXPECT_EQ(NULL, thread_ret);
+}
+
+/*
+ * Verifies that LANDLOCK_RESTRICT_SELF_TSYNC without
+ * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF overrides a sibling thread's
+ * log_subdomains_off, re-enabling audit logging on domains the sibling
+ * subsequently creates.
+ *
+ * Phase 1: the sibling sets log_subdomains_off, creates a muted domain, and
+ * triggers a denial that is NOT logged.
+ *
+ * Phase 2 (after TSYNC without LOG_SUBDOMAINS_OFF): the sibling stacks another
+ * domain and triggers a denial that IS logged, proving the muting was
+ * overridden.
+ */
+TEST_F(audit, tsync_override_log_subdomains_off)
+{
+ const struct landlock_ruleset_attr ruleset_attr = {
+ .scoped = LANDLOCK_SCOPE_SIGNAL,
+ };
+ struct audit_records records;
+ struct thread_data child_data;
+ int pipe_child[2], pipe_parent[2];
+ char buffer;
+ pthread_t thread;
+ void *thread_ret;
+
+ child_data.parent_pid = getppid();
+ ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
+ child_data.pipe_child = pipe_child[1];
+ ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
+ child_data.pipe_parent = pipe_parent[0];
+ child_data.ruleset_fd =
+ landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+ ASSERT_LE(0, child_data.ruleset_fd);
+
+ ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
+
+ child_data.mute_subdomains = true;
+
+ /* Creates the sibling thread. */
+ ASSERT_EQ(0, pthread_create(&thread, NULL, thread_sandbox_deny_twice,
+ &child_data));
+
+ /*
+ * Phase 1: the sibling mutes subdomain logs, creates a domain, and
+ * triggers a denial. The denial must not be logged.
+ */
+ ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+ ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
+
+ EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
+ child_data.parent_pid, NULL));
+
+ /* Drains any remaining records. */
+ EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+ EXPECT_EQ(0, records.access);
+
+ /*
+ * Overrides the sibling's log_subdomains_off by calling TSYNC without
+ * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF.
+ */
+ ASSERT_EQ(0, landlock_restrict_self(child_data.ruleset_fd,
+ LANDLOCK_RESTRICT_SELF_TSYNC));
+
+ /*
+ * Phase 2: the sibling stacks another domain and triggers a denial.
+ * Because TSYNC replaced its log_subdomains_off with 0, the new domain
+ * has log_status=LANDLOCK_LOG_PENDING.
+ */
+ ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
+ ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
+
+ /* The denial must be logged. */
+ EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd,
+ child_data.parent_pid, NULL));
+
+ EXPECT_EQ(0, close(pipe_child[0]));
+ EXPECT_EQ(0, close(pipe_parent[1]));
+ ASSERT_EQ(0, pthread_join(thread, &thread_ret));
+ EXPECT_EQ(NULL, thread_ret);
+}
+
FIXTURE(audit_flags)
{
struct audit_filter audit_filter;
diff --git a/tools/testing/selftests/landlock/tsync_test.c b/tools/testing/selftests/landlock/tsync_test.c
index 2b9ad4f154f4..abc290271a1a 100644
--- a/tools/testing/selftests/landlock/tsync_test.c
+++ b/tools/testing/selftests/landlock/tsync_test.c
@@ -247,4 +247,78 @@ TEST(tsync_interrupt)
EXPECT_EQ(0, close(ruleset_fd));
}
+/* clang-format off */
+FIXTURE(tsync_without_ruleset) {};
+/* clang-format on */
+
+FIXTURE_VARIANT(tsync_without_ruleset)
+{
+ const __u32 flags;
+ const int expected_errno;
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tsync_without_ruleset, tsync_only) {
+ /* clang-format on */
+ .flags = LANDLOCK_RESTRICT_SELF_TSYNC,
+ .expected_errno = EBADF,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off_same_exec_off) {
+ /* clang-format on */
+ .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
+ LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
+ LANDLOCK_RESTRICT_SELF_TSYNC,
+ .expected_errno = EBADF,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off_new_exec_on) {
+ /* clang-format on */
+ .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
+ LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
+ LANDLOCK_RESTRICT_SELF_TSYNC,
+ .expected_errno = EBADF,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tsync_without_ruleset, all_flags) {
+ /* clang-format on */
+ .flags = LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
+ LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
+ LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
+ LANDLOCK_RESTRICT_SELF_TSYNC,
+ .expected_errno = EBADF,
+};
+
+/* clang-format off */
+FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off) {
+ /* clang-format on */
+ .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
+ LANDLOCK_RESTRICT_SELF_TSYNC,
+ .expected_errno = 0,
+};
+
+FIXTURE_SETUP(tsync_without_ruleset)
+{
+}
+
+FIXTURE_TEARDOWN(tsync_without_ruleset)
+{
+}
+
+TEST_F(tsync_without_ruleset, check)
+{
+ int ret;
+
+ ret = landlock_restrict_self(-1, variant->flags);
+ if (variant->expected_errno) {
+ EXPECT_EQ(-1, ret);
+ EXPECT_EQ(variant->expected_errno, errno);
+ } else {
+ EXPECT_EQ(0, ret);
+ }
+}
+
TEST_HARNESS_MAIN
--
2.53.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork()
2026-04-04 8:49 [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Mickaël Salaün
2026-04-04 8:49 ` [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1 Mickaël Salaün
@ 2026-04-07 7:30 ` Günther Noack
2026-04-07 16:03 ` Mickaël Salaün
1 sibling, 1 reply; 7+ messages in thread
From: Günther Noack @ 2026-04-07 7:30 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, stable
Hello!
On Sat, Apr 04, 2026 at 10:49:57AM +0200, Mickaël Salaün wrote:
> hook_cred_transfer() only copies the Landlock security blob when the
> source credential has a domain. This is inconsistent with
> landlock_restrict_self() which can set log_subdomains_off on a
> credential without creating a domain (via the ruleset_fd=-1 path): the
> field is committed but not preserved across fork() because the child's
> prepare_creds() calls hook_cred_transfer() which skips the copy when
> domain is NULL.
>
> This breaks the documented use case where a process mutes subdomain logs
> before forking sandboxed children: the children lose the muting and
> their domains produce unexpected audit records.
>
> Fix this by unconditionally copying the Landlock credential blob.
> landlock_get_ruleset(NULL) is already a safe no-op.
>
> Cc: Günther Noack <gnoack@google.com>
> Cc: stable@vger.kernel.org
> Fixes: ead9079f7569 ("landlock: Add LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> security/landlock/cred.c | 6 +-
> tools/testing/selftests/landlock/audit_test.c | 88 +++++++++++++++++++
> 2 files changed, 90 insertions(+), 4 deletions(-)
>
> diff --git a/security/landlock/cred.c b/security/landlock/cred.c
> index 0cb3edde4d18..cc419de75cd6 100644
> --- a/security/landlock/cred.c
> +++ b/security/landlock/cred.c
> @@ -22,10 +22,8 @@ static void hook_cred_transfer(struct cred *const new,
> const struct landlock_cred_security *const old_llcred =
> landlock_cred(old);
>
> - if (old_llcred->domain) {
> - landlock_get_ruleset(old_llcred->domain);
> - *landlock_cred(new) = *old_llcred;
> - }
> + landlock_get_ruleset(old_llcred->domain);
> + *landlock_cred(new) = *old_llcred;
This fix looks correct for the hook_cred_prepare() case (and of
course, hook_cred_prepare() calls hook_cred_transfer() in Landlock).
But I'm afraid I might have spotted another issue here:
If I look at the code in security/keys/process_keys.c, where
security_tranfer_creds() is called, the "old" object is actually
already initialized, and if we are not checking for that, I think we
are leaking memory.
I would suggest to use the helper landlock_cred_copy() from cred.h for
that. This one is anyway supposed to be the central place for this
copying logic, and it is safe to use with zeroed-out target objects
(because the put is safe for the NULL-pointer).
Maybe this is worth updating while we are at it?
> }
>
> static int hook_cred_prepare(struct cred *const new,
> diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
> index 46d02d49835a..20099b8667e7 100644
> --- a/tools/testing/selftests/landlock/audit_test.c
> +++ b/tools/testing/selftests/landlock/audit_test.c
> @@ -279,6 +279,94 @@ TEST_F(audit, thread)
> &audit_tv_default, sizeof(audit_tv_default)));
> }
>
> +/*
> + * Verifies that log_subdomains_off set via the ruleset_fd=-1 path (without
> + * creating a domain) is inherited by children across fork(). This exercises
> + * the hook_cred_transfer() fix: the Landlock credential blob must be copied
> + * even when the source credential has no domain.
> + *
> + * Phase 1 (baseline): a child without muting creates a domain and triggers a
> + * denial that IS logged.
> + *
> + * Phase 2 (after muting): the parent mutes subdomain logs, forks another child
> + * who creates a domain and triggers a denial that is NOT logged.
> + */
> +TEST_F(audit, log_subdomains_off_fork)
> +{
> + const struct landlock_ruleset_attr ruleset_attr = {
> + .scoped = LANDLOCK_SCOPE_SIGNAL,
> + };
> + struct audit_records records;
> + int ruleset_fd, status;
> + pid_t child;
> +
> + 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));
> +
> + /*
> + * Phase 1: forks a child that creates a domain and triggers a denial
> + * before any muting. This proves the audit path works.
> + */
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
> + ASSERT_EQ(-1, kill(getppid(), 0));
> + ASSERT_EQ(EPERM, errno);
> + _exit(0);
> + return;
> + }
> +
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> + ASSERT_EQ(true, WIFEXITED(status));
> + ASSERT_EQ(0, WEXITSTATUS(status));
> +
> + /* The denial must be logged (baseline). */
> + EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd, getpid(),
> + NULL));
> +
> + /* Drains any remaining records (e.g. domain allocation). */
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> +
> + /*
> + * Mutes subdomain logs without creating a domain. The parent's
> + * credential has domain=NULL and log_subdomains_off=1.
> + */
> + ASSERT_EQ(0, landlock_restrict_self(
> + -1, LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF));
> +
> + /*
> + * Phase 2: forks a child that creates a domain and triggers a denial.
> + * Because log_subdomains_off was inherited via fork(), the child's
> + * domain has log_status=LANDLOCK_LOG_DISABLED.
> + */
> + child = fork();
> + ASSERT_LE(0, child);
> + if (child == 0) {
> + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
> + ASSERT_EQ(-1, kill(getppid(), 0));
> + ASSERT_EQ(EPERM, errno);
> + _exit(0);
> + return;
> + }
> +
> + ASSERT_EQ(child, waitpid(child, &status, 0));
> + ASSERT_EQ(true, WIFEXITED(status));
> + ASSERT_EQ(0, WEXITSTATUS(status));
> +
> + /* No denial record should appear. */
> + EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
> + getpid(), NULL));
> +
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> + EXPECT_EQ(0, records.access);
> +
> + EXPECT_EQ(0, close(ruleset_fd));
> +}
> +
> FIXTURE(audit_flags)
> {
> struct audit_filter audit_filter;
> --
> 2.53.0
>
Test looks fine.
While I do still think we should investigate the memory leak, this
commit is, as it is, already a strict improvement over what we had
before, so:
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
–Günther
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1
2026-04-04 8:49 ` [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1 Mickaël Salaün
@ 2026-04-07 8:25 ` Günther Noack
2026-04-07 16:06 ` Mickaël Salaün
0 siblings, 1 reply; 7+ messages in thread
From: Günther Noack @ 2026-04-07 8:25 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Günther Noack, linux-security-module, stable
Hello!
On Sat, Apr 04, 2026 at 10:49:58AM +0200, Mickaël Salaün wrote:
> LANDLOCK_RESTRICT_SELF_TSYNC does not allow
> LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with ruleset_fd=-1, preventing
> a multithreaded process from atomically propagating subdomain log muting
> to all threads without creating a domain layer. Relax the fd=-1
> condition to accept TSYNC alongside LOG_SUBDOMAINS_OFF, and update the
> documentation accordingly.
>
> Add flag validation tests for all TSYNC combinations with ruleset_fd=-1,
> and audit tests verifying both transition directions: muting via TSYNC
> (logged to not logged) and override via TSYNC (not logged to logged).
>
> Cc: Günther Noack <gnoack@google.com>
> Cc: stable@vger.kernel.org
> Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
> include/uapi/linux/landlock.h | 4 +-
> security/landlock/syscalls.c | 14 +-
> tools/testing/selftests/landlock/audit_test.c | 233 ++++++++++++++++++
> tools/testing/selftests/landlock/tsync_test.c | 74 ++++++
> 4 files changed, 319 insertions(+), 6 deletions(-)
>
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index f88fa1f68b77..d37603efc273 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -116,7 +116,9 @@ struct landlock_ruleset_attr {
> * ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``, this flag only affects
> * future nested domains, not the one being created. It can also be used
> * with a @ruleset_fd value of -1 to mute subdomain logs without creating a
> - * domain.
> + * domain. When combined with %LANDLOCK_RESTRICT_SELF_TSYNC and a
> + * @ruleset_fd value of -1, this configuration is propagated to all threads
> + * of the current process.
> *
> * The following flag supports policy enforcement in multithreaded processes:
> *
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index 0d66a68677b7..a0bb664e0d31 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -512,10 +512,13 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
>
> /*
> * It is allowed to set LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with
> - * -1 as ruleset_fd, but no other flag must be set.
> + * -1 as ruleset_fd, optionally combined with
> + * LANDLOCK_RESTRICT_SELF_TSYNC to propagate this configuration to all
> + * threads. No other flag must be set.
> */
> if (!(ruleset_fd == -1 &&
> - flags == LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
> + (flags & ~LANDLOCK_RESTRICT_SELF_TSYNC) ==
> + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
Well spotted, thanks!
> /* Gets and checks the ruleset. */
> ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);
> if (IS_ERR(ruleset))
> @@ -537,9 +540,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
>
> /*
> * The only case when a ruleset may not be set is if
> - * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set and ruleset_fd is -1.
> - * We could optimize this case by not calling commit_creds() if this flag
> - * was already set, but it is not worth the complexity.
> + * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set (optionally with
> + * LANDLOCK_RESTRICT_SELF_TSYNC) and ruleset_fd is -1. We could
> + * optimize this case by not calling commit_creds() if this flag was
> + * already set, but it is not worth the complexity.
> */
> if (ruleset) {
> /*
> diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
> index 20099b8667e7..a193d8a97560 100644
> --- a/tools/testing/selftests/landlock/audit_test.c
> +++ b/tools/testing/selftests/landlock/audit_test.c
> @@ -162,6 +162,7 @@ TEST_F(audit, layers)
> struct thread_data {
> pid_t parent_pid;
> int ruleset_fd, pipe_child, pipe_parent;
> + bool mute_subdomains;
> };
>
> static void *thread_audit_test(void *arg)
> @@ -367,6 +368,238 @@ TEST_F(audit, log_subdomains_off_fork)
> EXPECT_EQ(0, close(ruleset_fd));
> }
>
> +/*
> + * Thread function: runs two rounds of (create domain, trigger denial, signal
> + * back), waiting for the main thread before each round. When mute_subdomains
> + * is set, phase 1 also mutes subdomain logs via the fd=-1 path before creating
> + * the domain. The ruleset_fd is kept open across both rounds so each
> + * restrict_self call stacks a new domain layer.
> + */
> +static void *thread_sandbox_deny_twice(void *arg)
> +{
> + const struct thread_data *data = (struct thread_data *)arg;
> + uintptr_t err = 0;
> + char buffer;
> +
> + /* Phase 1: optionally mutes, creates a domain, and triggers a denial. */
> + if (read(data->pipe_parent, &buffer, 1) != 1) {
> + err = 1;
> + goto out;
> + }
> +
> + if (data->mute_subdomains &&
> + landlock_restrict_self(-1,
> + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
> + err = 2;
> + goto out;
> + }
> +
> + if (landlock_restrict_self(data->ruleset_fd, 0)) {
> + err = 3;
> + goto out;
> + }
> +
> + if (kill(data->parent_pid, 0) != -1 || errno != EPERM) {
> + err = 4;
> + goto out;
> + }
> +
> + if (write(data->pipe_child, ".", 1) != 1) {
> + err = 5;
> + goto out;
> + }
> +
> + /* Phase 2: stacks another domain and triggers a denial. */
> + if (read(data->pipe_parent, &buffer, 1) != 1) {
> + err = 6;
> + goto out;
> + }
> +
> + if (landlock_restrict_self(data->ruleset_fd, 0)) {
> + err = 7;
> + goto out;
> + }
> +
> + if (kill(data->parent_pid, 0) != -1 || errno != EPERM) {
> + err = 8;
> + goto out;
> + }
> +
> + if (write(data->pipe_child, ".", 1) != 1) {
> + err = 9;
> + goto out;
> + }
> +
> +out:
> + close(data->ruleset_fd);
> + close(data->pipe_child);
> + close(data->pipe_parent);
> + return (void *)err;
> +}
> +
> +/*
> + * Verifies that LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with
> + * LANDLOCK_RESTRICT_SELF_TSYNC and ruleset_fd=-1 propagates log_subdomains_off
> + * to a sibling thread, suppressing audit logging on domains it subsequently
> + * creates.
> + *
> + * Phase 1 (before TSYNC) acts as an inline baseline: the sibling creates a
> + * domain and triggers a denial that IS logged.
> + *
> + * Phase 2 (after TSYNC) verifies suppression: the sibling stacks another domain
> + * and triggers a denial that is NOT logged.
> + */
> +TEST_F(audit, log_subdomains_off_tsync)
> +{
> + const struct landlock_ruleset_attr ruleset_attr = {
> + .scoped = LANDLOCK_SCOPE_SIGNAL,
> + };
> + struct audit_records records;
> + struct thread_data child_data;
The child_data.mute_subdomains field stays uninitialized in this
function (and maybe others). Please fix.
struct thread_data child_data = {};
> + int pipe_child[2], pipe_parent[2];
> + char buffer;
> + pthread_t thread;
> + void *thread_ret;
> +
> + child_data.parent_pid = getppid();
> + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> + child_data.pipe_child = pipe_child[1];
> + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> + child_data.pipe_parent = pipe_parent[0];
> + child_data.ruleset_fd =
> + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, child_data.ruleset_fd);
> +
> + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> +
> + /* Creates the sibling thread. */
> + ASSERT_EQ(0, pthread_create(&thread, NULL, thread_sandbox_deny_twice,
> + &child_data));
> +
> + /*
> + * Phase 1: the sibling creates a domain and triggers a denial before
> + * any log muting. This proves the audit path works.
> + */
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> +
> + /* The denial must be logged. */
> + EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd,
> + child_data.parent_pid, NULL));
> +
> + /* Drains any remaining records (e.g. domain allocation). */
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> +
> + /*
> + * Mutes subdomain logs and propagates to the sibling thread via TSYNC,
> + * without creating a domain.
> + */
> + ASSERT_EQ(0, landlock_restrict_self(
> + -1, LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> + LANDLOCK_RESTRICT_SELF_TSYNC));
> +
> + /*
> + * Phase 2: the sibling stacks another domain and triggers a denial.
> + * Because log_subdomains_off was propagated via TSYNC, the new domain
> + * has log_status=LANDLOCK_LOG_DISABLED.
> + */
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> +
> + /* No denial record should appear. */
> + EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
> + child_data.parent_pid, NULL));
> +
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> + EXPECT_EQ(0, records.access);
> +
> + EXPECT_EQ(0, close(pipe_child[0]));
> + EXPECT_EQ(0, close(pipe_parent[1]));
> + ASSERT_EQ(0, pthread_join(thread, &thread_ret));
> + EXPECT_EQ(NULL, thread_ret);
> +}
> +
> +/*
> + * Verifies that LANDLOCK_RESTRICT_SELF_TSYNC without
> + * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF overrides a sibling thread's
> + * log_subdomains_off, re-enabling audit logging on domains the sibling
> + * subsequently creates.
> + *
> + * Phase 1: the sibling sets log_subdomains_off, creates a muted domain, and
> + * triggers a denial that is NOT logged.
> + *
> + * Phase 2 (after TSYNC without LOG_SUBDOMAINS_OFF): the sibling stacks another
> + * domain and triggers a denial that IS logged, proving the muting was
> + * overridden.
> + */
> +TEST_F(audit, tsync_override_log_subdomains_off)
> +{
> + const struct landlock_ruleset_attr ruleset_attr = {
> + .scoped = LANDLOCK_SCOPE_SIGNAL,
> + };
> + struct audit_records records;
> + struct thread_data child_data;
> + int pipe_child[2], pipe_parent[2];
> + char buffer;
> + pthread_t thread;
> + void *thread_ret;
> +
> + child_data.parent_pid = getppid();
> + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> + child_data.pipe_child = pipe_child[1];
> + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> + child_data.pipe_parent = pipe_parent[0];
> + child_data.ruleset_fd =
> + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> + ASSERT_LE(0, child_data.ruleset_fd);
> +
> + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> +
> + child_data.mute_subdomains = true;
> +
> + /* Creates the sibling thread. */
> + ASSERT_EQ(0, pthread_create(&thread, NULL, thread_sandbox_deny_twice,
> + &child_data));
> +
> + /*
> + * Phase 1: the sibling mutes subdomain logs, creates a domain, and
> + * triggers a denial. The denial must not be logged.
> + */
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> +
> + EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
> + child_data.parent_pid, NULL));
> +
> + /* Drains any remaining records. */
> + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> + EXPECT_EQ(0, records.access);
> +
> + /*
> + * Overrides the sibling's log_subdomains_off by calling TSYNC without
> + * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF.
> + */
> + ASSERT_EQ(0, landlock_restrict_self(child_data.ruleset_fd,
> + LANDLOCK_RESTRICT_SELF_TSYNC));
> +
> + /*
> + * Phase 2: the sibling stacks another domain and triggers a denial.
> + * Because TSYNC replaced its log_subdomains_off with 0, the new domain
> + * has log_status=LANDLOCK_LOG_PENDING.
> + */
> + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> +
> + /* The denial must be logged. */
> + EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd,
> + child_data.parent_pid, NULL));
> +
> + EXPECT_EQ(0, close(pipe_child[0]));
> + EXPECT_EQ(0, close(pipe_parent[1]));
> + ASSERT_EQ(0, pthread_join(thread, &thread_ret));
> + EXPECT_EQ(NULL, thread_ret);
> +}
> +
> FIXTURE(audit_flags)
> {
> struct audit_filter audit_filter;
> diff --git a/tools/testing/selftests/landlock/tsync_test.c b/tools/testing/selftests/landlock/tsync_test.c
> index 2b9ad4f154f4..abc290271a1a 100644
> --- a/tools/testing/selftests/landlock/tsync_test.c
> +++ b/tools/testing/selftests/landlock/tsync_test.c
> @@ -247,4 +247,78 @@ TEST(tsync_interrupt)
> EXPECT_EQ(0, close(ruleset_fd));
> }
>
> +/* clang-format off */
> +FIXTURE(tsync_without_ruleset) {};
> +/* clang-format on */
> +
> +FIXTURE_VARIANT(tsync_without_ruleset)
> +{
> + const __u32 flags;
> + const int expected_errno;
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(tsync_without_ruleset, tsync_only) {
> + /* clang-format on */
> + .flags = LANDLOCK_RESTRICT_SELF_TSYNC,
> + .expected_errno = EBADF,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off_same_exec_off) {
> + /* clang-format on */
> + .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> + LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> + LANDLOCK_RESTRICT_SELF_TSYNC,
> + .expected_errno = EBADF,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off_new_exec_on) {
> + /* clang-format on */
> + .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> + LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> + LANDLOCK_RESTRICT_SELF_TSYNC,
> + .expected_errno = EBADF,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(tsync_without_ruleset, all_flags) {
> + /* clang-format on */
> + .flags = LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> + LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> + LANDLOCK_RESTRICT_SELF_TSYNC,
> + .expected_errno = EBADF,
> +};
> +
> +/* clang-format off */
> +FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off) {
> + /* clang-format on */
> + .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> + LANDLOCK_RESTRICT_SELF_TSYNC,
> + .expected_errno = 0,
> +};
> +
> +FIXTURE_SETUP(tsync_without_ruleset)
> +{
> +}
> +
> +FIXTURE_TEARDOWN(tsync_without_ruleset)
> +{
> +}
> +
> +TEST_F(tsync_without_ruleset, check)
> +{
> + int ret;
> +
> + ret = landlock_restrict_self(-1, variant->flags);
> + if (variant->expected_errno) {
> + EXPECT_EQ(-1, ret);
> + EXPECT_EQ(variant->expected_errno, errno);
> + } else {
> + EXPECT_EQ(0, ret);
> + }
> +}
We are not setting the no_new_privs flag in this test, as we do in the
others.
no_new_privs or CAP_SYS_ADMIN are required in the implementation, even
when ruleset_fd == -1 and passing
LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF.
> +
> TEST_HARNESS_MAIN
> --
> 2.53.0
>
Reviewed-by: Günther Noack <gnoack3000@gmail.com>
But please fix the flaky test.
–Günther
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork()
2026-04-07 7:30 ` [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Günther Noack
@ 2026-04-07 16:03 ` Mickaël Salaün
2026-04-07 19:00 ` Günther Noack
0 siblings, 1 reply; 7+ messages in thread
From: Mickaël Salaün @ 2026-04-07 16:03 UTC (permalink / raw)
To: Günther Noack, Jann Horn
Cc: Günther Noack, linux-security-module, stable
On Tue, Apr 07, 2026 at 09:30:40AM +0200, Günther Noack wrote:
> Hello!
>
> On Sat, Apr 04, 2026 at 10:49:57AM +0200, Mickaël Salaün wrote:
> > hook_cred_transfer() only copies the Landlock security blob when the
> > source credential has a domain. This is inconsistent with
> > landlock_restrict_self() which can set log_subdomains_off on a
> > credential without creating a domain (via the ruleset_fd=-1 path): the
> > field is committed but not preserved across fork() because the child's
> > prepare_creds() calls hook_cred_transfer() which skips the copy when
> > domain is NULL.
> >
> > This breaks the documented use case where a process mutes subdomain logs
> > before forking sandboxed children: the children lose the muting and
> > their domains produce unexpected audit records.
> >
> > Fix this by unconditionally copying the Landlock credential blob.
> > landlock_get_ruleset(NULL) is already a safe no-op.
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: stable@vger.kernel.org
> > Fixes: ead9079f7569 ("landlock: Add LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF")
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> > security/landlock/cred.c | 6 +-
> > tools/testing/selftests/landlock/audit_test.c | 88 +++++++++++++++++++
> > 2 files changed, 90 insertions(+), 4 deletions(-)
> >
> > diff --git a/security/landlock/cred.c b/security/landlock/cred.c
> > index 0cb3edde4d18..cc419de75cd6 100644
> > --- a/security/landlock/cred.c
> > +++ b/security/landlock/cred.c
> > @@ -22,10 +22,8 @@ static void hook_cred_transfer(struct cred *const new,
> > const struct landlock_cred_security *const old_llcred =
> > landlock_cred(old);
> >
> > - if (old_llcred->domain) {
> > - landlock_get_ruleset(old_llcred->domain);
> > - *landlock_cred(new) = *old_llcred;
> > - }
> > + landlock_get_ruleset(old_llcred->domain);
> > + *landlock_cred(new) = *old_llcred;
>
> This fix looks correct for the hook_cred_prepare() case (and of
> course, hook_cred_prepare() calls hook_cred_transfer() in Landlock).
>
>
> But I'm afraid I might have spotted another issue here:
>
> If I look at the code in security/keys/process_keys.c, where
> security_tranfer_creds() is called, the "old" object is actually
> already initialized, and if we are not checking for that, I think we
> are leaking memory.
old is only a partially initialized credential, and the Landlock
part is not set yet, which is the goal of hook_transfer_creds(), so
there is no leak.
>
> I would suggest to use the helper landlock_cred_copy() from cred.h for
This is not required but if we would like to do it anyway, that would
not be backportable and would introduce a (minimal) performance penalty.
> that. This one is anyway supposed to be the central place for this
> copying logic, and it is safe to use with zeroed-out target objects
> (because the put is safe for the NULL-pointer).
>
> Maybe this is worth updating while we are at it?
>
>
> > }
> >
> > static int hook_cred_prepare(struct cred *const new,
> > diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
> > index 46d02d49835a..20099b8667e7 100644
> > --- a/tools/testing/selftests/landlock/audit_test.c
> > +++ b/tools/testing/selftests/landlock/audit_test.c
> > @@ -279,6 +279,94 @@ TEST_F(audit, thread)
> > &audit_tv_default, sizeof(audit_tv_default)));
> > }
> >
> > +/*
> > + * Verifies that log_subdomains_off set via the ruleset_fd=-1 path (without
> > + * creating a domain) is inherited by children across fork(). This exercises
> > + * the hook_cred_transfer() fix: the Landlock credential blob must be copied
> > + * even when the source credential has no domain.
> > + *
> > + * Phase 1 (baseline): a child without muting creates a domain and triggers a
> > + * denial that IS logged.
> > + *
> > + * Phase 2 (after muting): the parent mutes subdomain logs, forks another child
> > + * who creates a domain and triggers a denial that is NOT logged.
> > + */
> > +TEST_F(audit, log_subdomains_off_fork)
> > +{
> > + const struct landlock_ruleset_attr ruleset_attr = {
> > + .scoped = LANDLOCK_SCOPE_SIGNAL,
> > + };
> > + struct audit_records records;
> > + int ruleset_fd, status;
> > + pid_t child;
> > +
> > + 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));
> > +
> > + /*
> > + * Phase 1: forks a child that creates a domain and triggers a denial
> > + * before any muting. This proves the audit path works.
> > + */
> > + child = fork();
> > + ASSERT_LE(0, child);
> > + if (child == 0) {
> > + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
> > + ASSERT_EQ(-1, kill(getppid(), 0));
> > + ASSERT_EQ(EPERM, errno);
> > + _exit(0);
> > + return;
> > + }
> > +
> > + ASSERT_EQ(child, waitpid(child, &status, 0));
> > + ASSERT_EQ(true, WIFEXITED(status));
> > + ASSERT_EQ(0, WEXITSTATUS(status));
> > +
> > + /* The denial must be logged (baseline). */
> > + EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd, getpid(),
> > + NULL));
> > +
> > + /* Drains any remaining records (e.g. domain allocation). */
> > + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> > +
> > + /*
> > + * Mutes subdomain logs without creating a domain. The parent's
> > + * credential has domain=NULL and log_subdomains_off=1.
> > + */
> > + ASSERT_EQ(0, landlock_restrict_self(
> > + -1, LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF));
> > +
> > + /*
> > + * Phase 2: forks a child that creates a domain and triggers a denial.
> > + * Because log_subdomains_off was inherited via fork(), the child's
> > + * domain has log_status=LANDLOCK_LOG_DISABLED.
> > + */
> > + child = fork();
> > + ASSERT_LE(0, child);
> > + if (child == 0) {
> > + ASSERT_EQ(0, landlock_restrict_self(ruleset_fd, 0));
> > + ASSERT_EQ(-1, kill(getppid(), 0));
> > + ASSERT_EQ(EPERM, errno);
> > + _exit(0);
> > + return;
> > + }
> > +
> > + ASSERT_EQ(child, waitpid(child, &status, 0));
> > + ASSERT_EQ(true, WIFEXITED(status));
> > + ASSERT_EQ(0, WEXITSTATUS(status));
> > +
> > + /* No denial record should appear. */
> > + EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
> > + getpid(), NULL));
> > +
> > + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> > + EXPECT_EQ(0, records.access);
> > +
> > + EXPECT_EQ(0, close(ruleset_fd));
> > +}
> > +
> > FIXTURE(audit_flags)
> > {
> > struct audit_filter audit_filter;
> > --
> > 2.53.0
> >
>
> Test looks fine.
>
> While I do still think we should investigate the memory leak, this
> commit is, as it is, already a strict improvement over what we had
> before, so:
>
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
I'll keep your tag if this patch is ok with you as-is.
>
> –Günther
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1
2026-04-07 8:25 ` Günther Noack
@ 2026-04-07 16:06 ` Mickaël Salaün
0 siblings, 0 replies; 7+ messages in thread
From: Mickaël Salaün @ 2026-04-07 16:06 UTC (permalink / raw)
To: Günther Noack; +Cc: Günther Noack, linux-security-module, stable
On Tue, Apr 07, 2026 at 10:25:30AM +0200, Günther Noack wrote:
> Hello!
>
> On Sat, Apr 04, 2026 at 10:49:58AM +0200, Mickaël Salaün wrote:
> > LANDLOCK_RESTRICT_SELF_TSYNC does not allow
> > LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with ruleset_fd=-1, preventing
> > a multithreaded process from atomically propagating subdomain log muting
> > to all threads without creating a domain layer. Relax the fd=-1
> > condition to accept TSYNC alongside LOG_SUBDOMAINS_OFF, and update the
> > documentation accordingly.
> >
> > Add flag validation tests for all TSYNC combinations with ruleset_fd=-1,
> > and audit tests verifying both transition directions: muting via TSYNC
> > (logged to not logged) and override via TSYNC (not logged to logged).
> >
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: stable@vger.kernel.org
> > Fixes: 42fc7e6543f6 ("landlock: Multithreading support for landlock_restrict_self()")
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> > include/uapi/linux/landlock.h | 4 +-
> > security/landlock/syscalls.c | 14 +-
> > tools/testing/selftests/landlock/audit_test.c | 233 ++++++++++++++++++
> > tools/testing/selftests/landlock/tsync_test.c | 74 ++++++
> > 4 files changed, 319 insertions(+), 6 deletions(-)
> >
> > diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> > index f88fa1f68b77..d37603efc273 100644
> > --- a/include/uapi/linux/landlock.h
> > +++ b/include/uapi/linux/landlock.h
> > @@ -116,7 +116,9 @@ struct landlock_ruleset_attr {
> > * ``LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF``, this flag only affects
> > * future nested domains, not the one being created. It can also be used
> > * with a @ruleset_fd value of -1 to mute subdomain logs without creating a
> > - * domain.
> > + * domain. When combined with %LANDLOCK_RESTRICT_SELF_TSYNC and a
> > + * @ruleset_fd value of -1, this configuration is propagated to all threads
> > + * of the current process.
> > *
> > * The following flag supports policy enforcement in multithreaded processes:
> > *
> > diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> > index 0d66a68677b7..a0bb664e0d31 100644
> > --- a/security/landlock/syscalls.c
> > +++ b/security/landlock/syscalls.c
> > @@ -512,10 +512,13 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> >
> > /*
> > * It is allowed to set LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with
> > - * -1 as ruleset_fd, but no other flag must be set.
> > + * -1 as ruleset_fd, optionally combined with
> > + * LANDLOCK_RESTRICT_SELF_TSYNC to propagate this configuration to all
> > + * threads. No other flag must be set.
> > */
> > if (!(ruleset_fd == -1 &&
> > - flags == LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
> > + (flags & ~LANDLOCK_RESTRICT_SELF_TSYNC) ==
> > + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
>
> Well spotted, thanks!
>
>
> > /* Gets and checks the ruleset. */
> > ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_READ);
> > if (IS_ERR(ruleset))
> > @@ -537,9 +540,10 @@ SYSCALL_DEFINE2(landlock_restrict_self, const int, ruleset_fd, const __u32,
> >
> > /*
> > * The only case when a ruleset may not be set is if
> > - * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set and ruleset_fd is -1.
> > - * We could optimize this case by not calling commit_creds() if this flag
> > - * was already set, but it is not worth the complexity.
> > + * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF is set (optionally with
> > + * LANDLOCK_RESTRICT_SELF_TSYNC) and ruleset_fd is -1. We could
> > + * optimize this case by not calling commit_creds() if this flag was
> > + * already set, but it is not worth the complexity.
> > */
> > if (ruleset) {
> > /*
> > diff --git a/tools/testing/selftests/landlock/audit_test.c b/tools/testing/selftests/landlock/audit_test.c
> > index 20099b8667e7..a193d8a97560 100644
> > --- a/tools/testing/selftests/landlock/audit_test.c
> > +++ b/tools/testing/selftests/landlock/audit_test.c
> > @@ -162,6 +162,7 @@ TEST_F(audit, layers)
> > struct thread_data {
> > pid_t parent_pid;
> > int ruleset_fd, pipe_child, pipe_parent;
> > + bool mute_subdomains;
> > };
> >
> > static void *thread_audit_test(void *arg)
> > @@ -367,6 +368,238 @@ TEST_F(audit, log_subdomains_off_fork)
> > EXPECT_EQ(0, close(ruleset_fd));
> > }
> >
> > +/*
> > + * Thread function: runs two rounds of (create domain, trigger denial, signal
> > + * back), waiting for the main thread before each round. When mute_subdomains
> > + * is set, phase 1 also mutes subdomain logs via the fd=-1 path before creating
> > + * the domain. The ruleset_fd is kept open across both rounds so each
> > + * restrict_self call stacks a new domain layer.
> > + */
> > +static void *thread_sandbox_deny_twice(void *arg)
> > +{
> > + const struct thread_data *data = (struct thread_data *)arg;
> > + uintptr_t err = 0;
> > + char buffer;
> > +
> > + /* Phase 1: optionally mutes, creates a domain, and triggers a denial. */
> > + if (read(data->pipe_parent, &buffer, 1) != 1) {
> > + err = 1;
> > + goto out;
> > + }
> > +
> > + if (data->mute_subdomains &&
> > + landlock_restrict_self(-1,
> > + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF)) {
> > + err = 2;
> > + goto out;
> > + }
> > +
> > + if (landlock_restrict_self(data->ruleset_fd, 0)) {
> > + err = 3;
> > + goto out;
> > + }
> > +
> > + if (kill(data->parent_pid, 0) != -1 || errno != EPERM) {
> > + err = 4;
> > + goto out;
> > + }
> > +
> > + if (write(data->pipe_child, ".", 1) != 1) {
> > + err = 5;
> > + goto out;
> > + }
> > +
> > + /* Phase 2: stacks another domain and triggers a denial. */
> > + if (read(data->pipe_parent, &buffer, 1) != 1) {
> > + err = 6;
> > + goto out;
> > + }
> > +
> > + if (landlock_restrict_self(data->ruleset_fd, 0)) {
> > + err = 7;
> > + goto out;
> > + }
> > +
> > + if (kill(data->parent_pid, 0) != -1 || errno != EPERM) {
> > + err = 8;
> > + goto out;
> > + }
> > +
> > + if (write(data->pipe_child, ".", 1) != 1) {
> > + err = 9;
> > + goto out;
> > + }
> > +
> > +out:
> > + close(data->ruleset_fd);
> > + close(data->pipe_child);
> > + close(data->pipe_parent);
> > + return (void *)err;
> > +}
> > +
> > +/*
> > + * Verifies that LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF with
> > + * LANDLOCK_RESTRICT_SELF_TSYNC and ruleset_fd=-1 propagates log_subdomains_off
> > + * to a sibling thread, suppressing audit logging on domains it subsequently
> > + * creates.
> > + *
> > + * Phase 1 (before TSYNC) acts as an inline baseline: the sibling creates a
> > + * domain and triggers a denial that IS logged.
> > + *
> > + * Phase 2 (after TSYNC) verifies suppression: the sibling stacks another domain
> > + * and triggers a denial that is NOT logged.
> > + */
> > +TEST_F(audit, log_subdomains_off_tsync)
> > +{
> > + const struct landlock_ruleset_attr ruleset_attr = {
> > + .scoped = LANDLOCK_SCOPE_SIGNAL,
> > + };
> > + struct audit_records records;
> > + struct thread_data child_data;
>
> The child_data.mute_subdomains field stays uninitialized in this
> function (and maybe others). Please fix.
>
> struct thread_data child_data = {};
Well spotted!
>
>
> > + int pipe_child[2], pipe_parent[2];
> > + char buffer;
> > + pthread_t thread;
> > + void *thread_ret;
> > +
> > + child_data.parent_pid = getppid();
> > + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> > + child_data.pipe_child = pipe_child[1];
> > + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> > + child_data.pipe_parent = pipe_parent[0];
> > + child_data.ruleset_fd =
> > + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> > + ASSERT_LE(0, child_data.ruleset_fd);
> > +
> > + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> > +
> > + /* Creates the sibling thread. */
> > + ASSERT_EQ(0, pthread_create(&thread, NULL, thread_sandbox_deny_twice,
> > + &child_data));
> > +
> > + /*
> > + * Phase 1: the sibling creates a domain and triggers a denial before
> > + * any log muting. This proves the audit path works.
> > + */
> > + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> > + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> > +
> > + /* The denial must be logged. */
> > + EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd,
> > + child_data.parent_pid, NULL));
> > +
> > + /* Drains any remaining records (e.g. domain allocation). */
> > + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> > +
> > + /*
> > + * Mutes subdomain logs and propagates to the sibling thread via TSYNC,
> > + * without creating a domain.
> > + */
> > + ASSERT_EQ(0, landlock_restrict_self(
> > + -1, LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> > + LANDLOCK_RESTRICT_SELF_TSYNC));
> > +
> > + /*
> > + * Phase 2: the sibling stacks another domain and triggers a denial.
> > + * Because log_subdomains_off was propagated via TSYNC, the new domain
> > + * has log_status=LANDLOCK_LOG_DISABLED.
> > + */
> > + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> > + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> > +
> > + /* No denial record should appear. */
> > + EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
> > + child_data.parent_pid, NULL));
> > +
> > + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> > + EXPECT_EQ(0, records.access);
> > +
> > + EXPECT_EQ(0, close(pipe_child[0]));
> > + EXPECT_EQ(0, close(pipe_parent[1]));
> > + ASSERT_EQ(0, pthread_join(thread, &thread_ret));
> > + EXPECT_EQ(NULL, thread_ret);
> > +}
> > +
> > +/*
> > + * Verifies that LANDLOCK_RESTRICT_SELF_TSYNC without
> > + * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF overrides a sibling thread's
> > + * log_subdomains_off, re-enabling audit logging on domains the sibling
> > + * subsequently creates.
> > + *
> > + * Phase 1: the sibling sets log_subdomains_off, creates a muted domain, and
> > + * triggers a denial that is NOT logged.
> > + *
> > + * Phase 2 (after TSYNC without LOG_SUBDOMAINS_OFF): the sibling stacks another
> > + * domain and triggers a denial that IS logged, proving the muting was
> > + * overridden.
> > + */
> > +TEST_F(audit, tsync_override_log_subdomains_off)
> > +{
> > + const struct landlock_ruleset_attr ruleset_attr = {
> > + .scoped = LANDLOCK_SCOPE_SIGNAL,
> > + };
> > + struct audit_records records;
> > + struct thread_data child_data;
> > + int pipe_child[2], pipe_parent[2];
> > + char buffer;
> > + pthread_t thread;
> > + void *thread_ret;
> > +
> > + child_data.parent_pid = getppid();
> > + ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
> > + child_data.pipe_child = pipe_child[1];
> > + ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));
> > + child_data.pipe_parent = pipe_parent[0];
> > + child_data.ruleset_fd =
> > + landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> > + ASSERT_LE(0, child_data.ruleset_fd);
> > +
> > + ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
> > +
> > + child_data.mute_subdomains = true;
> > +
> > + /* Creates the sibling thread. */
> > + ASSERT_EQ(0, pthread_create(&thread, NULL, thread_sandbox_deny_twice,
> > + &child_data));
> > +
> > + /*
> > + * Phase 1: the sibling mutes subdomain logs, creates a domain, and
> > + * triggers a denial. The denial must not be logged.
> > + */
> > + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> > + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> > +
> > + EXPECT_EQ(-EAGAIN, matches_log_signal(_metadata, self->audit_fd,
> > + child_data.parent_pid, NULL));
> > +
> > + /* Drains any remaining records. */
> > + EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
> > + EXPECT_EQ(0, records.access);
> > +
> > + /*
> > + * Overrides the sibling's log_subdomains_off by calling TSYNC without
> > + * LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF.
> > + */
> > + ASSERT_EQ(0, landlock_restrict_self(child_data.ruleset_fd,
> > + LANDLOCK_RESTRICT_SELF_TSYNC));
> > +
> > + /*
> > + * Phase 2: the sibling stacks another domain and triggers a denial.
> > + * Because TSYNC replaced its log_subdomains_off with 0, the new domain
> > + * has log_status=LANDLOCK_LOG_PENDING.
> > + */
> > + ASSERT_EQ(1, write(pipe_parent[1], ".", 1));
> > + ASSERT_EQ(1, read(pipe_child[0], &buffer, 1));
> > +
> > + /* The denial must be logged. */
> > + EXPECT_EQ(0, matches_log_signal(_metadata, self->audit_fd,
> > + child_data.parent_pid, NULL));
> > +
> > + EXPECT_EQ(0, close(pipe_child[0]));
> > + EXPECT_EQ(0, close(pipe_parent[1]));
> > + ASSERT_EQ(0, pthread_join(thread, &thread_ret));
> > + EXPECT_EQ(NULL, thread_ret);
> > +}
> > +
> > FIXTURE(audit_flags)
> > {
> > struct audit_filter audit_filter;
> > diff --git a/tools/testing/selftests/landlock/tsync_test.c b/tools/testing/selftests/landlock/tsync_test.c
> > index 2b9ad4f154f4..abc290271a1a 100644
> > --- a/tools/testing/selftests/landlock/tsync_test.c
> > +++ b/tools/testing/selftests/landlock/tsync_test.c
> > @@ -247,4 +247,78 @@ TEST(tsync_interrupt)
> > EXPECT_EQ(0, close(ruleset_fd));
> > }
> >
> > +/* clang-format off */
> > +FIXTURE(tsync_without_ruleset) {};
> > +/* clang-format on */
> > +
> > +FIXTURE_VARIANT(tsync_without_ruleset)
> > +{
> > + const __u32 flags;
> > + const int expected_errno;
> > +};
> > +
> > +/* clang-format off */
> > +FIXTURE_VARIANT_ADD(tsync_without_ruleset, tsync_only) {
> > + /* clang-format on */
> > + .flags = LANDLOCK_RESTRICT_SELF_TSYNC,
> > + .expected_errno = EBADF,
> > +};
> > +
> > +/* clang-format off */
> > +FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off_same_exec_off) {
> > + /* clang-format on */
> > + .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> > + LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> > + LANDLOCK_RESTRICT_SELF_TSYNC,
> > + .expected_errno = EBADF,
> > +};
> > +
> > +/* clang-format off */
> > +FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off_new_exec_on) {
> > + /* clang-format on */
> > + .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> > + LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> > + LANDLOCK_RESTRICT_SELF_TSYNC,
> > + .expected_errno = EBADF,
> > +};
> > +
> > +/* clang-format off */
> > +FIXTURE_VARIANT_ADD(tsync_without_ruleset, all_flags) {
> > + /* clang-format on */
> > + .flags = LANDLOCK_RESTRICT_SELF_LOG_SAME_EXEC_OFF |
> > + LANDLOCK_RESTRICT_SELF_LOG_NEW_EXEC_ON |
> > + LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> > + LANDLOCK_RESTRICT_SELF_TSYNC,
> > + .expected_errno = EBADF,
> > +};
> > +
> > +/* clang-format off */
> > +FIXTURE_VARIANT_ADD(tsync_without_ruleset, subdomains_off) {
> > + /* clang-format on */
> > + .flags = LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF |
> > + LANDLOCK_RESTRICT_SELF_TSYNC,
> > + .expected_errno = 0,
> > +};
> > +
> > +FIXTURE_SETUP(tsync_without_ruleset)
> > +{
> > +}
> > +
> > +FIXTURE_TEARDOWN(tsync_without_ruleset)
> > +{
> > +}
> > +
> > +TEST_F(tsync_without_ruleset, check)
> > +{
> > + int ret;
> > +
I'll set NNP here.
> > + ret = landlock_restrict_self(-1, variant->flags);
> > + if (variant->expected_errno) {
> > + EXPECT_EQ(-1, ret);
> > + EXPECT_EQ(variant->expected_errno, errno);
> > + } else {
> > + EXPECT_EQ(0, ret);
> > + }
> > +}
>
> We are not setting the no_new_privs flag in this test, as we do in the
> others.
>
> no_new_privs or CAP_SYS_ADMIN are required in the implementation, even
> when ruleset_fd == -1 and passing
> LANDLOCK_RESTRICT_SELF_LOG_SUBDOMAINS_OFF.
Sure.
>
> > +
> > TEST_HARNESS_MAIN
> > --
> > 2.53.0
> >
>
> Reviewed-by: Günther Noack <gnoack3000@gmail.com>
>
> But please fix the flaky test.
>
> –Günther
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork()
2026-04-07 16:03 ` Mickaël Salaün
@ 2026-04-07 19:00 ` Günther Noack
0 siblings, 0 replies; 7+ messages in thread
From: Günther Noack @ 2026-04-07 19:00 UTC (permalink / raw)
To: Mickaël Salaün
Cc: Jann Horn, Günther Noack, linux-security-module, stable
On Tue, Apr 07, 2026 at 06:03:58PM +0200, Mickaël Salaün wrote:
> On Tue, Apr 07, 2026 at 09:30:40AM +0200, Günther Noack wrote:
> > On Sat, Apr 04, 2026 at 10:49:57AM +0200, Mickaël Salaün wrote:
> > > --- a/security/landlock/cred.c
> > > +++ b/security/landlock/cred.c
> > > @@ -22,10 +22,8 @@ static void hook_cred_transfer(struct cred *const new,
> > > const struct landlock_cred_security *const old_llcred =
> > > landlock_cred(old);
> > >
> > > - if (old_llcred->domain) {
> > > - landlock_get_ruleset(old_llcred->domain);
> > > - *landlock_cred(new) = *old_llcred;
> > > - }
> > > + landlock_get_ruleset(old_llcred->domain);
> > > + *landlock_cred(new) = *old_llcred;
> >
> > This fix looks correct for the hook_cred_prepare() case (and of
> > course, hook_cred_prepare() calls hook_cred_transfer() in Landlock).
> >
> >
> > But I'm afraid I might have spotted another issue here:
> >
> > If I look at the code in security/keys/process_keys.c, where
> > security_tranfer_creds() is called, the "old" object is actually
> > already initialized, and if we are not checking for that, I think we
> > are leaking memory.
>
> old is only a partially initialized credential, and the Landlock
> part is not set yet, which is the goal of hook_transfer_creds(), so
> there is no leak.
Ah, you are right. I think we might have mixed up the names "old" and
"new" in the discussion briefly, but it's still correct - the target
credential is only partially populated and its Landlock domain is not
set, so we don't need to call landlock_put_ruleset() on it.
> > I would suggest to use the helper landlock_cred_copy() from cred.h for
>
> This is not required but if we would like to do it anyway, that would
> not be backportable and would introduce a (minimal) performance penalty.
Fair enough, the backportability is a reasonable argument.
> > Test looks fine.
> >
> > While I do still think we should investigate the memory leak, this
> > commit is, as it is, already a strict improvement over what we had
> > before, so:
> >
> > Reviewed-by: Günther Noack <gnoack3000@gmail.com>
>
> I'll keep your tag if this patch is ok with you as-is.
Yes, absolutely.
–Günther
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-04-07 19:01 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-04 8:49 [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Mickaël Salaün
2026-04-04 8:49 ` [PATCH v1 2/2] landlock: Allow TSYNC with LOG_SUBDOMAINS_OFF and fd=-1 Mickaël Salaün
2026-04-07 8:25 ` Günther Noack
2026-04-07 16:06 ` Mickaël Salaün
2026-04-07 7:30 ` [PATCH v1 1/2] landlock: Fix log_subdomains_off inheritance across fork() Günther Noack
2026-04-07 16:03 ` Mickaël Salaün
2026-04-07 19:00 ` Günther Noack
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox