Re: [PATCH v5 07/10] selftests/landlock: add tests for quiet flag with fs rules

[Justin Suess <utilityemal77@gmail.com>]

Good morning,

Great job on the patch.

Small suggestion on the tests and samples. I saw you
added a bool quiet to some methods for the quiet flag.

> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 943b6e2ac53d..6aa65d344c72 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -718,11 +718,15 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
>  
>  static void add_path_beneath(struct __test_metadata *const _metadata,
>  			     const int ruleset_fd, const __u64 allowed_access,
> -			     const char *const path)
> +			     const char *const path, bool quiet)
>  {
>  	struct landlock_path_beneath_attr path_beneath = {
>  		.allowed_access = allowed_access,
>  	};
> +	__u32 flags = 0;
> +
> +	if (quiet)
> +		flags |= LANDLOCK_ADD_RULE_QUIET;
>  
>  	path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
>  	ASSERT_LE(0, path_beneath.parent_fd)


I think that the bool quiet could be replaced with a flags field
so it can support other flags.

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 6aa65d344c72..5c38a11f1a05 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -717,16 +717,12 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
 }
 
 static void add_path_beneath(struct __test_metadata *const _metadata,
-			     const int ruleset_fd, const __u64 allowed_access,
-			     const char *const path, bool quiet)
+			 const int ruleset_fd, const __u64 allowed_access,
+			 const char *const path, __u32 flags)
 {
 	struct landlock_path_beneath_attr path_beneath = {
 		.allowed_access = allowed_access,
 	};
-	__u32 flags = 0;
-
-	if (quiet)
-		flags |= LANDLOCK_ADD_RULE_QUIET;
 
 	path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
 	ASSERT_LE(0, path_beneath.parent_fd)

And then update the tests to account for the changed
function signature. 

I think the bool quiet in the landlock-sandboxer methods 
populate_ruleset_fs and populate_ruleset_net (in 
samples/landlock/sandboxer.c) should be updated as well, 
replacing the bool quiet with a general flags field. 

I have change this in my patch but it might make more sense in 
your patch since this is the first patch to add flags and would
make it easier if anyone else decides to add flags rebased on
your patch.

Great work and thank you for your help.

Kind Regards,
Justin Suess