[PATCH v4 0/5] landlock: Restrict whiteout object creation

[PATCH v4 0/5] landlock: Restrict whiteout object creation

[Günther Noack]

Hello!

As discussed in [1], the renameat2() syscall's RENAME_WHITEOUT flag allows
the creation of chardev directory entries with major=minor=0 as "whiteout
objects" in the location of the rename source file [2].

This functionality is available even without having any OverlayFS mounted
and can be invoked with the regular renameat2(2) syscall [3].

In V1 [5], it was discussed that whiteout objects are not the same as character
devices, and should therefore be treated differently.  After considering a new
access right in V2 and V3, we eventually settled on guarding it with the
existing LANDLOCK_ACCESS_FS_MAKE_REG access right and introducing a new erratum
[6].

Motivation
==========

The RENAME_WHITEOUT flag side-steps all of the existing Landlock access
rights, which are designed to restrict the creation of directory entries.
It is desirable to restrict that.

This patch set fixes that by adding a check in Landlock's path_rename and
path_mknod hooks.


[1] https://lore.kernel.org/all/adUBCQXrt7kmgqJT@google.com/
[2] https://docs.kernel.org/filesystems/overlayfs.html#whiteouts-and-opaque-directories
[3] https://man7.org/linux/man-pages/man2/renameat2.2.html#DESCRIPTION
[4] https://codesearch.debian.net/search?q=rename.*RENAME_WHITEOUT&literal=0
[5] https://lore.kernel.org/all/20260411090944.3131168-2-gnoack@google.com/
[6] https://lore.kernel.org/all/20260720.chow9ohYie5b@digikod.net/

Changelog
=========

v4:
 - Guard it with LANDLOCK_ACCESS_FS_MAKE_REG as discussed.
 - Selftests and documentation.

v3:
 - Do LANDLOCK_ACCESS_FS_MAKE_WHITEOUT check as part of
   current_check_refer_path().
 - https://lore.kernel.org/all/20260610092318.3868884-1-gnoack@google.com/

v2:
 - Introduce LANDLOCK_ACCESS_FS_MAKE_WHITEOUT access right
   and guard it with that.
 - Bump ABI version
 - https://lore.kernel.org/all/20260513160552.4022649-1-gnoack@google.com/

v1:
 - initial version
   https://lore.kernel.org/all/20260411090944.3131168-2-gnoack@google.com/


Günther Noack (5):
  selftests/landlock: Use an actual chardev for MAKE_CHAR audit test
  landlock: Require LANDLOCK_ACCESS_FS_MAKE_REG for whiteout creation
  selftests/landlock: Add tests for whiteout object creation
  selftests/landlock: Test whiteout object behaviour in OverlayFS
    renames
  landlock: Link the erratum documentation for whiteout objects

 Documentation/userspace-api/landlock.rst   |  3 ++
 include/uapi/linux/landlock.h              |  1 +
 security/landlock/errata/abi-1.h           | 26 +++++++++
 security/landlock/fs.c                     | 31 ++++++++---
 tools/testing/selftests/landlock/fs_test.c | 63 +++++++++++++++++++++-
 5 files changed, 116 insertions(+), 8 deletions(-)

