[PATCH v1 0/7] Minor Landlock fixes and new tests

[PATCH v1 0/7] Minor Landlock fixes and new tests

[Mickaël Salaün]

Hi,

This series contains landlock_add_rule(2) (cosmetic) signature and
documentation fixes.  There is also some miscellaneous new tests to
improve coverage and that may help for future access types (e.g.
networking).

Regards,

Mickaël Salaün (7):
  landlock: Fix landlock_add_rule(2) documentation
  landlock: Fix landlock_add_rule(2) signature
  selftest/landlock: Make tests build with old libc
  selftest/landlock: Extend tests for minimal valid attribute size
  selftest/landlock: Add tests for unknown access rights
  selftest/landlock: Extend access right tests to directories
  selftest/landlock: Fully test file rename with "remove" access

 include/linux/syscalls.h                     |  3 +-
 include/uapi/linux/landlock.h                |  5 +-
 security/landlock/syscalls.c                 | 14 ++--
 tools/testing/selftests/landlock/base_test.c |  5 ++
 tools/testing/selftests/landlock/fs_test.c   | 84 ++++++++++++++++----
 5 files changed, 86 insertions(+), 25 deletions(-)


base-commit: cfb92440ee71adcc2105b0890bb01ac3cddb8507
-- 
2.35.1

[PATCH v1 1/7] landlock: Fix landlock_add_rule(2) documentation

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

It is not mandatory to pass a file descriptor obtained with the O_PATH
flag.  Also, replace rule's accesses with ruleset's accesses.

Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-2-mic@digikod.net
---
 include/uapi/linux/landlock.h | 5 +++--
 security/landlock/syscalls.c  | 7 +++----
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index b3d952067f59..c0390e318a65 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -60,8 +60,9 @@ struct landlock_path_beneath_attr {
 	 */
 	__u64 allowed_access;
 	/**
-	 * @parent_fd: File descriptor, open with ``O_PATH``, which identifies
-	 * the parent directory of a file hierarchy, or just a file.
+	 * @parent_fd: File descriptor, preferably opened with ``O_PATH``,
+	 * which identifies the parent directory of a file hierarchy, or just a
+	 * file.
 	 */
 	__s32 parent_fd;
 	/*
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 32396962f04d..fd4b24022a06 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -290,14 +290,13 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
  *
  * - EOPNOTSUPP: Landlock is supported by the kernel but disabled at boot time;
  * - EINVAL: @flags is not 0, or inconsistent access in the rule (i.e.
- *   &landlock_path_beneath_attr.allowed_access is not a subset of the rule's
- *   accesses);
+ *   &landlock_path_beneath_attr.allowed_access is not a subset of the
+ *   ruleset handled accesses);
  * - ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access);
  * - EBADF: @ruleset_fd is not a file descriptor for the current thread, or a
  *   member of @rule_attr is not a file descriptor as expected;
  * - EBADFD: @ruleset_fd is not a ruleset file descriptor, or a member of
- *   @rule_attr is not the expected file descriptor type (e.g. file open
- *   without O_PATH);
+ *   @rule_attr is not the expected file descriptor type;
  * - EPERM: @ruleset_fd has no write access to the underlying ruleset;
  * - EFAULT: @rule_attr inconsistency.
  */
-- 
2.35.1

[PATCH v1 2/7] landlock: Fix landlock_add_rule(2) signature

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Replace the enum landlock_rule_type with an int in the syscall signature
of landlock_add_rule to avoid an implementation-defined size.  In
practice an enum type is like an int (at least with GCC and clang), but
compilers may accept options (e.g. -fshort-enums) that would have an
impact on that [1].  This change is mostly a cosmetic fix according to
the current kernel compilers and used options.

Link: https://lore.kernel.org/r/8a22a3c2-468c-e96c-6516-22a0f029aa34@gmail.com/ [1]
Reported-by: Alejandro Colomar <alx.manpages@gmail.com>
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-3-mic@digikod.net
---
 include/linux/syscalls.h     | 3 +--
 security/landlock/syscalls.c | 7 ++++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 819c0cb00b6d..a5956f91caf2 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -71,7 +71,6 @@ struct clone_args;
 struct open_how;
 struct mount_attr;
 struct landlock_ruleset_attr;
-enum landlock_rule_type;
 
 #include <linux/types.h>
 #include <linux/aio_abi.h>
@@ -1053,7 +1052,7 @@ asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
 asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
 asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
 		size_t size, __u32 flags);
-asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
+asmlinkage long sys_landlock_add_rule(int ruleset_fd, int rule_type,
 		const void __user *rule_attr, __u32 flags);
 asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
 asmlinkage long sys_memfd_secret(unsigned int flags);
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index fd4b24022a06..3b40fc5d0216 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -277,8 +277,9 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
  *
  * @ruleset_fd: File descriptor tied to the ruleset that should be extended
  *		with the new rule.
- * @rule_type: Identify the structure type pointed to by @rule_attr (only
- *             LANDLOCK_RULE_PATH_BENEATH for now).
+ * @rule_type: Identify the structure type pointed to by @rule_attr as defined
+ *             by enum landlock_rule_type (only LANDLOCK_RULE_PATH_BENEATH for
+ *             now).
  * @rule_attr: Pointer to a rule (only of type &struct
  *             landlock_path_beneath_attr for now).
  * @flags: Must be 0.
@@ -301,7 +302,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
  * - EFAULT: @rule_attr inconsistency.
  */
 SYSCALL_DEFINE4(landlock_add_rule,
-		const int, ruleset_fd, const enum landlock_rule_type, rule_type,
+		const int, ruleset_fd, const int, rule_type,
 		const void __user *const, rule_attr, const __u32, flags)
 {
 	struct landlock_path_beneath_attr path_beneath_attr;
-- 
2.35.1

[PATCH v1 3/7] selftest/landlock: Make tests build with old libc

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Replace SYS_<syscall> with __NR_<syscall>.  Using the __NR_<syscall>
notation, provided by UAPI, is useful to build tests on systems without
the SYS_<syscall> definitions.

Replace SYS_pivot_root with __NR_pivot_root, and SYS_move_mount with
__NR_move_mount.

Define renameat2() and RENAME_EXCHANGE if they are unknown to old build
systems.

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-4-mic@digikod.net
---
 tools/testing/selftests/landlock/fs_test.c | 23 +++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 10c9a1e4ebd9..699cda25a12a 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -22,6 +22,19 @@
 
 #include "common.h"
 
+#ifndef renameat2
+int renameat2(int olddirfd, const char *oldpath, int newdirfd,
+		const char *newpath, unsigned int flags)
+{
+	return syscall(__NR_renameat2, olddirfd, oldpath, newdirfd, newpath,
+			flags);
+}
+#endif
+
+#ifndef RENAME_EXCHANGE
+#define RENAME_EXCHANGE		(1 << 1)
+#endif
+
 #define TMP_DIR		"tmp"
 #define BINARY_PATH	"./true"
 
@@ -1249,7 +1262,7 @@ TEST_F_FORK(layout1, rule_inside_mount_ns)
 	int ruleset_fd;
 
 	set_cap(_metadata, CAP_SYS_ADMIN);
-	ASSERT_EQ(0, syscall(SYS_pivot_root, dir_s3d2, dir_s3d3)) {
+	ASSERT_EQ(0, syscall(__NR_pivot_root, dir_s3d2, dir_s3d3)) {
 		TH_LOG("Failed to pivot root: %s", strerror(errno));
 	};
 	ASSERT_EQ(0, chdir("/"));
@@ -1282,7 +1295,7 @@ TEST_F_FORK(layout1, mount_and_pivot)
 	set_cap(_metadata, CAP_SYS_ADMIN);
 	ASSERT_EQ(-1, mount(NULL, dir_s3d2, NULL, MS_RDONLY, NULL));
 	ASSERT_EQ(EPERM, errno);
-	ASSERT_EQ(-1, syscall(SYS_pivot_root, dir_s3d2, dir_s3d3));
+	ASSERT_EQ(-1, syscall(__NR_pivot_root, dir_s3d2, dir_s3d3));
 	ASSERT_EQ(EPERM, errno);
 	clear_cap(_metadata, CAP_SYS_ADMIN);
 }
@@ -1301,12 +1314,12 @@ TEST_F_FORK(layout1, move_mount)
 	ASSERT_LE(0, ruleset_fd);
 
 	set_cap(_metadata, CAP_SYS_ADMIN);
-	ASSERT_EQ(0, syscall(SYS_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
+	ASSERT_EQ(0, syscall(__NR_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
 				dir_s1d2, 0)) {
 		TH_LOG("Failed to move mount: %s", strerror(errno));
 	}
 
-	ASSERT_EQ(0, syscall(SYS_move_mount, AT_FDCWD, dir_s1d2, AT_FDCWD,
+	ASSERT_EQ(0, syscall(__NR_move_mount, AT_FDCWD, dir_s1d2, AT_FDCWD,
 				dir_s3d2, 0));
 	clear_cap(_metadata, CAP_SYS_ADMIN);
 
@@ -1314,7 +1327,7 @@ TEST_F_FORK(layout1, move_mount)
 	ASSERT_EQ(0, close(ruleset_fd));
 
 	set_cap(_metadata, CAP_SYS_ADMIN);
-	ASSERT_EQ(-1, syscall(SYS_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
+	ASSERT_EQ(-1, syscall(__NR_move_mount, AT_FDCWD, dir_s3d2, AT_FDCWD,
 				dir_s1d2, 0));
 	ASSERT_EQ(EPERM, errno);
 	clear_cap(_metadata, CAP_SYS_ADMIN);
-- 
2.35.1

[PATCH v1 4/7] selftest/landlock: Extend tests for minimal valid attribute size

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

This might be useful when the struct landlock_ruleset_attr will get more
fields.

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-5-mic@digikod.net
---
 tools/testing/selftests/landlock/base_test.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index ca40abe9daa8..38fa1e0dfa33 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -34,6 +34,8 @@ TEST(inconsistent_attr) {
 	ASSERT_EQ(EINVAL, errno);
 	ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, 1, 0));
 	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, 7, 0));
+	ASSERT_EQ(EINVAL, errno);
 
 	ASSERT_EQ(-1, landlock_create_ruleset(NULL, 1, 0));
 	/* The size if less than sizeof(struct landlock_attr_enforce). */
@@ -46,6 +48,9 @@ TEST(inconsistent_attr) {
 	ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, page_size + 1, 0));
 	ASSERT_EQ(E2BIG, errno);
 
+	/* Checks minimal valid attribute size. */
+	ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr, 8, 0));
+	ASSERT_EQ(ENOMSG, errno);
 	ASSERT_EQ(-1, landlock_create_ruleset(ruleset_attr,
 				sizeof(struct landlock_ruleset_attr), 0));
 	ASSERT_EQ(ENOMSG, errno);
-- 
2.35.1

[PATCH v1 5/7] selftest/landlock: Add tests for unknown access rights

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Make sure that trying to use unknown access rights returns an error.

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-6-mic@digikod.net
---
 tools/testing/selftests/landlock/fs_test.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 699cda25a12a..5506472a46ce 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -439,6 +439,22 @@ TEST_F_FORK(layout1, file_access_rights)
 	ASSERT_EQ(0, close(path_beneath.parent_fd));
 }
 
