Fix Landlock audit test flakiness

snprintf() returns the number of characters that would have been written, excluding the terminating NUL byte. When the output is truncated, this return value equals or exceeds the buffer size. Fix matches_log_domain_allocated() and matches_log_domain_deallocated() to detect truncation with ">=" instead of ">".

Cc Günther Noack <[email protected]>

Cc [email protected]

Fixes 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")

Reviewed-by Günther Noack <[email protected]>

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

---

Changes since v1:
https://lore.kernel.org/r/[email protected]
- New patch (split from the drain fix).

---
 tools/testing/selftests/landlock/audit.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

tools/testing/selftests/landlock/audit.h +2 -2

index 44eb433e9666..1049a0582af5 100644
@@ -309,7 +309,7 @@ static int __maybe_unused matches_log_domain_allocated(int audit_fd, pid_t pid,
 
 	log_match_len =
 		snprintf(log_match, sizeof(log_match), log_template, pid);
-	if (log_match_len > sizeof(log_match))
+	if (log_match_len >= sizeof(log_match))
 		return -E2BIG;
 
 	return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,

@@ -326,7 +326,7 @@ static int __maybe_unused matches_log_domain_deallocated(
 
 	log_match_len = snprintf(log_match, sizeof(log_match), log_template,
 				 num_denials);
-	if (log_match_len > sizeof(log_match))
+	if (log_match_len >= sizeof(log_match))
 		return -E2BIG;
 
 	return audit_match_record(audit_fd, AUDIT_LANDLOCK_DOMAIN, log_match,

Signature · 1 line

2.53.0

audit_init() opens a netlink socket and configures it, but leaks the file descriptor if audit_set_status() or setsockopt() fails. Fix this by jumping to an error path that closes the socket before returning.

Apply the same fix to audit_init_with_exe_filter(), which leaks the file descriptor from audit_init() if audit_init_filter_exe() or audit_filter_exe() fails, and to audit_cleanup(), which leaks it if audit_init_filter_exe() fails in FIXTURE_TEARDOWN_PARENT().

Cc Günther Noack <[email protected]>

Cc [email protected]

Fixes 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")

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

---

Changes since v1:
https://lore.kernel.org/r/[email protected]
- New patch (split from the drain fix, extended to audit_init_with_exe_filter() and audit_cleanup()).

---
 tools/testing/selftests/landlock/audit.h | 26 +++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

tools/testing/selftests/landlock/audit.h +19 -7

index 1049a0582af5..6422943fc69e 100644
@@ -379,19 +379,25 @@ static int audit_init(void)
 
 	err = audit_set_status(fd, AUDIT_STATUS_ENABLED, 1);
 	if (err)
-		return err;
+		goto err_close;
 
 	err = audit_set_status(fd, AUDIT_STATUS_PID, getpid());
 	if (err)
-		return err;
+		goto err_close;
 
 	/* Sets a timeout for negative tests. */
 	err = setsockopt(fd, SOL_SOCKET, SO_RCVTIMEO, &audit_tv_default,
 			 sizeof(audit_tv_default));
-	if (err)
-		return -errno;
+	if (err) {
+		err = -errno;
+		goto err_close;
+	}
 
 	return fd;
+
+err_close:
+	close(fd);
+	return err;
 }
 
 static int audit_init_filter_exe(struct audit_filter *filter, const char *path)

@@ -441,8 +447,10 @@ static int audit_cleanup(int audit_fd, struct audit_filter *filter)
 
 		filter = &new_filter;
 		err = audit_init_filter_exe(filter, NULL);
-		if (err)
+		if (err) {
+			close(audit_fd);
 			return err;
+		}
 	}
 
 	/* Filters might not be in place. */

@@ -468,11 +476,15 @@ static int audit_init_with_exe_filter(struct audit_filter *filter)
 
 	err = audit_init_filter_exe(filter, NULL);
 	if (err)
-		return err;
+		goto err_close;
 
 	err = audit_filter_exe(fd, filter, AUDIT_ADD_RULE);
 	if (err)
-		return err;
+		goto err_close;
 
 	return fd;
+
+err_close:
+	close(fd);
+	return err;
 }

Signature · 1 line

2.53.0

Non-audit Landlock tests generate audit records as side effects when audit_enabled is non-zero (e.g. from boot configuration). These records accumulate in the kernel audit backlog while no audit daemon socket is open. When the next test opens a new netlink socket and registers as the audit daemon, the stale backlog is delivered, causing baseline record count checks to fail spuriously.

Fix this by draining all pending records in audit_init() right after setting the receive timeout. The 1-usec SO_RCVTIMEO causes audit_recv() to return -EAGAIN once the backlog is empty, naturally terminating the drain loop.

