[LTP] [PATCH v4 0/5] landlock testing suite

[LTP] [PATCH v4 0/5] landlock testing suite

[Andrea Cervesato]

This testing suite is meant to test the following syscalls:

- landlock_create_ruleset
- landlock_add_rule
- landlock_restrict_self

Documentation can be found in kernel manuals and inside the official
kernel documentation at

https://www.kernel.org/doc/html/latest/userspace-api/landlock.html

Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
Changes in v4:
- landlock03: fix TBROK on -i usage
- landlock04: fix EINVAL caused by namespace sharing on kernel <=6.6
- Link to v3: https://lore.kernel.org/r/20240711-landlock-v3-0-c7b0e9edf9b0@suse.com

Changes in v3:
- landlock01: 1 byte less when
  HAVE_STRUCT_LANDLOCK_RULESET_ATTR_HANDLED_ACCESS_NET is defined
- landlock04: dynamically assign read/exec permissions to dependences
- landlock05: estetic fix and skip exfat
- landlock06: estetic fix and skip exfat
- Link to v2: https://lore.kernel.org/r/20240710-landlock-v2-0-ff79db017d57@suse.com

Changes in v2:
- remove -lc unused dependency from Makefile
- move SAFE_LANDLOCK_* macros in lapi/landlock.h
- define CAP_MKNOD in the lapi/capability.h
- fix landlock fallback in order to let LTP build properly
- fix landlock01 EINVAL test when "struct landlock_ruleset_attr" size is
  too small
- Link to v1: https://lore.kernel.org/r/20240701-landlock-v1-0-58e9af649a72@suse.com

---
Andrea Cervesato (5):
      Add landlock03 test
      Add CAP_MKNOD fallback in lapi/capability.h
      Add landlock04 test
      Add landlock05 test
      Add landlock06 test

 include/lapi/capability.h                          |  12 +-
 runtest/syscalls                                   |   4 +
 testcases/kernel/syscalls/landlock/.gitignore      |   5 +
 testcases/kernel/syscalls/landlock/landlock03.c    | 128 ++++++++
 testcases/kernel/syscalls/landlock/landlock04.c    | 214 +++++++++++++
 testcases/kernel/syscalls/landlock/landlock05.c    | 116 +++++++
 testcases/kernel/syscalls/landlock/landlock06.c    | 112 +++++++
 testcases/kernel/syscalls/landlock/landlock_exec.c |   9 +
 .../kernel/syscalls/landlock/landlock_tester.h     | 343 +++++++++++++++++++++
 9 files changed, 939 insertions(+), 4 deletions(-)
---
base-commit: c68112b0774bf4f13bfb7b121ecc6671d15d133e
change-id: 20240617-landlock-c48a4623a447

Best regards,
-- 
Andrea Cervesato <andrea.cervesato@suse.com>


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 1/5] Add landlock03 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that landlock_restrict_self syscall fails with the
right error codes:

- EINVAL flags is not 0
- EBADF ruleset_fd is not a file descriptor for the current thread
- EBADFD ruleset_fd is not a ruleset file descriptor
- EPERM ruleset doesn't have CAP_SYS_ADMIN in its namespace
- E2BIG The maximum number of stacked rulesets is reached for the
  current thread

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                |   1 +
 testcases/kernel/syscalls/landlock/.gitignore   |   1 +
 testcases/kernel/syscalls/landlock/landlock03.c | 128 ++++++++++++++++++++++++
 3 files changed, 130 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 8f9e06e73..9b3cba667 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -698,6 +698,7 @@ kill13 kill13
 
 landlock01 landlock01
 landlock02 landlock02
+landlock03 landlock03
 
 lchown01 lchown01
 lchown01_16 lchown01_16
diff --git a/testcases/kernel/syscalls/landlock/.gitignore b/testcases/kernel/syscalls/landlock/.gitignore
index ffed4abd2..f79cd090b 100644
--- a/testcases/kernel/syscalls/landlock/.gitignore
+++ b/testcases/kernel/syscalls/landlock/.gitignore
@@ -1,2 +1,3 @@
 landlock01
 landlock02
+landlock03
diff --git a/testcases/kernel/syscalls/landlock/landlock03.c b/testcases/kernel/syscalls/landlock/landlock03.c
new file mode 100644
index 000000000..c70d23885
--- /dev/null
+++ b/testcases/kernel/syscalls/landlock/landlock03.c
@@ -0,0 +1,128 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that landlock_restrict_self syscall fails with the right
+ * error codes:
+ *
+ * - EINVAL flags is not 0
+ * - EBADF ruleset_fd is not a file descriptor for the current thread
+ * - EBADFD ruleset_fd is not a ruleset file descriptor
+ * - EPERM ruleset doesn't have CAP_SYS_ADMIN in its namespace
+ * - E2BIG The maximum number of stacked rulesets is reached for the current
+ *   thread
+ */
+
+#include "landlock_common.h"
+
+#define MAX_STACKED_RULESETS 16
+
+static struct landlock_ruleset_attr *ruleset_attr;
+static int ruleset_fd = -1;
+static int ruleset_invalid = -1;
+static int file_fd = -1;
+
+static struct tst_cap dropadmin = {
+	.action = TST_CAP_DROP,
+	.id = CAP_SYS_ADMIN,
+	.name = "CAP_SYS_ADMIN",
+};
+
+static struct tst_cap needadmin = {
+	.action = TST_CAP_REQ,
+	.id = CAP_SYS_ADMIN,
+	.name = "CAP_SYS_ADMIN",
+};
+
+static struct tcase {
+	int *fd;
+	uint32_t flags;
+	int exp_errno;
+	char *msg;
+} tcases[] = {
+	{&ruleset_fd, -1, EINVAL, "Invalid flags"},
+	{&ruleset_invalid, 0, EBADF, "Invalid file descriptor"},
+	{&file_fd, 0, EBADFD, "Not a ruleset file descriptor"},
+	{&ruleset_fd, 0, EPERM, "File descriptor doesn't have CAP_SYS_ADMIN"},
+	{&ruleset_fd, 0, E2BIG, "Maximum number of stacked rulesets is reached"},
+};
+
+static void run_child(struct tcase *tc)
+{
+	if (tc->exp_errno == EPERM)
+		tst_cap_action(&dropadmin);
+
+	if (tc->exp_errno == E2BIG) {
+		for (int i = 0; i < MAX_STACKED_RULESETS; i++) {
+			TST_EXP_PASS_SILENT(tst_syscall(__NR_landlock_restrict_self,
+				*tc->fd, tc->flags));
+			if (TST_RET == -1)
+				return;
+		}
+	}
+
+	TST_EXP_FAIL(tst_syscall(__NR_landlock_restrict_self, *tc->fd, tc->flags),
+		tc->exp_errno,
+		"%s", tc->msg);
+
+	if (tc->exp_errno == EPERM)
+		tst_cap_action(&needadmin);
+}
+
+static void run(unsigned int n)
+{
+	struct tcase *tc = &tcases[n];
+
+	if (!SAFE_FORK()) {
+		run_child(tc);
+		_exit(0);
+	}
+}
+
+static void setup(void)
+{
+	verify_landlock_is_enabled();
+
+	ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_EXECUTE;
+
+	ruleset_fd = TST_EXP_FD_SILENT(tst_syscall(__NR_landlock_create_ruleset,
+		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0));
+
+	file_fd = SAFE_OPEN("junk.bin", O_CREAT, 0777);
+}
+
+static void cleanup(void)
+{
+	if (ruleset_fd != -1)
+		SAFE_CLOSE(ruleset_fd);
+
+	if (file_fd != -1)
+		SAFE_CLOSE(file_fd);
+}
+
+static struct tst_test test = {
+	.test = run,
+	.tcnt = ARRAY_SIZE(tcases),
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_kver = "5.13",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_SECURITY_LANDLOCK=y",
+		NULL
+	},
+	.bufs = (struct tst_buffers []) {
+		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{},
+	},
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN),
+		{}
+	},
+};

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 1/5] Add landlock03 test

[Petr Vorel]

Hi Andrea,

...
> +static struct tst_cap dropadmin = {
> +	.action = TST_CAP_DROP,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};
> +
> +static struct tst_cap needadmin = {
> +	.action = TST_CAP_REQ,
> +	.id = CAP_SYS_ADMIN,
> +	.name = "CAP_SYS_ADMIN",
> +};

I dared to merge this with very minor change:

+++ testcases/kernel/syscalls/landlock/landlock03.c
@@ -26,16 +26,15 @@ static int ruleset_fd = -1;
 static int ruleset_invalid = -1;
 static int file_fd = -1;
 