Range-diff against v3:
-:  ------------ > 1:  119b5b4b2fb6 selftests/landlock: Use an actual chardev for MAKE_CHAR audit test
1:  4a8c3fb9e707 ! 2:  3203d5da06e3 landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
    @@ Metadata
     Author: Günther Noack <gnoack@google.com>
     
      ## Commit message ##
    -    landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
    +    landlock: Require LANDLOCK_ACCESS_FS_MAKE_REG for whiteout creation
     
    -    renameat2(2) with the RENAME_WHITEOUT flag places a whiteout character
    -    device file in the source file location in place of the moved file.
    -    This creates a directory entry even in cases where all
    -    LANDLOCK_ACCESS_FS_MAKE_* rights are denied.
    +    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.
     
    -    Introduce the LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right, which is checked
    -    for the origin directory if RENAME_WHITEOUT is passed.
    +    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 OverlayFS invokes rename with RENAME_WHITEOUT as part of a
    -    "normal" rename operation, it does so in ovl_rename() using the
    -    credentials that were set at the time of mounting the OverlayFS.
    +    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.
     
    -    Bump the Landlock ABI version to 10.
    +    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 ##
     @@ include/uapi/linux/landlock.h: struct landlock_net_port_attr {
    -  *
    -  *   If multiple requirements are not met, the ``EACCES`` error code takes
    -  *   precedence over ``EXDEV``.
    -+ * - %LANDLOCK_ACCESS_FS_MAKE_WHITEOUT: Create a whiteout object through
    -+ *   :manpage:`rename(2)` with ``RENAME_WHITEOUT``.
    -  *
    -  * .. warning::
    -  *
    -@@ include/uapi/linux/landlock.h: struct landlock_net_port_attr {
    - #define LANDLOCK_ACCESS_FS_TRUNCATE			(1ULL << 14)
    - #define LANDLOCK_ACCESS_FS_IOCTL_DEV			(1ULL << 15)
    - #define LANDLOCK_ACCESS_FS_RESOLVE_UNIX			(1ULL << 16)
    -+#define LANDLOCK_ACCESS_FS_MAKE_WHITEOUT		(1ULL << 17)
    - /* clang-format on */
    - 
    - /**
    +  *   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.
     
    - ## security/landlock/audit.c ##
    -@@ security/landlock/audit.c: static const char *const fs_access_strings[] = {
    - 	[BIT_INDEX(LANDLOCK_ACCESS_FS_TRUNCATE)] = "fs.truncate",
    - 	[BIT_INDEX(LANDLOCK_ACCESS_FS_IOCTL_DEV)] = "fs.ioctl_dev",
    - 	[BIT_INDEX(LANDLOCK_ACCESS_FS_RESOLVE_UNIX)] = "fs.resolve_unix",
    -+	[BIT_INDEX(LANDLOCK_ACCESS_FS_MAKE_WHITEOUT)] = "fs.make_whiteout",
    - };
    - 
    - static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
    + ## security/landlock/errata/abi-1.h ##
    +@@
    +  * 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)
     
      ## security/landlock/fs.c ##
    +@@ security/landlock/fs.c: 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:
    +@@ security/landlock/fs.c: 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;
     @@ security/landlock/fs.c: 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.
    @@ security/landlock/fs.c: static bool collect_domain_accesses(const struct landloc
      	const struct landlock_cred_security *const subject =
      		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
     @@ security/landlock/fs.c: 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);
      	}
      
    @@ security/landlock/fs.c: static int current_check_refer_path(struct dentry *const
     +	 * right there.
     +	 */
     +	if (whiteout)
    -+		access_request_parent1 |= LANDLOCK_ACCESS_FS_MAKE_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) {
    @@ security/landlock/fs.c: static int hook_path_rename(const struct path *const old
      }
      
      static int hook_path_mkdir(const struct path *const dir,
    -
    - ## security/landlock/limits.h ##
    -@@
    - #define LANDLOCK_MAX_NUM_LAYERS		16
    - #define LANDLOCK_MAX_NUM_RULES		U32_MAX
    - 
    --#define LANDLOCK_LAST_ACCESS_FS		LANDLOCK_ACCESS_FS_RESOLVE_UNIX
    -+#define LANDLOCK_LAST_ACCESS_FS		LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
    - #define LANDLOCK_MASK_ACCESS_FS		((LANDLOCK_LAST_ACCESS_FS << 1) - 1)
    - #define LANDLOCK_NUM_ACCESS_FS		__const_hweight64(LANDLOCK_MASK_ACCESS_FS)
    - 
    -
    - ## security/landlock/syscalls.c ##
    -@@ security/landlock/syscalls.c: static const struct file_operations ruleset_fops = {
    -  * If the change involves a fix that requires userspace awareness, also update
    -  * the errata documentation in Documentation/userspace-api/landlock.rst .
    -  */
    --const int landlock_abi_version = 9;
    -+const int landlock_abi_version = 10;
    - 
    - /**
    -  * sys_landlock_create_ruleset - Create a new ruleset
    -
    - ## tools/testing/selftests/landlock/base_test.c ##
    -@@ tools/testing/selftests/landlock/base_test.c: TEST(abi_version)
    - 	const struct landlock_ruleset_attr ruleset_attr = {
    - 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
    - 	};
    --	ASSERT_EQ(9, landlock_create_ruleset(NULL, 0,
    --					     LANDLOCK_CREATE_RULESET_VERSION));
    -+	ASSERT_EQ(10, landlock_create_ruleset(NULL, 0,
    -+					      LANDLOCK_CREATE_RULESET_VERSION));
    - 
    - 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,
    - 					      LANDLOCK_CREATE_RULESET_VERSION));
    -
    - ## tools/testing/selftests/landlock/fs_test.c ##
    -@@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout1, inval)
    - 	LANDLOCK_ACCESS_FS_IOCTL_DEV | \
    - 	LANDLOCK_ACCESS_FS_RESOLVE_UNIX)
    - 
    --#define ACCESS_LAST LANDLOCK_ACCESS_FS_RESOLVE_UNIX
    -+#define ACCESS_LAST LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
    - 
    - #define ACCESS_ALL ( \
    - 	ACCESS_FILE | \
    -@@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout1, inval)
    - 	LANDLOCK_ACCESS_FS_MAKE_FIFO | \
    - 	LANDLOCK_ACCESS_FS_MAKE_BLOCK | \
    - 	LANDLOCK_ACCESS_FS_MAKE_SYM | \
    --	LANDLOCK_ACCESS_FS_REFER)
    -+	LANDLOCK_ACCESS_FS_REFER | \
    -+	LANDLOCK_ACCESS_FS_MAKE_WHITEOUT)
    - 
    - /* clang-format on */
    +@@ security/landlock/fs.c: 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:  063646822083 ! 3:  98f783636d6b selftests/landlock: Add test for RENAME_WHITEOUT denial
    @@ Metadata
     Author: Günther Noack <gnoack@google.com>
     
      ## Commit message ##
    -    selftests/landlock: Add test for RENAME_WHITEOUT denial
    +    selftests/landlock: Add tests for whiteout object creation
     
    -    Add a test to check that renames with RENAME_WHITEOUT are guarded by
    -    LANDLOCK_ACCESS_FS_MAKE_WHITEOUT.
    +    Add a test to check that whiteout object creation is guarded by
    +    LANDLOCK_ACCESS_FS_MAKE_REG, in the cases where these are created from
    +    userspace:
    +
    +    * Conventional creation with mknod()
    +    * Linking or renaming an existing whiteout object
    +    * renameat2() with RENAME_WHITEOUT,
    +      which creates a new whiteout object in the source location
     
         Signed-off-by: Günther Noack <gnoack@google.com>
     
    @@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout1, rename_file)
      
     +TEST_F_FORK(layout1, rename_whiteout_denied)
     +{
    -+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_WHITEOUT, NULL);
    ++	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, NULL);
     +
     +	/*
     +	 * Try to rename a file with RENAME_WHITEOUT.
    @@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout1, rename_file)
      TEST_F_FORK(layout1, rename_dir)
      {
      	const struct rule rules[] = {
    +@@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout1, make_char)
    + 		       makedev(1, 3));
    + }
    + 
    ++TEST_F_FORK(layout1, make_whiteout)
    ++{
    ++	/* Creates a whiteout object (creation guarded by MAKE_REG). */
    ++	set_cap(_metadata, CAP_MKNOD);
    ++	test_make_file(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, S_IFCHR,
    ++		       makedev(0, 0));
    ++}
    ++
    + TEST_F_FORK(layout1, make_block)
    + {
    + 	/* Creates a /dev/loop0 device. */
     @@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout2_overlay, same_content_different_file)
      	}
      }