Domain deallocation records are emitted asynchronously from a work queue, so they may still arrive after the drain. Remove records.domain == 0 checks that are not preceded by audit_match_record() calls, which would otherwise consume stale records before the count. Document this constraint above audit_count_records().

Increasing the drain timeout to catch in-flight deallocation records was considered but rejected: a longer timeout adds latency to every audit_init() call even when no stale record is pending, and any fixed timeout is still not guaranteed to catch all records under load. Removing the unprotected checks is simpler and avoids the spurious failures.

Cc Günther Noack <[email protected]>

Cc [email protected]

Fixes 6a500b22971c ("selftests/landlock: Add tests for audit flags and domain IDs")

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

---

Changes since v1:
https://lore.kernel.org/r/[email protected]
- Also remove domain checks from audit.trace and scoped_audit.connect_to_child.
- Document records.domain == 0 constraint above audit_count_records().
- Explain why a longer drain timeout was rejected.
- Drop Reviewed-by (new code comment not in v1).
- Split snprintf and fd leak fixes into separate patches.

---
 tools/testing/selftests/landlock/audit.h      | 19 +++++++++++++++++++
 tools/testing/selftests/landlock/audit_test.c |  2 --
 .../testing/selftests/landlock/ptrace_test.c  |  1 -
 .../landlock/scoped_abstract_unix_test.c      |  1 -
 4 files changed, 19 insertions(+), 4 deletions(-)

tools/testing/selftests/landlock/audit.h +19 -0

index 6422943fc69e..74e1c3d763be 100644
@@ -338,6 +338,15 @@ struct audit_records {
 	size_t domain;
 };
 
+/*
+ * WARNING: Do not assert records.domain == 0 without a preceding
+ * audit_match_record() call.  Domain deallocation records are emitted
+ * asynchronously from kworker threads and can arrive after the drain in
+ * audit_init(), corrupting the domain count.  A preceding audit_match_record()
+ * call consumes stale records while scanning, making the assertion safe in
+ * practice because stale deallocation records arrive before the expected access
+ * records.
+ */
 static int audit_count_records(int audit_fd, struct audit_records *records)
 {
 	struct audit_message msg;

@@ -393,6 +402,16 @@ static int audit_init(void)
 		goto err_close;
 	}
 
+	/*
+	 * Drains stale audit records that accumulated in the kernel backlog
+	 * while no audit daemon socket was open.  This happens when non-audit
+	 * Landlock tests generate records while audit_enabled is non-zero (e.g.
+	 * from boot configuration), or when domain deallocation records arrive
+	 * asynchronously after a previous test's socket was closed.
+	 */
+	while (audit_recv(fd, NULL) == 0)
+		;
+
 	return fd;
 
 err_close:

tools/testing/selftests/landlock/audit_test.c +0 -2

index 46d02d49835a..f92ba6774faa 100644
@@ -412,7 +412,6 @@ TEST_F(audit_flags, signal)
 		} else {
 			EXPECT_EQ(1, records.access);
 		}
-		EXPECT_EQ(0, records.domain);
 
 		/* Updates filter rules to match the drop record. */
 		set_cap(_metadata, CAP_AUDIT_CONTROL);

@@ -601,7 +600,6 @@ TEST_F(audit_exec, signal_and_open)
 	/* Tests that there was no denial until now. */
 	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
 	EXPECT_EQ(0, records.access);
-	EXPECT_EQ(0, records.domain);
 
 	/*
 	 * Wait for the child to do a first denied action by layer1 and

tools/testing/selftests/landlock/ptrace_test.c +0 -1

index 4f64c90583cd..1b6c8b53bf33 100644
@@ -342,7 +342,6 @@ TEST_F(audit, trace)
 	/* Makes sure there is no superfluous logged records. */
 	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
 	EXPECT_EQ(0, records.access);
-	EXPECT_EQ(0, records.domain);
 
 	yama_ptrace_scope = get_yama_ptrace_scope();
 	ASSERT_LE(0, yama_ptrace_scope);

tools/testing/selftests/landlock/scoped_abstract_unix_test.c +0 -1

index 72f97648d4a7..c47491d2d1c1 100644
@@ -312,7 +312,6 @@ TEST_F(scoped_audit, connect_to_child)
 	/* Makes sure there is no superfluous logged records. */
 	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
 	EXPECT_EQ(0, records.access);
-	EXPECT_EQ(0, records.domain);
 
 	ASSERT_EQ(0, pipe2(pipe_child, O_CLOEXEC));
 	ASSERT_EQ(0, pipe2(pipe_parent, O_CLOEXEC));

Signature · 1 line

2.53.0

On architectures where __u64 is unsigned long (e.g. powerpc64), using %llx to format a __u64 triggers a -Wformat warning because %llx expects unsigned long long. Cast the argument to unsigned long long.

Cc Günther Noack <[email protected]>