+#define ID_NAME(x) .id = x, .name = #x
 static struct tst_cap dropadmin = {
 	.action = TST_CAP_DROP,
-	.id = CAP_SYS_ADMIN,
-	.name = "CAP_SYS_ADMIN",
+	ID_NAME(CAP_SYS_ADMIN),
 };
 
 static struct tst_cap needadmin = {
 	.action = TST_CAP_REQ,
-	.id = CAP_SYS_ADMIN,
-	.name = "CAP_SYS_ADMIN",
+	ID_NAME(CAP_SYS_ADMIN),
 };
 
 static struct tcase {

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 1/5] Add landlock03 test

[Andrea Cervesato via ltp]

Hi Petr,

feel free to push with that.

Andrea

On 7/26/24 13:01, Petr Vorel wrote:
> Hi Andrea,
>
> ...
>> +static struct tst_cap dropadmin = {
>> +	.action = TST_CAP_DROP,
>> +	.id = CAP_SYS_ADMIN,
>> +	.name = "CAP_SYS_ADMIN",
>> +};
>> +
>> +static struct tst_cap needadmin = {
>> +	.action = TST_CAP_REQ,
>> +	.id = CAP_SYS_ADMIN,
>> +	.name = "CAP_SYS_ADMIN",
>> +};
> I dared to merge this with very minor change:
>
> +++ testcases/kernel/syscalls/landlock/landlock03.c
> @@ -26,16 +26,15 @@ static int ruleset_fd = -1;
>   static int ruleset_invalid = -1;
>   static int file_fd = -1;
>   
> +#define ID_NAME(x) .id = x, .name = #x
>   static struct tst_cap dropadmin = {
>   	.action = TST_CAP_DROP,
> -	.id = CAP_SYS_ADMIN,
> -	.name = "CAP_SYS_ADMIN",
> +	ID_NAME(CAP_SYS_ADMIN),
>   };
>   
>   static struct tst_cap needadmin = {
>   	.action = TST_CAP_REQ,
> -	.id = CAP_SYS_ADMIN,
> -	.name = "CAP_SYS_ADMIN",
> +	ID_NAME(CAP_SYS_ADMIN),
>   };
>   
>   static struct tcase {



-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 2/5] Add CAP_MKNOD fallback in lapi/capability.h

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 include/lapi/capability.h | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/include/lapi/capability.h b/include/lapi/capability.h
index 2b593797c..0f317d6d7 100644
--- a/include/lapi/capability.h
+++ b/include/lapi/capability.h
@@ -44,14 +44,18 @@
 # define CAP_SYS_TIME         25
 #endif
 
-#ifndef CAP_AUDIT_READ
-# define CAP_AUDIT_READ       37
-#endif
-
 #ifndef CAP_SYS_RESOURCE
 # define CAP_SYS_RESOURCE     24
 #endif
 
+#ifndef CAP_MKNOD
+# define CAP_MKNOD            27
+#endif
+
+#ifndef CAP_AUDIT_READ
+# define CAP_AUDIT_READ       37
+#endif
+
 #ifndef CAP_BPF
 # define CAP_BPF              39
 #endif

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 2/5] Add CAP_MKNOD fallback in lapi/capability.h

[Petr Vorel]

Hi Andrea,

obviously correct. I suppose it was needed otherwise old distros would fail,
right? Go ahead and merge (probably together with landlock04.c where it is
needed).

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 3/5] Add landlock04 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies that all landlock rules are working properly.
The way we do it is to verify that all disabled syscalls are not
working but the one we enabled via specifc landlock rules.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                   |   1 +
 testcases/kernel/syscalls/landlock/.gitignore      |   2 +
 testcases/kernel/syscalls/landlock/landlock04.c    | 214 +++++++++++++
 testcases/kernel/syscalls/landlock/landlock_exec.c |   9 +
 .../kernel/syscalls/landlock/landlock_tester.h     | 343 +++++++++++++++++++++
 5 files changed, 569 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 9b3cba667..67b2e2758 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -699,6 +699,7 @@ kill13 kill13
 landlock01 landlock01
 landlock02 landlock02
 landlock03 landlock03
+landlock04 landlock04
 
 lchown01 lchown01
 lchown01_16 lchown01_16
diff --git a/testcases/kernel/syscalls/landlock/.gitignore b/testcases/kernel/syscalls/landlock/.gitignore
index f79cd090b..4fe8d7cba 100644
--- a/testcases/kernel/syscalls/landlock/.gitignore
+++ b/testcases/kernel/syscalls/landlock/.gitignore
@@ -1,3 +1,5 @@
+landlock_exec
 landlock01
 landlock02
 landlock03
+landlock04
diff --git a/testcases/kernel/syscalls/landlock/landlock04.c b/testcases/kernel/syscalls/landlock/landlock04.c
new file mode 100644
index 000000000..e8d5fd478
--- /dev/null
+++ b/testcases/kernel/syscalls/landlock/landlock04.c
@@ -0,0 +1,214 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies that all landlock filesystem rules are working properly.
+ * The way we do it is to verify that all disabled syscalls are not working but
+ * the one we enabled via specifc landlock rules.
+ */
+
+#include "landlock_common.h"
+#include "landlock_tester.h"
+#include "tst_safe_stdio.h"
+
+#define ACCESS_NAME(x) #x
+
+static struct landlock_ruleset_attr *ruleset_attr;
+static struct landlock_path_beneath_attr *path_beneath_attr;
+
+static struct tvariant {
+	int access;
+	char *desc;
+} tvariants[] = {
+	{
+		LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_EXECUTE,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_EXECUTE)
+	},
+	{
+		LANDLOCK_ACCESS_FS_WRITE_FILE,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_WRITE_FILE)
+	},
+	{
+		LANDLOCK_ACCESS_FS_READ_FILE,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_READ_FILE)
+	},
+	{
+		LANDLOCK_ACCESS_FS_READ_DIR,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_READ_DIR)
+	},
+	{
+		LANDLOCK_ACCESS_FS_REMOVE_DIR,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_REMOVE_DIR)
+	},
+	{
+		LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_REMOVE_FILE)
+	},
+	{
+		LANDLOCK_ACCESS_FS_MAKE_CHAR,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_MAKE_CHAR)
+	},
+	{
+		LANDLOCK_ACCESS_FS_MAKE_BLOCK,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_MAKE_BLOCK)
+	},
+	{
+		LANDLOCK_ACCESS_FS_MAKE_REG,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_MAKE_REG)
+	},
+	{
+		LANDLOCK_ACCESS_FS_MAKE_SOCK,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_MAKE_SOCK)
+	},
+	{
+		LANDLOCK_ACCESS_FS_MAKE_FIFO,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_MAKE_FIFO)
+	},
+	{
+		LANDLOCK_ACCESS_FS_MAKE_SYM,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_MAKE_SYM)
+	},
+	{
+		LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_TRUNCATE,
+		ACCESS_NAME(LANDLOCK_ACCESS_FS_TRUNCATE)
+	},
+};
+
+static void run(void)
+{
+	if (!SAFE_FORK()) {
+		struct tvariant  variant = tvariants[tst_variant];
+
+		tester_run_all_fs_rules(variant.access);
+		_exit(0);
+	}
+}
+
+static void enable_exec_libs(const int ruleset_fd)
+{
+	FILE *fp;
+	char line[1024];
+	char path[PATH_MAX];
+	char dependency[8][PATH_MAX];
+	int count = 0;
+	int duplicate = 0;
+
+	fp = SAFE_FOPEN("/proc/self/maps", "r");
+
+	while (fgets(line, sizeof(line), fp)) {
+		if (strstr(line, ".so") == NULL)
+			continue;
+
+		SAFE_SSCANF(line, "%*x-%*x %*s %*x %*s %*d %s", path);
+
+		for (int i = 0; i < count; i++) {
+			if (strcmp(path, dependency[i]) == 0) {
+				duplicate = 1;
+				break;
+			}
+		}
+
+		if (duplicate) {
+			duplicate = 0;
+			continue;
+		}
+
+		strncpy(dependency[count], path, PATH_MAX);
+		count++;
+
+		tst_res(TINFO, "Enable read/exec permissions for %s", path);
+
+		path_beneath_attr->allowed_access =
+			LANDLOCK_ACCESS_FS_READ_FILE |
+			LANDLOCK_ACCESS_FS_EXECUTE;
+		path_beneath_attr->parent_fd = SAFE_OPEN(path, O_PATH | O_CLOEXEC);
+
+		SAFE_LANDLOCK_ADD_RULE(
+			ruleset_fd,
+			LANDLOCK_RULE_PATH_BENEATH,
+			path_beneath_attr,
+			0);
+
+		SAFE_CLOSE(path_beneath_attr->parent_fd);
+	}
+
+	SAFE_FCLOSE(fp);
+}
+
+static void setup(void)
+{
+	struct tvariant variant = tvariants[tst_variant];
+	int ruleset_fd;
+
+	verify_landlock_is_enabled();
+	tester_create_fs_tree();
+
+	tst_res(TINFO, "Testing %s", variant.desc);
+
+	ruleset_attr->handled_access_fs = tester_get_all_fs_rules();
+
+	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
+		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
+
+	/* since our binary is dynamically linked, we need to enable dependences
+	 * to be read and executed
+	 */
+	enable_exec_libs(ruleset_fd);
+
+	path_beneath_attr->allowed_access = variant.access;
+	path_beneath_attr->parent_fd = SAFE_OPEN(
+		SANDBOX_FOLDER, O_PATH | O_CLOEXEC);
+
+	SAFE_LANDLOCK_ADD_RULE(
+		ruleset_fd,
+		LANDLOCK_RULE_PATH_BENEATH,
+		path_beneath_attr,
+		0);
+
+	SAFE_CLOSE(path_beneath_attr->parent_fd);
+
+	enforce_ruleset(ruleset_fd);
+	SAFE_CLOSE(ruleset_fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.min_kver = "5.13",
+	.forks_child = 1,
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.test_variants = ARRAY_SIZE(tvariants),
+	.resource_files = (const char *[]) {
+		TESTAPP,
+		NULL,
+	},
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_SECURITY_LANDLOCK=y",
+		NULL
+	},
+	.bufs = (struct tst_buffers []) {
+		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
+		{},
+	},
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN),
+		TST_CAP(TST_CAP_REQ, CAP_MKNOD),
+		{}
+	},
+	.format_device = 1,
+	.mount_device = 1,
+	.mntpoint = SANDBOX_FOLDER,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *[]) {
+		"vfat",
+		"exfat",
+		NULL
+	},
+	.max_runtime = 3600,
+};
diff --git a/testcases/kernel/syscalls/landlock/landlock_exec.c b/testcases/kernel/syscalls/landlock/landlock_exec.c
new file mode 100644
index 000000000..aae5c76b2
--- /dev/null
+++ b/testcases/kernel/syscalls/landlock/landlock_exec.c
@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+int main(void)
+{
+	return 0;
+}
diff --git a/testcases/kernel/syscalls/landlock/landlock_tester.h b/testcases/kernel/syscalls/landlock/landlock_tester.h
new file mode 100644
index 000000000..b4a4c1f7d
--- /dev/null
+++ b/testcases/kernel/syscalls/landlock/landlock_tester.h
@@ -0,0 +1,343 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+#ifndef LANDLOCK_TESTER_H
+
+#include "tst_test.h"
+#include "lapi/landlock.h"
+#include <sys/sysmacros.h>
+
+#define PERM_MODE 0700
+
+#define SANDBOX_FOLDER	"sandbox"
+#define TESTAPP			"landlock_exec"
+
+#define FILE_EXEC		SANDBOX_FOLDER"/"TESTAPP
+#define FILE_READ		SANDBOX_FOLDER"/file_read"
+#define FILE_WRITE		SANDBOX_FOLDER"/file_write"
+#define FILE_REMOVE		SANDBOX_FOLDER"/file_remove"
+#define FILE_UNLINK		SANDBOX_FOLDER"/file_unlink"
+#define FILE_UNLINKAT	SANDBOX_FOLDER"/file_unlinkat"
+#define FILE_TRUNCATE	SANDBOX_FOLDER"/file_truncate"
+#define FILE_REGULAR	SANDBOX_FOLDER"/regular0"
+#define FILE_SOCKET		SANDBOX_FOLDER"/socket0"
+#define FILE_FIFO		SANDBOX_FOLDER"/fifo0"
+#define FILE_SYM0		SANDBOX_FOLDER"/symbolic0"
+#define FILE_SYM1		SANDBOX_FOLDER"/symbolic1"
+#define DIR_READDIR		SANDBOX_FOLDER"/dir_readdir"
+#define DIR_RMDIR		SANDBOX_FOLDER"/dir_rmdir"
+#define DEV_CHAR0		SANDBOX_FOLDER"/chardev0"
+#define DEV_BLK0		SANDBOX_FOLDER"/blkdev0"
+
+#define ALL_RULES (\
+	LANDLOCK_ACCESS_FS_EXECUTE | \
+	LANDLOCK_ACCESS_FS_WRITE_FILE | \
+	LANDLOCK_ACCESS_FS_READ_FILE | \
+	LANDLOCK_ACCESS_FS_READ_DIR | \
+	LANDLOCK_ACCESS_FS_REMOVE_DIR | \
+	LANDLOCK_ACCESS_FS_REMOVE_FILE | \
+	LANDLOCK_ACCESS_FS_MAKE_CHAR | \
+	LANDLOCK_ACCESS_FS_MAKE_DIR | \
+	LANDLOCK_ACCESS_FS_MAKE_REG | \
+	LANDLOCK_ACCESS_FS_MAKE_SOCK | \
+	LANDLOCK_ACCESS_FS_MAKE_FIFO | \
+	LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
+	LANDLOCK_ACCESS_FS_MAKE_SYM | \
+	LANDLOCK_ACCESS_FS_REFER | \
+	LANDLOCK_ACCESS_FS_TRUNCATE | \
+	LANDLOCK_ACCESS_FS_IOCTL_DEV)
+
+static char *readdir_files[] = {
+	DIR_READDIR"/file0",
+	DIR_READDIR"/file1",
+	DIR_READDIR"/file2",
+};
+
+static int dev_chr;
+static int dev_blk;
+
+static int tester_get_all_fs_rules(void)
+{
+	int abi;
+	int all_rules = ALL_RULES;
+
+	abi = SAFE_LANDLOCK_CREATE_RULESET(
+		NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
+
+	if (abi < 2)
+		all_rules &= ~LANDLOCK_ACCESS_FS_REFER;
+
+	if (abi < 3)
+		all_rules &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
+
+	if (abi < 5)
+		all_rules &= ~LANDLOCK_ACCESS_FS_IOCTL_DEV;
+
+	return all_rules;
+}
+
+static void tester_create_fs_tree(void)
+{
+	if (access(SANDBOX_FOLDER, F_OK) == -1)
+		SAFE_MKDIR(SANDBOX_FOLDER, PERM_MODE);
+
+	/* folders */
+	SAFE_MKDIR(DIR_RMDIR, PERM_MODE);
+	SAFE_MKDIR(DIR_READDIR, PERM_MODE);
+	for (size_t i = 0; i < ARRAY_SIZE(readdir_files); i++)
+		SAFE_TOUCH(readdir_files[i], PERM_MODE, NULL);
+
+	/* files */
+	tst_fill_file(FILE_READ, 'a', getpagesize(), 1);
+	SAFE_TOUCH(FILE_WRITE, PERM_MODE, NULL);
+	SAFE_TOUCH(FILE_REMOVE, PERM_MODE, NULL);
+	SAFE_TOUCH(FILE_UNLINK, PERM_MODE, NULL);
+	SAFE_TOUCH(FILE_UNLINKAT, PERM_MODE, NULL);
+	SAFE_TOUCH(FILE_TRUNCATE, PERM_MODE, NULL);
+	SAFE_TOUCH(FILE_SYM0, PERM_MODE, NULL);
+	SAFE_CP(TESTAPP, FILE_EXEC);
+
+	/* devices */
+	dev_chr = makedev(1, 3);
+	dev_blk = makedev(7, 0);
+}
+
+static void _test_exec(const int result)
+{
+	int status;
+	pid_t pid;
+	char *const args[] = {(char *)FILE_EXEC, NULL};
+
+	tst_res(TINFO, "Test binary execution");
+
+	pid = SAFE_FORK();
+	if (!pid) {
+		int rval;
+
+		if (result == TPASS) {
+			rval = execve(FILE_EXEC, args, NULL);
+			if (rval == -1)
+				tst_res(TFAIL | TERRNO, "Failed to execute test binary");
+		} else {
+			TST_EXP_FAIL(execve(FILE_EXEC, args, NULL), EACCES);
+		}
+
+		_exit(1);
+	}
+
+	SAFE_WAITPID(pid, &status, 0);
+	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
+		return;
+
+	tst_res(result, "Test binary has been executed");
+}
+
+static void _test_write(const int result)
+{
+	tst_res(TINFO, "Test writing file");
+
+	if (result == TPASS)
+		TST_EXP_FD(open(FILE_WRITE, O_WRONLY, PERM_MODE));
+	else
+		TST_EXP_FAIL(open(FILE_WRITE, O_WRONLY, PERM_MODE), EACCES);
+
+	if (TST_RET != -1)
+		SAFE_CLOSE(TST_RET);
+}
+
+static void _test_read(const int result)
+{
+	tst_res(TINFO, "Test reading file");
+
+	if (result == TPASS)
+		TST_EXP_FD(open(FILE_READ, O_RDONLY, PERM_MODE));
+	else
+		TST_EXP_FAIL(open(FILE_READ, O_RDONLY, PERM_MODE), EACCES);
+
+	if (TST_RET != -1)
+		SAFE_CLOSE(TST_RET);
+}
+
+static void _test_readdir(const int result)
+{
+	tst_res(TINFO, "Test reading directory");
+
+	DIR *dir;
+	struct dirent *de;
+	int files_counted = 0;
+
+	dir = opendir(DIR_READDIR);
+	if (!dir) {
+		tst_res(result == TPASS ? TFAIL : TPASS,
+			"Can't read '%s' directory", DIR_READDIR);
+
+		return;
+	}
+
+	tst_res(result, "Can read '%s' directory", DIR_READDIR);
+	if (result == TFAIL)
+		return;
+
+	while ((de = readdir(dir)) != NULL) {
+		if (de->d_type != DT_REG)
+			continue;
+
+		for (size_t i = 0; i < ARRAY_SIZE(readdir_files); i++) {
+			if (readdir_files[i] == NULL)
+				continue;
+
+			if (strstr(readdir_files[i], de->d_name) != NULL)
+				files_counted++;
+		}
+	}
+
+	SAFE_CLOSEDIR(dir);
+
+	TST_EXP_EQ_LI(files_counted, ARRAY_SIZE(readdir_files));
+}
+
+static void _test_rmdir(const int result)
+{
+	tst_res(TINFO, "Test removing directory");
+
+	if (result == TPASS)
+		TST_EXP_PASS(rmdir(DIR_RMDIR));
+	else
+		TST_EXP_FAIL(rmdir(DIR_RMDIR), EACCES);
+}
+
+static void _test_rmfile(const int result)
+{
+	tst_res(TINFO, "Test removing file");
+
+	if (result == TPASS) {
+		TST_EXP_PASS(unlink(FILE_UNLINK));
+		TST_EXP_PASS(remove(FILE_REMOVE));
+	} else {
+		TST_EXP_FAIL(unlink(FILE_UNLINK), EACCES);
+		TST_EXP_FAIL(remove(FILE_REMOVE), EACCES);
+	}
+}
+
+static void _test_make(
+	const char *path,
+	const int type,
+	const int dev,
+	const int result)
+{
+	tst_res(TINFO, "Test normal or special files creation");
+
+	if (result == TPASS)
+		TST_EXP_PASS(mknod(path, type | 0400, dev));
+	else
+		TST_EXP_FAIL(mknod(path, type | 0400, dev), EACCES);
+}
+
+static void _test_symbolic(const int result)
+{
+	tst_res(TINFO, "Test symbolic links");
+
+	if (result == TPASS)
+		TST_EXP_PASS(symlink(FILE_SYM0, FILE_SYM1));
+	else
+		TST_EXP_FAIL(symlink(FILE_SYM0, FILE_SYM1), EACCES);
+}
+
+static void _test_truncate(const int result)
+{
+	int fd;
+
+	tst_res(TINFO, "Test truncating file");
+
+	if (result == TPASS) {
+		TST_EXP_PASS(truncate(FILE_TRUNCATE, 10));
+
+		fd = TST_EXP_FD(open(FILE_TRUNCATE, O_WRONLY, PERM_MODE));
+		if (fd != -1) {
+			TST_EXP_PASS(ftruncate(fd, 10));
+			SAFE_CLOSE(fd);
+		}
+
+		fd = TST_EXP_FD(open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE));
+		if (fd != -1)
+			SAFE_CLOSE(fd);
+	} else {
+		TST_EXP_FAIL(truncate(FILE_TRUNCATE, 10), EACCES);
+
+		fd = open(FILE_TRUNCATE, O_WRONLY, PERM_MODE);
+		if (fd != -1) {
+			TST_EXP_FAIL(ftruncate(fd, 10), EACCES);
+			SAFE_CLOSE(fd);
+		}
+
+		TST_EXP_FAIL(open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE),
+			EACCES);
+
+		if (TST_RET != -1)
+			SAFE_CLOSE(TST_RET);
+	}
+}
+
+static void tester_run_fs_rules(const int rules, const int result)
+{
+	if (rules & LANDLOCK_ACCESS_FS_EXECUTE)
+		_test_exec(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_WRITE_FILE)
+		_test_write(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_READ_FILE)
+		_test_read(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_READ_DIR)
+		_test_readdir(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_REMOVE_DIR)
+		_test_rmdir(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_REMOVE_FILE)
+		_test_rmfile(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_MAKE_CHAR)
+		_test_make(DEV_CHAR0, S_IFCHR, dev_chr, result);
+
+	if (rules & LANDLOCK_ACCESS_FS_MAKE_BLOCK)
+		_test_make(DEV_BLK0, S_IFBLK, dev_blk, result);
+
+	if (rules & LANDLOCK_ACCESS_FS_MAKE_REG)
+		_test_make(FILE_REGULAR, S_IFREG, 0, result);
+
+	if (rules & LANDLOCK_ACCESS_FS_MAKE_SOCK)
+		_test_make(FILE_SOCKET, S_IFSOCK, 0, result);
+
+	if (rules & LANDLOCK_ACCESS_FS_MAKE_FIFO)
+		_test_make(FILE_FIFO, S_IFIFO, 0, result);
+
+	if (rules & LANDLOCK_ACCESS_FS_MAKE_SYM)
+		_test_symbolic(result);
+
+	if (rules & LANDLOCK_ACCESS_FS_TRUNCATE) {
+		if ((tst_kvercmp(6, 2, 0)) < 0) {
+			tst_res(TINFO, "Skip truncate test. Minimum kernel version is 6.2");
+			return;
+		}
+
+		_test_truncate(result);
+	}
+}
+
+static inline void tester_run_all_fs_rules(const int pass_rules)
+{
+	int fail_rules;
+	int all_rules;
+
+	all_rules = tester_get_all_fs_rules();
+	fail_rules = all_rules & ~pass_rules;
+
+	tester_run_fs_rules(pass_rules, TPASS);
+	tester_run_fs_rules(fail_rules, TFAIL);
+}
+
+#endif

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/5] Add landlock04 test

[Petr Vorel]

Hi Andrea,

unfortunately one minor things: cleanup needed to fix running with -i
(obviously on all filesystems):

LTP_SINGLE_FS_TYPE=btrfs ./landlock04 -i2 2>&1 |grep TFAIL
landlock_tester.h:206: TFAIL: rmdir(DIR_RMDIR) failed: ENOENT (2)
landlock_tester.h:216: TFAIL: unlink(FILE_UNLINK) failed: ENOENT (2)
landlock_tester.h:217: TFAIL: remove(FILE_REMOVE) failed: ENOENT (2)
landlock_tester.h:233: TFAIL: mknod(path, type | 0400, dev) failed: EEXIST (17)
landlock_tester.h:233: TFAIL: mknod(path, type | 0400, dev) failed: EEXIST (17)
landlock_tester.h:233: TFAIL: mknod(path, type | 0400, dev) failed: EEXIST (17)
landlock_tester.h:233: TFAIL: mknod(path, type | 0400, dev) failed: EEXIST (17)
landlock_tester.h:233: TFAIL: mknod(path, type | 0400, dev) failed: EEXIST (17)
landlock_tester.h:243: TFAIL: symlink(FILE_SYM0, FILE_SYM1) failed: EEXIST (17)

_test_symbolic() and other test functions in landlock_tester.h need to unlink()
after creating files.

TL;DR
The test itself LGTM. Interesting, quite extensive testing, thanks for that!

With:
* fixing on -i
* replaced TST_TO_STR_() instead of ACCESS_NAME()
* lower .max_runtime (or none .max_runtime)
* removing .format_device = 1 and .needs_tmpdir = 1
* add #define LANDLOCK_TESTER_H + append '__' to the definition

you can merge with my:

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Some notes below.

...
> +++ b/testcases/kernel/syscalls/landlock/landlock04.c
> @@ -0,0 +1,214 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +/*\
> + * [Description]
> + *
> + * This test verifies that all landlock filesystem rules are working properly.
> + * The way we do it is to verify that all disabled syscalls are not working but
> + * the one we enabled via specifc landlock rules.
very nit (ignore): I would avoid "This" (bogus word) and use passive form "we do
it". If you look on generated docs, it'd be nice to use a bit consistent style.
But it's minor (the least important thing).

> + */
> +
> +#include "landlock_common.h"
> +#include "landlock_tester.h"
> +#include "tst_safe_stdio.h"
> +
> +#define ACCESS_NAME(x) #x
We have TST_TO_STR_() for this, could you please use it?

> +
> +static struct landlock_ruleset_attr *ruleset_attr;
> +static struct landlock_path_beneath_attr *path_beneath_attr;
> +
> +static struct tvariant {
> +	int access;
> +	char *desc;
> +} tvariants[] = {
> +	{
> +		LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_EXECUTE,
> +		ACCESS_NAME(LANDLOCK_ACCESS_FS_EXECUTE)
s/ACCESS_NAME/TST_TO_STR_/
> +	},
...
> +};
...
> +static void enable_exec_libs(const int ruleset_fd)
> +{
> +	FILE *fp;
> +	char line[1024];
> +	char path[PATH_MAX];
> +	char dependency[8][PATH_MAX];
> +	int count = 0;
> +	int duplicate = 0;
> +
> +	fp = SAFE_FOPEN("/proc/self/maps", "r");
> +
> +	while (fgets(line, sizeof(line), fp)) {
> +		if (strstr(line, ".so") == NULL)
> +			continue;
> +
> +		SAFE_SSCANF(line, "%*x-%*x %*s %*x %*s %*d %s", path);
> +
> +		for (int i = 0; i < count; i++) {
> +			if (strcmp(path, dependency[i]) == 0) {
> +				duplicate = 1;
> +				break;
> +			}
> +		}
> +
> +		if (duplicate) {
> +			duplicate = 0;
> +			continue;
> +		}
> +
> +		strncpy(dependency[count], path, PATH_MAX);
> +		count++;
> +
> +		tst_res(TINFO, "Enable read/exec permissions for %s", path);
> +
> +		path_beneath_attr->allowed_access =
> +			LANDLOCK_ACCESS_FS_READ_FILE |
> +			LANDLOCK_ACCESS_FS_EXECUTE;
> +		path_beneath_attr->parent_fd = SAFE_OPEN(path, O_PATH | O_CLOEXEC);
> +
> +		SAFE_LANDLOCK_ADD_RULE(
> +			ruleset_fd,
> +			LANDLOCK_RULE_PATH_BENEATH,
> +			path_beneath_attr,
> +			0);
very nit (ignore): for me would be more readable:
		SAFE_LANDLOCK_ADD_RULE(
			ruleset_fd, LANDLOCK_RULE_PATH_BENEATH, path_beneath_attr, 0);

The same applies to landlock_tester.h below:
static void _test_make(
	const char *path,
	const int type,
	const int dev,
	const int result)

> +
> +		SAFE_CLOSE(path_beneath_attr->parent_fd);
> +	}
> +
> +	SAFE_FCLOSE(fp);
> +}
> +
> +static void setup(void)
> +{
> +	struct tvariant variant = tvariants[tst_variant];
> +	int ruleset_fd;
> +
> +	verify_landlock_is_enabled();
> +	tester_create_fs_tree();
> +
> +	tst_res(TINFO, "Testing %s", variant.desc);
> +
> +	ruleset_attr->handled_access_fs = tester_get_all_fs_rules();
> +
> +	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
> +		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
> +
> +	/* since our binary is dynamically linked, we need to enable dependences
> +	 * to be read and executed
> +	 */
> +	enable_exec_libs(ruleset_fd);
> +
> +	path_beneath_attr->allowed_access = variant.access;
> +	path_beneath_attr->parent_fd = SAFE_OPEN(
> +		SANDBOX_FOLDER, O_PATH | O_CLOEXEC);
> +
> +	SAFE_LANDLOCK_ADD_RULE(
> +		ruleset_fd,
> +		LANDLOCK_RULE_PATH_BENEATH,
> +		path_beneath_attr,
> +		0);
> +
> +	SAFE_CLOSE(path_beneath_attr->parent_fd);
> +
> +	enforce_ruleset(ruleset_fd);
> +	SAFE_CLOSE(ruleset_fd);
> +}
> +
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.min_kver = "5.13",
> +	.forks_child = 1,
> +	.needs_tmpdir = 1,

If you generate docs, you will see:
testcases/kernel/syscalls/landlock/landlock04.c: useless tag: format_device
testcases/kernel/syscalls/landlock/landlock04.c: useless tag: needs_tmpdir

Please remove them (implied by .all_filesystems and others).

> +	.needs_root = 1,
> +	.test_variants = ARRAY_SIZE(tvariants),
> +	.resource_files = (const char *[]) {
> +		TESTAPP,
> +		NULL,
> +	},
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_SECURITY_LANDLOCK=y",
> +		NULL
> +	},
> +	.bufs = (struct tst_buffers []) {
> +		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
> +		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
> +		{},
> +	},
> +	.caps = (struct tst_cap []) {
> +		TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN),
> +		TST_CAP(TST_CAP_REQ, CAP_MKNOD),
> +		{}
> +	},
> +	.format_device = 1,
> +	.mount_device = 1,
> +	.mntpoint = SANDBOX_FOLDER,
> +	.all_filesystems = 1,
> +	.skip_filesystems = (const char *[]) {
> +		"vfat",
> +		"exfat",

Interesting, ntfs over FUSE works.

> +		NULL
> +	},
> +	.max_runtime = 3600,

Is it really needed for test to run 1 hour? On random VM with 6.5 kernel it runs
with the default (which is 30s).


> +};
> diff --git a/testcases/kernel/syscalls/landlock/landlock_exec.c b/testcases/kernel/syscalls/landlock/landlock_exec.c
> new file mode 100644
> index 000000000..aae5c76b2
> --- /dev/null
> +++ b/testcases/kernel/syscalls/landlock/landlock_exec.c
> @@ -0,0 +1,9 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +int main(void)
> +{
> +	return 0;
> +}
> diff --git a/testcases/kernel/syscalls/landlock/landlock_tester.h b/testcases/kernel/syscalls/landlock/landlock_tester.h
> new file mode 100644
> index 000000000..b4a4c1f7d
> --- /dev/null
> +++ b/testcases/kernel/syscalls/landlock/landlock_tester.h
> @@ -0,0 +1,343 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
> + */
> +
> +#ifndef LANDLOCK_TESTER_H
You need to have here definition:
#define LANDLOCK_TESTER_H

otherwise the guarder does not work.

While at it, could you please rename it to LANDLOCK_TESTER_H__
(we usually append '__' to avoid clashes with random headers?
See my other fix: 2d24a286f1e35fe6ad7e1e307b86375658e23bee.

> +
> +#include "tst_test.h"
> +#include "lapi/landlock.h"
> +#include <sys/sysmacros.h>
> +
> +#define PERM_MODE 0700
> +
> +#define SANDBOX_FOLDER	"sandbox"
> +#define TESTAPP			"landlock_exec"
> +
> +#define FILE_EXEC		SANDBOX_FOLDER"/"TESTAPP
> +#define FILE_READ		SANDBOX_FOLDER"/file_read"
> +#define FILE_WRITE		SANDBOX_FOLDER"/file_write"
> +#define FILE_REMOVE		SANDBOX_FOLDER"/file_remove"
> +#define FILE_UNLINK		SANDBOX_FOLDER"/file_unlink"
> +#define FILE_UNLINKAT	SANDBOX_FOLDER"/file_unlinkat"
> +#define FILE_TRUNCATE	SANDBOX_FOLDER"/file_truncate"
> +#define FILE_REGULAR	SANDBOX_FOLDER"/regular0"
> +#define FILE_SOCKET		SANDBOX_FOLDER"/socket0"
> +#define FILE_FIFO		SANDBOX_FOLDER"/fifo0"
> +#define FILE_SYM0		SANDBOX_FOLDER"/symbolic0"
> +#define FILE_SYM1		SANDBOX_FOLDER"/symbolic1"
> +#define DIR_READDIR		SANDBOX_FOLDER"/dir_readdir"
> +#define DIR_RMDIR		SANDBOX_FOLDER"/dir_rmdir"
> +#define DEV_CHAR0		SANDBOX_FOLDER"/chardev0"
> +#define DEV_BLK0		SANDBOX_FOLDER"/blkdev0"
> +
> +#define ALL_RULES (\
> +	LANDLOCK_ACCESS_FS_EXECUTE | \
> +	LANDLOCK_ACCESS_FS_WRITE_FILE | \
> +	LANDLOCK_ACCESS_FS_READ_FILE | \
> +	LANDLOCK_ACCESS_FS_READ_DIR | \
> +	LANDLOCK_ACCESS_FS_REMOVE_DIR | \
> +	LANDLOCK_ACCESS_FS_REMOVE_FILE | \
> +	LANDLOCK_ACCESS_FS_MAKE_CHAR | \
> +	LANDLOCK_ACCESS_FS_MAKE_DIR | \
> +	LANDLOCK_ACCESS_FS_MAKE_REG | \
> +	LANDLOCK_ACCESS_FS_MAKE_SOCK | \
> +	LANDLOCK_ACCESS_FS_MAKE_FIFO | \
> +	LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
> +	LANDLOCK_ACCESS_FS_MAKE_SYM | \
> +	LANDLOCK_ACCESS_FS_REFER | \
> +	LANDLOCK_ACCESS_FS_TRUNCATE | \
> +	LANDLOCK_ACCESS_FS_IOCTL_DEV)

I see we haven't covered LANDLOCK_ACCESS_NET_BIND_TCP and
LANDLOCK_ACCESS_NET_CONNECT_TCP (which we added into the LAPI header). This is
not complaining, but it'd be good to remember that. I wanted to note that in the
ticket, but we have no landlock ticket, thus I created one:

https://github.com/linux-test-project/ltp/issues/1181

> +
> +static char *readdir_files[] = {
> +	DIR_READDIR"/file0",
> +	DIR_READDIR"/file1",
> +	DIR_READDIR"/file2",
> +};
> +
> +static int dev_chr;
> +static int dev_blk;
> +
> +static int tester_get_all_fs_rules(void)
> +{
> +	int abi;
> +	int all_rules = ALL_RULES;
> +
> +	abi = SAFE_LANDLOCK_CREATE_RULESET(
> +		NULL, 0, LANDLOCK_CREATE_RULESET_VERSION);
> +
> +	if (abi < 2)
> +		all_rules &= ~LANDLOCK_ACCESS_FS_REFER;
> +
> +	if (abi < 3)
> +		all_rules &= ~LANDLOCK_ACCESS_FS_TRUNCATE;
> +
> +	if (abi < 5)
> +		all_rules &= ~LANDLOCK_ACCESS_FS_IOCTL_DEV;
> +
> +	return all_rules;
> +}
> +
> +static void tester_create_fs_tree(void)
> +{
> +	if (access(SANDBOX_FOLDER, F_OK) == -1)
> +		SAFE_MKDIR(SANDBOX_FOLDER, PERM_MODE);
> +
> +	/* folders */
> +	SAFE_MKDIR(DIR_RMDIR, PERM_MODE);
> +	SAFE_MKDIR(DIR_READDIR, PERM_MODE);
> +	for (size_t i = 0; i < ARRAY_SIZE(readdir_files); i++)
> +		SAFE_TOUCH(readdir_files[i], PERM_MODE, NULL);
> +
> +	/* files */
> +	tst_fill_file(FILE_READ, 'a', getpagesize(), 1);
> +	SAFE_TOUCH(FILE_WRITE, PERM_MODE, NULL);
> +	SAFE_TOUCH(FILE_REMOVE, PERM_MODE, NULL);
> +	SAFE_TOUCH(FILE_UNLINK, PERM_MODE, NULL);
> +	SAFE_TOUCH(FILE_UNLINKAT, PERM_MODE, NULL);
> +	SAFE_TOUCH(FILE_TRUNCATE, PERM_MODE, NULL);
> +	SAFE_TOUCH(FILE_SYM0, PERM_MODE, NULL);
> +	SAFE_CP(TESTAPP, FILE_EXEC);
> +
> +	/* devices */
> +	dev_chr = makedev(1, 3);
> +	dev_blk = makedev(7, 0);
Shouldn't we check makedev result?

i.g.
if (dev_chr != 0 && errno != EEXIST)
	tst_brk("can't create dev_chr");

We probably can ignore it, as we don't test that even in new API tests
(fgetxattr02.c, statx01.c). I suppose it's unlikely to fail (only permissions),
but maybe in the future we should add SAFE_MAKEDEV().

> +}
> +
> +static void _test_exec(const int result)
> +{
> +	int status;
> +	pid_t pid;
> +	char *const args[] = {(char *)FILE_EXEC, NULL};
> +
> +	tst_res(TINFO, "Test binary execution");
> +
> +	pid = SAFE_FORK();
> +	if (!pid) {
> +		int rval;
> +
> +		if (result == TPASS) {
> +			rval = execve(FILE_EXEC, args, NULL);
> +			if (rval == -1)
> +				tst_res(TFAIL | TERRNO, "Failed to execute test binary");
> +		} else {
> +			TST_EXP_FAIL(execve(FILE_EXEC, args, NULL), EACCES);
> +		}
> +
> +		_exit(1);
> +	}
> +
> +	SAFE_WAITPID(pid, &status, 0);
> +	if (!WIFEXITED(status) || WEXITSTATUS(status) != 0)
Why don't we TFAIL on result == TPASS? What am I missing?

> +		return;
> +
> +	tst_res(result, "Test binary has been executed");
> +}
> +
> +static void _test_write(const int result)
> +{
> +	tst_res(TINFO, "Test writing file");
> +
> +	if (result == TPASS)
> +		TST_EXP_FD(open(FILE_WRITE, O_WRONLY, PERM_MODE));
> +	else
> +		TST_EXP_FAIL(open(FILE_WRITE, O_WRONLY, PERM_MODE), EACCES);
> +
> +	if (TST_RET != -1)
> +		SAFE_CLOSE(TST_RET);
> +}
> +
> +static void _test_read(const int result)
> +{
> +	tst_res(TINFO, "Test reading file");
> +
> +	if (result == TPASS)
> +		TST_EXP_FD(open(FILE_READ, O_RDONLY, PERM_MODE));
> +	else
> +		TST_EXP_FAIL(open(FILE_READ, O_RDONLY, PERM_MODE), EACCES);
> +
> +	if (TST_RET != -1)
> +		SAFE_CLOSE(TST_RET);
> +}
> +
> +static void _test_readdir(const int result)
> +{
> +	tst_res(TINFO, "Test reading directory");
> +
> +	DIR *dir;
> +	struct dirent *de;
> +	int files_counted = 0;
> +
> +	dir = opendir(DIR_READDIR);
> +	if (!dir) {
> +		tst_res(result == TPASS ? TFAIL : TPASS,
> +			"Can't read '%s' directory", DIR_READDIR);
> +
> +		return;
> +	}
> +
> +	tst_res(result, "Can read '%s' directory", DIR_READDIR);
> +	if (result == TFAIL)
> +		return;
NOTE the check above is needed only if 'dir = opendir(DIR_READDIR);' passes on
result == TFAIL. I'm not sure if this can happen, but probably better to assume it can.

> +
> +	while ((de = readdir(dir)) != NULL) {
> +		if (de->d_type != DT_REG)
> +			continue;
> +
> +		for (size_t i = 0; i < ARRAY_SIZE(readdir_files); i++) {
> +			if (readdir_files[i] == NULL)
> +				continue;
> +
> +			if (strstr(readdir_files[i], de->d_name) != NULL)
> +				files_counted++;
> +		}
> +	}
> +
> +	SAFE_CLOSEDIR(dir);
> +
> +	TST_EXP_EQ_LI(files_counted, ARRAY_SIZE(readdir_files));
> +}
> +
> +static void _test_rmdir(const int result)
> +{
> +	tst_res(TINFO, "Test removing directory");
> +
> +	if (result == TPASS)
> +		TST_EXP_PASS(rmdir(DIR_RMDIR));
> +	else
> +		TST_EXP_FAIL(rmdir(DIR_RMDIR), EACCES);
> +}
> +
> +static void _test_rmfile(const int result)
> +{
> +	tst_res(TINFO, "Test removing file");
> +
> +	if (result == TPASS) {
> +		TST_EXP_PASS(unlink(FILE_UNLINK));
> +		TST_EXP_PASS(remove(FILE_REMOVE));
> +	} else {
> +		TST_EXP_FAIL(unlink(FILE_UNLINK), EACCES);
> +		TST_EXP_FAIL(remove(FILE_REMOVE), EACCES);
> +	}
> +}
> +
> +static void _test_make(
> +	const char *path,
> +	const int type,
> +	const int dev,
> +	const int result)
...
> +{
> +	tst_res(TINFO, "Test normal or special files creation");
> +
> +	if (result == TPASS)
> +		TST_EXP_PASS(mknod(path, type | 0400, dev));
> +	else
> +		TST_EXP_FAIL(mknod(path, type | 0400, dev), EACCES);

Here I guess we need to remove file to enable running with -i.
> +}
> +
> +static void _test_symbolic(const int result)
> +{
> +	tst_res(TINFO, "Test symbolic links");
> +
> +	if (result == TPASS)
> +		TST_EXP_PASS(symlink(FILE_SYM0, FILE_SYM1));
> +	else
> +		TST_EXP_FAIL(symlink(FILE_SYM0, FILE_SYM1), EACCES);

Also here.
> +}
> +
> +static void _test_truncate(const int result)
> +{
> +	int fd;
> +
> +	tst_res(TINFO, "Test truncating file");
> +
> +	if (result == TPASS) {
> +		TST_EXP_PASS(truncate(FILE_TRUNCATE, 10));
> +
> +		fd = TST_EXP_FD(open(FILE_TRUNCATE, O_WRONLY, PERM_MODE));
Shouldn't we use SAFE_OPEN() here? To quit early on error.

> +		if (fd != -1) {
> +			TST_EXP_PASS(ftruncate(fd, 10));
> +			SAFE_CLOSE(fd);
> +		}
> +
> +		fd = TST_EXP_FD(open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE));
> +		if (fd != -1)
> +			SAFE_CLOSE(fd);
> +	} else {
> +		TST_EXP_FAIL(truncate(FILE_TRUNCATE, 10), EACCES);
> +
> +		fd = open(FILE_TRUNCATE, O_WRONLY, PERM_MODE);
> +		if (fd != -1) {
> +			TST_EXP_FAIL(ftruncate(fd, 10), EACCES);
> +			SAFE_CLOSE(fd);
> +		}
> +
> +		TST_EXP_FAIL(open(FILE_TRUNCATE, O_WRONLY | O_TRUNC, PERM_MODE),
> +			EACCES);
> +
> +		if (TST_RET != -1)
> +			SAFE_CLOSE(TST_RET);
> +	}
> +}
> +
...

