On Fri, May 24, 2024 at 05:30:06PM +0800, Mikhail Ivanov wrote:

It looked to me like these patches came from Huawei?
Was this left by accident?

+ */
+
+#define _GNU_SOURCE
+
+#include <errno.h>
+#include <linux/landlock.h>
+#include <sched.h>
+#include <string.h>
+#include <sys/prctl.h>
+#include <sys/socket.h>
+
+#include "common.h"
+
+/* clang-format off */
+
+#define ACCESS_LAST LANDLOCK_ACCESS_SOCKET_CREATE
+
+#define ACCESS_ALL ( \
+	LANDLOCK_ACCESS_SOCKET_CREATE)
+
+/* clang-format on */

It does not look like clang-format would really mess up this format in a bad
way.  Maybe we can remove the "clang-format off" section here and just write the
"#define"s on one line?

ACCESS_ALL is unused in this commit.
Should it be introduced in a subsequent commit instead?

+static int test_socket(const struct service_fixture *const srv)
+{
+	int fd;
+
+	fd = socket(srv->protocol.family, srv->protocol.type | SOCK_CLOEXEC, 0);
+	if (fd < 0)
+		return errno;
+	/*
+	 * Mixing error codes from close(2) and socket(2) should not lead to any
+	 * (access type) confusion for this test.
+	 */
+	if (close(fd) != 0)
+		return errno;
+	return 0;
+}

I personally find that it helps me remember if these test helpers have the same
signature as the syscall that they are exercising.  (But I don't feel very
strongly about it.  Just a suggestion.)

[...]

+TEST_F(protocol, create)
+{
+	const struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_socket = LANDLOCK_ACCESS_SOCKET_CREATE,
+	};
+	const struct landlock_socket_attr create_socket_attr = {
+		.allowed_access = LANDLOCK_ACCESS_SOCKET_CREATE,
+		.family = self->srv0.protocol.family,
+		.type = self->srv0.protocol.type,
+	};
+
+	int ruleset_fd;
+
+	/* Allowed create */
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_SOCKET,
+				       &create_socket_attr, 0));
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	ASSERT_EQ(0, test_socket(&self->srv0));
+	ASSERT_EQ(EAFNOSUPPORT, test_socket(&self->unspec_srv0));
+
+	/* Denied create */
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	EXPECT_EQ(0, close(ruleset_fd));
+
+	ASSERT_EQ(EACCES, test_socket(&self->srv0));
+	ASSERT_EQ(EAFNOSUPPORT, test_socket(&self->unspec_srv0));

Should we exhaustively try out the other combinations (other than selv->srv0)
here?  I assume socket() should always fail for these?

(If you are alredy doing this in another commit that I have not looked at yet,
please ignore this comment.)

—Günther