Linux Security Modules development
 help / color / mirror / Atom feed
From: "Mickaël Salaün" <mic@digikod.net>
To: "Günther Noack" <gnoack@google.com>
Cc: Christian Brauner <brauner@kernel.org>,
	 linux-security-module@vger.kernel.org,
	Paul Moore <paul@paul-moore.com>,
	 Amir Goldstein <amir73il@gmail.com>,
	Miklos Szeredi <miklos@szeredi.hu>,
	 Serge Hallyn <serge@hallyn.com>,
	Stephen Smalley <stephen.smalley.work@gmail.com>
Subject: Re: [PATCH v2 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
Date: Tue, 9 Jun 2026 18:09:51 +0200	[thread overview]
Message-ID: <20260609.pait5oaTheHi@digikod.net> (raw)
In-Reply-To: <20260513160552.4022649-2-gnoack@google.com>

On Wed, May 13, 2026 at 06:05:50PM +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                       | 15 +++++++++++++++
>  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, 26 insertions(+), 6 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..09de6ba5c3a3 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -1519,6 +1519,21 @@ 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);

We should not need a second path walk, even if whiteouts are rare.
Please propose another way.

> +		if (err)
> +			return err;
> +	}
> +
>  	return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
>  					!!(flags & RENAME_EXCHANGE));
>  }
> 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.563.g4f69b47b94-goog
> 
> 

  reply	other threads:[~2026-06-09 16:09 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-13 16:05 [PATCH v2 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT Günther Noack
2026-05-13 16:05 ` [PATCH v2 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
2026-06-09 16:09   ` Mickaël Salaün [this message]
2026-05-13 16:05 ` [PATCH v2 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial Günther Noack
2026-05-13 16:05 ` [PATCH v2 3/3] selftests/landlock: Test OverlayFS renames w/o LANDLOCK_ACCESS_FS_MAKE_WHITEOUT Günther Noack

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260609.pait5oaTheHi@digikod.net \
    --to=mic@digikod.net \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=gnoack@google.com \
    --cc=linux-security-module@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=stephen.smalley.work@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox