Linux Security Modules development
 help / color / mirror / Atom feed
From: "Farid Zakaria" <farid.m.zakaria@gmail.com>
To: "Günther Noack" <gnoack3000@gmail.com>,
	"Farid Zakaria" <farid.m.zakaria@gmail.com>
Cc: "Mickaël Salaün" <mic@digikod.net>,
	"Günther Noack" <gnoack@google.com>,
	"Paul Moore" <paul@paul-moore.com>,
	"James Morris" <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"Shuah Khan" <shuah@kernel.org>,
	"Jonathan Corbet" <corbet@lwn.net>,
	"John Ericson" <mail@johnericson.me>,
	linux-security-module@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-doc@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH RFC 0/3] landlock: Add access rights on created files
Date: Sun, 25 Oct 2026 10:55:54 -0700	[thread overview]
Message-ID: <DME42HRSRKWQ.2FMOK9VWSRP32@gmail.com> (raw)
In-Reply-To: <20261025.769b43a39e24@gnoack.org>

On Sun Oct 25, 2026 at 4:07 AM PDT, Günther Noack wrote:
> Hello Farid!
>
> On Sat, Oct 24, 2026 at 01:39:16PM -0700, Farid Zakaria wrote:
>> This is a proposal for an idea to add support for landlock to work
>> better with build systems such as Nix where they want to apply
>> restrictions but the paths may not yet exist.
>> 
>> TL;DR;
>> ========
>> 
>> Landlock rules attach to inodes that exist when a ruleset is built.
>> That makes a directory shared between trusted and sandboxed tasks hard
>> to confine: the sandbox may create entries there with
>> LANDLOCK_ACCESS_FS_MAKE_*, but no rule can name those entries ahead of
>> time, and granting read access on the directory to cover them exposes
>> every other entry as well. This series adds a rule flag that grants
>> access rights on the entries a domain creates in a directory, instead
>> of on the directory.
>> 
>> Motivation
>> ==========
>> 
>> The motivating user is the Nix build sandbox. Every Nix build runs in
>> its own sandbox, and a build may only read its declared inputs in
>> /nix/store and must be able to write and read back the outputs it
>> creates there. Nix enforces this with a private mount namespace: it
>> creates an empty store directory and bind-mounts each input path into
>> it, one mount per input. Nix has paid this per-input cost since the
>> sandbox was introduced [1].
>> 
>> The cost is paid per build and grows with the size of the input
>> closure. Nix is moving towards much finer-grained builds (many small
>> derivations instead of a few large ones), so the number of sandboxes
>> grows while each build's own work shrinks, and the sandbox setup is
>> becoming the bottleneck. A typical build has hundreds of inputs; large
>> ones have thousands.
>> 
>> A microbenchmark sets up and tears down a Nix-like sandbox (user,
>> mount, PID, IPC, UTS and network namespaces, pivot_root into a chroot,
>> removing the chroot afterwards). It compares one bind mount per input
>> with a single bind mount of the whole store plus one Landlock rule per
>> input. Median latency per sandbox, one sandbox at a time, Linux 7.1,
>> ext4, 16 cores:
>> 
>>   inputs   bind mount per input   one bind mount + Landlock   speedup
>>       10         3.2 ms                    2.3 ms               1.4x
>>      100        11.6 ms                    2.8 ms               4.1x
>>     1000        79.3 ms                    6.4 ms              12.4x
>>     5000       400.1 ms                   24.4 ms              16.4x
>> 
>> With Landlock, an input costs an O_PATH open and a landlock_add_rule()
>> call: there is a single mount, and no mountpoint directory to create
>> before the build and remove after it (removing the 1000 empty
>> mountpoints alone took 17 ms).
>> 
>> The single bind mount is only sound if the store listing and the
>> undeclared store paths stay unreadable while the build can still use
>> its outputs. Without this series, covering the outputs requires a rule
>> on /nix/store itself, which makes every store path readable again.
>> 
>> [1] https://github.com/NixOS/nix/issues/179
>
> I have not done a thorough code review yet, and will focus on the
> general approach.
>
> I am generally in favor of this approach where we keep track of the
> directory inode plus the file name.
>
> Other projects that have run into the problem before are:
>
> (1) Justine Tunney's Landlocked Make experiment
>     (https://github.com/jart/landlock-make).  This one shares the same
>     "build system" problem, I believe the topic came up during code
>     review at the time.
>
>     So I think this is a general problem in build systems that use
>     Landlock.
>
> (2) Michael Stapelberg's Go rsync implementation.
>
>     https://github.com/gokrazy/rsync/commit/f85bc9b98cdda705ff8e76ebcd8fba03c6560467
>
> In the rsync case, the issue is not a rule for *creation*, but for
> *reading* files.  But it could still be resolved with the same
> mechanism: When allow-listing the /etc/resolv.conf file, the policy
> works for a while, but eventually a system-level network management
> daemon might exchange the file for a new one with an atomic rename,
> changing the inode number.  It would be desirable to express a policy
> in this case as well, which is tied to the /etc inode plus the
> "resolv.conf" filename, for opening files for reading.
>
> The same problem also exists for allow-listing of Pathname Unix Domain
> Sockets, where the common way to restart these is to unlink the socket
> file and recreate it during bind(2).
>
> In both cases, the best current workaround is to grant the access
> right on the parent directory, which is arguably often granting more
> than necessary.
>
> Do you think your patch would generalize for the non-creation use
> cases as well?
>
>

This is covered in a separate follow-up.

>> Series
>> ======
>> 
>> Patch 1 lets an LSM observe inode creation through the
>> inode_init_security hook when no LSM provides an xattr. Patch 2 adds
>> the LANDLOCK_ADD_RULE_CREATED flag for path-beneath rules and bumps the
>> ABI to 13. Patch 3 adds selftests.
>
> I only had a short look so far, but stumbled across the words "package
> store" / "store" etc.  In other packaging systems, I have not come
> across the term "package store" before.  In the kernel code, please
> let's not assume that readers have a familiarity with Nix, and let's
> please explain features in more general terms.  (Note to self, on
> review, grep for the word "store".)
>
> (FWIW, I suspect that this wording might have been put there by an AI.
> I've observed in the past that AIs sometimes have an unfortunate
> tendency to blur the lines between abstraction layers and
> cross-reference implementation internals across abstraction layers.
> Maybe something to watch out for?)
>
>

Feedback understood. I reviewed the wording and it sounded "fine" to me
but your comment makes sense; that this shouldn't mention that. I guess
I wanted to articulate the motivation or example for the feature,
but I can do that without mentioning Nix specifically.

>> Testing
>> =======
>> 
>> The series applies to the Landlock next branch (3e271a27c46f) and was
>> tested on Linux 7.3 with that branch applied.
>> 
>> Without patch 2, the five new tests and the ABI version check fail.
>> 
>> Open questions
>> ==============
>> 
>> - A created inode stays pinned until the layer's hierarchy is freed, so a
>>   sandbox that creates many entries in such a directory keeps them all in
>>   memory. Should this be bounded?
>
> Is that a preexisting problem that predates your patch?
>
>

This is a fall out of this design because the inodes grow after the
landlock rule has been applied based on what the sandbox does;

If we go forward with your other suggestion, this is moot anyways.

>> - Creation is detected in inode_init_security, so on filesystems that do
>>   not call security_inode_init_security() created entries get no rights.
>>   Is there a better hook?
>
> We are already hooking into hook_path_mknod(), hook_path_symlink()
> etc.  Is the feature not possible based on that hook as well?
>
> In fact, I would have assumed that you would only need to change the
> logic in the path walk so that it can pick up the special rule type?
> Does that not work?
>
>> - Through overlayfs, created entries get no rights either, because the
>>   upper inode is created with the mounter's credentials. Should overlayfs
>>   be handled, e.g. via security_dentry_create_files_as()?
>
> Landlock does generally provide the normal semantics when you use
> files through an OverlayFS mount.  We have exhaustive tests for it in
> fs_test.c as well.
>
>
>> - A file renamed or linked in keeps only the rights it had at its source.
>>   Should it get none instead?
>
> Please see the logic and discussions around the
> LANDLOCK_ACCESS_FS_REFER access right: When linking or renaming a file
> to a new location, we must make sure that a landlocked program can not
> move it to a place where it suddenly has more access rights on the
> file because it is now accessible under different parent directories.
>

This dos follow the refer rule fwiw.

Thank you for taking the time to respond :)