The rest LGTM.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 3/5] Add landlock04 test

[Petr Vorel]

Hi all,

...
> +static void tester_run_fs_rules(const int rules, const int result)
> +{
> +	if (rules & LANDLOCK_ACCESS_FS_EXECUTE)
> +		_test_exec(result);
> +
> +	if (rules & LANDLOCK_ACCESS_FS_WRITE_FILE)
> +		_test_write(result);
> +
> +	if (rules & LANDLOCK_ACCESS_FS_READ_FILE)
> +		_test_read(result);
> +
> +	if (rules & LANDLOCK_ACCESS_FS_READ_DIR)
> +		_test_readdir(result);
> +
> +	if (rules & LANDLOCK_ACCESS_FS_REMOVE_DIR)
> +		_test_rmdir(result);
> +
> +	if (rules & LANDLOCK_ACCESS_FS_REMOVE_FILE)
> +		_test_rmfile(result);
> +

> +	if (rules & LANDLOCK_ACCESS_FS_MAKE_CHAR)
> +		_test_make(DEV_CHAR0, S_IFCHR, dev_chr, result);
FYI no need to skip vfat and exfat (.skip_filesystems), just certain tests. This
one above would need to be skipped.

> +
> +	if (rules & LANDLOCK_ACCESS_FS_MAKE_BLOCK)
> +		_test_make(DEV_BLK0, S_IFBLK, dev_blk, result);
and this...

> +
> +	if (rules & LANDLOCK_ACCESS_FS_MAKE_REG)
> +		_test_make(FILE_REGULAR, S_IFREG, 0, result);
> +
> +	if (rules & LANDLOCK_ACCESS_FS_MAKE_SOCK)
> +		_test_make(FILE_SOCKET, S_IFSOCK, 0, result);
and this...

> +
> +	if (rules & LANDLOCK_ACCESS_FS_MAKE_FIFO)
> +		_test_make(FILE_FIFO, S_IFIFO, 0, result);
and this...

> +
> +	if (rules & LANDLOCK_ACCESS_FS_MAKE_SYM)
> +		_test_symbolic(result);
and this.

Kind regards,
Petr


> +
> +	if (rules & LANDLOCK_ACCESS_FS_TRUNCATE) {
> +		if ((tst_kvercmp(6, 2, 0)) < 0) {
> +			tst_res(TINFO, "Skip truncate test. Minimum kernel version is 6.2");
> +			return;
> +		}
> +
> +		_test_truncate(result);
> +	}
> +}

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 4/5] Add landlock05 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies LANDLOCK_ACCESS_FS_REFER access in the
landlock sandbox. The feature is available since kernel 5.19.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                |   1 +
 testcases/kernel/syscalls/landlock/.gitignore   |   1 +
 testcases/kernel/syscalls/landlock/landlock05.c | 116 ++++++++++++++++++++++++
 3 files changed, 118 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 67b2e2758..6522f5bc7 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -700,6 +700,7 @@ landlock01 landlock01
 landlock02 landlock02
 landlock03 landlock03
 landlock04 landlock04
+landlock05 landlock05
 
 lchown01 lchown01
 lchown01_16 lchown01_16
diff --git a/testcases/kernel/syscalls/landlock/.gitignore b/testcases/kernel/syscalls/landlock/.gitignore
index 4fe8d7cba..a7ea6be2e 100644
--- a/testcases/kernel/syscalls/landlock/.gitignore
+++ b/testcases/kernel/syscalls/landlock/.gitignore
@@ -3,3 +3,4 @@ landlock01
 landlock02
 landlock03
 landlock04
+landlock05
diff --git a/testcases/kernel/syscalls/landlock/landlock05.c b/testcases/kernel/syscalls/landlock/landlock05.c
new file mode 100644
index 000000000..6ad1fdb79
--- /dev/null
+++ b/testcases/kernel/syscalls/landlock/landlock05.c
@@ -0,0 +1,116 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies LANDLOCK_ACCESS_FS_REFER access in the
+ * landlock sandbox.
+ *
+ * [Algorithm]
+ *
+ * - apply LANDLOCK_ACCESS_FS_REFER in the folder1
+ * - apply LANDLOCK_ACCESS_FS_REFER in the folder2
+ * - create folder3
+ * - verify that file can be moved from folder1 to folder2
+ * - verify that file can't be moved from folder1 to folder3
+ */
+
+#include "landlock_common.h"
+
+#define MNTPOINT "sandbox"
+#define DIR1 MNTPOINT"/folder1"
+#define DIR2 MNTPOINT"/folder2"
+#define DIR3 MNTPOINT"/folder3"
+#define FILENAME1 DIR1"/file"
+#define FILENAME2 DIR2"/file"
+#define FILENAME3 DIR3"/file"
+
+static struct landlock_ruleset_attr *ruleset_attr;
+static struct landlock_path_beneath_attr *path_beneath_attr;
+
+static void run(void)
+{
+	if (SAFE_FORK())
+		return;
+
+	TST_EXP_PASS(rename(FILENAME1, FILENAME2));
+	if (TST_RET == -1)
+		return;
+
+	TST_EXP_FAIL(rename(FILENAME2, FILENAME3), EXDEV);
+	TST_EXP_PASS(rename(FILENAME2, FILENAME1));
+
+	_exit(0);
+}
+
+static void setup(void)
+{
+	int ruleset_fd;
+
+	verify_landlock_is_enabled();
+
+	SAFE_MKDIR(DIR1, 0640);
+	SAFE_MKDIR(DIR2, 0640);
+	SAFE_MKDIR(DIR3, 0640);
+	SAFE_TOUCH(FILENAME1, 0640, NULL);
+
+	tst_res(TINFO, "Applying LANDLOCK_ACCESS_FS_REFER");
+
+	ruleset_attr->handled_access_fs =
+		LANDLOCK_ACCESS_FS_READ_FILE |
+		LANDLOCK_ACCESS_FS_WRITE_FILE |
+		LANDLOCK_ACCESS_FS_REFER;
+
+	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
+		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
+
+	apply_landlock_rule(
+		path_beneath_attr,
+		ruleset_fd,
+		LANDLOCK_ACCESS_FS_REFER,
+		DIR1);
+
+	apply_landlock_rule(
+		path_beneath_attr,
+		ruleset_fd,
+		LANDLOCK_ACCESS_FS_REFER,
+		DIR2);
+
+	enforce_ruleset(ruleset_fd);
+
+	SAFE_CLOSE(ruleset_fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.min_kver = "5.19",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_SECURITY_LANDLOCK=y",
+		NULL
+	},
+	.bufs = (struct tst_buffers []) {
+		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
+		{},
+	},
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN),
+		{}
+	},
+	.format_device = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *[]) {
+		"vfat",
+		"exfat",
+		NULL
+	},
+};

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

[LTP] [PATCH v4 5/5] Add landlock06 test

[Andrea Cervesato]

From: Andrea Cervesato <andrea.cervesato@suse.com>

This test verifies LANDLOCK_ACCESS_FS_IOCTL_DEV access in the
landlock sandbox by creating a pipe and testing that ioctl() can
be executed on it. The test is also verifying that some of the I/O
operations can be always executed no matter the sandbox rules.
This feature is available since kernel 6.10.

Reviewed-by: Li Wang <liwang@redhat.com>
Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com>
---
 runtest/syscalls                                |   1 +
 testcases/kernel/syscalls/landlock/.gitignore   |   1 +
 testcases/kernel/syscalls/landlock/landlock06.c | 112 ++++++++++++++++++++++++
 3 files changed, 114 insertions(+)

diff --git a/runtest/syscalls b/runtest/syscalls
index 6522f5bc7..7ebdb41d8 100644
--- a/runtest/syscalls
+++ b/runtest/syscalls
@@ -701,6 +701,7 @@ landlock02 landlock02
 landlock03 landlock03
 landlock04 landlock04
 landlock05 landlock05
+landlock06 landlock06
 
 lchown01 lchown01
 lchown01_16 lchown01_16
diff --git a/testcases/kernel/syscalls/landlock/.gitignore b/testcases/kernel/syscalls/landlock/.gitignore
index a7ea6be2e..315ac1dca 100644
--- a/testcases/kernel/syscalls/landlock/.gitignore
+++ b/testcases/kernel/syscalls/landlock/.gitignore
@@ -4,3 +4,4 @@ landlock02
 landlock03
 landlock04
 landlock05
