[PATCH v2] Add Landlock test for io_uring IORING_OP_OPENAT operation

[PATCH v2] Add Landlock test for io_uring IORING_OP_OPENAT operation

[Dorine Tipo]

This patch expands Landlock test coverage to include io_uring operations.
It introduces a test for IORING_OP_OPENAT with Landlock rules, verifying
allowed and disallowed access. This mitigates potential security
vulnerabilities by ensuring Landlock controls access through io_uring.

It also updates the Makefile to include -luring in the LDLIBS.
This ensures the test code has access to the necessary liburing
library for io_uring operations.

Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
---
Changes since V1:
V2: - Consolidated two dependent patches in the V1 series into one patch
      as suggested by <fabio.maria.de.francesco@linux.intel.com>
    - Updated the subject line to be more descriptive.

 tools/testing/selftests/landlock/Makefile  |   4 +-
 tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
 2 files changed, 134 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
index 348e2dbdb4e0..ab47d1dadb62 100644
--- a/tools/testing/selftests/landlock/Makefile
+++ b/tools/testing/selftests/landlock/Makefile
@@ -13,11 +13,11 @@ TEST_GEN_PROGS := $(src_test:.c=)
 TEST_GEN_PROGS_EXTENDED := true

 # Short targets:
-$(TEST_GEN_PROGS): LDLIBS += -lcap
+$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
 $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static

 include ../lib.mk

 # Targets with $(OUTPUT)/ prefix:
-$(TEST_GEN_PROGS): LDLIBS += -lcap
+$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
 $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 9a6036fbf289..9c8247995d42 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -21,7 +21,10 @@
 #include <sys/stat.h>
 #include <sys/sysmacros.h>
 #include <sys/vfs.h>
+#include <sys/types.h>
 #include <unistd.h>
+#include <liburing.h>
+#include <linux/io_uring.h>

 #include "common.h"

@@ -4874,4 +4877,133 @@ TEST_F_FORK(layout3_fs, release_inodes)
 	ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY));
 }

+/* Test io_uring openat access control with landlock rules */
+static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths, const int paths_size)
+{
+	struct io_uring ring;
+	struct io_uring_sqe *sqe;
+
+	const char *allowed_paths[] = {
+		file1_s1d1, file2_s1d1,
+		file1_s1d2, file2_s1d2,
+		file1_s1d3, file2_s1d3,
+		file1_s2d1, file1_s2d2,
+		file1_s2d3, file2_s2d3,
+		file1_s3d1,
+	};
+	const char *disallowed_paths[] = {
+		/* dir_s3d2 is a mount point. */
+		dir_s3d2,
+		dir_s3d3,
+	};
+
+	/* Test Allowed Access */
+	const struct rule allowed_rule[] = {
+		{
+			.path = allowed_paths[0],
+			.access = LANDLOCK_ACCESS_FS_READ_FILE |
+				  LANDLOCK_ACCESS_FS_WRITE_FILE,
+		},
+	};
+	int allowed_ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);
+
+	ASSERT_LE(0, allowed_ruleset_fd);
+
+	int ret = io_uring_queue_init(32, &ring, 0);
+
+	ASSERT_EQ(0, ret);
+
+	/* Test each allowed path */
+	for (int i = 0; i < ARRAY_SIZE(allowed_paths); ++i) {
+		sqe = io_uring_get_sqe(&ring);
+		io_uring_prep_openat(sqe, AT_FDCWD, allowed_paths[i], O_RDONLY,
+				     allowed_ruleset_fd);
+		/* Verify successful SQE preparation */
+		ASSERT_EQ(0, ret);
+
+		if (ret != 0)
+			return ret;
+
+		ret = io_uring_submit(&ring);
+		/* Verify 1 submission completed */
+		ASSERT_EQ(1, ret);
+	}
+
+	/* Test Disallowed Access */
+	const struct rule disallowed_rule[] = {
+		{
+			.path = disallowed_paths[0],
+			.access = 0,
+		}
+
+	};
+	int disallowed_ruleset_fd = create_ruleset(_metadata, disallowed_rule[0].access, disallowed_rule);
+
+	ASSERT_LE(0, disallowed_ruleset_fd);
+
+	/* Test each disallowed path */
+	for (int i = 0; i < ARRAY_SIZE(disallowed_paths); ++i) {
+		sqe = io_uring_get_sqe(&ring);
+		io_uring_prep_openat(sqe, AT_FDCWD, disallowed_paths[i], O_RDONLY, disallowed_ruleset_fd);
+		/* Verify successful SQE preparation */
+		ASSERT_EQ(1, ret);
+
+		if (ret != 0)
+			return ret;
+
+		ret = io_uring_submit(&ring);
+		/* Verify 1 submission completed */
+		ASSERT_EQ(0, ret);
+	}
+
+	/*  Cleanup: close ruleset fds, etc. */
+	close(allowed_ruleset_fd);
+	close(disallowed_ruleset_fd);
+
+	return 0;
+}
+
+/* clang-format off */
+FIXTURE(openat_test) {
+	struct __test_metadata *metadata;
+	const char *allowed_paths[11];
+	const char *disallowed_paths[2];
+};
+
+/* clang-format on */
+
+FIXTURE_SETUP(openat_test)
+{
+	/* initialize metadata, allowed_paths, and disallowed_paths */
+	self->metadata = _metadata;
+	const char *temp_allowed_paths[] = {
+		file1_s1d1, file2_s1d1, file1_s1d2, file2_s1d2,
+		file1_s1d3, file2_s1d3, file1_s2d1, file1_s2d2,
+		file1_s2d3, file2_s2d3, file1_s3d1};
+
+	memcpy(self->allowed_paths, temp_allowed_paths, sizeof(temp_allowed_paths));
+
+	const char *temp_disallowed_paths[] = {dir_s3d2, dir_s3d3};
+
+	memcpy(self->disallowed_paths, temp_disallowed_paths, sizeof(temp_disallowed_paths));
+}
+
+FIXTURE_TEARDOWN(openat_test)
+{
+	/* Clean up test environment */
+}
+
+TEST_F_FORK(openat_test, test_ioring_op_openat_allowed)
+{
+	test_ioring_op_openat(self->metadata, LANDLOCK_ACCESS_FS_READ_FILE |
+			      LANDLOCK_ACCESS_FS_WRITE_FILE, self->allowed_paths,
+			      ARRAY_SIZE(self->allowed_paths));
+}
+
+TEST_F_FORK(openat_test, test_ioring_op_openat_disallowed)
+{
+	test_ioring_op_openat(self->metadata, 0, self->disallowed_paths,
+			      ARRAY_SIZE(self->disallowed_paths));
+}
+
 TEST_HARNESS_MAIN
--
2.25.1

Re: [PATCH v2] Add Landlock test for io_uring IORING_OP_OPENAT operation

[Shuah Khan]

On 3/27/24 07:20, Dorine Tipo wrote:
> This patch expands Landlock test coverage to include io_uring operations.
> It introduces a test for IORING_OP_OPENAT with Landlock rules, verifying
> allowed and disallowed access. This mitigates potential security
> vulnerabilities by ensuring Landlock controls access through io_uring.
> 
> It also updates the Makefile to include -luring in the LDLIBS.
> This ensures the test code has access to the necessary liburing
> library for io_uring operations.
> 
> Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>

You are missing linux-kselftest and linux-kernel mailing lists
when you send kselftest patches.

cc linux-kernel-metees since you are a LFX mentee

Adding missing lists
   
> ---
> Changes since V1:
> V2: - Consolidated two dependent patches in the V1 series into one patch
>        as suggested by <fabio.maria.de.francesco@linux.intel.com>
>      - Updated the subject line to be more descriptive.
> 
>   tools/testing/selftests/landlock/Makefile  |   4 +-
>   tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
>   2 files changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index 348e2dbdb4e0..ab47d1dadb62 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -13,11 +13,11 @@ TEST_GEN_PROGS := $(src_test:.c=)
>   TEST_GEN_PROGS_EXTENDED := true
> 
>   # Short targets:
> -$(TEST_GEN_PROGS): LDLIBS += -lcap
> +$(TEST_GEN_PROGS): LDLIBS += -lcap -luring

Check if .gitignore needs updates since you are adding
a new executable.

The rest look good to me.

Acked-by: Shuah Khan <skhan@linuxfoundation.org>

thanks,
-- Shuah

Re: [PATCH v2] Add Landlock test for io_uring IORING_OP_OPENAT operation

[Mickaël Salaün]

Please use the "selftests/landlock: " subject prefix.

On Wed, Mar 27, 2024 at 01:20:01PM +0000, Dorine Tipo wrote:
> This patch expands Landlock test coverage to include io_uring operations.
> It introduces a test for IORING_OP_OPENAT with Landlock rules, verifying
> allowed and disallowed access. This mitigates potential security