3:  5d4606bc1e84 ! 4:  98aab13d9f3f selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
    @@ Metadata
     Author: Günther Noack <gnoack@google.com>
     
      ## Commit message ##
    -    selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
    +    selftests/landlock: Test whiteout object behaviour in OverlayFS renames
     
         Even though OverlayFS uses vfs_rename() with RENAME_WHITEOUT, and even
    -    though RENAME_WHITEOUT requires LANDLOCK_ACCESS_FS_MAKE_WHITEOUT, a process
    -    that renames files in an OverlayFS can do so without having the
    -    LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right in that location.
    +    though RENAME_WHITEOUT requires LANDLOCK_ACCESS_FS_MAKE_REG, a process that
    +    renames non-regular files in an OverlayFS can do so without having the
    +    LANDLOCK_ACCESS_FS_MAKE_REG right in that location.
     
         This works, and is supposed to work, because OverlayFS uses the credentials
         determined at mount time for the internal vfs_rename() operation. -- The
    @@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout2_overlay, same_co
      	}
      }
      
    -+TEST_F_FORK(layout2_overlay, rename_in_overlay_without_make_whiteout)
    ++TEST_F_FORK(layout2_overlay, rename_in_overlay_without_make_reg)
     +{
     +	struct stat st;
     +	const char *merge_fl1_renamed = MERGE_DATA "/fl1_renamed";
    @@ tools/testing/selftests/landlock/fs_test.c: TEST_F_FORK(layout2_overlay, same_co
     +	if (self->skip_test)
     +		SKIP(return, "overlayfs is not supported (test)");
     +
    -+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_WHITEOUT, NULL);
    ++	/*
    ++	 * In this test, merge_fl1 is a FIFO file.  MAKE_REG is restricted, but
    ++	 * MAKE_FIFO is allowed.  Despite MAKE_REG being restricted, the rename
    ++	 * on the OverlayFS works and creates a whiteout file in the underlying
    ++	 * upper file system.
    ++	 */
    ++	ASSERT_EQ(0, unlink(merge_fl1));
    ++	ASSERT_EQ(0, mknod(merge_fl1, S_IFIFO, 0));
    ++	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, NULL);
     +
     +	/*
     +	 * Execute a regular file rename within OverlayFS.
-:  ------------ > 5:  d79f3daff3ed landlock: Link the erratum documentation for whiteout objects
-- 
2.55.0.229.g6434b31f56-goog

[PATCH v4 1/5] selftests/landlock: Use an actual chardev for MAKE_CHAR audit test

[Günther Noack]

By passing a (0, 0) device number, the audit test for
LANDLOCK_ACCESS_FS_MAKE_CHAR was accidentally creating a whiteout object
rather than a char device.  In preparation to treating whiteout objects
differently, use an actual character device instead.

Signed-off-by: Günther Noack <gnoack@google.com>
---
 tools/testing/selftests/landlock/fs_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 86e08aa6e0a7..e82b56a74c5f 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -7436,7 +7436,7 @@ TEST_F(audit_layout1, make_char)
 
 	enforce_fs(_metadata, ACCESS_ALL, NULL);
 
-	EXPECT_EQ(-1, mknod(file1_s1d3, S_IFCHR | 0644, 0));
+	EXPECT_EQ(-1, mknod(file1_s1d3, S_IFCHR | 0644, makedev(7, 0)));
 	EXPECT_EQ(EACCES, errno);
 	EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd, "fs\\.make_char",
 				    dir_s1d3));
-- 
2.55.0.229.g6434b31f56-goog

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

[Günther Noack]

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

[PATCH v4 3/5] selftests/landlock: Add tests for whiteout object creation

[Günther Noack]

Add a test to check that whiteout object creation is guarded by
LANDLOCK_ACCESS_FS_MAKE_REG, in the cases where these are created from
userspace:

* Conventional creation with mknod()
* Linking or renaming an existing whiteout object
* renameat2() with RENAME_WHITEOUT,
  which creates a new whiteout object in the source location

Signed-off-by: Günther Noack <gnoack@google.com>
---
 tools/testing/selftests/landlock/fs_test.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index e82b56a74c5f..fe5faeca83eb 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2247,6 +2247,19 @@ TEST_F_FORK(layout1, rename_file)
 			       RENAME_EXCHANGE));
 }
 
+TEST_F_FORK(layout1, rename_whiteout_denied)
+{
+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, NULL);
+
+	/*
+	 * Try to rename a file with RENAME_WHITEOUT.
+	 * file1_s3d3 is in dir_s3d2 (tmpfs), so it supports RENAME_WHITEOUT.
+	 */
+	EXPECT_EQ(-1, renameat2(AT_FDCWD, file1_s3d3, AT_FDCWD,
+				TMP_DIR "/s3d1/s3d2/s3d3/f2", RENAME_WHITEOUT));
+	EXPECT_EQ(EACCES, errno);
+}
+
 TEST_F_FORK(layout1, rename_dir)
 {
 	const struct rule rules[] = {
@@ -3270,6 +3283,14 @@ TEST_F_FORK(layout1, make_char)
 		       makedev(1, 3));
 }
 