+landlock06
diff --git a/testcases/kernel/syscalls/landlock/landlock06.c b/testcases/kernel/syscalls/landlock/landlock06.c
new file mode 100644
index 000000000..647ebbe48
--- /dev/null
+++ b/testcases/kernel/syscalls/landlock/landlock06.c
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2024 SUSE LLC Andrea Cervesato <andrea.cervesato@suse.com>
+ */
+
+/*\
+ * [Description]
+ *
+ * This test verifies LANDLOCK_ACCESS_FS_IOCTL_DEV access in the
+ * landlock sandbox by creating a pipe and testing that ioctl() can be executed
+ * on it. The test is also verifying that some of the I/O operations can be
+ * always executed no matter the sandbox rules.
+ */
+
+#include "landlock_common.h"
+#include <sys/ioctl.h>
+
+#define MNTPOINT "sandbox"
+#define FILENAME MNTPOINT"/fifo"
+
+static struct landlock_ruleset_attr *ruleset_attr;
+static struct landlock_path_beneath_attr *path_beneath_attr;
+static int file_fd;
+static int dev_fd;
+
+static void run(void)
+{
+	if (SAFE_FORK())
+		return;
+
+	int flag;
+	size_t sz = 0;
+
+	TST_EXP_PASS(ioctl(file_fd, FIONREAD, &sz));
+
+	/* check unrestrictable commands */
+	TST_EXP_PASS(ioctl(dev_fd, FIOCLEX));
+	TST_EXP_PASS(ioctl(dev_fd, FIONCLEX));
+	TST_EXP_PASS(ioctl(dev_fd, FIONBIO, &flag));
+	TST_EXP_PASS(ioctl(dev_fd, FIOASYNC, &flag));
+
+	_exit(0);
+}
+
+static void setup(void)
+{
+	int ruleset_fd;
+
+	verify_landlock_is_enabled();
+
+	SAFE_MKFIFO(FILENAME, 0640);
+
+	file_fd = SAFE_OPEN(FILENAME, O_RDONLY | O_NONBLOCK, 0640);
+	dev_fd = SAFE_OPEN("/dev/zero", O_RDONLY | O_NONBLOCK, 0640);
+
+	tst_res(TINFO, "Applying LANDLOCK_ACCESS_FS_IOCTL_DEV");
+
+	ruleset_attr->handled_access_fs = LANDLOCK_ACCESS_FS_IOCTL_DEV;
+
+	ruleset_fd = SAFE_LANDLOCK_CREATE_RULESET(
+		ruleset_attr, sizeof(struct landlock_ruleset_attr), 0);
+
+	apply_landlock_layer(
+		ruleset_attr,
+		path_beneath_attr,
+		MNTPOINT,
+		LANDLOCK_ACCESS_FS_IOCTL_DEV
+	);
+
+	SAFE_CLOSE(ruleset_fd);
+}
+
+static void cleanup(void)
+{
+	if (dev_fd != -1)
+		SAFE_CLOSE(dev_fd);
+
+	if (file_fd != -1)
+		SAFE_CLOSE(file_fd);
+}
+
+static struct tst_test test = {
+	.test_all = run,
+	.setup = setup,
+	.cleanup = cleanup,
+	.min_kver = "6.10",
+	.needs_tmpdir = 1,
+	.needs_root = 1,
+	.forks_child = 1,
+	.needs_kconfigs = (const char *[]) {
+		"CONFIG_SECURITY_LANDLOCK=y",
+		NULL
+	},
+	.bufs = (struct tst_buffers []) {
+		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
+		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
+		{},
+	},
+	.caps = (struct tst_cap []) {
+		TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN),
+		{}
+	},
+	.format_device = 1,
+	.mount_device = 1,
+	.mntpoint = MNTPOINT,
+	.all_filesystems = 1,
+	.skip_filesystems = (const char *[]) {
+		"vfat",
+		"exfat",
+		NULL
+	},
+};

-- 
2.43.0


-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 5/5] Add landlock06 test

[Petr Vorel]

Hi Andrea,

Reviewed-by: Petr Vorel <pvorel@suse.cz>

Few notes below.

...
> +static struct tst_test test = {
> +	.test_all = run,
> +	.setup = setup,
> +	.cleanup = cleanup,
> +	.min_kver = "6.10",
nit: would not be able to check the availablity via landlock ABI == 5?

> +	.needs_tmpdir = 1,
> +	.needs_root = 1,
> +	.forks_child = 1,
> +	.needs_kconfigs = (const char *[]) {
> +		"CONFIG_SECURITY_LANDLOCK=y",
> +		NULL
Maybe we don't need this in any landlock test, if we are checking
verify_landlock_is_enabled(), right?

> +	},
> +	.bufs = (struct tst_buffers []) {
> +		{&ruleset_attr, .size = sizeof(struct landlock_ruleset_attr)},
> +		{&path_beneath_attr, .size = sizeof(struct landlock_path_beneath_attr)},
> +		{},
> +	},
> +	.caps = (struct tst_cap []) {
> +		TST_CAP(TST_CAP_REQ, CAP_SYS_ADMIN),
> +		{}
> +	},
> +	.format_device = 1,
> +	.mount_device = 1,
Please remove these two before merge:

$ cd metadata && make
...
testcases/kernel/syscalls/landlock/landlock06.c: useless tag: format_device
testcases/kernel/syscalls/landlock/landlock06.c: useless tag: needs_tmpdir

> +	.mntpoint = MNTPOINT,
> +	.all_filesystems = 1,
> +	.skip_filesystems = (const char *[]) {
> +		"vfat",
> +		"exfat",
I wonder why landlock is not supported...

> +		NULL
> +	},
> +};

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 5/5] Add landlock06 test

[Mickaël Salaün]

On Fri, Jul 26, 2024 at 03:24:40PM +0200, Petr Vorel wrote:
> Hi Andrea,
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> 
> Few notes below.
> 
> ...
> > +static struct tst_test test = {
> > +	.test_all = run,
> > +	.setup = setup,
> > +	.cleanup = cleanup,
> > +	.min_kver = "6.10",
> nit: would not be able to check the availablity via landlock ABI == 5?

Because Landlock is available since 5.13, I guess min_kver should be set
to the same version.

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 5/5] Add landlock06 test

[Petr Vorel]

> On Fri, Jul 26, 2024 at 03:24:40PM +0200, Petr Vorel wrote:
> > Hi Andrea,

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > Few notes below.

> > ...
> > > +static struct tst_test test = {
> > > +	.test_all = run,
> > > +	.setup = setup,
> > > +	.cleanup = cleanup,
> > > +	.min_kver = "6.10",
> > nit: would not be able to check the availablity via landlock ABI == 5?

> Because Landlock is available since 5.13, I guess min_kver should be set
> to the same version.

If anybody backport this, kernel version will not match. IMHO it's better to
avoid specifying version if we can detect with something else (ABI version in
this case). Also avoid kernel config reading if not needed is IMHO better.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 5/5] Add landlock06 test

[Petr Vorel]

> > On Fri, Jul 26, 2024 at 03:24:40PM +0200, Petr Vorel wrote:
> > > Hi Andrea,

> > > Reviewed-by: Petr Vorel <pvorel@suse.cz>

> > > Few notes below.

> > > ...
> > > > +static struct tst_test test = {
> > > > +	.test_all = run,
> > > > +	.setup = setup,
> > > > +	.cleanup = cleanup,
> > > > +	.min_kver = "6.10",
> > > nit: would not be able to check the availablity via landlock ABI == 5?

> > Because Landlock is available since 5.13, I guess min_kver should be set
> > to the same version.

> If anybody backport this, kernel version will not match. IMHO it's better to
> avoid specifying version if we can detect with something else (ABI version in
> this case). Also avoid kernel config reading if not needed is IMHO better.

Although functionality is not backported often to the enterprise kernels (e.g.
SLES, RHEL), certain things are backported, e.g. [1]. And that is the reason why
LTP even has a way to detect enterprise kernel [2]. Obviously the easiest thing
is to avoid kernel version if there is way to detect functionality by different
way.

Kind regards,
Petr

[1] https://github.com/linux-test-project/ltp/commit/c354ba291da3e255c135ef78da0c7b8c5556da07
[2] https://github.com/linux-test-project/ltp/blob/master/lib/tst_kvercmp.c#L131

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp

Re: [LTP] [PATCH v4 5/5] Add landlock06 test

[Petr Vorel]

Hi all,

...
> > +	.mntpoint = MNTPOINT,
> > +	.all_filesystems = 1,
> > +	.skip_filesystems = (const char *[]) {
> > +		"vfat",
> > +		"exfat",
> I wonder why landlock is not supported...

It was probably obvious for others:

not only ioctl(file_fd, FIONREAD, &sz), but even mkfifo(sandbox/fifo, 0640)
fails on exfat and vfat.

Kind regards,
Petr

> > +		NULL
> > +	},
> > +};

> Kind regards,
> Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp