All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT
@ 2026-06-10  9:23 Günther Noack
  2026-06-10  9:23 ` [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Günther Noack @ 2026-06-10  9:23 UTC (permalink / raw)
  To: Mickaël Salaün, Christian Brauner
  Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
	Serge Hallyn, Stephen Smalley, 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 guarded with a separate access
right.  We are therefore guarding the operation with the new access right
LANDLOCK_ACCESS_FS_MAKE_WHITEOUT now.

By introducing a new access right, that change is also exposed by
incrementing the ABI level and does not require a Landlock erratum.

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 hook.


[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/


Changelog
=========

v3:
 - Do LANDLOCK_ACCESS_FS_MAKE_WHITEOUT check as part of
   current_check_refer_path().

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 (3):
  landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
  selftests/landlock: Add test for RENAME_WHITEOUT denial
  selftests/landlock: Test OverlayFS renames w/o
    LANDLOCK_ACCESS_FS_MAKE_WHITEOUT

 include/uapi/linux/landlock.h                |  3 ++
 security/landlock/audit.c                    |  1 +
 security/landlock/fs.c                       | 17 +++++--
 security/landlock/limits.h                   |  2 +-
 security/landlock/syscalls.c                 |  2 +-
 tools/testing/selftests/landlock/base_test.c |  4 +-
 tools/testing/selftests/landlock/fs_test.c   | 50 +++++++++++++++++++-
 7 files changed, 70 insertions(+), 9 deletions(-)

Range-diff against v2:
1:  1f2b7f49b927 ! 1:  4a8c3fb9e707 landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
    @@ security/landlock/audit.c: static const char *const fs_access_strings[] = {
      static_assert(ARRAY_SIZE(fs_access_strings) == LANDLOCK_NUM_ACCESS_FS);
     
      ## security/landlock/fs.c ##
    +@@ 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.
    +  * @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
    +@@ security/landlock/fs.c: 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);
    +@@ security/landlock/fs.c: static int current_check_refer_path(struct dentry *const 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_WHITEOUT;
    ++
    + 	/* 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_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,
     @@ security/landlock/fs.c: static int hook_path_rename(const struct path *const old_dir,
    - 			    const unsigned int flags)
      {
      	/* old_dir refers to old_dentry->d_parent and new_dir->mnt */
    -+	if (flags & RENAME_WHITEOUT) {
    -+		int err;
    -+
    -+		/*
    -+		 * Rename with RENAME_WHITEOUT creates a whiteout object in the
    -+		 * old location, so we check the access right for creating that.
    -+		 *
    -+		 * See Documentation/filesystems/overlayfs.rst and renameat2(2).
    -+		 */
    -+		err = current_check_access_path(
    -+			old_dir, LANDLOCK_ACCESS_FS_MAKE_WHITEOUT);
    -+		if (err)
    -+			return err;
    -+	}
    -+
      	return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
    - 					!!(flags & RENAME_EXCHANGE));
    +-					!!(flags & RENAME_EXCHANGE));
    ++					!!(flags & RENAME_EXCHANGE),
    ++					!!(flags & RENAME_WHITEOUT));
      }
    + 
    + static int hook_path_mkdir(const struct path *const dir,
     
      ## security/landlock/limits.h ##
     @@
2:  aa4e4aeb5884 = 2:  063646822083 selftests/landlock: Add test for RENAME_WHITEOUT denial
3:  6660d70a1eda = 3:  5d4606bc1e84 selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
-- 
2.54.0.1099.g489fc7bff1-goog


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
  2026-06-10  9:23 [PATCH v3 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT Günther Noack
@ 2026-06-10  9:23 ` Günther Noack
  2026-06-10 13:38   ` Mickaël Salaün
  2026-06-10  9:23 ` [PATCH v3 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial Günther Noack
  2026-06-10  9:23 ` [PATCH v3 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT Günther Noack
  2 siblings, 1 reply; 6+ messages in thread
From: Günther Noack @ 2026-06-10  9:23 UTC (permalink / raw)
  To: Mickaël Salaün, Christian Brauner
  Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
	Serge Hallyn, Stephen Smalley, Günther Noack

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.

Introduce the LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right, which is checked
for the origin directory if RENAME_WHITEOUT is passed.

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.

Bump the Landlock ABI version to 10.

Suggested-by: Christian Brauner <brauner@kernel.org>
Suggested-by: Mickaël Salaün <mic@digikod.net>
Signed-off-by: Günther Noack <gnoack@google.com>
---
 include/uapi/linux/landlock.h                |  3 +++
 security/landlock/audit.c                    |  1 +
 security/landlock/fs.c                       | 17 ++++++++++++++---
 security/landlock/limits.h                   |  2 +-
 security/landlock/syscalls.c                 |  2 +-
 tools/testing/selftests/landlock/base_test.c |  4 ++--
 tools/testing/selftests/landlock/fs_test.c   |  5 +++--
 7 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 10a346e55e95..1f8a1d6d25f1 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -328,6 +328,8 @@ 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::
  *
@@ -356,6 +358,7 @@ 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 */
 
 /**
diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index 8d0edf94037d..09c97083f599 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -38,6 +38,7 @@ 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);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index c1ecfe239032..67810d5242b2 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -1080,6 +1080,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
@@ -1127,7 +1128,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);
@@ -1159,6 +1161,14 @@ static int current_check_refer_path(struct dentry *const 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_WHITEOUT;
+
 	/* The mount points are the same for old and new paths, cf. EXDEV. */
 	if (old_dentry->d_parent == new_dir->dentry) {
 		/*
@@ -1509,7 +1519,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,
@@ -1520,7 +1530,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,
diff --git a/security/landlock/limits.h b/security/landlock/limits.h
index b454ad73b15e..e59378e8e897 100644
--- a/security/landlock/limits.h
+++ b/security/landlock/limits.h
@@ -19,7 +19,7 @@
 #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)
 
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index accfd2e5a0cd..d45469d5d464 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -166,7 +166,7 @@ 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
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 30d37234086c..6c8113c2ded1 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,8 +76,8 @@ 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));
diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index cdb47fc1fc0a..53d1b659849f 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -579,7 +579,7 @@ 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 | \
@@ -593,7 +593,8 @@ 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 */
 
-- 
2.54.0.1099.g489fc7bff1-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial
  2026-06-10  9:23 [PATCH v3 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT Günther Noack
  2026-06-10  9:23 ` [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
@ 2026-06-10  9:23 ` Günther Noack
  2026-06-10  9:23 ` [PATCH v3 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT Günther Noack
  2 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-06-10  9:23 UTC (permalink / raw)
  To: Mickaël Salaün, Christian Brauner
  Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
	Serge Hallyn, Stephen Smalley, Günther Noack

Add a test to check that renames with RENAME_WHITEOUT are guarded by
LANDLOCK_ACCESS_FS_MAKE_WHITEOUT.

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

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 53d1b659849f..bdad92195f62 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2248,6 +2248,19 @@ TEST_F_FORK(layout1, rename_file)
 			       RENAME_EXCHANGE));
 }
 