Cc [email protected]

Fixes a549d055a22e ("selftests/landlock: Add network tests")

Reported-by kernel test robot <[email protected]>

Closes https://lore.kernel.org/r/[email protected]/ open here

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

---

Changes since v2:
- New patch.

---
 tools/testing/selftests/landlock/net_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tools/testing/selftests/landlock/net_test.c +1 -1

index b34b139b3f89..4c528154ea92 100644
@@ -1356,7 +1356,7 @@ TEST_F(mini, network_access_rights)
 					    &net_port, 0))
 		{
 			TH_LOG("Failed to add rule with access 0x%llx: %s",
-			       access, strerror(errno));
+			       (unsigned long long)access, strerror(errno));
 		}
 	}
 	EXPECT_EQ(0, close(ruleset_fd));

Signature · 1 line

2.53.0

This series fixes two classes of audit selftest failures plus two minor bugs in the audit test helpers.

The main issue is that domain deallocation audit records are emitted asynchronously from kworker threads and can arrive after a previous test's socket has been closed. This causes two distinct failure modes:

- audit_match_record() picks up a stale deallocation record from a previous test instead of the expected one, causing a domain ID mismatch. The audit.layers test (which reads 16 deallocation records in sequence) is particularly vulnerable because the large read window allows stale records to interleave. Patch 4 fixes this by filtering deallocation records by domain ID and skipping type-matching records with wrong content patterns.

- audit_count_records() counts stale deallocation records from a previous test, incrementing records.domain from the expected 0 to 1. Patch 3 fixes this by draining stale records at audit_init() time and removing records.domain == 0 checks that are not preceded by audit_match_record() calls (which would consume stale records).

These races are more likely to manifest when additional instrumentation changes kworker timing in the deallocation path (e.g. with the upcoming Landlock tracepoints work).

The two minor fixes (patches 1-2) correct a snprintf truncation check off-by-one and socket file descriptor leaks on error paths in audit_init(), audit_init_with_exe_filter(), and audit_cleanup(). Patch 5 fixes a __u64 format warning reported by the kbuild bot on powerpc64.

Patch 1 is an exact subset of the v1 combined patch, which is why it carries the Reviewed-by tag. Patches 2 and 3 extend beyond what was in v1, so the Reviewed-by is not carried. Patches 4 and 5 are new.

Changes since v2:
https://lore.kernel.org/r/[email protected]
- Patches 4-5: fix __u64 format warnings on powerpc64 (cast to unsigned long long for %llx). Patch 5 is new.

Changes since v1:
https://lore.kernel.org/r/[email protected]
- Split the combined drain fix into four separate patches.
- Patch 2: extend fd leak fix to audit_init_with_exe_filter() and audit_cleanup().
- Patch 3: also remove domain checks from audit.trace and scoped_audit.connect_to_child, document constraint, explain why a longer drain timeout was rejected.
- Patch 4: new, add domain ID filtering and timeout management to matches_log_domain_deallocated(), skip stale records in audit_match_record().

Mickaël Salaün (5):
selftests/landlock: Fix snprintf truncation checks in audit helpers selftests/landlock: Fix socket file descriptor leaks in audit helpers selftests/landlock: Drain stale audit records on init selftests/landlock: Skip stale records in audit_match_record() selftests/landlock: Fix format warning for __u64 in net_test

 tools/testing/selftests/landlock/audit.h      | 133 ++++++++++++++----
 tools/testing/selftests/landlock/audit_test.c |  36 ++---
 tools/testing/selftests/landlock/net_test.c   |   2 +-
 .../testing/selftests/landlock/ptrace_test.c  |   1 -
 .../landlock/scoped_abstract_unix_test.c      |   1 -
 5 files changed, 119 insertions(+), 54 deletions(-)

Signature · 1 line

2.53.0

My kernel config is this:

    make defconfig
    make kvm_guest.config
    KCONFIG_CONFIG="${KBUILD_OUTPUT}/.config" ./scripts/kconfig/merge_config.sh "${KBUILD_OUTPUT}/.config" tools/testing/selftests/landlock/config
    make debug.config
    echo "CONFIG_RANDOMIZE_BASE=n" >> "${KBUILD_OUTPUT}/.config"
    make olddefconfig

Hello!

Quoted 59 lines · Mickaël Salaün

This series fixes two classes of audit selftest failures plus two minor bugs in the audit test helpers.

The main issue is that domain deallocation audit records are emitted asynchronously from kworker threads and can arrive after a previous test's socket has been closed. This causes two distinct failure modes:

- audit_match_record() picks up a stale deallocation record from a previous test instead of the expected one, causing a domain ID mismatch. The audit.layers test (which reads 16 deallocation records in sequence) is particularly vulnerable because the large read window allows stale records to interleave. Patch 4 fixes this by filtering deallocation records by domain ID and skipping type-matching records with wrong content patterns.

