Re: [PATCH RFC -next 09/12] landlock: Implement metadata access hooks

Implement the LANDLOCK_ACCESS_FS_READ_METADATA and
LANDLOCK_ACCESS_FS_WRITE_METADATA access rights by hooking the
inode_getattr, inode_setattr, inode_setxattr, inode_getxattr,
inode_listxattr, inode_removexattr, inode_set_acl, inode_get_acl and
inode_remove_acl LSM hooks, which now receive a struct path thanks to
the preceding VFS and LSM refactoring.

The following system calls are now controlled:

- stat(2), fstat(2), lstat(2), newfstatat(2), getxattr(2) and
  friends, listxattr(2) and friends, and POSIX ACL reads via
  inode_getattr, inode_getxattr, inode_listxattr and inode_get_acl
  (READ_METADATA)
- chmod(2), fchmod(2), fchmodat(2), fchmodat2(2), chown(2), fchown(2),
  lchown(2), fchownat(2), chgrp(2), utimensat(2), futimens(2),
  utime(2), setxattr(2) and friends, removexattr(2) and friends, and
  POSIX ACL set and remove via inode_setattr, inode_setxattr,
  inode_removexattr, inode_set_acl and inode_remove_acl
  (WRITE_METADATA)

Both new rights are added to ACCESS_FILE as they apply to both files
and directories.

hook_inode_setattr only restricts explicit metadata changes, i.e. it
checks WRITE_METADATA only when the ia_valid mask contains
ATTR_MODE, ATTR_UID, ATTR_GID, ATTR_TIMES_SET or ATTR_TOUCH.
Metadata changes that the kernel performs implicitly, such as
timestamp updates on write(2) or size changes on truncate(2), are
therefore not restricted, and neither are chmod(2)/chown(2) calls
that do not change any attribute (e.g. chown(2) with -1/-1, which is
a no-op that never reaches the hook), matching the behavior of the
SELinux inode_setattr hook.

Kernel-internal accesses performed with override_creds() (e.g.
overlayfs and cachefiles) are not affected because Landlock domains
are attached to credentials, and kernel threads without a Landlock
domain (e.g. nfsd and ksmbd) are not restricted either.

Assisted-by: opencode: glm-5.3
Signed-off-by: Cai Xinchen <redacted>
---
 security/landlock/fs.c | 86 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index cab43892ec2f..e58b2aa0da65 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c

@@ -318,6 +318,8 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
 	LANDLOCK_ACCESS_FS_EXECUTE | \
 	LANDLOCK_ACCESS_FS_WRITE_FILE | \
 	LANDLOCK_ACCESS_FS_READ_FILE | \
+	LANDLOCK_ACCESS_FS_READ_METADATA | \
+	LANDLOCK_ACCESS_FS_WRITE_METADATA | \
 	LANDLOCK_ACCESS_FS_TRUNCATE | \
 	LANDLOCK_ACCESS_FS_IOCTL_DEV | \
 	LANDLOCK_ACCESS_FS_RESOLVE_UNIX)

@@ -1676,6 +1678,81 @@ static int hook_path_truncate(const struct path *const path)
 	return current_check_access_path(path, LANDLOCK_ACCESS_FS_TRUNCATE);
 }
 
+static int hook_inode_getattr(const struct path *const path)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_READ_METADATA);
+}
+
+static int hook_inode_setattr(const struct path *const path,
+			      struct iattr *const attr)
+{
+	/*
+	 * Explicit metadata changes (i.e. mode, ownership, and timestamps
+	 * set with utimes() and friends) require
+	 * LANDLOCK_ACCESS_FS_WRITE_METADATA.  Implicit timestamp updates
+	 * (e.g. ATTR_CTIME set for a write) and size changes (handled by
+	 * the truncate hooks) are not restricted.

+	 */
+	if (!(attr->ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID |
+				ATTR_TIMES_SET | ATTR_TOUCH)))
+		return 0;
+
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_WRITE_METADATA);
+}
+
+static int hook_inode_setxattr(const struct path *const path,
+			       const char *const name,
+			       const void *const value, const size_t size,
+			       const int flags)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_WRITE_METADATA);
+}
+
+static int hook_inode_getxattr(const struct path *const path,
+			       const char *const name)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_READ_METADATA);
+}
+
+static int hook_inode_listxattr(const struct path *const path)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_READ_METADATA);
+}
+
+static int hook_inode_removexattr(const struct path *const path,
+				  const char *const name)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_WRITE_METADATA);
+}
+
+static int hook_inode_set_acl(const struct path *const path,
+			      const char *const acl_name,
+			      struct posix_acl *const kacl)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_WRITE_METADATA);
+}
+
+static int hook_inode_get_acl(const struct path *const path,
+			      const char *const acl_name)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_READ_METADATA);
+}
+
+static int hook_inode_remove_acl(const struct path *const path,
+				 const char *const acl_name)
+{
+	return current_check_access_path(path,
+					LANDLOCK_ACCESS_FS_WRITE_METADATA);
+}
+
 /**
  * unmask_scoped_access - Remove access right bits in @masks in all layers
  *                        where @client and @server have the same domain

@@ -2100,6 +2177,15 @@ static struct security_hook_list landlock_hooks[] __ro_after_init = {
 	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(inode_getattr, hook_inode_getattr),
+	LSM_HOOK_INIT(inode_setattr, hook_inode_setattr),
+	LSM_HOOK_INIT(inode_setxattr, hook_inode_setxattr),
+	LSM_HOOK_INIT(inode_getxattr, hook_inode_getxattr),
+	LSM_HOOK_INIT(inode_listxattr, hook_inode_listxattr),
+	LSM_HOOK_INIT(inode_removexattr, hook_inode_removexattr),
+	LSM_HOOK_INIT(inode_set_acl, hook_inode_set_acl),
+	LSM_HOOK_INIT(inode_get_acl, hook_inode_get_acl),
+	LSM_HOOK_INIT(inode_remove_acl, hook_inode_remove_acl),
 	LSM_HOOK_INIT(unix_find, hook_unix_find),
 
 	LSM_HOOK_INIT(file_alloc_security, hook_file_alloc_security),

-- 
2.18.0.huawei.25