+TEST_F_FORK(layout1, unknown_access_rights)
+{
+	__u64 access_mask;
+
+	for (access_mask = 1ULL << 63; access_mask != ACCESS_LAST;
+			access_mask >>= 1) {
+		struct landlock_ruleset_attr ruleset_attr = {
+			.handled_access_fs = access_mask,
+		};
+
+		ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr,
+					sizeof(ruleset_attr), 0));
+		ASSERT_EQ(EINVAL, errno);
+	}
+}
+
 static void add_path_beneath(struct __test_metadata *const _metadata,
 		const int ruleset_fd, const __u64 allowed_access,
 		const char *const path)
-- 
2.35.1

[PATCH v1 6/7] selftest/landlock: Extend access right tests to directories

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

Make sure that all filesystem access rights can be tied to directories.

Rename layout1/file_access_rights to layout1/file_and_dir_access_rights
to reflect this change.

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-7-mic@digikod.net
---
 tools/testing/selftests/landlock/fs_test.c | 29 ++++++++++++++++------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 5506472a46ce..3736253c9582 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -409,11 +409,12 @@ TEST_F_FORK(layout1, inval)
 	LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
 	ACCESS_LAST)
 
-TEST_F_FORK(layout1, file_access_rights)
+TEST_F_FORK(layout1, file_and_dir_access_rights)
 {
 	__u64 access;
 	int err;
-	struct landlock_path_beneath_attr path_beneath = {};
+	struct landlock_path_beneath_attr path_beneath_file = {},
+					  path_beneath_dir = {};
 	struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = ACCESS_ALL,
 	};
