Re: [PATCH 1/2] landlock: Support truncate(2).

Mickaël Salaün <mic@xxxxxxxxxxx> · Fri, 8 Jul 2022 13:17:32 +0200

No final dot for a subject please.

On 07/07/2022 22:06, Günther Noack wrote:
Add support for restricting the use of the truncate(2) and
ftruncate(2) family of syscalls with Landlock.

This change also updates the Landlock ABI version and updates the
existing Landlock tests to match the new ABI version.

Technically, unprivileged processes can already restrict the use of
truncate(2) with seccomp-bpf.

Using Landlock instead of seccomp-bpf has the folowwing advantages:

typo: following


- it doesn't require the use of BPF (conceptually simpler)

- callers don't need to keep track of lists of syscall numbers for
   different architectures and kernel versions

- the restriction policy can be configured per file hierarchy.

Signed-off-by: Günther Noack <gnoack3000@xxxxxxxxx>
---
  include/uapi/linux/landlock.h                | 2 ++
  security/landlock/fs.c                       | 9 ++++++++-
  security/landlock/limits.h                   | 2 +-
  security/landlock/syscalls.c                 | 2 +-
  tools/testing/selftests/landlock/base_test.c | 2 +-
  tools/testing/selftests/landlock/fs_test.c   | 7 ++++---
  6 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 23df4e0e8ace..2351050d4773 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -134,6 +134,7 @@ struct landlock_path_beneath_attr {
   *   directory) parent.  Otherwise, such actions are denied with errno set to
   *   EACCES.  The EACCES errno prevails over EXDEV to let user space
   *   efficiently deal with an unrecoverable error.
+ * - %LANDLOCK_ACCESS_FS_TRUNCATE%: Truncate a file.

We need to specify the ABI version starting to support this right.

   *
   * .. warning::

You need to remove truncate(2) from this warning block.

   *
@@ -160,6 +161,7 @@ struct landlock_path_beneath_attr {
  #define LANDLOCK_ACCESS_FS_MAKE_BLOCK			(1ULL << 11)
  #define LANDLOCK_ACCESS_FS_MAKE_SYM			(1ULL << 12)
  #define LANDLOCK_ACCESS_FS_REFER			(1ULL << 13)
+#define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
  /* clang-format on */
  
  #endif /* _UAPI_LINUX_LANDLOCK_H */
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index ec5a6247cd3e..c57f581a9cd5 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -146,7 +146,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
  #define ACCESS_FILE ( \
  	LANDLOCK_ACCESS_FS_EXECUTE | \
  	LANDLOCK_ACCESS_FS_WRITE_FILE | \
-	LANDLOCK_ACCESS_FS_READ_FILE)
+	LANDLOCK_ACCESS_FS_READ_FILE | \
+	LANDLOCK_ACCESS_FS_TRUNCATE)
  /* clang-format on */
  
  /*
@@ -1140,6 +1141,11 @@ static int hook_path_rmdir(const struct path *const dir,
  	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
  }
  
+static int hook_path_truncate(const struct path *const path)
+{
+	return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
+}
+
  /* File hooks */
  
  static inline access_mask_t get_file_access(const struct file *const file)
@@ -1192,6 +1198,7 @@ static struct security_hook_list landlock_hooks[] __lsm_ro_after_init = {
  	LSM_HOOK_INIT(path_symlink, hook_path_symlink),
  	LSM_HOOK_INIT(path_unlink, hook_path_unlink),
  	LSM_HOOK_INIT(path_rmdir, hook_path_rmdir),
+	LSM_HOOK_INIT(path_truncate, hook_path_truncate),
  
  	LSM_HOOK_INIT(file_open, hook_file_open),
  };
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index b54184ab9439..82288f0e9e5e 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -18,7 +18,7 @@
  #define LANDLOCK_MAX_NUM_LAYERS		16
  #define LANDLOCK_MAX_NUM_RULES		U32_MAX
  
-#define LANDLOCK_LAST_ACCESS_FS		LANDLOCK_ACCESS_FS_REFER
+#define LANDLOCK_LAST_ACCESS_FS		LANDLOCK_ACCESS_FS_TRUNCATE
  #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
  #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
  
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 735a0865ea11..f4d6fc7ed17f 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -129,7 +129,7 @@ static const struct file_operations ruleset_fops = {
  	.write = fop_dummy_write,
  };
  
-#define LANDLOCK_ABI_VERSION 2
+#define LANDLOCK_ABI_VERSION 3
  
  /**
   * sys_landlock_create_ruleset - Create a new ruleset
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index da9290817866..72cdae277b02 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(2, landlock_create_ruleset(NULL, 0,
+	ASSERT_EQ(3, landlock_create_ruleset(NULL, 0,
  					     LANDLOCK_CREATE_RULESET_VERSION));
  
  	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 21a2ce8fa739..cb77eaa01c91 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -399,9 +399,10 @@ TEST_F_FORK(layout1, inval)
  #define ACCESS_FILE ( \
  	LANDLOCK_ACCESS_FS_EXECUTE | \
  	LANDLOCK_ACCESS_FS_WRITE_FILE | \
-	LANDLOCK_ACCESS_FS_READ_FILE)
+	LANDLOCK_ACCESS_FS_READ_FILE | \
+	LANDLOCK_ACCESS_FS_TRUNCATE)
  
-#define ACCESS_LAST LANDLOCK_ACCESS_FS_REFER
+#define ACCESS_LAST LANDLOCK_ACCESS_FS_TRUNCATE
  
  #define ACCESS_ALL ( \
  	ACCESS_FILE | \
@@ -415,7 +416,7 @@ TEST_F_FORK(layout1, inval)
  	LANDLOCK_ACCESS_FS_MAKE_FIFO | \
  	LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
  	LANDLOCK_ACCESS_FS_MAKE_SYM | \
-	ACCESS_LAST)
+	LANDLOCK_ACCESS_FS_REFER)

I created ACCESS_LAST to store the last access right while avoiding to 
copy it in ACCESS_FILE or ACCESS_ALL, and then avoid forgetting about 
new access right, but I now think it is not worth it and I prefer your 
approach which will be easier to maintain.

  
  /* clang-format on */
  