I wouldn't say "This mitigate..." but "This makes sure that IORING_OP_OPENAT is
well covered...".

> vulnerabilities by ensuring Landlock controls access through io_uring.
> 
> It also updates the Makefile to include -luring in the LDLIBS.
> This ensures the test code has access to the necessary liburing
> library for io_uring operations.

When running tests I get:

#  RUN           openat_test.test_ioring_op_openat_allowed ...
# fs_test.c:683:test_ioring_op_openat_allowed:Expected 0 (0) <= path_beneath.parent_fd (-1)
# fs_test.c:685:test_ioring_op_openat_allowed:Failed to open directory "tmp/s1d1/f1": No such file or directory
# test_ioring_op_openat_allowed: Test failed at step #5
#          FAIL  openat_test.test_ioring_op_openat_allowed
not ok 112 openat_test.test_ioring_op_openat_allowed


> 
> Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
> ---
> Changes since V1:
> V2: - Consolidated two dependent patches in the V1 series into one patch
>       as suggested by <fabio.maria.de.francesco@linux.intel.com>
>     - Updated the subject line to be more descriptive.
> 
>  tools/testing/selftests/landlock/Makefile  |   4 +-
>  tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
>  2 files changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/Makefile b/tools/testing/selftests/landlock/Makefile
> index 348e2dbdb4e0..ab47d1dadb62 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -13,11 +13,11 @@ TEST_GEN_PROGS := $(src_test:.c=)
>  TEST_GEN_PROGS_EXTENDED := true
> 
>  # Short targets:
> -$(TEST_GEN_PROGS): LDLIBS += -lcap
> +$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
>  $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static
> 
>  include ../lib.mk
> 
>  # Targets with $(OUTPUT)/ prefix:
> -$(TEST_GEN_PROGS): LDLIBS += -lcap
> +$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
>  $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 9a6036fbf289..9c8247995d42 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -21,7 +21,10 @@
>  #include <sys/stat.h>
>  #include <sys/sysmacros.h>
>  #include <sys/vfs.h>
> +#include <sys/types.h>
>  #include <unistd.h>
> +#include <liburing.h>
> +#include <linux/io_uring.h>

You can sort include lines with `sort -u`

> 
>  #include "common.h"
> 
> @@ -4874,4 +4877,133 @@ TEST_F_FORK(layout3_fs, release_inodes)
>  	ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY));
>  }
> 
> +/* Test io_uring openat access control with landlock rules */
> +static int test_ioring_op_openat(struct __test_metadata *const _metadata, const __u64 access, const char **paths, const int paths_size)
> +{
> +	struct io_uring ring;
> +	struct io_uring_sqe *sqe;
> +
> +	const char *allowed_paths[] = {
> +		file1_s1d1, file2_s1d1,
> +		file1_s1d2, file2_s1d2,
> +		file1_s1d3, file2_s1d3,
> +		file1_s2d1, file1_s2d2,
> +		file1_s2d3, file2_s2d3,
> +		file1_s3d1,
> +	};
> +	const char *disallowed_paths[] = {
> +		/* dir_s3d2 is a mount point. */
> +		dir_s3d2,
> +		dir_s3d3,
> +	};
> +
> +	/* Test Allowed Access */
> +	const struct rule allowed_rule[] = {
> +		{
> +			.path = allowed_paths[0],
> +			.access = LANDLOCK_ACCESS_FS_READ_FILE |
> +				  LANDLOCK_ACCESS_FS_WRITE_FILE,
> +		},
> +	};
> +	int allowed_ruleset_fd = create_ruleset(_metadata, allowed_rule[0].access, allowed_rule);

Just use ruleset_fd.

> +
> +	ASSERT_LE(0, allowed_ruleset_fd);
> +
> +	int ret = io_uring_queue_init(32, &ring, 0);

All variable declarations need to be at the top of the function.

> +
> +	ASSERT_EQ(0, ret);
> +
> +	/* Test each allowed path */

We don't need to test all paths, only one that is allowed and another
that is denied.

> +	for (int i = 0; i < ARRAY_SIZE(allowed_paths); ++i) {
> +		sqe = io_uring_get_sqe(&ring);

Please add a comment explaining that we test IORING_OP_OPENAT.

> +		io_uring_prep_openat(sqe, AT_FDCWD, allowed_paths[i], O_RDONLY,
> +				     allowed_ruleset_fd);

Please explain what is happening here...

> +		/* Verify successful SQE preparation */
> +		ASSERT_EQ(0, ret);
> +
> +		if (ret != 0)
> +			return ret;
> +
> +		ret = io_uring_submit(&ring);
> +		/* Verify 1 submission completed */
> +		ASSERT_EQ(1, ret);
> +	}
> +
> +	/* Test Disallowed Access */
> +	const struct rule disallowed_rule[] = {
> +		{
> +			.path = disallowed_paths[0],
> +			.access = 0,
> +		}
> +
> +	};
> +	int disallowed_ruleset_fd = create_ruleset(_metadata, disallowed_rule[0].access, disallowed_rule);
> +
> +	ASSERT_LE(0, disallowed_ruleset_fd);
> +
> +	/* Test each disallowed path */
> +	for (int i = 0; i < ARRAY_SIZE(disallowed_paths); ++i) {
> +		sqe = io_uring_get_sqe(&ring);
> +		io_uring_prep_openat(sqe, AT_FDCWD, disallowed_paths[i], O_RDONLY, disallowed_ruleset_fd);
> +		/* Verify successful SQE preparation */
> +		ASSERT_EQ(1, ret);
> +
> +		if (ret != 0)
> +			return ret;
> +
> +		ret = io_uring_submit(&ring);
> +		/* Verify 1 submission completed */
> +		ASSERT_EQ(0, ret);
> +	}
> +
> +	/*  Cleanup: close ruleset fds, etc. */
> +	close(allowed_ruleset_fd);
> +	close(disallowed_ruleset_fd);
> +
> +	return 0;
> +}
> +
> +/* clang-format off */