@@ -423,20 +424,32 @@ TEST_F_FORK(layout1, file_access_rights)
 	ASSERT_LE(0, ruleset_fd);
 
 	/* Tests access rights for files. */
-	path_beneath.parent_fd = open(file1_s1d2, O_PATH | O_CLOEXEC);
-	ASSERT_LE(0, path_beneath.parent_fd);
+	path_beneath_file.parent_fd = open(file1_s1d2, O_PATH | O_CLOEXEC);
+	ASSERT_LE(0, path_beneath_file.parent_fd);
+
+	/* Tests access rights for directories. */
+	path_beneath_dir.parent_fd = open(dir_s1d2, O_PATH | O_DIRECTORY |
+			O_CLOEXEC);
+	ASSERT_LE(0, path_beneath_dir.parent_fd);
+
 	for (access = 1; access <= ACCESS_LAST; access <<= 1) {
-		path_beneath.allowed_access = access;
+		path_beneath_dir.allowed_access = access;
+		ASSERT_EQ(0, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+				&path_beneath_dir, 0));
+
+		path_beneath_file.allowed_access = access;
 		err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
-				&path_beneath, 0);
-		if ((access | ACCESS_FILE) == ACCESS_FILE) {
+				&path_beneath_file, 0);
+		if (access & ACCESS_FILE) {
 			ASSERT_EQ(0, err);
 		} else {
 			ASSERT_EQ(-1, err);
 			ASSERT_EQ(EINVAL, errno);
 		}
 	}
-	ASSERT_EQ(0, close(path_beneath.parent_fd));
+	ASSERT_EQ(0, close(path_beneath_file.parent_fd));
+	ASSERT_EQ(0, close(path_beneath_dir.parent_fd));
+	ASSERT_EQ(0, close(ruleset_fd));
 }
 
 TEST_F_FORK(layout1, unknown_access_rights)
-- 
2.35.1

[PATCH v1 7/7] selftest/landlock: Fully test file rename with "remove" access

[Mickaël Salaün]

From: Mickaël Salaün <mic@linux.microsoft.com>

These tests were missing to check the check_access_path() call with all
combinations of maybe_remove(old_dentry) and maybe_remove(new_dentry).

Extend layout1/link with a new complementary test.

Cc: Shuah Khan <shuah@kernel.org>
Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
Link: https://lore.kernel.org/r/20220221155311.166278-8-mic@digikod.net
---
 tools/testing/selftests/landlock/fs_test.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 3736253c9582..62b88406419d 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -1640,11 +1640,14 @@ TEST_F_FORK(layout1, link)
 
 	ASSERT_EQ(-1, link(file2_s1d1, file1_s1d1));
 	ASSERT_EQ(EACCES, errno);
