Merge tag 'landlock-7.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/mic/linuxHEADmaster

Pull Landlock fixes from Mickaël Salaün:
 "This fixes a use-after-free and a lockdep assert NULL dereferencing,
  and properly truncates too-long strings printed by a Landlock
  tracepoint. Most of the changes are brought by new tests"

* tag 'landlock-7.3-rc3' of git://git.kernel.org/pub/scm/linux/kernel/git/mic/linux:
  landlock: Test trace path output boundaries
  landlock: Bound escaped trace path output
  landlock: Clean up ruleset validation checks
  selftests/landlock: Test abstract socket trace name limits
  landlock: Fix use-after-free of the source's parent directory

2 files changed, 214 insertions, 22 deletions

diff --git a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
index 6dbe863ea571..5dc0debacb2a 100644
--- a/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
+++ b/tools/testing/selftests/landlock/scoped_abstract_unix_test.c
@@ -1222,7 +1222,7 @@ FIXTURE_SETUP(trace_unix)
 	int ret;
 
 	set_cap(_metadata, CAP_SYS_ADMIN);
-	ASSERT_EQ(0, unshare(CLONE_NEWNS));
+	ASSERT_EQ(0, unshare(CLONE_NEWNS | CLONE_NEWNET));
 	ASSERT_EQ(0, mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL));
 
 	ret = tracefs_fixture_setup();
@@ -1252,6 +1252,11 @@ FIXTURE_TEARDOWN(trace_unix)
 	clear_cap(_metadata, CAP_SYS_ADMIN);
 }
 
