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

On Fri, Jul 31, 2026 at 03:43:24PM +0200, Mickaël Salaün wrote:
> On Fri, Jul 24, 2026 at 06:10:03PM +0200, Günther Noack wrote:
> > 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
> 
> This "--" doesn't seem useful.

Reworded this and removed the "--"

(The paragraph was previously slightly misleading - vfs_rename() is
used by OverlayFS, but does not actually call into the Landlock rename
hook, so the credentials at the time of OverlayFS mount do not matter
for the in-kernel OverlayFS.  So the old paragraph was a bit
misleading, but the larger point still applies - the operations done
by OverlayFS on the underlying upper and lower file systems are not
subject to the Landlock policy of the userspace process that triggered
the operation through an operation on the unified OverlayFS file
system.)

> 
> > rename happens with the credentials of the user who mounted the OverlayFS.
> > 
> > Signed-off-by: Günther Noack <[email protected]>
> > ---
> >  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));
> 
> merge_fl1 just became a whiteout with this unlink, so I think the test
> is wrong because it doesn't check RENAME_WHITEOUT against the lower file.

OK, changed to this approach:

* Creating a FIFO at lower/data/pl1 as part of the fixture
  (For well-defined behaviour, this needs to happen before mounting
  the OverlayFS, according to Documentation/filesystems/overlayfs.rst,
  section "Changes to underlying filesystems")

Now, as usual:

* Do a rename from merge_pl1 to merge_pl1_renamed.  Because the
  original FIFO came from the lower layer, and modifications through
  OverlayFS take effect on the upper layer, this creates the whiteout
  in the upper layer to hide the FIFO in the lower layer.

I also started using is_whiteout() and is_missing() helpers to avoid
the repeated stat() dance - it reads a bit closer to the test's intent
that way.

> > +	ASSERT_EQ(0, mknod(merge_fl1, S_IFIFO, 0));
> 
> The mode should be 0600.

Done.


> > +	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_MAKE_REG, NULL);
> > +
> > +	/*
> > +	 * Execute a regular file rename within OverlayFS.
> 
> merge_fl1 is a fifo.

Done.


> > +	 * 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
> > 
> >