+
 	/* Denies linking because of reparenting. */
 	ASSERT_EQ(-1, link(file1_s2d1, file1_s1d2));
 	ASSERT_EQ(EXDEV, errno);
 	ASSERT_EQ(-1, link(file2_s1d2, file1_s1d3));
 	ASSERT_EQ(EXDEV, errno);
+	ASSERT_EQ(-1, link(file2_s1d3, file1_s1d2));
+	ASSERT_EQ(EXDEV, errno);
 
 	ASSERT_EQ(0, link(file2_s1d2, file1_s1d2));
 	ASSERT_EQ(0, link(file2_s1d3, file1_s1d3));
@@ -1668,7 +1671,6 @@ TEST_F_FORK(layout1, rename_file)
 
 	ASSERT_LE(0, ruleset_fd);
 
-	ASSERT_EQ(0, unlink(file1_s1d1));
 	ASSERT_EQ(0, unlink(file1_s1d2));
 
 	enforce_ruleset(_metadata, ruleset_fd);
@@ -1704,9 +1706,15 @@ TEST_F_FORK(layout1, rename_file)
 	ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s2d2, AT_FDCWD, file1_s2d1,
 				RENAME_EXCHANGE));
 	ASSERT_EQ(EACCES, errno);
+	/* Checks that file1_s2d1 cannot be removed (instead of ENOTDIR). */
+	ASSERT_EQ(-1, rename(dir_s2d2, file1_s2d1));
+	ASSERT_EQ(EACCES, errno);
 	ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s2d1, AT_FDCWD, dir_s2d2,
 				RENAME_EXCHANGE));
 	ASSERT_EQ(EACCES, errno);
+	/* Checks that file1_s1d1 cannot be removed (instead of EISDIR). */
+	ASSERT_EQ(-1, rename(file1_s1d1, dir_s1d2));
+	ASSERT_EQ(EACCES, errno);
 
 	/* Renames files with different parents. */
 	ASSERT_EQ(-1, rename(file1_s2d2, file1_s1d2));
@@ -1769,9 +1777,15 @@ TEST_F_FORK(layout1, rename_dir)
 	ASSERT_EQ(-1, renameat2(AT_FDCWD, dir_s1d1, AT_FDCWD, dir_s2d1,
 				RENAME_EXCHANGE));
 	ASSERT_EQ(EACCES, errno);
+	/* Checks that dir_s1d2 cannot be removed (instead of ENOTDIR). */
+	ASSERT_EQ(-1, rename(dir_s1d2, file1_s1d1));
+	ASSERT_EQ(EACCES, errno);
 	ASSERT_EQ(-1, renameat2(AT_FDCWD, file1_s1d1, AT_FDCWD, dir_s1d2,
 				RENAME_EXCHANGE));
 	ASSERT_EQ(EACCES, errno);