> Thanks,
> –Günther



      parent reply	other threads:[~2026-10-25 17:55 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-10-24 20:39 [PATCH RFC 0/3] landlock: Add access rights on created files Farid Zakaria
2026-10-24 20:39 ` [PATCH RFC 1/3] lsm: Call inode_init_security hooks without xattrs Farid Zakaria
2026-10-24 21:38   ` sashiko-bot
2026-10-24 20:39 ` [PATCH RFC 2/3] landlock: Add access rights on created files Farid Zakaria
2026-10-24 22:06   ` sashiko-bot
2026-10-24 20:39 ` [PATCH RFC 3/3] selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED Farid Zakaria
2026-10-24 21:41   ` sashiko-bot
2026-10-24 21:43 ` [PATCH RFC 0/3] landlock: Add access rights on created files Justin Suess
2026-10-24 22:03   ` Justin Suess
2026-10-24 23:59     ` Farid Zakaria
2026-10-25 11:07 ` Günther Noack
2026-10-25 14:57   ` Günther Noack
2026-10-25 17:52     ` Farid Zakaria
2026-10-25 17:55   ` Farid Zakaria [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=DME42HRSRKWQ.2FMOK9VWSRP32@gmail.com \
    --to=farid.m.zakaria@gmail.com \
    --cc=corbet@lwn.net \
    --cc=gnoack3000@gmail.com \
    --cc=gnoack@google.com \
    --cc=jmorris@namei.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mail@johnericson.me \
    --cc=mic@digikod.net \
    --cc=paul@paul-moore.com \
    --cc=serge@hallyn.com \
    --cc=shuah@kernel.org \
    /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