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

On Fri, Jul 31, 2026 at 03:22:40PM +0200, Mickaël Salaün wrote:
> On Fri, Jul 24, 2026 at 06:10:02PM +0200, Günther Noack wrote:
> > 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 <[email protected]>
> > ---
> >  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);
> > +}
> 
> rename_whiteout_denied could not fail.  It moves a regular file, whose
> own creation already requires MAKE_REG, and a same-directory rename
> merges both parent directories' requirements, so its EACCES came from
> the moved file and not from the whiteout: it passes unchanged with the
> whiteout checks removed from fs.c .  Moving a named pipe with MAKE_FIFO
> granted leaves MAKE_REG required for the whiteout alone.

Good catch -- renaming a FIFO makes this test more useful. Fixed (and double
checked by breaking the implementation, to see that it gets caught).


> Four cases are then unexercised, each covering something a bug could
> have broken silently:
> 
> - allowing RENAME_WHITEOUT where MAKE_REG is granted, since a check that
>   denied unconditionally would have passed the denial test;
> 
> - reparenting, since a same-directory rename merges both parents and so
>   cannot show that the whiteout is charged to the source directory;
> 
> - the audit record, since the denial now reports fs.make_reg where it
>   used to report fs.make_char, and nothing pinned which;
> 
> - RENAME_EXCHANGE of an existing whiteout, the only operation needing
>   that right in the source directory, and the case that shows the
>   reclassification covers moving a whiteout and not only creating one.

(Note to self, I still need to look into these)

 - needs a rename_whiteout_allowed or similar check
 - by testing reparenting, the test can check in which of the two directories
   MAKE_REG is allowed
 - updated audit test for whiteouts
 - rename_exchange case -- maybe double check whether this should be tested
   in test_make_file() in a generic way?

> > +
> >  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);
> 
> CAP_MKNOD was never needed for whiteout.

Done.


—Günther