+static const char
+	trace_unix_max_name[sizeof(((struct sockaddr_un *)0)->sun_path)] = {
+		[0 ... sizeof(trace_unix_max_name) - 2] = 'x',
+	};
+
 /* clang-format off */
 FIXTURE_VARIANT(trace_unix) {
 	/* clang-format on */
@@ -1259,6 +1264,8 @@ FIXTURE_VARIANT(trace_unix) {
 	bool sandbox;
 	bool sandbox_target; /* Peer owned by a domain: peer_domain != 0. */
 	int expect_denied;
+	const char *name; /* NULL generates a PID-based binary name. */
+	size_t name_len;
 };
 
 /* clang-format off */
@@ -1281,6 +1288,26 @@ FIXTURE_VARIANT_ADD(trace_unix, stream_allowed) {
 	.sandbox_target = false, .expect_denied = 0,
 };
 
+/* Stream: lower abstract-name length boundary. */
+FIXTURE_VARIANT_ADD(trace_unix, stream_denied_empty_name) {
+	.sock_type = SOCK_STREAM,
+	.sandbox = true,
+	.sandbox_target = false,
+	.expect_denied = 1,
+	.name = "",
+	.name_len = 0,
+};
+
+/* Stream: upper abstract-name length boundary. */
+FIXTURE_VARIANT_ADD(trace_unix, stream_denied_max_name) {
+	.sock_type = SOCK_STREAM,
+	.sandbox = true,
+	.sandbox_target = false,
+	.expect_denied = 1,
+	.name = trace_unix_max_name,
+	.name_len = sizeof(trace_unix_max_name) - 1,
+};
+
 /* Datagram: sandboxed client sendto() an unsandboxed peer (peer_domain=0). */
 FIXTURE_VARIANT_ADD(trace_unix, dgram_denied) {
 	.sock_type = SOCK_DGRAM, .sandbox = true,
@@ -1304,12 +1331,11 @@ FIXTURE_VARIANT_ADD(trace_unix, dgram_allowed) {
 /*
  * A sandboxed thread reaching an abstract unix socket peer through connect(2)
  * (stream) or sendto(2) (datagram) is denied and emits
- * landlock_deny_scope_abstract_unix_socket.  The abstract name is crafted with
- * a space and an embedded NUL followed by an "END" marker to check the
- * tracepoint escaping and its length handling (a raw space would break the
- * sun_path field regex; strlen() would truncate at the NUL and drop "END").
- * peer_pid is only meaningful for a stream peer (a datagram peer has no
- * SO_PEERCRED), so it is asserted only there.
+ * landlock_deny_scope_abstract_unix_socket.  The default abstract name has a
+ * space and an embedded NUL followed by an "END" marker to check escaping and
+ * binary length handling.  Additional stream variants cover the minimum and
+ * maximum abstract-name lengths.  peer_pid is only meaningful for a stream peer
+ * (a datagram peer has no SO_PEERCRED), so it is asserted only there.
  */
 TEST_F(trace_unix, deny_scope_unix)
 {
@@ -1336,12 +1362,19 @@ TEST_F(trace_unix, deny_scope_unix)
 	ASSERT_LE(0, server_fd);
 
 	addr.sun_path[0] = '\0';
-	name_len = snprintf(addr.sun_path + 1, sizeof(addr.sun_path) - 1,
-			    "landlock_trace_test_%d ", getpid());
-	addr.sun_path[1 + name_len] = '\0';
-	memcpy(addr.sun_path + 1 + name_len + 1, "END", 3);
-	addr_len =
-		offsetof(struct sockaddr_un, sun_path) + 1 + name_len + 1 + 3;
+	if (variant->name) {
+		ASSERT_LE(variant->name_len, sizeof(addr.sun_path) - 1);
+		memcpy(addr.sun_path + 1, variant->name, variant->name_len);
+		name_len = variant->name_len;
+	} else {
+		name_len = snprintf(addr.sun_path + 1,
+				    sizeof(addr.sun_path) - 1,
+				    "landlock_trace_test_%d ", getpid());
+		addr.sun_path[1 + name_len] = '\0';
+		memcpy(addr.sun_path + 1 + name_len + 1, "END", 3);
+		name_len += 1 + 3;
+	}
+	addr_len = offsetof(struct sockaddr_un, sun_path) + 1 + name_len;
 
 	ASSERT_EQ(0, bind(server_fd, (struct sockaddr *)&addr, addr_len));
 	if (variant->sock_type == SOCK_STREAM)
@@ -1430,19 +1463,18 @@ TEST_F(trace_unix, deny_scope_unix)
 		       count, buf);
 	}
 
-	/*
-	 * sun_path is escaped: a raw space would break this field's [^ ]*$
-	 * regex, so a successful extract proves the space was escaped, and its
-	 * full length is honored: the "END" marker after the embedded NUL must
-	 * survive (strlen() would truncate it at the NUL).
-	 */
 	ASSERT_EQ(0, tracefs_extract_field(
 			     buf,
 			     REGEX_DENY_SCOPE_ABSTRACT_UNIX_SOCKET(TRACE_TASK),
 			     "sun_path", field, sizeof(field)));
-	EXPECT_NE(NULL, strstr(field, "END"))
-	{
-		TH_LOG("sun_path truncated or unescaped: %s", field);
+	if (variant->name) {
+		EXPECT_STREQ(variant->name, field);
+	} else {
+		/* An embedded NUL must not truncate the following marker. */
+		EXPECT_NE(NULL, strstr(field, "END"))
+		{
+			TH_LOG("sun_path truncated or unescaped: %s", field);
+		}
 	}
 
 	/* peer_pid is the parent's PID for a stream peer (0 for datagram). */
diff --git a/tools/testing/selftests/landlock/trace_fs_test.c b/tools/testing/selftests/landlock/trace_fs_test.c
index 5220f6a4bee1..4543a25c1f55 100644
--- a/tools/testing/selftests/landlock/trace_fs_test.c
+++ b/tools/testing/selftests/landlock/trace_fs_test.c
@@ -6,8 +6,10 @@
  */
 
 #define _GNU_SOURCE
+#include <assert.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <limits.h>
 #include <linux/landlock.h>
 #include <sched.h>
 #include <stdio.h>
@@ -23,6 +25,63 @@
 
 #define TRACE_TASK "trace_fs_test"
 
+/* Mirrors TRACE_SEQ_SIZE, conservatively larger than the usable buffer. */
+#define TRACE_SEQUENCE_SIZE 8192
+#define OCTAL_ESCAPE_LEN 4
+#define LONG_PATH_COMPONENT_COUNT 11
+#define LONG_PATH_COMPONENT_LEN 240
+#define LONG_PATH_LEN                                                \
+	(LONG_PATH_COMPONENT_COUNT * (LONG_PATH_COMPONENT_LEN + 1) + \
+	 sizeof("/tmp"))
+#define LONG_ESCAPED_PATH_LEN \
+	(LONG_PATH_COMPONENT_COUNT * LONG_PATH_COMPONENT_LEN * OCTAL_ESCAPE_LEN)
+
+static_assert(LONG_ESCAPED_PATH_LEN > TRACE_SEQUENCE_SIZE,
+	      "escaped path must exceed the trace sequence");
+static_assert(LONG_PATH_LEN < PATH_MAX, "path must fit in PATH_MAX");
+
+static void create_long_path(struct __test_metadata *const _metadata,
+			     char *path)
+{
+	size_t path_len;
+
+	strcpy(path, "/tmp");
+	path_len = strlen(path);
+
+	set_cap(_metadata, CAP_SYS_ADMIN);
+	ASSERT_EQ(0, mount("tmpfs", "/tmp", "tmpfs", 0, NULL));
+	clear_cap(_metadata, CAP_SYS_ADMIN);
+
+	for (int i = 0; i < LONG_PATH_COMPONENT_COUNT; i++) {
+		path[path_len++] = '/';
+		memset(path + path_len, ' ', LONG_PATH_COMPONENT_LEN);
+		path_len += LONG_PATH_COMPONENT_LEN;
+		path[path_len] = '\0';
+		ASSERT_EQ(0, mkdir(path, 0700));
+	}
+}
+
+static void expect_truncated_path(struct __test_metadata *const _metadata,
+				  const char *const trace,
+				  const char *const event_regex)
+{
+	static const char marker[] = "\xe2\x80\xa6";
+	char *path;
+	size_t path_len;
+
+	path = malloc(TRACE_SEQUENCE_SIZE);
+	ASSERT_NE(NULL, path);
+	ASSERT_EQ(0, tracefs_extract_field(trace, event_regex, "path", path,
+					   TRACE_SEQUENCE_SIZE));
+	EXPECT_EQ(path, strstr(path, "/tmp/"));
+	EXPECT_NE(NULL, strstr(path, "\\040"));
+
+	path_len = strlen(path);
+	ASSERT_LE(sizeof(marker) - 1, path_len);
+	EXPECT_STREQ(marker, path + path_len - (sizeof(marker) - 1));
+	free(path);
+}
+
 /*
  * Like REGEX_DENY_ACCESS_FS(), but pins the logged field to a specific value
  * ("0" or "1") so a test can tell a suppressed (quiet) denial from a logged
@@ -184,6 +243,107 @@ TEST_F(trace_fs, add_rule_fs)
 }
 
 /*
+ * Verifies that a path whose escaping exceeds the trace scratch sequence does
+ * not corrupt a sibling symbolic field.
+ */
+TEST_F(trace_fs, add_rule_fs_escaped_path_overflow)
+{
+	static const char access_prefix[] = "execute|write_file|read_file|";
+	static const char access_suffix[] = "|ioctl_dev|resolve_unix";
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
+	};
+	struct landlock_path_beneath_attr path_beneath = {
+		.allowed_access = LANDLOCK_ACCESS_FS_READ_FILE,
+	};
+	char path[PATH_MAX];
+	char *buf, field_buf[256];
+	size_t field_len;
+	int ruleset_fd, count;
+
+	create_long_path(_metadata, path);
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+	path_beneath.parent_fd = open(path, O_PATH | O_DIRECTORY | O_CLOEXEC);
+	ASSERT_LE(0, path_beneath.parent_fd);
+
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+				       &path_beneath, 0));
+	ASSERT_EQ(0, close(path_beneath.parent_fd));
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	buf = tracefs_read_buf();
+	ASSERT_NE(NULL, buf);
+
+	count = tracefs_count_matches(buf, REGEX_ADD_RULE_FS(TRACE_TASK));
+	EXPECT_EQ(1, count)
+	{
+		TH_LOG("Expected 1 add_rule_fs event, got %d\n%s", count, buf);
+	}
+
+	/*
+	 * The marker catches a full revert with any compiler.  The symbolic
+	 * field also catches scratch-sequence poisoning when the compiler
+	 * evaluates the overflowing path first, as GCC currently does.
+	 */
+	ASSERT_EQ(0, tracefs_extract_field(buf, REGEX_ADD_RULE_FS(TRACE_TASK),
+					   "access_rights", field_buf,
+					   sizeof(field_buf)));
+	EXPECT_EQ(0,
+		  strncmp(field_buf, access_prefix, sizeof(access_prefix) - 1));
+	EXPECT_EQ(NULL, strstr(field_buf, "|refer|"));
+	field_len = strlen(field_buf);
+	ASSERT_LE(sizeof(access_suffix) - 1, field_len);
+	EXPECT_STREQ(access_suffix,
+		     field_buf + field_len - (sizeof(access_suffix) - 1));
+	expect_truncated_path(_metadata, buf, REGEX_ADD_RULE_FS(TRACE_TASK));
+
+	free(buf);
+}
+
+/*
+ * Verifies that an overflowing denied path does not corrupt its sibling
+ * symbolic blockers field.
+ */
+TEST_F(trace_fs, deny_access_fs_escaped_path_overflow)
+{
+	char path[PATH_MAX];
+	char *buf, field_buf[64];
+	int count, err;
+
+	create_long_path(_metadata, path);
+	ASSERT_EQ(0, tracefs_clear_buf());
+
+	sandbox_child_fs_access(_metadata, "/usr", LANDLOCK_ACCESS_FS_READ_DIR,
+				LANDLOCK_ACCESS_FS_READ_DIR, path);
+
+	buf = tracefs_read_buf();
+	ASSERT_NE(NULL, buf);
+
+	count = tracefs_count_matches(buf, REGEX_DENY_ACCESS_FS(TRACE_TASK));
+	EXPECT_EQ(1, count)
+	{
+		TH_LOG("Expected 1 deny_access_fs event, got %d\n%s", count,
+		       buf);
+	}
+
+	/*
+	 * The marker catches a full revert with any compiler.  The symbolic
+	 * field also catches scratch-sequence poisoning when the compiler
+	 * evaluates the overflowing path first, as GCC currently does.
+	 */
+	err = tracefs_extract_field(buf, REGEX_DENY_ACCESS_FS(TRACE_TASK),
+				    "blockers", field_buf, sizeof(field_buf));
+	ASSERT_EQ(0, err);
+	EXPECT_STREQ("read_dir", field_buf);
+	expect_truncated_path(_metadata, buf, REGEX_DENY_ACCESS_FS(TRACE_TASK));
+
+	free(buf);
+}
+
+/*
  * Verifies that an allowed access emits check_rule events (rule matched during
  * pathwalk) but does NOT emit deny_access events (no denial).
  */