Re: [PATCH v1] selftests/landlock: Add tests for O_TMPFILE

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

On Wed, Jul 22, 2026 at 05:13:52PM +0200, Mickaël Salaün wrote:
> open(2) with O_TMPFILE creates its unnamed inode through vfs_tmpfile(),
> which, unlike normal file creation, calls neither security_path_mknod()
> nor security_inode_create().  It is nonetheless mediated: vfs_tmpfile()
> opens the inode through the filesystem's ->tmpfile() operation, which
> reaches security_file_open() via finish_open().  The open is therefore
> checked like any other, and materializing the file with linkat(2) is
> checked like any other link.
> 
> Add tests for both paths so O_TMPFILE cannot bypass Landlock.
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  tools/testing/selftests/landlock/fs_test.c | 251 +++++++++++++++++++++
>  1 file changed, 251 insertions(+)
> 
> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index cdb47fc1fc0a..b826b2c6a8f0 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -450,6 +450,25 @@ static int test_open(const char *const path, const int flags)
>  	return test_open_rel(AT_FDCWD, path, flags);
>  }
>  
> +/*
> + * Opens an anonymous O_TMPFILE inode in the directory dir.  O_TMPFILE is always
> + * combined with O_WRONLY or O_RDWR, so the caller must pass one of them in
> + * flags.
> + */
> +static int test_tmpfile(const char *const dir, const int flags)
> +{
> +	int fd;
> +
> +	fd = open(dir, O_TMPFILE | flags | O_CLOEXEC, 0700);
                                                      ^^^^
The file mode doesn't seem to be used in the test.  According to the man page,
it's only relevant when you later do a linkat(), but even then this
functionality seems unrelated to Landlock and is not tested here, I think.

> +	if (fd < 0)
> +		return errno;
> +
> +	if (close(fd) != 0)
> +		return errno;
> +
> +	return 0;
> +}
> +

The test_open() function does the exact same thing, except that it doesn't
enforce O_TMPFILE.  But because the test_open() function mirrors the syscall
parameters, you can pass O_TMPFILE as a flag from the outside, by calling it
with:

    EXPECT_EQ(0, test_open(dir_s1d2, O_TMPFILE|O_WRONLY));

I find that preferrable over using a special test_tmpfile() helper, because it
is not much longer than what you have now, and it is very implicit here what
parameters we are passing to open(2) -- and the interface of open(2) is already
well-known.  For a test_tmpfile() helper, you'd always have to look up what that
function does.