+	/* Checks that dir_s1d2 cannot be removed (instead of EISDIR). */
+	ASSERT_EQ(-1, rename(file1_s1d1, dir_s1d2));
+	ASSERT_EQ(EACCES, errno);
 
 	/*
 	 * Exchanges and renames directory to the same parent, which allows
-- 
2.35.1

Re: [PATCH v1 2/7] landlock: Fix landlock_add_rule(2) signature

[Alejandro Colomar (man-pages)]

Hi Mickaël,

On 21/2/22 16:53, Mickaël Salaün wrote:
> From: Mickaël Salaün <mic@linux.microsoft.com>
> 
> Replace the enum landlock_rule_type with an int in the syscall signature
> of landlock_add_rule to avoid an implementation-defined size.  In
> practice an enum type is like an int (at least with GCC and clang), but
> compilers may accept options (e.g. -fshort-enums) that would have an
> impact on that [1].  This change is mostly a cosmetic fix according to
> the current kernel compilers and used options.

There are two proposals for C2x that might bring C++ syntax to C for 
enums, i.e., being able to specify the underlying type of an enum.

See:
<http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2904.htm>
<http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2908.htm>

Since the current kernel is safe from that enum problem, it may be 
better to wait and see what the standard decides to do with enum.  I 
guess they'll add this feature sooner or later.

Regards,
Alex

> 
> Link: https://lore.kernel.org/r/8a22a3c2-468c-e96c-6516-22a0f029aa34@gmail.com/ [1]
> Reported-by: Alejandro Colomar <alx.manpages@gmail.com>
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Nick Desaulniers <ndesaulniers@google.com>
> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
> Link: https://lore.kernel.org/r/20220221155311.166278-3-mic@digikod.net
> ---
>   include/linux/syscalls.h     | 3 +--
>   security/landlock/syscalls.c | 7 ++++---
>   2 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 819c0cb00b6d..a5956f91caf2 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -71,7 +71,6 @@ struct clone_args;
>   struct open_how;
>   struct mount_attr;
>   struct landlock_ruleset_attr;
> -enum landlock_rule_type;
>   
>   #include <linux/types.h>
>   #include <linux/aio_abi.h>
> @@ -1053,7 +1052,7 @@ asmlinkage long sys_pidfd_send_signal(int pidfd, int sig,
>   asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
>   asmlinkage long sys_landlock_create_ruleset(const struct landlock_ruleset_attr __user *attr,
>   		size_t size, __u32 flags);
> -asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum landlock_rule_type rule_type,
> +asmlinkage long sys_landlock_add_rule(int ruleset_fd, int rule_type,
>   		const void __user *rule_attr, __u32 flags);
>   asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 flags);
>   asmlinkage long sys_memfd_secret(unsigned int flags);
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index fd4b24022a06..3b40fc5d0216 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -277,8 +277,9 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
>    *
>    * @ruleset_fd: File descriptor tied to the ruleset that should be extended
>    *		with the new rule.
> - * @rule_type: Identify the structure type pointed to by @rule_attr (only
> - *             LANDLOCK_RULE_PATH_BENEATH for now).
> + * @rule_type: Identify the structure type pointed to by @rule_attr as defined
> + *             by enum landlock_rule_type (only LANDLOCK_RULE_PATH_BENEATH for
> + *             now).
>    * @rule_attr: Pointer to a rule (only of type &struct
>    *             landlock_path_beneath_attr for now).
>    * @flags: Must be 0.
> @@ -301,7 +302,7 @@ static int get_path_from_fd(const s32 fd, struct path *const path)
>    * - EFAULT: @rule_attr inconsistency.
>    */
>   SYSCALL_DEFINE4(landlock_add_rule,
> -		const int, ruleset_fd, const enum landlock_rule_type, rule_type,
> +		const int, ruleset_fd, const int, rule_type,
>   		const void __user *const, rule_attr, const __u32, flags)
>   {
>   	struct landlock_path_beneath_attr path_beneath_attr;

Re: [PATCH v1 2/7] landlock: Fix landlock_add_rule(2) signature

[Mickaël Salaün]

On 26/02/2022 22:26, Alejandro Colomar (man-pages) wrote:
> Hi Mickaël,
> 
> On 21/2/22 16:53, Mickaël Salaün wrote:
>> From: Mickaël Salaün <mic@linux.microsoft.com>
>>
>> Replace the enum landlock_rule_type with an int in the syscall signature
>> of landlock_add_rule to avoid an implementation-defined size.  In
>> practice an enum type is like an int (at least with GCC and clang), but
>> compilers may accept options (e.g. -fshort-enums) that would have an
>> impact on that [1].  This change is mostly a cosmetic fix according to
>> the current kernel compilers and used options.
> 
> There are two proposals for C2x that might bring C++ syntax to C for 
> enums, i.e., being able to specify the underlying type of an enum.
> 
> See:
> <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2904.htm>
> <http://www.open-std.org/jtc1/sc22/wg14/www/docs/n2908.htm>
> 
> Since the current kernel is safe from that enum problem, it may be 
> better to wait and see what the standard decides to do with enum.  I 
> guess they'll add this feature sooner or later.

Ok, interesting, I'll remove this patch then. I'd be curious to know 
when this will impact Linux though.

Thanks!

> 
> Regards,
> Alex
> 
>>
>> Link: 
>> https://lore.kernel.org/r/8a22a3c2-468c-e96c-6516-22a0f029aa34@gmail.com/ 
>> [1]
>> Reported-by: Alejandro Colomar <alx.manpages@gmail.com>
>> Cc: Nathan Chancellor <nathan@kernel.org>
>> Cc: Nick Desaulniers <ndesaulniers@google.com>
>> Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
>> Link: https://lore.kernel.org/r/20220221155311.166278-3-mic@digikod.net
>> ---
>>   include/linux/syscalls.h     | 3 +--
>>   security/landlock/syscalls.c | 7 ++++---
>>   2 files changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
>> index 819c0cb00b6d..a5956f91caf2 100644
>> --- a/include/linux/syscalls.h
>> +++ b/include/linux/syscalls.h
>> @@ -71,7 +71,6 @@ struct clone_args;
>>   struct open_how;
>>   struct mount_attr;
>>   struct landlock_ruleset_attr;
>> -enum landlock_rule_type;
>>   #include <linux/types.h>
>>   #include <linux/aio_abi.h>
>> @@ -1053,7 +1052,7 @@ asmlinkage long sys_pidfd_send_signal(int pidfd, 
>> int sig,
>>   asmlinkage long sys_pidfd_getfd(int pidfd, int fd, unsigned int flags);
>>   asmlinkage long sys_landlock_create_ruleset(const struct 
>> landlock_ruleset_attr __user *attr,
>>           size_t size, __u32 flags);
>> -asmlinkage long sys_landlock_add_rule(int ruleset_fd, enum 
>> landlock_rule_type rule_type,
>> +asmlinkage long sys_landlock_add_rule(int ruleset_fd, int rule_type,
>>           const void __user *rule_attr, __u32 flags);
>>   asmlinkage long sys_landlock_restrict_self(int ruleset_fd, __u32 
>> flags);
>>   asmlinkage long sys_memfd_secret(unsigned int flags);
>> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
>> index fd4b24022a06..3b40fc5d0216 100644
>> --- a/security/landlock/syscalls.c
>> +++ b/security/landlock/syscalls.c
>> @@ -277,8 +277,9 @@ static int get_path_from_fd(const s32 fd, struct 
>> path *const path)
>>    *
>>    * @ruleset_fd: File descriptor tied to the ruleset that should be 
>> extended
>>    *        with the new rule.
>> - * @rule_type: Identify the structure type pointed to by @rule_attr 
>> (only
>> - *             LANDLOCK_RULE_PATH_BENEATH for now).
>> + * @rule_type: Identify the structure type pointed to by @rule_attr 
>> as defined
>> + *             by enum landlock_rule_type (only 
>> LANDLOCK_RULE_PATH_BENEATH for
>> + *             now).
>>    * @rule_attr: Pointer to a rule (only of type &struct
>>    *             landlock_path_beneath_attr for now).
>>    * @flags: Must be 0.
>> @@ -301,7 +302,7 @@ static int get_path_from_fd(const s32 fd, struct 
>> path *const path)
>>    * - EFAULT: @rule_attr inconsistency.
>>    */
>>   SYSCALL_DEFINE4(landlock_add_rule,
>> -        const int, ruleset_fd, const enum landlock_rule_type, rule_type,
>> +        const int, ruleset_fd, const int, rule_type,
>>           const void __user *const, rule_attr, const __u32, flags)
>>   {
>>       struct landlock_path_beneath_attr path_beneath_attr;
>