We don't need these clang-format markers here, but you need to format
this patch with clang-format.

> +FIXTURE(openat_test) {
> +	struct __test_metadata *metadata;
> +	const char *allowed_paths[11];
> +	const char *disallowed_paths[2];
> +};
> +
> +/* clang-format on */
> +
> +FIXTURE_SETUP(openat_test)
> +{
> +	/* initialize metadata, allowed_paths, and disallowed_paths */
> +	self->metadata = _metadata;
> +	const char *temp_allowed_paths[] = {
> +		file1_s1d1, file2_s1d1, file1_s1d2, file2_s1d2,
> +		file1_s1d3, file2_s1d3, file1_s2d1, file1_s2d2,
> +		file1_s2d3, file2_s2d3, file1_s3d1};
> +
> +	memcpy(self->allowed_paths, temp_allowed_paths, sizeof(temp_allowed_paths));
> +
> +	const char *temp_disallowed_paths[] = {dir_s3d2, dir_s3d3};
> +
> +	memcpy(self->disallowed_paths, temp_disallowed_paths, sizeof(temp_disallowed_paths));
> +}
> +
> +FIXTURE_TEARDOWN(openat_test)
> +{
> +	/* Clean up test environment */
> +}
> +
> +TEST_F_FORK(openat_test, test_ioring_op_openat_allowed)
> +{
> +	test_ioring_op_openat(self->metadata, LANDLOCK_ACCESS_FS_READ_FILE |
> +			      LANDLOCK_ACCESS_FS_WRITE_FILE, self->allowed_paths,
> +			      ARRAY_SIZE(self->allowed_paths));
> +}
> +
> +TEST_F_FORK(openat_test, test_ioring_op_openat_disallowed)
> +{
> +	test_ioring_op_openat(self->metadata, 0, self->disallowed_paths,
> +			      ARRAY_SIZE(self->disallowed_paths));
> +}
> +
>  TEST_HARNESS_MAIN
> --
> 2.25.1
> 
> 

Re: [PATCH v2] Add Landlock test for io_uring IORING_OP_OPENAT operation

[Fabio M. De Francesco]

On Wednesday, 27 March 2024 14:20:01 CET Dorine Tipo wrote:
> This patch expands Landlock test coverage to include io_uring operations.
> It introduces a test for IORING_OP_OPENAT with Landlock rules, verifying
> allowed and disallowed access. This mitigates potential security
> vulnerabilities by ensuring Landlock controls access through io_uring.
> 
> It also updates the Makefile to include -luring in the LDLIBS.
> This ensures the test code has access to the necessary liburing
> library for io_uring operations.
> 
> Signed-off-by: Dorine Tipo <dorine.a.tipo@gmail.com>
> ---

