All of lore.kernel.org
 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>
Subject: Re: [PATCH 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT
Date: Tue, 14 Apr 2026 15:40:59 +0200	[thread overview]
Message-ID: <20260414.Lae5ida1eeGh@digikod.net> (raw)
In-Reply-To: <20260411090944.3131168-2-gnoack@google.com>

Thanks for bringing this up.

By default, creating whiteout files is not privileged e.g.,
mknod whiteout c 0 0 is allowed for unprivileged users.  Landlock should
follow this semantic.  So there are two issues:
1. LANDLOCK_ACCESS_FS_MAKE_CHAR should not apply for whiteouts.
2. Whiteouts creation should be controllable by Landlock (e.g. through
mknod and renameat2).

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.

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.

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.

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

I think option 3 is the more pragmatic.  Landlock should properly log
the right blocker wrt handled access rights though.


On Sat, Apr 11, 2026 at 11:09:42AM +0200, Günther Noack wrote:
> 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].
> 
> 
> Motivation
> ==========
> 
> The RENAME_WHITEOUT flag side-steps Landlock's LANDLOCK_ACCESS_FS_MAKE_CHAR
> right, which is designed to restrict the creation of chardev device files.
> 
> This patch set fixes that by adding a check in Landlock's path_rename hook.
> 
> 
> Tradeoffs considered in the implementation
> ==========================================
> 
> Q: Should we guard it with a dedicated LANDLOCK_ACCESS_FS_MAKE_WHITEOUT
>    right?
> 
>    Pros:
>    * This would be the fully backwards compatible solution,
>      and Linux always strives for full backward compatibility.
> 
>    Cons:
>    * Complicates the Landlock API surface for a very minor use case.
> 
>      In Debian Code search, the only use of RENAME_WHITEOUT from userspace
>      seems to be for fuse-overlayfs.  It is used there for the same purpose
>      as in the kernel OverlayFS and it likely does not run in a Landlock
>      domain.
> 
>    The tradeoff does not seem worth it to me.  The chances that we break
>    anyone with this seem very low, and I'm inclined to treat it as a bugfix
>    for the existing LANDLOCK_ACCESS_FS_MAKE_CHAR right.
> 
> 
> Q: Should we add a Landlock erratum for this?
> 
>    I punted on it for now, but we can do it if needed.
> 
> Q: Should the access right check be merged into the longer
>    current_check_refer_path() function?
> 
>    I am leaning towards keeping it as a special case earlier.  This means
>    that we traverse the source path twice, but as we have seen in Debian
>    Code Search, there are apparently no legitimate callers of renameat2()
>    with RENAME_WHITEOUT who are calling this from within a Landlock domain.
>    (fuse-overlayfs is legitimate, but is not landlocked)
> 
>    It doesn't seem worth complicating our common rename code for a corner
>    case that doesn't happen in practice.
> 
> 
> [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
> 
> 
> Günther Noack (3):
>   landlock: Require LANDLOCK_ACCESS_FS_MAKE_CHAR for RENAME_WHITEOUT
>   selftests/landlock: Add test for RENAME_WHITEOUT denial
>   selftests/landlock: Test OverlayFS renames w/o
>     LANDLOCK_ACCESS_FS_MAKE_CHAR
> 
>  security/landlock/fs.c                     | 16 ++++++++
>  tools/testing/selftests/landlock/fs_test.c | 45 ++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
> 
> -- 
> 2.54.0.rc0.605.g598a273b03-goog
> 
> 

      parent reply	other threads:[~2026-04-14 13:41 UTC|newest]

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

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=20260414.Lae5ida1eeGh@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 \
    /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 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.