>  TEST_F_FORK(layout1, no_restriction)
>  {
>  	ASSERT_EQ(0, test_open(dir_s1d1, O_RDONLY));
> @@ -2140,6 +2159,238 @@ TEST_F_FORK(layout1, link)
>  	ASSERT_EQ(0, link(file1_s1d3, file2_s1d3));
>  }
>  
> +/*
> + * O_TMPFILE does not go through the path_mknod hook: vfs_tmpfile() creates the
> + * inode without calling security_path_mknod().  These tests verify that the
> + * resulting file is still mediated, via the file_open hook, so O_TMPFILE cannot
> + * be used to bypass Landlock.
> + */
> +
> +/*
> + * An O_TMPFILE open requires WRITE_FILE (and READ_FILE for O_RDWR) on the
> + * directory hierarchy, exactly like any other writable open.  It does not
> + * require (nor is it granted by) MAKE_REG: the anonymous inode is not yet a
> + * named file.  O_TMPFILE always implies write access, so a read-only request is
> + * rejected by the VFS with EINVAL before Landlock is consulted; Landlock must
> + * not change that into EACCES.
> + */
> +TEST_F_FORK(layout1, open_tmpfile)
> +{
> +	const struct rule rules[] = {
> +		/* Write allowed, but neither MAKE_REG nor READ_FILE. */
> +		{
> +			.path = dir_s1d1,
> +			.access = LANDLOCK_ACCESS_FS_WRITE_FILE,
> +		},
> +		/* Both read and write allowed. */
> +		{
> +			.path = dir_s1d2,
> +			.access = LANDLOCK_ACCESS_FS_READ_FILE |
> +				  LANDLOCK_ACCESS_FS_WRITE_FILE,
> +		},
> +		/* File-creation right without write. */
> +		{
> +			.path = dir_s2d1,
> +			.access = LANDLOCK_ACCESS_FS_MAKE_REG,
> +		},
> +		{},
> +	};
> +
> +	/* Baseline: an unsandboxed O_TMPFILE open works. */
> +	EXPECT_EQ(0, test_tmpfile(dir_s1d1, O_WRONLY));
> +	EXPECT_EQ(0, test_tmpfile(dir_s2d1, O_RDWR));
> +	EXPECT_EQ(0, test_tmpfile(dir_s3d1, O_RDWR));

Nit: Should they all threee use O_RDWR?


> +
> +	/* O_TMPFILE requires write access: read-only is EINVAL at the VFS. */
> +	EXPECT_EQ(EINVAL, test_tmpfile(dir_s1d1, O_RDONLY));
> +
> +	enforce_fs(_metadata,
> +		   LANDLOCK_ACCESS_FS_READ_FILE |
> +			   LANDLOCK_ACCESS_FS_WRITE_FILE |
> +			   LANDLOCK_ACCESS_FS_MAKE_REG,
> +		   rules);
> +
> +	/* Write is enough for an O_WRONLY tmpfile; MAKE_REG is not needed. */
> +	EXPECT_EQ(0, test_tmpfile(dir_s1d1, O_WRONLY));
> +	/* O_RDWR additionally needs READ_FILE, which is absent here. */
> +	EXPECT_EQ(EACCES, test_tmpfile(dir_s1d1, O_RDWR));
> +
> +	/* Read and write allowed: both open modes succeed. */
> +	EXPECT_EQ(0, test_tmpfile(dir_s1d2, O_WRONLY));
> +	EXPECT_EQ(0, test_tmpfile(dir_s1d2, O_RDWR));
> +
> +	/* MAKE_REG without WRITE_FILE does not allow the open. */
> +	EXPECT_EQ(EACCES, test_tmpfile(dir_s2d1, O_WRONLY));
> +	EXPECT_EQ(EACCES, test_tmpfile(dir_s2d1, O_RDWR));
> +
> +	/* No rule at all: the open is denied. */
> +	EXPECT_EQ(EACCES, test_tmpfile(dir_s3d1, O_WRONLY));
> +	EXPECT_EQ(EACCES, test_tmpfile(dir_s3d1, O_RDWR));
> +
> +	/*
> +	 * A read-only O_TMPFILE stays EINVAL under Landlock, whether the
> +	 * directory is fully allowed or has no rule: the VFS rejects the flag
> +	 * combination before the file_open hook, so Landlock never turns it
> +	 * into EACCES.
> +	 */
> +	EXPECT_EQ(EINVAL, test_tmpfile(dir_s1d2, O_RDONLY));
> +	EXPECT_EQ(EINVAL, test_tmpfile(dir_s3d1, O_RDONLY));
> +}
> +
> +/*
> + * When the ruleset handles neither file read nor write access, Landlock has no
> + * opinion on an O_TMPFILE open and must not interfere with it.

If the intent is to demonstrate that only READ_FILE and WRITE_FILE interfere
with O_TMPFILE, I would suggest that (a) the ruleset should "handle" everything
but these two, and (b) we do not need to pass any rules as exceptions to that?
        
enforce_fs(_metadata, ACCESS_ALL & ~(READ_FILE | WRITE_FILE), NULL);


> + */
> +TEST_F_FORK(layout1, open_tmpfile_unhandled)
> +{
> +	const struct rule rules[] = {
> +		{
> +			.path = dir_s1d2,
> +			.access = LANDLOCK_ACCESS_FS_READ_DIR,
> +		},
> +		{},
> +	};
> +
> +	enforce_fs(_metadata, LANDLOCK_ACCESS_FS_READ_DIR, rules);
> +
> +	EXPECT_EQ(0, test_tmpfile(dir_s1d1, O_WRONLY));
> +	EXPECT_EQ(0, test_tmpfile(dir_s1d3, O_RDWR));
> +	EXPECT_EQ(0, test_tmpfile(dir_s3d1, O_RDWR));
> +}
> +
> +/*
> + * Materializing an anonymous O_TMPFILE into its creation directory with
> + * linkat(AT_EMPTY_PATH) is gated by MAKE_REG on that directory, even though
> + * obtaining the writable tmpfile only required WRITE_FILE.  This is the check
> + * that stops O_TMPFILE from creating a named file where the sandbox forbids
> + * file creation.  Linking into the same directory does not involve reparenting,
> + * so REFER is not required.
> + */
> +TEST_F_FORK(layout1, link_tmpfile)
> +{
> +	int fd;
> +	const struct rule rules[] = {
> +		/* Write only: the tmpfile opens but cannot be linked. */
> +		{
> +			.path = dir_s1d1,
> +			.access = LANDLOCK_ACCESS_FS_WRITE_FILE,
> +		},
> +		/* Write and MAKE_REG: the tmpfile opens and can be linked. */
> +		{
> +			.path = dir_s2d1,
> +			.access = LANDLOCK_ACCESS_FS_WRITE_FILE |
> +				  LANDLOCK_ACCESS_FS_MAKE_REG,
> +		},
> +		{},
> +	};
> +
> +	/* Frees names in the two directories for the new links. */
> +	ASSERT_EQ(0, unlink(file1_s1d1));
> +	ASSERT_EQ(0, unlink(file1_s2d1));
> +
> +	enforce_fs(_metadata,
> +		   LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_MAKE_REG,
> +		   rules);
> +
> +	/*
> +	 * WRITE_FILE is enough to obtain the anonymous tmpfile.  linkat(2) with
> +	 * AT_EMPTY_PATH needs no capability because the fd's open-time
> +	 * credentials match the caller's.  Linking into the same directory does
> +	 * not require REFER (no reparenting), only MAKE_REG, which is absent
> +	 * here.
> +	 */
> +	fd = open(dir_s1d1, O_TMPFILE | O_WRONLY | O_CLOEXEC, 0700);
> +	ASSERT_LE(0, fd);
> +	ASSERT_EQ(-1, linkat(fd, "", AT_FDCWD, file1_s1d1, AT_EMPTY_PATH));
> +	EXPECT_EQ(EACCES, errno);
> +	EXPECT_EQ(0, close(fd));
> +
> +	/* With MAKE_REG on the directory, the same link is allowed. */
> +	fd = open(dir_s2d1, O_TMPFILE | O_WRONLY | O_CLOEXEC, 0700);
> +	ASSERT_LE(0, fd);
> +	EXPECT_EQ(0, linkat(fd, "", AT_FDCWD, file1_s2d1, AT_EMPTY_PATH));
> +	EXPECT_EQ(0, close(fd));
> +}
> +
> +/*
> + * Linking a tmpfile into a different directory is a reparenting operation: like
> + * any cross-directory link it requires LANDLOCK_ACCESS_FS_REFER.  Without it,
> + * materializing the tmpfile outside its creation directory is denied with
> + * EXDEV, so a tmpfile cannot escape its origin hierarchy.

Because a O_TMPFILE is always a regular file, I think for most Landlock access
rights, it does not make a difference.  If I understand this correctly, the only
two access rights where it does make a difference would be EXECUTE and TRUNCATE,
because they are the ones which apply to regular files where the refer rule
matters.  Is that correct?  Maybe they would be worth mentioning as a
motivational example here, to show that this makes an actual difference for
O_TMPFILE?

> + */
> +TEST_F_FORK(layout1, link_tmpfile_reparent_without_refer)
> +{
> +	int fd;
> +	const struct rule rules[] = {
> +		/* Source directory: only the tmpfile open is allowed. */
> +		{
> +			.path = dir_s1d1,
> +			.access = LANDLOCK_ACCESS_FS_WRITE_FILE,
> +		},
> +		/* Destination directory: file creation is allowed. */
> +		{
> +			.path = dir_s2d1,
> +			.access = LANDLOCK_ACCESS_FS_MAKE_REG,
> +		},
> +		{},
> +	};
> +
> +	/* Frees a name in the destination directory for the new link. */
> +	ASSERT_EQ(0, unlink(file1_s2d1));
> +
> +	enforce_fs(_metadata,
> +		   LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_MAKE_REG,
> +		   rules);
> +
> +	fd = open(dir_s1d1, O_TMPFILE | O_WRONLY | O_CLOEXEC, 0700);
> +	ASSERT_LE(0, fd);
> +	/* Cross-directory link without REFER is denied with EXDEV. */
> +	ASSERT_EQ(-1, linkat(fd, "", AT_FDCWD, file1_s2d1, AT_EMPTY_PATH));
> +	EXPECT_EQ(EXDEV, errno);
> +	EXPECT_EQ(0, close(fd));
> +}
> +
> +/*
> + * With LANDLOCK_ACCESS_FS_REFER on both directories, a tmpfile created in one
> + * directory can be linked into another.  The destination needs only MAKE_REG
> + * (plus REFER), not WRITE_FILE: the reparenting check compares file access
> + * rights, and the tmpfile gains none by moving to a directory that grants only
> + * the directory-level creation right.

In other words, linking the tmpfile into a directory that grants fewer rights on
it is fine.  The only required rights in the destination directory are REFER and
MAKE_REG, because those are required for the linking itself.  This checks out,
but it's already a basic property that REFER has in the normal case as well.  We
can test it for completeness, but it doesn't seem particularly special to the
tmpfile case.  (Would be OK to drop imho, but I'm not feeling strongly about
it.)

> + */
> +TEST_F_FORK(layout1, link_tmpfile_reparent_with_refer)
> +{
> +	int fd;
> +	const struct rule rules[] = {
> +		/* Source: tmpfile open (write) and reparenting. */
> +		{
> +			.path = dir_s1d1,
> +			.access = LANDLOCK_ACCESS_FS_WRITE_FILE |
> +				  LANDLOCK_ACCESS_FS_REFER,
> +		},
> +		/* Destination: file creation and reparenting, but no write. */
> +		{
> +			.path = dir_s2d1,
> +			.access = LANDLOCK_ACCESS_FS_MAKE_REG |
> +				  LANDLOCK_ACCESS_FS_REFER,
> +		},
> +		{},
> +	};
> +
> +	/* Frees a name in the destination directory for the new link. */
> +	ASSERT_EQ(0, unlink(file1_s2d1));
> +
> +	enforce_fs(_metadata,
> +		   LANDLOCK_ACCESS_FS_WRITE_FILE | LANDLOCK_ACCESS_FS_MAKE_REG |
> +			   LANDLOCK_ACCESS_FS_REFER,
> +		   rules);
> +
> +	fd = open(dir_s1d1, O_TMPFILE | O_WRONLY | O_CLOEXEC, 0700);
> +	ASSERT_LE(0, fd);
> +	/* REFER on both sides plus MAKE_REG on the destination allows it. */
> +	EXPECT_EQ(0, linkat(fd, "", AT_FDCWD, file1_s2d1, AT_EMPTY_PATH));
> +	EXPECT_EQ(0, close(fd));
> +}
> +
>  static int test_rename(const char *const oldpath, const char *const newpath)
>  {
>  	if (rename(oldpath, newpath))
> -- 
> 2.54.0
> 

P.S.: I checked the Sashiko review as well and it did not spot any issues in these tests.

https://sashiko.dev/#/patchset/20260722151354.2736457-1-mic%40digikod.net

(It believes that it spotted a bug in the kernel code, but that is better
investigated separately of this patch.)

—Günther