[PATCH 0/2] selftests: landlock: fix runs on older systems

[PATCH 0/2] selftests: landlock: fix runs on older systems

[Andre Przywara]

When naively running all kselftests on some systems, it was observed
that the landlock selftest is quite picky and reports failures, even
though the system is fine.

Those two patches relax some tests to make them pass on older kernels:
- The landlock ABI version is only "3" in recent kernels, so patch 1/2
  relaxes the test to accept other numbers.
- Older kernels or some defconfig based kernels might not implement
  the landlock syscall at all. Patch 2/2 catches this.

I couldn't find an easy way to not check for the syscall availability in
*every* test in base_test.c, short of not using TEST_HARNESS_MAIN at all.
If someone has a better idea, I am all ears, especially as this approach
will get quite annoying in fs_base.c.

Cheers,
Andre

Andre Przywara (2):
  selftests: landlock: allow other ABI versions
  selftests: landlock: skip all tests without landlock syscall

 tools/testing/selftests/landlock/base_test.c | 29 +++++++++++++++++++-
 1 file changed, 28 insertions(+), 1 deletion(-)

-- 
2.25.1

[PATCH 1/2] selftests: landlock: allow other ABI versions

[Andre Przywara]

At the moment the abi_version subtest of the landlock selftest expects
exactly version 3 of the landlock syscall ABI. However older kernels
returned a smaller number (or even -1, for the initial code), and the
kselftest documentation states that older kernels should still be
supported.

Relax the test for the return value, to just not accept 0, which was
never a value returned by this syscall (the initial ABI version was 1).

This fixes kselftests runs on older kernels like on my Ubuntu 20.04
system.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/testing/selftests/landlock/base_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 792c3f0a59b4f..1e3b6de57e80e 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -75,7 +75,7 @@ TEST(abi_version)
 	const struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
-	ASSERT_EQ(3, landlock_create_ruleset(NULL, 0,
+	ASSERT_NE(0, landlock_create_ruleset(NULL, 0,
 					     LANDLOCK_CREATE_RULESET_VERSION));
 
 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
-- 
2.25.1

[PATCH 2/2] selftests: landlock: skip all tests without landlock syscall

[Andre Przywara]

"landlock" is a relatively new syscall, and most defconfigs do not enable
it (yet). On systems without this syscall available, the selftests fail
at the moment, instead of being skipped.

Check the availability of the landlock system call before executing each
test, and skip the rest of the tests if we get an ENOSYS back.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 tools/testing/selftests/landlock/base_test.c | 27 ++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 1e3b6de57e80e..c539cec775fba 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -21,12 +21,20 @@
 #define O_PATH 010000000
 #endif
 
+static bool has_syscall(void)
+{
+	return landlock_create_ruleset(NULL, 0, 0) == -1 && errno != ENOSYS;
+}
+
 TEST(inconsistent_attr)
 {
 	const long page_size = sysconf(_SC_PAGESIZE);
 	char *const buf = malloc(page_size + 1);
 	struct landlock_ruleset_attr *const ruleset_attr = (void *)buf;
 
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	ASSERT_NE(NULL, buf);
 
 	/* Checks copy_from_user(). */
@@ -75,6 +83,10 @@ TEST(abi_version)
 	const struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
+
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	ASSERT_NE(0, landlock_create_ruleset(NULL, 0,
 					     LANDLOCK_CREATE_RULESET_VERSION));
 
@@ -107,6 +119,9 @@ TEST(create_ruleset_checks_ordering)
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
 
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	/* Checks priority for invalid flags. */
 	ASSERT_EQ(-1, landlock_create_ruleset(NULL, 0, invalid_flag));
 	ASSERT_EQ(EINVAL, errno);
@@ -153,6 +168,9 @@ TEST(add_rule_checks_ordering)
 	const int ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
 
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	ASSERT_LE(0, ruleset_fd);
 
 	/* Checks invalid flags. */
@@ -200,6 +218,9 @@ TEST(restrict_self_checks_ordering)
 	const int ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
 
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	ASSERT_LE(0, ruleset_fd);
 	path_beneath_attr.parent_fd =
 		open("/tmp", O_PATH | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC);
@@ -240,6 +261,9 @@ TEST(ruleset_fd_io)
 	int ruleset_fd;
 	char buf;
 
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	drop_caps(_metadata);
 	ruleset_fd =
 		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
@@ -267,6 +291,9 @@ TEST(ruleset_fd_transfer)
 	pid_t child;
 	int status;
 
+	if (!has_syscall())
+		SKIP(return, "landlock syscall not available");
+
 	drop_caps(_metadata);
 
 	/* Creates a test ruleset with a simple rule. */
-- 
2.25.1

Re: [PATCH 2/2] selftests: landlock: skip all tests without landlock syscall

[Mickaël Salaün]

On Wed, Aug 09, 2023 at 06:04:35PM +0100, Andre Przywara wrote:
> "landlock" is a relatively new syscall, and most defconfigs do not enable
> it (yet). On systems without this syscall available, the selftests fail
> at the moment, instead of being skipped.
> 
> Check the availability of the landlock system call before executing each
> test, and skip the rest of the tests if we get an ENOSYS back.
> 
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  tools/testing/selftests/landlock/base_test.c | 27 ++++++++++++++++++++
>  1 file changed, 27 insertions(+)
> 
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 1e3b6de57e80e..c539cec775fba 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -21,12 +21,20 @@
>  #define O_PATH 010000000
>  #endif
>  
> +static bool has_syscall(void)
> +{
> +	return landlock_create_ruleset(NULL, 0, 0) == -1 && errno != ENOSYS;
> +}

We could replace most TEST*(name) macros with
TEST*_LANDLOCK(name, minimal_abi_version).

These TEST*_LANDLOCK() macros would simply prepend a header like this:

const int abi_version = landlock_create_ruleset(NULL, 0,
	LANDLOCK_CREATE_RULESET_VERSION);
if (abi_version < minimal_abi_version)
	SKIP(return, "only supported since Landlock ABI %d (instead of
		%d)", minimal_abi_version, abi_version);

These helpers need to be defined in common.h to be easily usable everywhere.

> +
>  TEST(inconsistent_attr)
>  {
>  	const long page_size = sysconf(_SC_PAGESIZE);
>  	char *const buf = malloc(page_size + 1);
>  	struct landlock_ruleset_attr *const ruleset_attr = (void *)buf;
>  
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	ASSERT_NE(NULL, buf);
>  
>  	/* Checks copy_from_user(). */
> @@ -75,6 +83,10 @@ TEST(abi_version)
>  	const struct landlock_ruleset_attr ruleset_attr = {
>  		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
>  	};
> +
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	ASSERT_NE(0, landlock_create_ruleset(NULL, 0,
>  					     LANDLOCK_CREATE_RULESET_VERSION));
>  
> @@ -107,6 +119,9 @@ TEST(create_ruleset_checks_ordering)
>  		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
>  	};
>  
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	/* Checks priority for invalid flags. */
>  	ASSERT_EQ(-1, landlock_create_ruleset(NULL, 0, invalid_flag));
>  	ASSERT_EQ(EINVAL, errno);
> @@ -153,6 +168,9 @@ TEST(add_rule_checks_ordering)
>  	const int ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
>  
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	ASSERT_LE(0, ruleset_fd);
>  
>  	/* Checks invalid flags. */
> @@ -200,6 +218,9 @@ TEST(restrict_self_checks_ordering)
>  	const int ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
>  
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	ASSERT_LE(0, ruleset_fd);
>  	path_beneath_attr.parent_fd =
>  		open("/tmp", O_PATH | O_NOFOLLOW | O_DIRECTORY | O_CLOEXEC);
> @@ -240,6 +261,9 @@ TEST(ruleset_fd_io)
>  	int ruleset_fd;
>  	char buf;
>  
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	drop_caps(_metadata);
>  	ruleset_fd =
>  		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
> @@ -267,6 +291,9 @@ TEST(ruleset_fd_transfer)
>  	pid_t child;
>  	int status;
>  
> +	if (!has_syscall())
> +		SKIP(return, "landlock syscall not available");
> +
>  	drop_caps(_metadata);
>  
>  	/* Creates a test ruleset with a simple rule. */
> -- 
> 2.25.1
> 

Re: [PATCH 0/2] selftests: landlock: fix runs on older systems

[Mickaël Salaün]

Hi Andre,

On Wed, Aug 09, 2023 at 06:04:33PM +0100, Andre Przywara wrote:
> When naively running all kselftests on some systems, it was observed
> that the landlock selftest is quite picky and reports failures, even
> though the system is fine.

Indeed, the current Landlock test suite only checks for the Landlock ABI
of the same source tree as the kselftest files, hence the strict
abi_version test.

> 
> Those two patches relax some tests to make them pass on older kernels:
> - The landlock ABI version is only "3" in recent kernels, so patch 1/2
>   relaxes the test to accept other numbers.
> - Older kernels or some defconfig based kernels might not implement
>   the landlock syscall at all. Patch 2/2 catches this.
> 
> I couldn't find an easy way to not check for the syscall availability in
> *every* test in base_test.c, short of not using TEST_HARNESS_MAIN at all.
> If someone has a better idea, I am all ears, especially as this approach
> will get quite annoying in fs_base.c.

I'd like to take such changes but we need to be more generic, and if
possible avoid being too verbose.

For the more generic part, tests should be skipped according to the
Landlock ABI of the running kernel: i.e. a test should pass iff
ABI >= N.

For the verbosity improvements, we can rely on new macros as explain in
the following email.

> 
> Cheers,
> Andre
> 
> Andre Przywara (2):
>   selftests: landlock: allow other ABI versions
>   selftests: landlock: skip all tests without landlock syscall
> 
>  tools/testing/selftests/landlock/base_test.c | 29 +++++++++++++++++++-
>  1 file changed, 28 insertions(+), 1 deletion(-)
> 
> -- 
> 2.25.1
>