- audit_count_records() counts stale deallocation records from a previous test, incrementing records.domain from the expected 0 to 1. Patch 3 fixes this by draining stale records at audit_init() time and removing records.domain == 0 checks that are not preceded by audit_match_record() calls (which would consume stale records).

These races are more likely to manifest when additional instrumentation changes kworker timing in the deallocation path (e.g. with the upcoming Landlock tracepoints work).

The two minor fixes (patches 1-2) correct a snprintf truncation check off-by-one and socket file descriptor leaks on error paths in audit_init(), audit_init_with_exe_filter(), and audit_cleanup(). Patch 5 fixes a __u64 format warning reported by the kbuild bot on powerpc64.

Patch 1 is an exact subset of the v1 combined patch, which is why it carries the Reviewed-by tag. Patches 2 and 3 extend beyond what was in v1, so the Reviewed-by is not carried. Patches 4 and 5 are new.

Changes since v2:
https://lore.kernel.org/r/[email protected]
- Patches 4-5: fix __u64 format warnings on powerpc64 (cast to unsigned long long for %llx). Patch 5 is new.

Changes since v1:
https://lore.kernel.org/r/[email protected]
- Split the combined drain fix into four separate patches.
- Patch 2: extend fd leak fix to audit_init_with_exe_filter() and audit_cleanup().
- Patch 3: also remove domain checks from audit.trace and scoped_audit.connect_to_child, document constraint, explain why a longer drain timeout was rejected.
- Patch 4: new, add domain ID filtering and timeout management to matches_log_domain_deallocated(), skip stale records in audit_match_record().

Mickaël Salaün (5):
selftests/landlock: Fix snprintf truncation checks in audit helpers selftests/landlock: Fix socket file descriptor leaks in audit helpers selftests/landlock: Drain stale audit records on init selftests/landlock: Skip stale records in audit_match_record() selftests/landlock: Fix format warning for __u64 in net_test

Quoted 10 lines

 tools/testing/selftests/landlock/audit.h      | 133 ++++++++++++++----
 tools/testing/selftests/landlock/audit_test.c |  36 ++---
 tools/testing/selftests/landlock/net_test.c   |   2 +-
 .../testing/selftests/landlock/ptrace_test.c  |   1 -
 .../landlock/scoped_abstract_unix_test.c      |   1 -
 5 files changed, 119 insertions(+), 54 deletions(-)

Signature · 1 line

2.53.0

I am still getting flaky audit tests even with these patches, I am afraid. It differs which of these tests is flaking, some of them still do, for example:

# RUN audit_layout1.remove_dir ...
# fs_test.c:7281:remove_dir:Expected 0 (0) == matches_log_fs(_metadata, self->audit_fd, "fs\\.remove_dir", dir_s1d2) (-11) # remove_dir: Test failed
# ❌ FAIL audit_layout1.remove_dir
not ok 191 audit_layout1.remove_dir
# RUN audit_layout1.read_dir ...
# ✅ OK audit_layout1.read_dir
ok 192 audit_layout1.read_dir
# RUN audit_layout1.read_file ...

# ✅ OK audit_layout1.read_file
ok 193 audit_layout1.read_file
# RUN audit_layout1.write_file ...
# fs_test.c:7221:write_file:Expected 0 (0) == matches_log_fs(_metadata, self->audit_fd, "fs\\.write_file", file1_s1d1) (-11) # fs_test.c:7224:write_file:Expected 0 (0) == records.access (1)
# write_file: Test failed
# ❌ FAIL audit_layout1.write_file
not ok 194 audit_layout1.write_file

My kernel config is this:

    make defconfig
    make kvm_guest.config
    KCONFIG_CONFIG="${KBUILD_OUTPUT}/.config" ./scripts/kconfig/merge_config.sh "${KBUILD_OUTPUT}/.config" tools/testing/selftests/landlock/config
    make debug.config
    echo "CONFIG_RANDOMIZE_BASE=n" >> "${KBUILD_OUTPUT}/.config"
    make olddefconfig

and then I run the selftests in Qemu with these flags:

qemu-system-x86_64 \
    -nographic \
    -m 4G \
    -enable-kvm \
    -append "console=ttyS0 lsm=landlock no_hash_pointers" \
    -kernel "${KBUILD_OUTPUT}/arch/x86/boot/bzImage" \
    -initrd "${INITRAMFS}"

This is using my own selftest runner scripts which builds an initramfs with the statically linked selftests.

Do you have a hunch what might be missing there? In the test run above, I have applied your V4 patch set on top of the current master, 5619b098e2fbf3a23bf13d91897056a1fe238c6d ("Merge tag 'for-7.0-rc6-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux").