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(-)
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,
"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. */
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
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
linux-kselftest-mirror@lists.linaro.org
-
Andre Przywara -
Mickaël Salaün