Hi Dorine,

I'm adding inline a few comments and questions for you. However, since my 
experience with io_uring (inexistent with Landlock) is limited, I won't 
comment on the use of its API.

> Changes since V1:
> V2: - Consolidated two dependent patches in the V1 series into one patch
>       as suggested by <fabio.maria.de.francesco@linux.intel.com>

NIT: No need for an entire email address. Next time"Fabio" will suffice  :)

>     - Updated the subject line to be more descriptive.
> 
>  tools/testing/selftests/landlock/Makefile  |   4 +-
>  tools/testing/selftests/landlock/fs_test.c | 132 +++++++++++++++++++++
>  2 files changed, 134 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/landlock/Makefile
> b/tools/testing/selftests/landlock/Makefile index
> 348e2dbdb4e0..ab47d1dadb62 100644
> --- a/tools/testing/selftests/landlock/Makefile
> +++ b/tools/testing/selftests/landlock/Makefile
> @@ -13,11 +13,11 @@ TEST_GEN_PROGS := $(src_test:.c=)
>  TEST_GEN_PROGS_EXTENDED := true
> 
>  # Short targets:
> -$(TEST_GEN_PROGS): LDLIBS += -lcap
> +$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
>  $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static
> 
>  include ../lib.mk
> 
>  # Targets with $(OUTPUT)/ prefix:
> -$(TEST_GEN_PROGS): LDLIBS += -lcap
> +$(TEST_GEN_PROGS): LDLIBS += -lcap -luring
>  $(TEST_GEN_PROGS_EXTENDED): LDFLAGS += -static
> diff --git a/tools/testing/selftests/landlock/fs_test.c
> b/tools/testing/selftests/landlock/fs_test.c index
> 9a6036fbf289..9c8247995d42 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -21,7 +21,10 @@
>  #include <sys/stat.h>
>  #include <sys/sysmacros.h>
>  #include <sys/vfs.h>
> +#include <sys/types.h>
>  #include <unistd.h>
> +#include <liburing.h>
> +#include <linux/io_uring.h>
> 
>  #include "common.h"
> 
> @@ -4874,4 +4877,133 @@ TEST_F_FORK(layout3_fs, release_inodes)
>  	ASSERT_EQ(EACCES, test_open(TMP_DIR, O_RDONLY));
>  }
> 
> +/* Test io_uring openat access control with landlock rules */
> +static int test_ioring_op_openat(struct __test_metadata *const
> _metadata, const __u64 access, const char **paths, const int paths_size)

"access", "paths", and "paths_size" are not used anywhere. Why are there 
three unused arguments in the test_ioring_op_openat() signature?