+TEST_F_FORK(layout1, rename_whiteout_denied)
+{
+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_WHITEOUT, 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[] = {
@@ -6950,6 +6963,7 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
 	}
 }
 
+
 FIXTURE(layout3_fs)
 {
 	bool has_created_dir;
-- 
2.54.0.1099.g489fc7bff1-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v3 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
  2026-06-10  9:23 [PATCH v3 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT Günther Noack
  2026-06-10  9:23 ` [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
  2026-06-10  9:23 ` [PATCH v3 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial Günther Noack
@ 2026-06-10  9:23 ` Günther Noack
  2 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-06-10  9:23 UTC (permalink / raw)
  To: Mickaël Salaün, Christian Brauner
  Cc: linux-security-module, Paul Moore, Amir Goldstein, Miklos Szeredi,
	Serge Hallyn, Stephen Smalley, Günther Noack

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.

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 | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index bdad92195f62..0c29887278d0 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -6963,6 +6963,37 @@ TEST_F_FORK(layout2_overlay, same_content_different_file)
 	}
 }
 
+TEST_F_FORK(layout2_overlay, rename_in_overlay_without_make_whiteout)
+{
+	struct stat st;
+	const char *merge_fl1_renamed = MERGE_DATA "/fl1_renamed";
+
+	if (self->skip_test)
+		SKIP(return, "overlayfs is not supported (test)");
+
+	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_WHITEOUT, 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.54.0.1099.g489fc7bff1-goog


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
  2026-06-10  9:23 ` [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
@ 2026-06-10 13:38   ` Mickaël Salaün
  2026-06-12  8:34     ` Günther Noack
  0 siblings, 1 reply; 6+ messages in thread
From: Mickaël Salaün @ 2026-06-10 13:38 UTC (permalink / raw)
  To: Günther Noack
  Cc: Christian Brauner, linux-security-module, Paul Moore,
	Amir Goldstein, Miklos Szeredi, Serge Hallyn, Stephen Smalley

Making MAKE_CHAR not covering MAKE_WHITEOUT is not addressed (see
previous discussion).  MAKE_CHAR should not restrict whiteout creation
*if* MAKE_WHITEOUT is handled.  Specific tests should check that all
these cases are proprely handled.

There is no documentation update related to the new feature.  A note
should also explain what exactly is a whiteout and why it is not
considered a character device (see previous discussions).

The sandboxer is not updated.

There is no audit tests.


On Wed, Jun 10, 2026 at 11:23:16AM +0200, Günther Noack wrote:
> 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.
> 
> Introduce the LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right, which is checked
> for the origin directory if RENAME_WHITEOUT is passed.
> 
> 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.
> 
> Bump the Landlock ABI version to 10.
> 
> Suggested-by: Christian Brauner <brauner@kernel.org>
> Suggested-by: Mickaël Salaün <mic@digikod.net>
> Signed-off-by: Günther Noack <gnoack@google.com>
> ---
>  include/uapi/linux/landlock.h                |  3 +++
>  security/landlock/audit.c                    |  1 +
>  security/landlock/fs.c                       | 17 ++++++++++++++---
>  security/landlock/limits.h                   |  2 +-
>  security/landlock/syscalls.c                 |  2 +-
>  tools/testing/selftests/landlock/base_test.c |  4 ++--
>  tools/testing/selftests/landlock/fs_test.c   |  5 +++--
>  7 files changed, 25 insertions(+), 9 deletions(-)
> 
> diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
> index 10a346e55e95..1f8a1d6d25f1 100644
> --- a/include/uapi/linux/landlock.h
> +++ b/include/uapi/linux/landlock.h
> @@ -328,6 +328,8 @@ 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::
>   *
> @@ -356,6 +358,7 @@ 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 */
>  
>  /**
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index 8d0edf94037d..09c97083f599 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -38,6 +38,7 @@ 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);
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index c1ecfe239032..67810d5242b2 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1080,6 +1080,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
> @@ -1127,7 +1128,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);
> @@ -1159,6 +1161,14 @@ static int current_check_refer_path(struct dentry *const 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_WHITEOUT;
> +
>  	/* The mount points are the same for old and new paths, cf. EXDEV. */
>  	if (old_dentry->d_parent == new_dir->dentry) {
>  		/*
> @@ -1509,7 +1519,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,
> @@ -1520,7 +1530,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,
> diff --git a/security/landlock/limits.h b/security/landlock/limits.h
> index b454ad73b15e..e59378e8e897 100644
> --- a/security/landlock/limits.h
> +++ b/security/landlock/limits.h
> @@ -19,7 +19,7 @@
>  #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)
>  
> diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
> index accfd2e5a0cd..d45469d5d464 100644
> --- a/security/landlock/syscalls.c
> +++ b/security/landlock/syscalls.c
> @@ -166,7 +166,7 @@ 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
> diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
> index 30d37234086c..6c8113c2ded1 100644
> --- a/tools/testing/selftests/landlock/base_test.c
> +++ b/tools/testing/selftests/landlock/base_test.c
> @@ -76,8 +76,8 @@ 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));
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index cdb47fc1fc0a..53d1b659849f 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -579,7 +579,7 @@ 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 | \
> @@ -593,7 +593,8 @@ 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 */
>  
> -- 
> 2.54.0.1099.g489fc7bff1-goog
> 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
  2026-06-10 13:38   ` Mickaël Salaün
@ 2026-06-12  8:34     ` Günther Noack
  0 siblings, 0 replies; 6+ messages in thread
From: Günther Noack @ 2026-06-12  8:34 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Christian Brauner, linux-security-module, Paul Moore,
	Amir Goldstein, Miklos Szeredi, Serge Hallyn, Stephen Smalley

Thanks for the review!

On Wed, Jun 10, 2026 at 03:38:56PM +0200, Mickaël Salaün wrote:
> Making MAKE_CHAR not covering MAKE_WHITEOUT is not addressed (see
> previous discussion).  MAKE_CHAR should not restrict whiteout creation
> *if* MAKE_WHITEOUT is handled.

(This is option (3) from your reply to V1 [1].)
        
I am skeptical of this approach, because it complicates how userspace
needs to deal with this access right.  Consider the following
scenario: A program wants to install the policy:

 * DENY  MAKE_WHITEOUT, MAKE_CHAR
 * ALLOW MAKE_WHITEOUT             in /foo  (path_beneath rule)

Then, if the kernel ABI predates make-whiteout, with the usual
best-effort fallback (clearing out the unsupported bits), this ruleset
becomes:

 * DENY  MAKE_CHAR
 * (no ALLOW rule)

But this ruleset is incorrect, because it denies mknod("/foo/x",
S_IFCHR | mode, makedev(0, 0)) in /foo, which was explicitly allowed
in the earlier ruleset.

So in order to implement the best-effort fallback, I guess userspace
libraries would now have to take into account whether there are any
rules where MAKE_WHITEOUT is specifically allowed, and if so, they
can't restrict MAKE_CHAR either?  I find this a bit complicated and I
think it's foreseeable that library implementers will predominantly
get this wrong.


Let me circle back to the other options you mentioned in [1], quoting
them here for reference:
> I see four options:
> 
> 1. Consider whiteouts as regular files and make them handled by
>    LANDLOCK_ACCESS_FS_MAKE_REG.  This would require an erratum and would
>    make sense for direct mknod calls, but it would be weird for
>    renameat2 calls than move a file and should only require
>    LANDLOCK_ACCESS_FS_REMOVE_FILE from the user point of view.

It would be weird for renameat2 calls to require MAKE_REG in the
source directory, but the weirdness would only affect
fuse-overlayfs-style programs and could be documented explicitly for
them for the case that they start using Landlock.

Normal programs that just call rename() on an existing FUSE-Overlayfs
filesystem would *not* require the MAKE_REG right, because the FUSE
process would do that on their behalf with the FUSE processes'
credentials.

> 
> 2. Add a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right to handle whitout
>    creation (direct and indirect?) and keep LANDLOCK_ACCESS_FS_MAKE_CHAR
>    handle direct whiteout creation (and don't backport anything).  It
>    looks inconsistent from an access control point of view.

MAKE_WHITEOUT to handle rename(RENAME_WHITEOUT) and MAKE_CHAR to
handle mknod(chardev (0, 0)) -- This is a bit inconsistent, but it
does not make a difference for any programs other than the ones
calling rename(RENAME_WHITEOUT) (i.e., overlayfs-fuse), and it could
be documented for that one use case.

I find this a pragmatic balance, and it does not require special logic
for the best-effort fallback either.  Could you be persuaded to go
this route instead?

> 3. Add a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right and, when handled,
>    make LANDLOCK_ACCESS_FS_MAKE_CHAR not handle whiteout.  This would be
>    a bit weird from a kernel point of view but it should work well for
>    users while still forbidding direct whiteout creation.

Except for the best-effort fallback, which is IMHO prone to
implementation bugs. (see above)

On the side, the implementation of this is also non-trivial: In order
to check for mknod(..., makedev(0, 0)), we need to check
layer-by-layer whether the layer handles MAKE_WHITEOUT and then either
check for MAKE_CHAR or MAKE_WHITEOUT.


> 4. Add a new LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right and make
>    LANDLOCK_ACCESS_FS_MAKE_CHAR never handle whiteout (and backport
>    MAKE_CHAR fix with an errata).  This would be consistent but backport
>    a way to directly create whiteouts (e.g. with mknod).

It's mostly theoretical, but lifting the mknod(chardev (0,0))
restriction for normal mknod() calls and calling it an erratum seems
surprising as well, because it would relax security guarantees for
existing programs.

I also pondered the alternative of creating an erratum but
intentionally *not* backporting it, but even in that case, that
surprising erratum still affects older programs which are deployed on
newer kernels.


Revisiting this discussion, I'd lean towards option 1 or 2 -- could
you be persuaded towards one of these?

I have a slight preference for option 1 (using MAKE_REG) because it
would be a narrow fix that could be backported to older kernels as
well and would not require a new access right.  Given that the use
case for RENAME_WHITEOUT is really only for FUSE-OverlayFS and given
that FUSE-OverlayFS anyway needs MAKE_REG permissions there, I have
trouble imagining a scenario where a separate access right for
MAKE_WHITEOUT is needed in a policy.  It seems like a pragmatic
choice.


> Specific tests should check that all
> these cases are proprely handled.
>
> There is no documentation update related to the new feature.  A note
> should also explain what exactly is a whiteout and why it is not
> considered a character device (see previous discussions).
> 
> The sandboxer is not updated.
> 
> There is no audit tests.

Acknowledged, these were missing.

(I was initially hoping that this bug report wouldn't expand into a
full-fledged feature with its own access right constant, but it is
correct that this is all required in that case... :-/)

Will add this for the next patch set revision if it is still needed.

—Günther

[1] https://lore.kernel.org/all/20260414.Lae5ida1eeGh@digikod.net/

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2026-06-12  8:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-10  9:23 [PATCH v3 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT Günther Noack
2026-06-10  9:23 ` [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
2026-06-10 13:38   ` Mickaël Salaün
2026-06-12  8:34     ` Günther Noack
2026-06-10  9:23 ` [PATCH v3 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial Günther Noack
2026-06-10  9:23 ` [PATCH v3 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT Günther Noack

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.