All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Günther Noack" <gnoack@google.com>
To: "Mickaël Salaün" <mic@digikod.net>
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 v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT
Date: Fri, 24 Jul 2026 16:52:13 +0200	[thread overview]
Message-ID: <amN8Hb2pxhXr8drB@google.com> (raw)
In-Reply-To: <20260720.chow9ohYie5b@digikod.net>

On Mon, Jul 20, 2026 at 12:58:00PM +0200, Mickaël Salaün wrote:
> On Fri, Jun 12, 2026 at 10:34:43AM +0200, Günther Noack wrote:
> > > 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.
> > 
> > Except for the best-effort fallback, which is IMHO prone to
> > implementation bugs. (see above)
> > 
> > On the side, the implementation of this is also non-trivial: In order
> > to check for mknod(..., makedev(0, 0)), we need to check
> > layer-by-layer whether the layer handles MAKE_WHITEOUT and then either
> > check for MAKE_CHAR or MAKE_WHITEOUT.
> 
> Canonicalizing MAKE_WHITEOUT at domain creation time (similarly to how
> unhandled access rights are actually handled) should make this much
> easier, and the following proposal would enable that:
> 
> First, a backportable patch would make MAKE_CHAR also control/deny
> RENAME_WHITEOUT to be consistent with direct whiteout creation (i.e.
> MAKE_CHAR control any S_IFCHR file, including whiteout ones).  This
> would be a real "Fixes" patch (without erratum).  We should keep in mind
> that whiteout creation doesn't require CAP_MKNOD, so in most cases,
> sandboxed processes shouldn't be allowed to create real character
> devices in the first place.  Also, current fuse-overlayfs processes need
> both ways to create whiteouts, so the current state is inconsistent to
> them too.
> 
> Then, to be able to differentiate real character devices from whiteouts,
> a new patch would add a LANDLOCK_ACCESS_FS_MAKE_WHITEOUT right,
> controlling only whiteouts.  As with this third option, when this right
> is handled by a ruleset/layer, MAKE_CHAR doesn't control whiteout
> creation.
> 
> The best-effort compatibility is handled by userspace libraries.  The
> tricky part is when a policy handles MAKE_CHAR | MAKE_WHITEOUT, and has
> a rule with MAKE_WHITEOUT but not MAKE_CHAR: when running such policy in
> an old kernel (not supporting MAKE_WHITEOUT), the library should replace
> the MAKE_WHITEOUT in the rules with a MAKE_CHAR.  For the other cases,
> simple masking should work as expected.  This pattern could apply to
> potentially future splits of existing access right.
> 
> I find this approach good to add a new dedicated access right, but I now
> think it's overkill for this whiteout case.  Anyway, this pattern could
> be used if we want, one day, to split an access right.

Agreed, this is an approach which would have simplified the implementation
complexity for option 3. - Mapping old invocation variants onto the
more modern but compatible internal representation at an early stage
during ruleset enablement would be an elegant way to keep the
backwards compatibility concerns out of our core logic.  (Maybe we can move
the "refer" implementation more in that direction as well?)


> > Revisiting this discussion, I'd lean towards option 1 or 2 -- could
> > you be persuaded towards one of these?
> > 
> > I have a slight preference for option 1 (using MAKE_REG) because it
> > would be a narrow fix that could be backported to older kernels as
> > well and would not require a new access right.  Given that the use
> > case for RENAME_WHITEOUT is really only for FUSE-OverlayFS and given
> > that FUSE-OverlayFS anyway needs MAKE_REG permissions there, I have
> > trouble imagining a scenario where a separate access right for
> > MAKE_WHITEOUT is needed in a policy.  It seems like a pragmatic
> > choice.
> 
> I was reluctant at first but I now think it's the best approach.
> whiteouts are technically S_IFCHR but they are handled by the kernel/VFS
> as regular files wrt access checks (unprivileged: don't need CAP_MKNOD),
> so it would be inconsistent for Landlock to diverge from that.
> Whiteouts are safe (no device can bind to them), and the goal of
> LANDLOCK_ACCESS_FS_MAKE_CHAR is really to restrict the creation of new
> interfaces to the kernel (through an arbitrary device driver).  This
> case and rationale should be explained in the doc (also see other VFS
> replies wrt what are whiteouts).
> 
> So, LANDLOCK_ACCESS_FS_MAKE_REG should be the *only* access right to
> restrict whiteout creation, either directly with mknod or indirectly
> with RENAME_WHITEOUT.  fuse-overlayfs already needs MAKE_REG, so this
> should not change much its current status, but fuse-overlayfs could now
> drop MAKE_CHAR.
> 
> Let's go with option 1!

Awesome, sounds good.  I'll go and implement option 1, where MAKE_REG guards
whiteout creation directly from userspace, for both the mknod() and
RENAME_WHITEOUT case, and also introducing an erratum for it.  This will be
simpler than introducing a new access right and also benefit users on
older kernels when it gets backported.

—Günther

  reply	other threads:[~2026-07-24 14:52 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10  9:23 [PATCH v3 0/3] landlock: Restrict renameat2 with RENAME_WHITEOUT Günther Noack
2026-06-10  9:23 ` [PATCH v3 1/3] landlock: Require LANDLOCK_ACCESS_FS_MAKE_WHITEOUT for RENAME_WHITEOUT Günther Noack
2026-06-10 13:38   ` Mickaël Salaün
2026-06-12  8:34     ` Günther Noack
2026-07-17  9:23       ` Günther Noack
2026-07-20 10:58       ` Mickaël Salaün
2026-07-24 14:52         ` Günther Noack [this message]
2026-06-10  9:23 ` [PATCH v3 2/3] selftests/landlock: Add test for RENAME_WHITEOUT denial Günther Noack
2026-06-10  9:23 ` [PATCH v3 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=amN8Hb2pxhXr8drB@google.com \
    --to=gnoack@google.com \
    --cc=amir73il@gmail.com \
    --cc=brauner@kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mic@digikod.net \
    --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 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.