+TEST_F_FORK(layout1, make_whiteout)
+{
+	/* Creates a whiteout object (creation guarded by MAKE_REG). */
+	set_cap(_metadata, CAP_MKNOD);
+	test_make_file(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, S_IFCHR,
+		       makedev(0, 0));
+}
+
 TEST_F_FORK(layout1, make_block)
 {
 	/* Creates a /dev/loop0 device. */
@@ -6951,6 +6972,7 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
 	}
 }
 
+
 FIXTURE(layout3_fs)
 {
 	bool has_created_dir;
-- 
2.55.0.229.g6434b31f56-goog

[PATCH v4 4/5] selftests/landlock: Test whiteout object behaviour in OverlayFS renames

[Günther Noack]

Even though OverlayFS uses vfs_rename() with RENAME_WHITEOUT, and even
though RENAME_WHITEOUT requires LANDLOCK_ACCESS_FS_MAKE_REG, a process that
renames non-regular files in an OverlayFS can do so without having the
LANDLOCK_ACCESS_FS_MAKE_REG right in that location.

This works, and is supposed to work, because OverlayFS uses the credentials
determined at mount time for the internal vfs_rename() operation. -- The
rename happens with the credentials of the user who mounted the OverlayFS.

Signed-off-by: Günther Noack <gnoack@google.com>
---
 tools/testing/selftests/landlock/fs_test.c | 39 ++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index fe5faeca83eb..73770dbb0592 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -6972,6 +6972,45 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
 	}
 }
 