> +{
> +	struct io_uring ring;
> +	struct io_uring_sqe *sqe;
> +
> +	const char *allowed_paths[] = {
> +		file1_s1d1, file2_s1d1,
> +		file1_s1d2, file2_s1d2,
> +		file1_s1d3, file2_s1d3,
> +		file1_s2d1, file1_s2d2,
> +		file1_s2d3, file2_s2d3,
> +		file1_s3d1,
> +	};
> +
> +	const char *disallowed_paths[] = {
> +		/* dir_s3d2 is a mount point. */
> +		dir_s3d2,
> +		dir_s3d3,
> +	};
> +
> +	/* Test Allowed Access */
> +	const struct rule allowed_rule[] = {
> +		{
> +			.path = allowed_paths[0],
> +			.access = LANDLOCK_ACCESS_FS_READ_FILE |
> +				  LANDLOCK_ACCESS_FS_WRITE_FILE,
> +		},

I suspect that you are forgetting the other elements as well as the final 
NULL element. Don't we need to fill allowed_rule[] with the other pairs of 
allowed paths and their respective access?

> +	};
> +	int allowed_ruleset_fd = create_ruleset(_metadata,
> allowed_rule[0].access, allowed_rule);

Please check how create_ruleset() is implemented. And pay special attention 
at this snippet:

for (i = 0; rules[i].path; i++) {
		add_path_beneath(_metadata, ruleset_fd, rules[i].access,
				 rules[i].path); 

It loops on the elements of rules[], working on the fields of each element 
struct (i.e., "path" and  "access"), but "allowed_rule" is an array with a 
single element. 

Furthermore, I think that you are forgetting a NULL final struct that marks 
the last element of allowed_rule[]. Am I overlooking something?
 
> +	ASSERT_LE(0, allowed_ruleset_fd);
> +
> +	int ret = io_uring_queue_init(32, &ring, 0);
> +
> +	ASSERT_EQ(0, ret);
> +
> +	/* Test each allowed path */
> +	for (int i = 0; i < ARRAY_SIZE(allowed_paths); ++i) {
> +		sqe = io_uring_get_sqe(&ring);
> +		io_uring_prep_openat(sqe, AT_FDCWD, allowed_paths[i], 
O_RDONLY,
> +				     allowed_ruleset_fd);
> +		/* Verify successful SQE preparation */
> +		ASSERT_EQ(0, ret);
> +
> +		if (ret != 0)
> +			return ret;
> +
> +		ret = io_uring_submit(&ring);
> +		/* Verify 1 submission completed */
> +		ASSERT_EQ(1, ret);
> +	}
> +
> +	/* Test Disallowed Access */
> +	const struct rule disallowed_rule[] = {
> +		{
> +			.path = disallowed_paths[0],
> +			.access = 0,
> +		}

Again, what happened to the other disallowed path and to the final NULL 
element of disallowed_rule[]?

> +
> +	};
> +	int disallowed_ruleset_fd = create_ruleset(_metadata,
> disallowed_rule[0].access, disallowed_rule); +
> +	ASSERT_LE(0, disallowed_ruleset_fd);
> +
> +	/* Test each disallowed path */
> +	for (int i = 0; i < ARRAY_SIZE(disallowed_paths); ++i) {
> +		sqe = io_uring_get_sqe(&ring);
> +		io_uring_prep_openat(sqe, AT_FDCWD, disallowed_paths[i], 
O_RDONLY,
> disallowed_ruleset_fd); +		/* Verify successful SQE 
preparation */
> +		ASSERT_EQ(1, ret);
> +
> +		if (ret != 0)
> +			return ret;
> +
> +		ret = io_uring_submit(&ring);
> +		/* Verify 1 submission completed */
> +		ASSERT_EQ(0, ret);
> +	}
> +
> +	/*  Cleanup: close ruleset fds, etc. */
> +	close(allowed_ruleset_fd);
> +	close(disallowed_ruleset_fd);
> +
> +	return 0;
> +}
> +
> +/* clang-format off */
> +FIXTURE(openat_test) {
> +	struct __test_metadata *metadata;
> +	const char *allowed_paths[11];
> +	const char *disallowed_paths[2];
> +};
> +
> +/* clang-format on */
> +
> +FIXTURE_SETUP(openat_test)
> +{
> +	/* initialize metadata, allowed_paths, and disallowed_paths */
> +	self->metadata = _metadata;
> +	const char *temp_allowed_paths[] = {
> +		file1_s1d1, file2_s1d1, file1_s1d2, file2_s1d2,
> +		file1_s1d3, file2_s1d3, file1_s2d1, file1_s2d2,
> +		file1_s2d3, file2_s2d3, file1_s3d1};
> +

Can you please say why you are defining both temp_allowed_paths[] and 
allowed_paths[]? 

> +	memcpy(self->allowed_paths, temp_allowed_paths,
> sizeof(temp_allowed_paths)); +
> +	const char *temp_disallowed_paths[] = {dir_s3d2, dir_s3d3};

I have the same question for temp_disallowed_paths[] and 
disallowed_paths[]. 

I see that they are initialised with the same set of information. Why don't 
you define them only once and pass references where needed?

Thanks,

Fabio 

> +
> +	memcpy(self->disallowed_paths, temp_disallowed_paths,
> sizeof(temp_disallowed_paths)); +}
> +
> +FIXTURE_TEARDOWN(openat_test)
> +{
> +	/* Clean up test environment */
> +}
> +
> +TEST_F_FORK(openat_test, test_ioring_op_openat_allowed)
> +{
> +	test_ioring_op_openat(self->metadata, LANDLOCK_ACCESS_FS_READ_FILE 
|
> +			      LANDLOCK_ACCESS_FS_WRITE_FILE, self-
>allowed_paths,
> +			      ARRAY_SIZE(self->allowed_paths));
> +}
> +
> +TEST_F_FORK(openat_test, test_ioring_op_openat_disallowed)
> +{
> +	test_ioring_op_openat(self->metadata, 0, self->disallowed_paths,
> +			      ARRAY_SIZE(self->disallowed_paths));
> +}
> +
>  TEST_HARNESS_MAIN
> --
> 2.25.1