[PATCH v4 2/5] landlock: Require LANDLOCK_ACCESS_FS_MAKE_REG for whiteout creation

["Günther Noack" <gnoack@google.com>]

Whiteout files are used in the upper layer of an Overlayfs to indicate
that the file with this name does not exist in the unified view, even
if it is present in one of the lower layer file systems.

For userspace implementations of Overlay file systems (fuse-overlayfs),
whiteout files can be created from userspace as well:

* mknod(2) with S_IFCHR and makedev(0, 0)
* renameat2(2) with RENAME_WHITEOUT,
  creating the whiteout in the old place of the moved file.

This commit guards whiteout creation in both of these cases with
LANDLOCK_ACCESS_FS_MAKE_REG.  Whiteout files are *not* considered
character devices and are not bound to a driver.

Before this commit, renameat2(2) with RENAME_WHITEOUT would create a
directory entry even when all LANDLOCK_ACCESS_FS_MAKE_* rights are
denied.

This does not affect normal renames within layered OverlayFS mounts:
When doing a regular rename() on a mounted fuse-overlayfs, it is the
fuse-overlayfs daemon that exercises renameat2() with RENAME_WHITEOUT,
and only the Landlock domain of that daemon is checked there.

This also adds a Landlock erratum for that case.

Suggested-by: Christian Brauner <brauner@kernel.org>
Suggested-by: Mickaël Salaün <mic@digikod.net>
Fixes: cb2c7d1a1776 ("landlock: Support filesystem access-control")
Signed-off-by: Günther Noack <gnoack@google.com>
---
 include/uapi/linux/landlock.h    |  1 +
 security/landlock/errata/abi-1.h | 26 ++++++++++++++++++++++++++
 security/landlock/fs.c           | 31 ++++++++++++++++++++++++-------
 3 files changed, 51 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 7ffe2ef127ee..9c1102ebf06e 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -351,6 +351,7 @@ struct landlock_net_port_attr {
  *   device.
  * - %LANDLOCK_ACCESS_FS_MAKE_DIR: Create (or rename) a directory.
  * - %LANDLOCK_ACCESS_FS_MAKE_REG: Create (or rename or link) a regular file.
+ *   This also guards the creation of whiteout objects as used in OverlayFS.
  * - %LANDLOCK_ACCESS_FS_MAKE_SOCK: Create (or rename or link) a UNIX domain
  *   socket.
  * - %LANDLOCK_ACCESS_FS_MAKE_FIFO: Create (or rename or link) a named pipe.
diff --git a/security/landlock/errata/abi-1.h b/security/landlock/errata/abi-1.h
index 3f099555f059..ee43bf53f6e2 100644
--- a/security/landlock/errata/abi-1.h
+++ b/security/landlock/errata/abi-1.h
@@ -22,3 +22,29 @@
  * from their original mount points.
  */
 LANDLOCK_ERRATUM(3)
+
+/**
+ * DOC: erratum_4
+ *
+ * Erratum 4: Creation of whiteout objects
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ * This fix addresses an issue through which it was possible to create whiteout
+ * objects, even when all file creation is restricted using Landlock.
+ *
+ * With this fix, the creation of whiteout objects is now guarded using
+ * ``LANDLOCK_ACCESS_FS_MAKE_REG``, both when it is done through
+ * :manpage:`renameat2(2)` with `RENAME_WHITEOUT`, and when it is done through
+ * :manpage:`mknod(2)` with ``S_IFCHR`` and ``makedev(0, 0)`` (which previously
+ * required ``LANDLOCK_ACCESS_FS_MAKE_CHAR``).
+ *
+ * Whiteout objects are special file types used in OverlayFS to mark the absence
+ * of a file in an upper file system, even when the lower (often read-only) file
+ * system does have a file with the same name.
+ *
+ * Impact:
+ *
+ * Without this fix, it was possible to create whiteout files from userspace
+ * using :manpage:`renameat2(2)` with the ``RENAME_WHITEOUT`` flag.
+ */
+LANDLOCK_ERRATUM(4)
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index f7e5e4ef9eac..570f9ff21344 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -983,7 +983,8 @@ static int current_check_access_path(const struct path *const path,
 	return -EACCES;
 }
 
-static __attribute_const__ access_mask_t get_mode_access(const umode_t mode)
+static __attribute_const__ access_mask_t get_mode_access(const umode_t mode,
+							 const unsigned int dev)
 {
 	switch (mode & S_IFMT) {
 	case S_IFLNK:
@@ -991,6 +992,9 @@ static __attribute_const__ access_mask_t get_mode_access(const umode_t mode)
 	case S_IFDIR:
 		return LANDLOCK_ACCESS_FS_MAKE_DIR;
 	case S_IFCHR:
+		/* Whiteout objects are guarded with MAKE_REG. */
+		if (dev == WHITEOUT_DEV)
+			return LANDLOCK_ACCESS_FS_MAKE_REG;
 		return LANDLOCK_ACCESS_FS_MAKE_CHAR;
 	case S_IFBLK:
 		return LANDLOCK_ACCESS_FS_MAKE_BLOCK;
@@ -1093,6 +1097,7 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
  * @new_dentry: Destination file or directory.
  * @removable: Sets to true if it is a rename operation.
  * @exchange: Sets to true if it is a rename operation with RENAME_EXCHANGE.
+ * @whiteout: Sets to true if it is a rename operation with RENAME_WHITEOUT.
  *
  * Because of its unprivileged constraints, Landlock relies on file hierarchies
  * (and not only inodes) to tie access rights to files.  Being able to link or
@@ -1140,7 +1145,8 @@ static bool collect_domain_accesses(const struct landlock_ruleset *const domain,
 static int current_check_refer_path(struct dentry *const old_dentry,
 				    const struct path *const new_dir,
 				    struct dentry *const new_dentry,
-				    const bool removable, const bool exchange)
+				    const bool removable, const bool exchange,
+				    const bool whiteout)
 {
 	const struct landlock_cred_security *const subject =
 		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
@@ -1160,17 +1166,27 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 		if (unlikely(d_is_negative(new_dentry)))
 			return -ENOENT;
 		access_request_parent1 =
-			get_mode_access(d_backing_inode(new_dentry)->i_mode);
+			get_mode_access(d_backing_inode(new_dentry)->i_mode,
+					d_backing_inode(new_dentry)->i_rdev);
 	} else {
 		access_request_parent1 = 0;
 	}
 	access_request_parent2 =
-		get_mode_access(d_backing_inode(old_dentry)->i_mode);
+		get_mode_access(d_backing_inode(old_dentry)->i_mode,
+				d_backing_inode(old_dentry)->i_rdev);
 	if (removable) {
 		access_request_parent1 |= maybe_remove(old_dentry);
 		access_request_parent2 |= maybe_remove(new_dentry);
 	}
 
+	/*
+	 * In case of renameat2(2) with RENAME_WHITEOUT, a whiteout object is
+	 * created in the source location, so we require an additional access
+	 * right there.
+	 */
+	if (whiteout)
+		access_request_parent1 |= LANDLOCK_ACCESS_FS_MAKE_REG;
+
 	/* The mount points are the same for old and new paths, cf. EXDEV. */
 	if (old_dentry->d_parent == new_dir->dentry) {
 		/*
@@ -1520,7 +1536,7 @@ static int hook_path_link(struct dentry *const old_dentry,
 			  struct dentry *const new_dentry)
 {
 	return current_check_refer_path(old_dentry, new_dir, new_dentry, false,
-					false);
+					false, false);
 }
 
 static int hook_path_rename(const struct path *const old_dir,
@@ -1531,7 +1547,8 @@ static int hook_path_rename(const struct path *const old_dir,
 {
 	/* old_dir refers to old_dentry->d_parent and new_dir->mnt */
 	return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
-					!!(flags & RENAME_EXCHANGE));
+					!!(flags & RENAME_EXCHANGE),
+					!!(flags & RENAME_WHITEOUT));
 }
 
 static int hook_path_mkdir(const struct path *const dir,
@@ -1544,7 +1561,7 @@ static int hook_path_mknod(const struct path *const dir,
 			   struct dentry *const dentry, const umode_t mode,
 			   const unsigned int dev)
 {
-	return current_check_access_path(dir, get_mode_access(mode));
+	return current_check_access_path(dir, get_mode_access(mode, dev));
 }
 
 static int hook_path_symlink(const struct path *const dir,
-- 
2.55.0.229.g6434b31f56-goog