+TEST_F_FORK(layout2_overlay, rename_in_overlay_without_make_reg)
+{
+	struct stat st;
+	const char *merge_fl1_renamed = MERGE_DATA "/fl1_renamed";
+
+	if (self->skip_test)
+		SKIP(return, "overlayfs is not supported (test)");
+
+	/*
+	 * In this test, merge_fl1 is a FIFO file.  MAKE_REG is restricted, but
+	 * MAKE_FIFO is allowed.  Despite MAKE_REG being restricted, the rename
+	 * on the OverlayFS works and creates a whiteout file in the underlying
+	 * upper file system.
+	 */
+	ASSERT_EQ(0, unlink(merge_fl1));
+	ASSERT_EQ(0, mknod(merge_fl1, S_IFIFO, 0));
+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, NULL);
+
+	/*
+	 * Execute a regular file rename within OverlayFS.
+	 * merge_fl1 originates from lower layer, so this triggers a copy-up
+	 * and creation of a whiteout in the upper layer.
+	 */
+	EXPECT_EQ(0, rename(merge_fl1, merge_fl1_renamed));
+
+	/* Check that the rename worked. */
+	EXPECT_EQ(0, stat(merge_fl1_renamed, &st));
+	EXPECT_EQ(-1, stat(merge_fl1, &st));
+	EXPECT_EQ(ENOENT, errno);
+
+	/*
+	 * Check that the whiteout object on the underlying "upper" filesystem
+	 * exists after the rename.  This is OK because it was done with the
+	 * credentials of the OverlayFS.
+	 */
+	EXPECT_EQ(0, stat(UPPER_DATA "/fl1", &st));
+	EXPECT_TRUE(S_ISCHR(st.st_mode));
+	EXPECT_EQ(0, st.st_rdev);
+}
 
 FIXTURE(layout3_fs)
 {
-- 
2.55.0.229.g6434b31f56-goog

[PATCH v4 5/5] landlock: Link the erratum documentation for whiteout objects

[Günther Noack]

The documentation embeds the canonical erratum documentation from the
header file, which is already a self-contained description of the issue.

Signed-off-by: Günther Noack <gnoack@google.com>
---
 Documentation/userspace-api/landlock.rst | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 5a63d4476c1c..df996ccaee34 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -556,6 +556,9 @@ in the running kernel.
 .. kernel-doc:: security/landlock/errata/abi-1.h
     :doc: erratum_3
 
+.. kernel-doc:: security/landlock/errata/abi-1.h
+    :doc: erratum_4
+
 How to check for errata
 ~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
2.55.0.229.g6434b31f56-goog