[PATCH RFC 0/3] landlock: Add access rights on created files

[PATCH RFC 0/3] landlock: Add access rights on created files

[Farid Zakaria]

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

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.

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?

- 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?

- 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()?

- A file renamed or linked in keeps only the rights it had at its source.
  Should it get none instead?

---
Farid Zakaria (3):
      lsm: Call inode_init_security hooks without xattrs
      landlock: Add access rights on created files
      selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED

 Documentation/userspace-api/landlock.rst     |  42 +++++
 include/uapi/linux/landlock.h                |  18 +++
 security/landlock/domain.c                   | 182 ++++++++++++++++++++-
 security/landlock/domain.h                   |  28 ++++
 security/landlock/fs.c                       | 176 +++++++++++++++++++--
 security/landlock/object.h                   |   6 +
 security/landlock/ruleset.c                  |   6 +
 security/landlock/ruleset.h                  |  14 ++
 security/landlock/syscalls.c                 |  20 ++-
 security/security.c                          |   9 +-
 tools/testing/selftests/landlock/base_test.c |   2 +-
 tools/testing/selftests/landlock/fs_test.c   | 227 +++++++++++++++++++++++++++
 12 files changed, 703 insertions(+), 27 deletions(-)
---
base-commit: 3e271a27c46fd591d1011c7de5a644a41e2010f1
change-id: 20261024-landlock-created-rights-9340b00a8c4d

Best regards,
--  
Farid Zakaria <farid.m.zakaria@gmail.com>

[PATCH RFC 1/3] lsm: Call inode_init_security hooks without xattrs

[Farid Zakaria]

security_inode_init_security() returns early when no LSM reserves an
xattr slot, so the inode_init_security hooks never run on a kernel
whose LSMs provide no xattr. An LSM can then not use the hook to
observe inode creation unless some other LSM happens to label inodes.

Call the hooks regardless of the reserved xattr count, and only skip
allocating the xattr array when no slot is reserved. LSMs that write
xattrs still need a reserved slot, so they are unaffected. Landlock
uses the hook in the next patch to track the files a sandboxed task
creates.

Assisted-by: LLM
Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
---
 security/security.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/security/security.c b/security/security.c
index b284e24e3771..bb94c94d97a5 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1353,10 +1353,11 @@ int security_inode_init_security(struct inode *inode, struct inode *dir,
 	if (unlikely(IS_PRIVATE(inode)))
 		return 0;
 
-	if (!blob_sizes.lbs_xattr_count)
-		return 0;
-
-	if (initxattrs) {
+	/*
+	 * Even without any xattr to provide, LSMs (e.g. Landlock) may need to
+	 * track inode creations.
+	 */
+	if (initxattrs && blob_sizes.lbs_xattr_count) {
 		/* Allocate +1 as terminator. */
 		new_xattrs = kcalloc(blob_sizes.lbs_xattr_count + 1,
 				     sizeof(*new_xattrs), GFP_NOFS);

-- 
2.55.0

Re: [PATCH RFC 1/3] lsm: Call inode_init_security hooks without xattrs

[sashiko-bot]

> security_inode_init_security() returns early when no LSM reserves an
> xattr slot, so the inode_init_security hooks never run on a kernel
> whose LSMs provide no xattr. An LSM can then not use the hook to
> observe inode creation unless some other LSM happens to label inodes.
> 
> Call the hooks regardless of the reserved xattr count, and only skip
> allocating the xattr array when no slot is reserved. LSMs that write
> xattrs still need a reserved slot, so they are unaffected. Landlock
> uses the hook in the next patch to track the files a sandboxed task
> creates.
> 
> Assisted-by: LLM
> Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024-landlock-created-rights-v1-0-77c8db3f6bd2@gmail.com?part=1

[PATCH RFC 2/3] landlock: Add access rights on created files

[Farid Zakaria]

Landlock ties access rights to inodes that exist when a ruleset is
built. A directory where a sandboxed task may only create entries is
then hard to use: LANDLOCK_ACCESS_FS_MAKE_* lets the task create
entries, but the entries are only covered by the rules of their parent
directories, and granting read access on the directory to cover them
also exposes every entry already there.

A shared package store such as /nix/store has this shape. A build
sandbox should read only its declared inputs and use the outputs it
creates, without being able to list or read the rest of the store. The
outputs do not exist when the ruleset is built, so no rule can name
them.

Add a LANDLOCK_ADD_RULE_CREATED flag for LANDLOCK_RULE_PATH_BENEATH
rules on a directory. Its access rights are not granted on the
directory but on each file hierarchy that a task of the domain, or of a
nested domain, creates directly in it. The inode_init_security hook
observes the creation, and each layer's hierarchy keeps the granted
rights and pins the created inodes until the hierarchy is freed. A file
linked or renamed into the directory only gets the rights it already
had at its source, which keeps the LANDLOCK_ACCESS_FS_REFER guarantee
that a reparented file gains nothing.

This bumps the Landlock ABI version to 13.

Assisted-by: LLM
Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
---
 Documentation/userspace-api/landlock.rst     |  42 +++++++
 include/uapi/linux/landlock.h                |  18 +++
 security/landlock/domain.c                   | 182 ++++++++++++++++++++++++++-
 security/landlock/domain.h                   |  28 +++++
 security/landlock/fs.c                       | 176 ++++++++++++++++++++++++--
 security/landlock/object.h                   |   6 +
 security/landlock/ruleset.c                  |   6 +
 security/landlock/ruleset.h                  |  14 +++
 security/landlock/syscalls.c                 |  20 ++-
 tools/testing/selftests/landlock/base_test.c |   2 +-
 10 files changed, 471 insertions(+), 23 deletions(-)

diff --git a/Documentation/userspace-api/landlock.rst b/Documentation/userspace-api/landlock.rst
index 3d45b956b640..d347f157fb79 100644
--- a/Documentation/userspace-api/landlock.rst
+++ b/Documentation/userspace-api/landlock.rst
@@ -418,6 +418,41 @@ In this case, granting read-write access to ``~/tmp/``, instead of write-only
 access, would potentially allow moving ``~/tmp/`` to a non-readable directory
 and still keep the ability to list the content of ``~/tmp/``.
 
+Created file hierarchies
+------------------------
+
+Some directories are shared between trusted and sandboxed tasks.  For instance,
+a package store may hold the declared inputs of a build sandbox, other packages
+that this sandbox must not read, and the outputs that this sandbox creates.
+Granting the ``LANDLOCK_ACCESS_FS_MAKE_*`` access rights on such a directory
+enables the sandbox to create new entries, but these entries are then only
+covered by the rules of their parent directories.  They cannot be the target of
+a rule either, because they do not exist yet when the ruleset is created.
+
+A rule added with the ``LANDLOCK_ADD_RULE_CREATED`` flag grants its access
+rights on each file hierarchy that the sandboxed tasks create directly in a
+directory, instead of on the directory itself.  For instance, the following
+rule enables a sandboxed task to use the entries it creates in ``/nix/store``,
+but not the other entries of this directory:
+
+.. code-block:: c
+
+    struct landlock_path_beneath_attr created_attr = {
+        .allowed_access = ruleset_attr.handled_access_fs,
+        .parent_fd = open("/nix/store", O_PATH | O_CLOEXEC),
+    };
+
+    err = landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+                            &created_attr, LANDLOCK_ADD_RULE_CREATED);
+
+A file hierarchy linked or renamed into such a directory only gets these access
+rights if it already had them at its source.  This enables building an entry in
+another directory and then moving it in place.  These access rights are tied to
+the created inodes, which are kept in memory for as long as the domain (or one
+of its nested domains) exists.  Entries created through an OverlayFS merge
+directory do not get these access rights, because OverlayFS creates them on its
+upper layer on behalf of the task that mounted it.
+
 Layers of file path access rights
 ---------------------------------
 
@@ -1051,6 +1086,13 @@ fd-reference acquisition (:manpage:`open_tree(2)`, :manpage:`fsmount(2)`) with
 the new ``LANDLOCK_PERMISSION_NAMESPACE_USE`` permission flag and
 ``LANDLOCK_RULE_NAMESPACE`` rule type.
 
+Created file hierarchies (ABI < 13)
+-----------------------------------
+
+Starting with the Landlock ABI version 13, it is possible to grant access
+rights on the file hierarchies created by a sandboxed task in a directory with
+the new ``LANDLOCK_ADD_RULE_CREATED`` rule flag.
+
 .. _kernel_support:
 
 Kernel support
diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index 24510e5d8479..a92324bba41e 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -129,10 +129,28 @@ struct landlock_ruleset_attr {
  *     allowed_access in the passed in rule_attr.  When this flag is
  *     present, the caller is also allowed to pass in an empty
  *     allowed_access.
+ *
+ * %LANDLOCK_ADD_RULE_CREATED
+ *     Only valid for a %LANDLOCK_RULE_PATH_BENEATH rule on a directory.
+ *     Instead of granting allowed_access on the directory hierarchy, this
+ *     grants it on each file hierarchy that a task of the resulting domain
+ *     (or of a nested domain) creates directly in this directory.  A file
+ *     hierarchy linked or renamed into this directory only gets these access
+ *     rights if it already had them at its source.
+ *
+ *     This enables a sandboxed task to use the files it creates in a
+ *     directory without being able to read the rest of this directory, e.g.
+ *     build outputs in a shared package store.  Creating files still requires
+ *     the related ``LANDLOCK_ACCESS_FS_MAKE_*`` rights on the directory.
+ *
+ *     These access rights are tied to the created inodes for as long as the
+ *     domain exists.  This flag cannot be combined with
+ *     %LANDLOCK_ADD_RULE_QUIET.
  */
 
 /* clang-format off */
 #define LANDLOCK_ADD_RULE_QUIET			(1U << 0)
+#define LANDLOCK_ADD_RULE_CREATED		(1U << 1)
 /* clang-format on */
 
 /**
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index 4c0b1f0e1db5..f8dc771724e1 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -57,6 +57,7 @@ static struct landlock_domain *create_domain(const u32 num_layers)
 
 	refcount_set(&new_domain->usage, 1);
 	new_domain->rules.root_inode = RB_ROOT;
+	new_domain->rules.root_inode_created = RB_ROOT;
 
 #if IS_ENABLED(CONFIG_INET)
 	new_domain->rules.root_net_port = RB_ROOT;
@@ -98,9 +99,9 @@ void landlock_put_domain_deferred(struct landlock_domain *const domain)
 }
 
 /* The returned access has the same lifetime as the domain. */
-static const struct landlock_rule *
-find_rule(const struct landlock_domain *const domain,
-	  const struct landlock_id id)
+const struct landlock_rule *
+landlock_find_rule(const struct landlock_domain *const domain,
+		   const struct landlock_id id)
 {
 	const struct rb_root *root;
 	const struct rb_node *node;
@@ -152,7 +153,7 @@ bool landlock_unmask_layers(const struct landlock_domain *const domain,
 	if (!masks)
 		return true;
 
-	rule = find_rule(domain, id);
+	rule = landlock_find_rule(domain, id);
 	if (!rule)
 		return false;
 
@@ -198,6 +199,166 @@ bool landlock_unmask_layers(const struct landlock_domain *const domain,
 	return true;
 }
 
+/**
+ * struct landlock_created - Access rights granted on a created inode
+ */
+struct landlock_created {
+	/**
+	 * @node: Node in &landlock_hierarchy.created .
+	 */
+	struct rb_node node;
+	/**
+	 * @object: Object tied to the created inode.
+	 */
+	struct landlock_object *object;
+	/**
+	 * @access: Access rights granted on the created file hierarchy.
+	 */
+	access_mask_t access;
+};
+
+static struct landlock_hierarchy *
+get_layer_hierarchy(const struct landlock_domain *const domain,
+		    const size_t layer_level)
+{
+	struct landlock_hierarchy *hierarchy = domain->hierarchy;
+
+	for (size_t i = domain->num_layers - 1; i > layer_level && hierarchy;
+	     i--)
+		hierarchy = hierarchy->parent;
+	return hierarchy;
+}
+
+/**
+ * landlock_add_created - Grant access rights on a created inode
+ *
+ * @domain: The domain of the task which created the inode.
+ * @layer_level: The layer of @domain which granted @access.
+ * @object: The object tied to the created inode.
+ * @access: The access rights to grant on the created file hierarchy.
+ *
+ * The access rights are tied to the hierarchy of the layer, which is shared
+ * with nested domains, and keep @object alive until this hierarchy is freed.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+int landlock_add_created(const struct landlock_domain *const domain,
+			 const size_t layer_level,
+			 struct landlock_object *const object,
+			 const access_mask_t access)
+{
+	struct landlock_hierarchy *const hierarchy =
+		get_layer_hierarchy(domain, layer_level);
+	struct landlock_created *new_created;
+	struct rb_node **walker_node, *parent_node = NULL;
+
+	if (WARN_ON_ONCE(!hierarchy))
+		return -EINVAL;
+
+	new_created = kzalloc_obj(*new_created, GFP_KERNEL_ACCOUNT);
+	if (!new_created)
+		return -ENOMEM;
+
+	spin_lock(&hierarchy->created_lock);
+	walker_node = &hierarchy->created.rb_node;
+	while (*walker_node) {
+		struct landlock_created *const this =
+			rb_entry(*walker_node, struct landlock_created, node);
+
+		if (this->object == object) {
+			this->access |= access;
+			spin_unlock(&hierarchy->created_lock);
+			kfree(new_created);
+			return 0;
+		}
+		parent_node = *walker_node;
+		if (this->object < object)
+			walker_node = &(*walker_node)->rb_right;
+		else
+			walker_node = &(*walker_node)->rb_left;
+	}
+	landlock_get_object(object);
+	new_created->object = object;
+	new_created->access = access;
+	rb_link_node(&new_created->node, parent_node, walker_node);
+	rb_insert_color(&new_created->node, &hierarchy->created);
+	spin_unlock(&hierarchy->created_lock);
+	WRITE_ONCE(object->created, true);
+	return 0;
+}
+
+static access_mask_t get_created(struct landlock_hierarchy *const hierarchy,
+				 const struct landlock_object *const object)
+{
+	const struct rb_node *node;
+	access_mask_t access = 0;
+
+	spin_lock(&hierarchy->created_lock);
+	node = hierarchy->created.rb_node;
+	while (node) {
+		const struct landlock_created *const this =
+			rb_entry(node, struct landlock_created, node);
+
+		if (this->object == object) {
+			access = this->access;
+			break;
+		}
+		if (this->object < object)
+			node = node->rb_right;
+		else
+			node = node->rb_left;
+	}
+	spin_unlock(&hierarchy->created_lock);
+	return access;
+}
+
+/**
+ * landlock_unmask_created - Remove the access rights in @masks which are
+ *                           granted on a created inode
+ *
+ * @domain: The Landlock domain to check against.
+ * @object: The object tied to the inode, which may have been created by a
+ *          task of @domain, a parent domain, or a nested domain.
+ * @masks: A matrix of unfulfilled access rights for each layer.
+ *
+ * Return: True if the request is allowed (i.e. @masks has no leftover set
+ * bits).
+ */
+bool landlock_unmask_created(const struct landlock_domain *const domain,
+			     const struct landlock_object *const object,
+			     struct layer_masks *masks)
+{
+	struct landlock_hierarchy *hierarchy = domain->hierarchy;
+
+	if (!masks)
+		return true;
+
+	if (!READ_ONCE(object->created))
+		return false;
+
+	for (size_t i = domain->num_layers; i > 0 && hierarchy;
+	     i--, hierarchy = hierarchy->parent)
+		masks->layers[i - 1].access &= ~get_created(hierarchy, object);
+
+	for (size_t i = 0; i < ARRAY_SIZE(masks->layers); i++) {
+		if (masks->layers[i].access)
+			return false;
+	}
+	return true;
+}
+
+void landlock_free_created(struct landlock_hierarchy *const hierarchy)
+{
+	struct landlock_created *freeme, *next;
+
+	might_sleep();
+	rbtree_postorder_for_each_entry_safe(freeme, next, &hierarchy->created,
+					     node) {
+		landlock_put_object(freeme->object);
+		kfree(freeme);
+	}
+}
+
 typedef access_mask_t
 get_access_mask_t(const struct landlock_domain *const domain,
 		  const u16 layer_level);
@@ -331,11 +492,15 @@ static int merge_ruleset(struct landlock_domain *const dst,
 	dst->layers[dst->num_layers - 1] =
 		landlock_upgrade_handled_layer_config(src->layer);
 
-	/* Merges the @src inode tree. */
+	/* Merges the @src inode trees. */
 	err = merge_tree(dst, src, LANDLOCK_KEY_INODE);
 	if (err)
 		return err;
 
+	err = merge_tree(dst, src, LANDLOCK_KEY_INODE_CREATED);
+	if (err)
+		return err;
+
 #if IS_ENABLED(CONFIG_INET)
 	/* Merges the @src network port tree. */
 	err = merge_tree(dst, src, LANDLOCK_KEY_NET_PORT);
@@ -386,11 +551,15 @@ static int inherit_ruleset(struct landlock_domain *const parent,
 	if (!parent)
 		return 0;
 
-	/* Copies the @parent inode tree. */
+	/* Copies the @parent inode trees. */
 	err = inherit_tree(parent, child, LANDLOCK_KEY_INODE);
 	if (err)
 		return err;
 
+	err = inherit_tree(parent, child, LANDLOCK_KEY_INODE_CREATED);
+	if (err)
+		return err;
+
 #if IS_ENABLED(CONFIG_INET)
 	/* Copies the @parent network port tree. */
 	err = inherit_tree(parent, child, LANDLOCK_KEY_NET_PORT);
@@ -463,6 +632,7 @@ landlock_merge_ruleset(struct landlock_domain *const parent,
 		return ERR_PTR(-ENOMEM);
 
 	refcount_set(&new_dom->hierarchy->usage, 1);
+	spin_lock_init(&new_dom->hierarchy->created_lock);
 
 	/* ...as a child of @parent... */
 	err = inherit_ruleset(parent, new_dom);
diff --git a/security/landlock/domain.h b/security/landlock/domain.h
index 935e18b12b3c..21d6bb4453d1 100644
--- a/security/landlock/domain.h
+++ b/security/landlock/domain.h
@@ -19,6 +19,7 @@
 #include <linux/refcount.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
+#include <linux/spinlock.h>
 #include <linux/workqueue.h>
 
 #include "access.h"
@@ -98,6 +99,17 @@ struct landlock_hierarchy {
 	 * domain.
 	 */
 	refcount_t usage;
+	/**
+	 * @created_lock: Protects @created.
+	 */
+	spinlock_t created_lock;
+	/**
+	 * @created: Red-black tree of &struct landlock_created, one per inode
+	 * which got access rights from a %LANDLOCK_ADD_RULE_CREATED rule of
+	 * this layer.  Each node holds a reference to its object, and thus to
+	 * its inode, until this hierarchy is freed.
+	 */
+	struct rb_root created;
 
 #ifdef CONFIG_SECURITY_LANDLOCK_LOG
 	/**
@@ -187,6 +199,8 @@ landlock_get_hierarchy(struct landlock_hierarchy *const hierarchy)
 		refcount_inc(&hierarchy->usage);
 }
 
+void landlock_free_created(struct landlock_hierarchy *const hierarchy);
+
 static inline void landlock_put_hierarchy(struct landlock_hierarchy *hierarchy)
 {
 	while (hierarchy && refcount_dec_and_test(&hierarchy->usage)) {
@@ -194,6 +208,7 @@ static inline void landlock_put_hierarchy(struct landlock_hierarchy *hierarchy)
 
 		landlock_log_free_domain(hierarchy);
 		landlock_free_hierarchy_details(hierarchy);
+		landlock_free_created(hierarchy);
 		hierarchy = hierarchy->parent;
 		kfree(freeme);
 	}
@@ -321,6 +336,19 @@ bool landlock_unmask_layers(const struct landlock_domain *const domain,
 			    struct layer_masks *masks,
 			    const struct landlock_rule **matched_rule);
 
+const struct landlock_rule *
+landlock_find_rule(const struct landlock_domain *const domain,
+		   const struct landlock_id id);
+
+int landlock_add_created(const struct landlock_domain *const domain,
+			 const size_t layer_level,
+			 struct landlock_object *const object,
+			 const access_mask_t access);
+
+bool landlock_unmask_created(const struct landlock_domain *const domain,
+			     const struct landlock_object *const object,
+			     struct layer_masks *masks);
+
 access_mask_t
 landlock_init_layer_masks(const struct landlock_domain *const domain,
 			  const access_mask_t access_request,
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 2ab67bcad198..64a344725433 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -332,12 +332,15 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 {
 	int err;
 	struct landlock_id id = {
-		.type = LANDLOCK_KEY_INODE,
+		.type = (flags & LANDLOCK_ADD_RULE_CREATED) ?
+				LANDLOCK_KEY_INODE_CREATED :
+				LANDLOCK_KEY_INODE,
 	};
 
 	/* Files only get access rights that make sense. */
 	if (!d_is_dir(path->dentry) &&
-	    !access_mask_subset(access_rights, ACCESS_FILE))
+	    ((flags & LANDLOCK_ADD_RULE_CREATED) ||
+	     !access_mask_subset(access_rights, ACCESS_FILE)))
 		return -EINVAL;
 
 	/* Transforms relative access rights to absolute ones. */
@@ -428,6 +431,16 @@ static bool unmask_layers_fs(const struct landlock_domain *const domain,
 	return ret;
 }
 
+static bool unmask_layers_created(const struct landlock_domain *const domain,
+				  const struct landlock_id id,
+				  struct layer_masks *masks)
+{
+	if (RB_EMPTY_ROOT(&domain->rules.root_inode_created))
+		return false;
+
+	return landlock_unmask_created(domain, id.key.object, masks);
+}
+
 /*
  * Allows access to pseudo filesystems that will never be mountable (e.g.
  * sockfs, pipefs), but can still be reachable through
@@ -869,9 +882,11 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 						    LANDLOCK_MASK_ACCESS_FS,
 						    &_layer_masks_child1,
 						    LANDLOCK_KEY_INODE);
-		if (handled && get_inode_id(dentry_child1, &id))
+		if (handled && get_inode_id(dentry_child1, &id)) {
 			unmask_layers_fs(domain, id, handled,
 					 &_layer_masks_child1, dentry_child1);
+			unmask_layers_created(domain, id, &_layer_masks_child1);
+		}
 		layer_masks_child1 = &_layer_masks_child1;
 		child1_is_directory = d_is_dir(dentry_child1);
 	}
@@ -885,9 +900,11 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 						    LANDLOCK_MASK_ACCESS_FS,
 						    &_layer_masks_child2,
 						    LANDLOCK_KEY_INODE);
-		if (handled && get_inode_id(dentry_child2, &id))
+		if (handled && get_inode_id(dentry_child2, &id)) {
 			unmask_layers_fs(domain, id, handled,
 					 &_layer_masks_child2, dentry_child2);
+			unmask_layers_created(domain, id, &_layer_masks_child2);
+		}
 		layer_masks_child2 = &_layer_masks_child2;
 		child2_is_directory = d_is_dir(dentry_child2);
 	}
@@ -945,13 +962,17 @@ is_access_to_paths_allowed(const struct landlock_domain *const domain,
 				unmask_layers_fs(domain, id,
 						 access_masked_parent1,
 						 layer_masks_parent1,
-						 walker_path.dentry);
+						 walker_path.dentry) ||
+				unmask_layers_created(domain, id,
+						      layer_masks_parent1);
 			allowed_parent2 =
 				allowed_parent2 ||
 				unmask_layers_fs(domain, id,
 						 access_masked_parent2,
 						 layer_masks_parent2,
-						 walker_path.dentry);
+						 walker_path.dentry) ||
+				unmask_layers_created(domain, id,
+						      layer_masks_parent2);
 		}
 
 		/* Stops when a rule from each layer grants access. */
@@ -1148,8 +1169,9 @@ static bool collect_domain_accesses(const struct landlock_domain *const domain,
 
 		/* Gets all layers allowing all domain accesses. */
 		if (get_inode_id(dir, &id) &&
-		    unmask_layers_fs(domain, id, access_masked_dom,
-				     layer_masks_dom, dir)) {
+		    (unmask_layers_fs(domain, id, access_masked_dom,
+				      layer_masks_dom, dir) ||
+		     unmask_layers_created(domain, id, layer_masks_dom))) {
 			/*
 			 * Stops when all handled accesses are allowed by at
 			 * least one rule in each layer.
@@ -1359,8 +1381,130 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	return -EXDEV;
 }
 
+static const struct landlock_rule *
+get_created_rule(const struct landlock_domain *const domain,
+		 const struct inode *const dir)
+{
+	const struct landlock_id id = {
+		.key.object = rcu_access_pointer(landlock_inode(dir)->object),
+		.type = LANDLOCK_KEY_INODE_CREATED,
+	};
+
+	if (!id.key.object)
+		return NULL;
+
+	return landlock_find_rule(domain, id);
+}
+
+/**
+ * grant_created - Grant the access rights of a LANDLOCK_ADD_RULE_CREATED rule
+ *
+ * @domain: The domain of the current task.
+ * @rule: The rule tied to the parent directory of @inode.
+ * @inode: The created, linked, or renamed inode.
+ * @src_masks: If not NULL, the unfulfilled access rights of @inode at its
+ *             source.  Only the layers which already granted their access
+ *             rights there grant them on @inode.
+ *
+ * Return: 0 on success, -errno on failure.
+ */
+static int grant_created(const struct landlock_domain *const domain,
+			 const struct landlock_rule *const rule,
+			 struct inode *const inode,
+			 const struct layer_masks *const src_masks)
+{
+	struct landlock_object *object;
+	int err = 0;
+
+	object = get_inode_object(inode);
+	if (IS_ERR(object))
+		return PTR_ERR(object);
+
+	for (size_t i = 0; i < rule->num_layers && !err; i++) {
+		const struct landlock_layer *const layer = &rule->layers[i];
+
+		if (src_masks &&
+		    (src_masks->layers[layer->level - 1].access & layer->access))
+			continue;
+
+		err = landlock_add_created(domain, layer->level - 1, object,
+					   layer->access);
+	}
+	landlock_put_object(object);
+	return err;
+}
+
+/*
+ * Grants the access rights of @new_dir's LANDLOCK_ADD_RULE_CREATED rule on
+ * the reparented @old_dentry, for the layers which already granted them at
+ * its source.  This must only be called once the refer action is allowed.
+ */
+static int refer_created(struct dentry *const old_dentry,
+			 const struct path *const new_dir)
+{
+	const struct landlock_cred_security *const subject =
+		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+	/* The mount points are the same for old and new paths, cf. EXDEV. */
+	const struct path old_path = {
+		.mnt = new_dir->mnt,
+		.dentry = old_dentry,
+	};
+	const struct landlock_rule *rule;
+	struct layer_masks layer_masks;
+	access_mask_t access = 0;
+
+	if (!subject ||
+	    RB_EMPTY_ROOT(&subject->domain->rules.root_inode_created) ||
+	    old_dentry->d_parent == new_dir->dentry)
+		return 0;
+
+	rule = get_created_rule(subject->domain,
+				d_backing_inode(new_dir->dentry));
+	if (!rule)
+		return 0;
+
+	for (size_t i = 0; i < rule->num_layers; i++)
+		access |= rule->layers[i].access;
+
+	/* Only compares the access rights which make sense for files. */
+	if (!d_is_dir(old_dentry))
+		access &= ACCESS_FILE;
+
+	access = landlock_init_layer_masks(subject->domain, access,
+					   &layer_masks, LANDLOCK_KEY_INODE);
+	is_access_to_paths_allowed(subject->domain, &old_path, access,
+				   &layer_masks, NULL, NULL, 0, NULL, NULL,
+				   NULL);
+	return grant_created(subject->domain, rule,
+			     d_backing_inode(old_dentry), &layer_masks);
+}
+
 /* Inode hooks */
 
+static int hook_inode_init_security(struct inode *const inode,
+				    struct inode *const dir,
+				    const struct qstr *const qstr,
+				    struct xattr *const xattrs,
+				    int *const xattr_count)
+{
+	const struct landlock_cred_security *const subject =
+		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+	const struct landlock_rule *rule;
+	int err;
+
+	if (!subject || !dir ||
+	    RB_EMPTY_ROOT(&subject->domain->rules.root_inode_created))
+		return -EOPNOTSUPP;
+
+	rule = get_created_rule(subject->domain, dir);
+	if (!rule)
+		return -EOPNOTSUPP;
+
+	/* Landlock does not provide any xattr. */
+	err = grant_created(subject->domain, rule, inode, NULL);
+	return err ?: -EOPNOTSUPP;
+}
+
 static void hook_inode_free_security_rcu(void *inode_security)
 {
 	struct landlock_inode_security *inode_sec;
@@ -1623,7 +1767,8 @@ static int hook_path_link(struct dentry *const old_dentry,
 			  struct dentry *const new_dentry)
 {
 	return current_check_refer_path(old_dentry, new_dir, new_dentry, false,
-					false, false);
+					false, false) ?:
+		       refer_created(old_dentry, new_dir);
 }
 
 static int hook_path_rename(const struct path *const old_dir,
@@ -1632,10 +1777,16 @@ static int hook_path_rename(const struct path *const old_dir,
 			    struct dentry *const new_dentry,
 			    const unsigned int flags)
 {
+	int err;
+
 	/* old_dir refers to old_dentry->d_parent and new_dir->mnt */
-	return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
-					!!(flags & RENAME_EXCHANGE),
-					!!(flags & RENAME_WHITEOUT));
+	err = current_check_refer_path(old_dentry, new_dir, new_dentry, true,
+				       !!(flags & RENAME_EXCHANGE),
+				       !!(flags & RENAME_WHITEOUT));
+	if (err || (flags & RENAME_EXCHANGE))
+		return err;
+
+	return refer_created(old_dentry, new_dir);
 }
 
 static int hook_path_mkdir(const struct path *const dir,
@@ -2083,6 +2234,7 @@ static void hook_file_free_security(struct file *file)
 }
 
 static struct security_hook_list landlock_hooks[] __ro_after_init = {
+	LSM_HOOK_INIT(inode_init_security, hook_inode_init_security),
 	LSM_HOOK_INIT(inode_free_security_rcu, hook_inode_free_security_rcu),
 
 	LSM_HOOK_INIT(sb_delete, hook_sb_delete),
diff --git a/security/landlock/object.h b/security/landlock/object.h
index 5f28c35e8aa8..61bd8589862d 100644
--- a/security/landlock/object.h
+++ b/security/landlock/object.h
@@ -54,6 +54,12 @@ struct landlock_object {
 	 * Lock ordering: inode->i_lock nests inside this.
 	 */
 	spinlock_t lock;
+	/**
+	 * @created: Set once the underlying inode got access rights from a
+	 * %LANDLOCK_ADD_RULE_CREATED rule, which enables to skip looking up
+	 * such access rights for other inodes.
+	 */
+	bool created;
 	/**
 	 * @underobj: Used when cleaning up an object and to mark an object as
 	 * tied to its underlying kernel structure.  This pointer is protected
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 11776b138a56..44168537648c 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -49,6 +49,7 @@ struct landlock_ruleset *landlock_create_ruleset(
 	refcount_set(&new_ruleset->usage, 1);
 	mutex_init(&new_ruleset->lock);
 	new_ruleset->rules.root_inode = RB_ROOT;
+	new_ruleset->rules.root_inode_created = RB_ROOT;
 
 #if IS_ENABLED(CONFIG_INET)
 	new_ruleset->rules.root_net_port = RB_ROOT;
@@ -110,6 +111,7 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
 {
 	switch (key_type) {
 	case LANDLOCK_KEY_INODE:
+	case LANDLOCK_KEY_INODE_CREATED:
 		return true;
 
 #if IS_ENABLED(CONFIG_INET)
@@ -342,6 +344,10 @@ void landlock_free_rules(struct landlock_rules *const rules)
 					     node)
 		free_rule(freeme, LANDLOCK_KEY_INODE);
 
+	rbtree_postorder_for_each_entry_safe(freeme, next,
+					     &rules->root_inode_created, node)
+		free_rule(freeme, LANDLOCK_KEY_INODE_CREATED);
+
 #if IS_ENABLED(CONFIG_INET)
 	rbtree_postorder_for_each_entry_safe(freeme, next,
 					     &rules->root_net_port, node)
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 3a38d7079dc2..4ed347a0c3a5 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -74,6 +74,11 @@ enum landlock_key_type {
 	 * keys.
 	 */
 	LANDLOCK_KEY_NET_PORT,
+	/**
+	 * @LANDLOCK_KEY_INODE_CREATED: Type of
+	 * &landlock_rules.root_inode_created's node keys.
+	 */
+	LANDLOCK_KEY_INODE_CREATED,
 };
 
 /**
@@ -130,6 +135,13 @@ struct landlock_rules {
 	 */
 	struct rb_root root_inode;
 
+	/**
+	 * @root_inode_created: Root of a red-black tree containing &struct
+	 * landlock_rule nodes with directory inode object, for rules added with
+	 * %LANDLOCK_ADD_RULE_CREATED.  Immutable for domains.
+	 */
+	struct rb_root root_inode_created;
+
 #if IS_ENABLED(CONFIG_INET)
 	/**
 	 * @root_net_port: Root of a red-black tree containing &struct
@@ -237,6 +249,8 @@ landlock_get_rule_root(struct landlock_rules *const rules,
 	switch (key_type) {
 	case LANDLOCK_KEY_INODE:
 		return &rules->root_inode;
+	case LANDLOCK_KEY_INODE_CREATED:
+		return &rules->root_inode_created;
 
 #if IS_ENABLED(CONFIG_INET)
 	case LANDLOCK_KEY_NET_PORT:
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index fb5af318b69b..755f6574539d 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -191,7 +191,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 = 12;
+const int landlock_abi_version = 13;
 
 /**
  * sys_landlock_create_ruleset - Create a new ruleset
@@ -397,9 +397,15 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
 	 * are ignored in path walks.  However, the rule is not useless if it is
 	 * there to hold a quiet flag.
 	 */
-	if (!flags && !path_beneath_attr.allowed_access)
+	if (!(flags & LANDLOCK_ADD_RULE_QUIET) &&
+	    !path_beneath_attr.allowed_access)
 		return -ENOMSG;
 
+	/* Created file hierarchies are not covered by quiet flags. */
+	if ((flags & LANDLOCK_ADD_RULE_QUIET) &&
+	    (flags & LANDLOCK_ADD_RULE_CREATED))
+		return -EINVAL;
+
 	/* Checks that allowed_access matches the @ruleset constraints. */
 	mask = ruleset->layer.handled.fs;
 	if ((path_beneath_attr.allowed_access | mask) != mask)
@@ -450,6 +456,9 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
 	if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_access.net)
 		return -EINVAL;
 
+	if (flags & LANDLOCK_ADD_RULE_CREATED)
+		return -EINVAL;
+
 	/* Denies inserting a rule with port greater than 65535. */
 	if (net_port_attr.port > U16_MAX)
 		return -EINVAL;
@@ -618,7 +627,7 @@ static int add_rule_capability(struct landlock_ruleset *const ruleset,
  *             %LANDLOCK_RULE_PATH_BENEATH, %LANDLOCK_RULE_NET_PORT,
  *             %LANDLOCK_RULE_NAMESPACE, or %LANDLOCK_RULE_CAPABILITY.
  * @rule_attr: Pointer to a rule (matching the @rule_type).
- * @flags: Must be 0 or %LANDLOCK_ADD_RULE_QUIET.
+ * @flags: Must be 0, %LANDLOCK_ADD_RULE_QUIET, or %LANDLOCK_ADD_RULE_CREATED.
  *
  * This system call enables to define a new rule and add it to an existing
  * ruleset.
@@ -640,6 +649,9 @@ static int add_rule_capability(struct landlock_ruleset *const ruleset,
  * - %EINVAL: &landlock_net_port_attr.port is greater than 65535;
  * - %EINVAL: LANDLOCK_ADD_RULE_QUIET is passed but the ruleset has no
  *   quiet access bits set for the corresponding rule type.
+ * - %EINVAL: LANDLOCK_ADD_RULE_CREATED is passed with
+ *   LANDLOCK_ADD_RULE_QUIET, for a rule which is not a
+ *   %LANDLOCK_RULE_PATH_BENEATH one, or for a file which is not a directory.
  * - %ENOMSG: Empty accesses (e.g. &landlock_path_beneath_attr.allowed_access is
  *   0) and no flags;
  * - %ENOMSG: &landlock_namespace_attr.permissions is 0, or both
@@ -667,7 +679,7 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
 	if (!is_initialized())
 		return -EOPNOTSUPP;
 
-	if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+	if (flags & ~(LANDLOCK_ADD_RULE_QUIET | LANDLOCK_ADD_RULE_CREATED))
 		return -EINVAL;
 
 	/* Gets and checks the ruleset. */
diff --git a/tools/testing/selftests/landlock/base_test.c b/tools/testing/selftests/landlock/base_test.c
index 89c0d96607de..14078ad203d8 100644
--- a/tools/testing/selftests/landlock/base_test.c
+++ b/tools/testing/selftests/landlock/base_test.c
@@ -76,7 +76,7 @@ TEST(abi_version)
 	const struct landlock_ruleset_attr ruleset_attr = {
 		.handled_access_fs = LANDLOCK_ACCESS_FS_READ_FILE,
 	};
-	ASSERT_EQ(12, landlock_create_ruleset(NULL, 0,
+	ASSERT_EQ(13, landlock_create_ruleset(NULL, 0,
 					      LANDLOCK_CREATE_RULESET_VERSION));
 
 	ASSERT_EQ(-1, landlock_create_ruleset(&ruleset_attr, 0,

-- 
2.55.0

Re: [PATCH RFC 2/3] landlock: Add access rights on created files

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 7 potential issue(s) to consider:
- [Critical] landlock: use-after-free in landlock_unmask_created() during path walk
- [High] landlock: uninitialized layer_masks read in refer_created()
- [High] landlock: refer_created() mutates state and pins inodes before VFS checks
- [High] landlock: hook_inode_init_security() pins I_NEW inodes without error rollback
- [High] landlock: GFP_KERNEL_ACCOUNT allocation in hook_inode_init_security()
- [Medium] landlock: grant_created() miscomputes reparented access rights
- [Medium] landlock: missing hook_inode_init_security() calls and unlinked inode pinning
--

commit 5e3bc0f81c353fca29a705bd4d2b123bb4da9434
Author: Farid Zakaria <farid.m.zakaria@gmail.com>

landlock: Add access rights on created files

This commit adds the LANDLOCK_ADD_RULE_CREATED flag for
LANDLOCK_RULE_PATH_BENEATH rules on directories so that access rights are
granted on file hierarchies created directly in the directory by tasks in the
domain (or nested domains), and bumps the Landlock ABI version to 13.

> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 4c0b1f0e1db5d..f8dc771724e14 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c

[ ... ]

> @@ -198,6 +199,166 @@ bool landlock_unmask_layers(const struct landlock_domain *const domain,

[ ... ]

> +bool landlock_unmask_created(const struct landlock_domain *const domain,
> +			     const struct landlock_object *const object,
> +			     struct layer_masks *masks)
> +{
> +	struct landlock_hierarchy *hierarchy = domain->hierarchy;
> +
> +	if (!masks)
> +		return true;
> +
> +	if (!READ_ONCE(object->created))
> +		return false;
> +
> +	for (size_t i = domain->num_layers; i > 0 && hierarchy;
> +	     i--, hierarchy = hierarchy->parent)
> +		masks->layers[i - 1].access &= ~get_created(hierarchy, object);
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(masks->layers); i++) {
> +		if (masks->layers[i].access)
> +			return false;
> +	}
> +	return true;
> +}

[Severity: Critical]
Can this dereference of object->created in landlock_unmask_created() trigger a
use-after-free or grant access to the wrong inode via slab pointer reuse?

During the sleepable path walks in is_access_to_paths_allowed() and
collect_domain_accesses() in security/landlock/fs.c, get_inode_id() fetches
the raw object pointer without holding rcu_read_lock() or taking a reference:

security/landlock/fs.c:get_inode_id() {
	...
	id->key.object = rcu_access_pointer(
		landlock_inode(d_backing_inode(dentry))->object);
	return !!id->key.object;
}

Previously, id.key.object was never dereferenced and was only compared
numerically against domain->rules.root_inode, which is immutable and pins its
objects for the lifetime of the domain.

With unmask_layers_created() passing id.key.object to
landlock_unmask_created(), if an inode visited during the path walk has a
landlock_object attached by another ruleset or domain that is concurrently
freed via landlock_put_object() -> kfree_rcu(object, rcu_free), the RCU grace
period can elapse while the path walk is running. Could READ_ONCE(object->created)
then read from freed slab memory?

Furthermore, because hierarchy->created is updated at runtime by
landlock_add_created(), if that freed landlock_object slab slot is reallocated
for a newly created inode in this domain and inserted into hierarchy->created,
would get_created(hierarchy, object) match the reused pointer address and
grant the created inode's rights to the unrelated inode?

> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 2ab67bcad1983..64a3447254337 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c

[ ... ]

> @@ -1359,8 +1381,130 @@ static int current_check_refer_path(struct dentry *const old_dentry,

[ ... ]

> +static int grant_created(const struct landlock_domain *const domain,
> +			 const struct landlock_rule *const rule,
> +			 struct inode *const inode,
> +			 const struct layer_masks *const src_masks)
> +{
> +	struct landlock_object *object;
> +	int err = 0;
> +
> +	object = get_inode_object(inode);
> +	if (IS_ERR(object))
> +		return PTR_ERR(object);
> +
> +	for (size_t i = 0; i < rule->num_layers && !err; i++) {
> +		const struct landlock_layer *const layer = &rule->layers[i];
> +
> +		if (src_masks &&
> +		    (src_masks->layers[layer->level - 1].access & layer->access))
> +			continue;
> +
> +		err = landlock_add_created(domain, layer->level - 1, object,
> +					   layer->access);
> +	}
> +	landlock_put_object(object);
> +	return err;
> +}

[Severity: Medium]
When reparenting a file or directory via refer_created(), does grant_created()
drop the entire layer whenever any single bit of layer->access was not granted
at the source?

In refer_created(), layer_masks (passed as src_masks) holds the access bits
from rule that were unfulfilled at old_path. Checking:

	if (src_masks &&
	    (src_masks->layers[layer->level - 1].access & layer->access))
		continue;

skips the entire layer whenever any bit in layer->access remains unfulfilled
at the source (for instance, when created_attr.allowed_access is set to
ruleset_attr.handled_access_fs as shown in the documentation, but the source
staging directory did not grant LANDLOCK_ACCESS_FS_MAKE_BLOCK or
LANDLOCK_ACCESS_FS_IOCTL_DEV), rather than granting the intersection of rights
that were already allowed at the source
(layer->access & ~src_masks->layers[layer->level - 1].access).

Also, when old_dentry is not a directory, refer_created() masks
access &= ACCESS_FILE before calling landlock_init_layer_masks(), leaving
non-ACCESS_FILE bits zeroed in src_masks, so grant_created() passes the
unmasked layer->access (including directory access rights) to
landlock_add_created(). Should this grant only the fulfilled subset of
applicable rights instead?

> +
> +/*
> + * Grants the access rights of @new_dir's LANDLOCK_ADD_RULE_CREATED rule on
> + * the reparented @old_dentry, for the layers which already granted them at
> + * its source.  This must only be called once the refer action is allowed.
> + */
> +static int refer_created(struct dentry *const old_dentry,
> +			 const struct path *const new_dir)
> +{
> +	const struct landlock_cred_security *const subject =
> +		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
> +	/* The mount points are the same for old and new paths, cf. EXDEV. */
> +	const struct path old_path = {
> +		.mnt = new_dir->mnt,
> +		.dentry = old_dentry,
> +	};
> +	const struct landlock_rule *rule;
> +	struct layer_masks layer_masks;
> +	access_mask_t access = 0;
> +
> +	if (!subject ||
> +	    RB_EMPTY_ROOT(&subject->domain->rules.root_inode_created) ||
> +	    old_dentry->d_parent == new_dir->dentry)
> +		return 0;
> +
> +	rule = get_created_rule(subject->domain,
> +				d_backing_inode(new_dir->dentry));
> +	if (!rule)
> +		return 0;
> +
> +	for (size_t i = 0; i < rule->num_layers; i++)
> +		access |= rule->layers[i].access;
> +
> +	/* Only compares the access rights which make sense for files. */
> +	if (!d_is_dir(old_dentry))
> +		access &= ACCESS_FILE;
> +
> +	access = landlock_init_layer_masks(subject->domain, access,
> +					   &layer_masks, LANDLOCK_KEY_INODE);
> +	is_access_to_paths_allowed(subject->domain, &old_path, access,
> +				   &layer_masks, NULL, NULL, 0, NULL, NULL,
> +				   NULL);
> +	return grant_created(subject->domain, rule,
> +			     d_backing_inode(old_dentry), &layer_masks);
> +}

[Severity: High]
Can refer_created() pass an uninitialized layer_masks struct on the stack to
grant_created()?

When old_dentry is a non-directory (!d_is_dir(old_dentry)), access is masked
with ACCESS_FILE. If the ruleset handles all ACCESS_FILE rights and the
LANDLOCK_ADD_RULE_CREATED rule on new_dir only grants directory rights (such
as LANDLOCK_ACCESS_FS_READ_DIR | LANDLOCK_ACCESS_FS_MAKE_REG), access becomes
0 after access &= ACCESS_FILE.

When access is 0, landlock_init_layer_masks() in security/landlock/domain.c
returns early without initializing layer_masks:

	if (!access_request)
		return 0;

and is_access_to_paths_allowed() in security/landlock/fs.c also returns early
without writing to layer_masks:

	if (!access_request_parent1 && !access_request_parent2)
		return true;

Wouldn't grant_created() then read uninitialized stack memory from
src_masks->layers[layer->level - 1].access?

> +
> +/* Inode hooks */
> +
> +static int hook_inode_init_security(struct inode *const inode,
> +				    struct inode *const dir,
> +				    const struct qstr *const qstr,
> +				    struct xattr *const xattrs,
> +				    int *const xattr_count)
> +{
> +	const struct landlock_cred_security *const subject =
> +		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
> +	const struct landlock_rule *rule;
> +	int err;
> +
> +	if (!subject || !dir ||
> +	    RB_EMPTY_ROOT(&subject->domain->rules.root_inode_created))
> +		return -EOPNOTSUPP;
> +
> +	rule = get_created_rule(subject->domain, dir);
> +	if (!rule)
> +		return -EOPNOTSUPP;
> +
> +	/* Landlock does not provide any xattr. */
> +	err = grant_created(subject->domain, rule, inode, NULL);
> +	return err ?: -EOPNOTSUPP;
> +}

[Severity: High]
What happens to the newly allocated I_NEW | I_CREATING inode if creation fails
after hook_inode_init_security() pins it in hierarchy->created?

In hook_inode_init_security(), grant_created() calls get_inode_object(inode)
(which calls ihold(inode), incrementing i_count from 1 to 2) and
landlock_add_created() (which holds a reference to object in
hierarchy->created until the domain hierarchy is freed).

If inode creation subsequently fails — for example, in grant_created() itself
when layer 0 succeeds and a subsequent layer fails with -ENOMEM, in a later
LSM hook or initxattrs() callback inside security_inode_init_security(), when
the filesystem fails to add the directory entry (e.g. -ENOSPC, -EDQUOT,
-EMLINK, -ENOMEM), or when vfs_tmpfile() later fails in may_open() or
hook_file_open() — the filesystem's error path calls discard_new_inode(),
iget_failed(), or iput():

fs/inode.c:discard_new_inode() {
	...
	inode_state_clear(inode, I_NEW);
	inode_wake_up_bit(inode, __I_NEW);
	spin_unlock(&inode->i_lock);
	iput(inode);
}

Because hierarchy->created still holds a reference to the inode, iput(inode)
only drops i_count from 2 to 1 instead of running iput_final() -> evict() ->
remove_inode_hash(). For filesystems that hash new inodes via
insert_inode_locked() before calling security_inode_init_security() (such as
ext2, jfs, gfs2, btrfs, or sysv), doesn't this leave an uninstantiated inode
with I_CREATING set in inode_hashtable until the domain exits, causing
subsequent allocations of the reused on-disk inode number to fail with -EBUSY
in insert_inode_locked() or -ESTALE in find_inode_fast()?

[Severity: High]
Is it safe for hook_inode_init_security() to allocate memory with
GFP_KERNEL_ACCOUNT via grant_created()?

Filesystems invoke security_inode_init_security() in the middle of inode
creation while holding internal filesystem transactions, mutexes, glocks, or
locked metadata folios without memalloc_nofs_save() (for example, jfs_create()
holds a transaction and commit_mutex, f2fs_init_inode_metadata() holds
f2fs_lock_op() and a locked inode folio, btrfs_create_new_inode() holds a
transaction handle, and gfs2_create_inode() holds glocks), which is why
security_inode_init_security() in security/security.c uses GFP_NOFS.

However, hook_inode_init_security() -> grant_created() allocates struct
landlock_object in get_inode_object() -> landlock_create_object() and struct
landlock_created in landlock_add_created() using GFP_KERNEL_ACCOUNT (which
includes __GFP_FS). Can direct reclaim recurse into the filesystem and
deadlock on those held locks or transactions?

[Severity: Medium]
Does relying on hook_inode_init_security() miss filesystems that do not call
security_inode_init_security(), and keep unlinked files' backing storage
allocated until the domain exits?

First, landlock_append_fs_rule() accepts LANDLOCK_ADD_RULE_CREATED on any
directory, but security_inode_init_security() is not called at the VFS layer:
in ext4, ext2, f2fs, jfs, jffs2, and ubifs it is only compiled in when
CONFIG_*_FS_SECURITY=y (where CONFIG_EXT4_FS_SECURITY and
CONFIG_F2FS_FS_SECURITY default to n), and in filesystems without security
xattr support (such as fuse/virtiofs, nfs, 9p, cifs, vfat, exfat, ntfs3, udf,
and nilfs2) it is never called at all. On those filesystems or kernel
configurations, creating a file with open(O_CREAT | O_RDWR) in a
LANDLOCK_ADD_RULE_CREATED directory will never trigger
hook_inode_init_security(), causing the file to be created on disk and then
rejected with -EACCES in hook_file_open().

Second, because landlock_add_created() pins each created inode via ihold() in
hierarchy->created until landlock_free_created() runs when the layer's
hierarchy is freed, iput_final() -> evict() cannot run when created files are
unlinked. If a long-lived sandbox or build daemon creates and unlinks temporary
files in a LANDLOCK_ADD_RULE_CREATED directory, won't their on-disk blocks and
tmpfs pages remain allocated until every task in that domain layer exits?

> @@ -1623,7 +1767,8 @@ static int hook_path_link(struct dentry *const old_dentry,
>  			  struct dentry *const new_dentry)
>  {
>  	return current_check_refer_path(old_dentry, new_dir, new_dentry, false,
> -					false, false);
> +					false, false) ?:
> +		       refer_created(old_dentry, new_dir);
>  }
>  
>  static int hook_path_rename(const struct path *const old_dir,
> @@ -1632,10 +1777,16 @@ static int hook_path_rename(const struct path *const old_dir,
>  			    struct dentry *const new_dentry,
>  			    const unsigned int flags)
>  {
> +	int err;
> +
>  	/* old_dir refers to old_dentry->d_parent and new_dir->mnt */
> -	return current_check_refer_path(old_dentry, new_dir, new_dentry, true,
> -					!!(flags & RENAME_EXCHANGE),
> -					!!(flags & RENAME_WHITEOUT));
> +	err = current_check_refer_path(old_dentry, new_dir, new_dentry, true,
> +				       !!(flags & RENAME_EXCHANGE),
> +				       !!(flags & RENAME_WHITEOUT));
> +	if (err || (flags & RENAME_EXCHANGE))
> +		return err;
> +
> +	return refer_created(old_dentry, new_dir);
>  }

[Severity: High]
Doesn't calling refer_created() in hook_path_link() and hook_path_rename()
permanently grant access rights and pin the inode in hierarchy->created before
the VFS validates or performs the link or rename?

In fs/namei.c, security_path_link() and security_path_rename() run before
vfs_link() and vfs_rename():

fs/namei.c:filename_renameat2() {
	...
	error = security_path_rename(&old_path, rd.old_dentry,
				     &new_path, rd.new_dentry, flags);
	if (error)
		goto exit_unlock;

	error = vfs_rename(&rd);
	...
}

At the point hook_path_link() and hook_path_rename() run, the VFS has not yet
checked DAC permissions (may_create(), may_delete_dentry(),
inode_permission()), file-type constraints (such as S_ISDIR() returning -EPERM
in vfs_link(), or -EISDIR / -ENOTDIR / -ENOTEMPTY in vfs_rename()),
subsequent LSM hooks (security_inode_link(), security_inode_rename()), or the
filesystem's ->link() and ->rename() operations.

If vfs_link() or vfs_rename() fails, refer_created() has already inserted
old_dentry's inode into hierarchy->created and pinned it via ihold(). Could a
sandboxed task issue a link() or rename() that is guaranteed to fail in the
VFS (for example, calling link() on a directory, which fails with -EPERM in
vfs_link(), or renaming onto an incompatible target or DAC-unwritable
directory) to permanently stamp new_dir's LANDLOCK_ADD_RULE_CREATED rights
onto an inode in another directory, and then move that inode into a more
restricted directory while retaining those rights?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024-landlock-created-rights-v1-0-77c8db3f6bd2@gmail.com?part=2

[PATCH RFC 3/3] selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED

[Farid Zakaria]

Cover the new flag: entries created directly in the directory are
usable while the directory and its existing entries stay unreadable, a
renamed file keeps the rights it had at its source and gains none, a
parent layer covers the files a nested domain creates, an unrelated
domain with the same policy cannot use them, and invalid flag
combinations are rejected.

Assisted-by: LLM
Signed-off-by: Farid Zakaria <farid.m.zakaria@gmail.com>
---
 tools/testing/selftests/landlock/fs_test.c | 227 +++++++++++++++++++++++++++++
 1 file changed, 227 insertions(+)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 7e25a6f6073c..84980d426436 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -2640,6 +2640,233 @@ TEST_F_FORK(layout1, refer_mount_root_deny)
 	EXPECT_EQ(0, close(root_fd));
 }
 
+/* clang-format off */
+#define ACCESS_CREATED ( \
+	ACCESS_RW | \
+	LANDLOCK_ACCESS_FS_MAKE_REG | \
+	LANDLOCK_ACCESS_FS_MAKE_DIR)
+/* clang-format on */
+
+/*
+ * Creates a ruleset where dir_s2d1 plays a shared store: tasks may create and
+ * remove entries in it and use what they create, but not read it otherwise.
+ * dir_s1d1 is a read-write source and dir_s3d1 a source without read access.
+ */
+static int create_created_ruleset(struct __test_metadata *const _metadata)
+{
+	const struct rule rules[] = {
+		{
+			.path = dir_s1d1,
+			.access = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+				  LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		},
+		{
+			.path = dir_s2d1,
+			.access = LANDLOCK_ACCESS_FS_MAKE_REG |
+				  LANDLOCK_ACCESS_FS_MAKE_DIR |
+				  LANDLOCK_ACCESS_FS_REFER |
+				  LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		},
+		{
+			.path = dir_s3d1,
+			.access = LANDLOCK_ACCESS_FS_REFER |
+				  LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		},
+		{},
+	};
+	const int ruleset_fd = create_ruleset(
+		_metadata, ACCESS_CREATED | LANDLOCK_ACCESS_FS_REFER |
+				   LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		rules);
+
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_CREATED, dir_s2d1,
+			 LANDLOCK_ADD_RULE_CREATED);
+	return ruleset_fd;
+}
+
+/* Creates a new regular file and returns 0 or the open(2) errno. */
+static int create_file_at(const char *const path)
+{
+	const int fd = open(path, O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0700);
+
+	if (fd < 0)
+		return errno;
+	close(fd);
+	return 0;
+}
+
+/*
+ * Checks that entries created directly in a LANDLOCK_ADD_RULE_CREATED
+ * directory are usable, while the directory, its pre-existing entries, and
+ * entries created deeper in pre-existing subdirectories stay inaccessible.
+ */
+TEST_F_FORK(layout1, created_rule)
+{
+	const char *const new_file = TMP_DIR "/s2d1/new";
+	const char *const new_dir = TMP_DIR "/s2d1/newdir";
+	const char *const new_nested = TMP_DIR "/s2d1/newdir/f";
+	const int ruleset_fd = create_created_ruleset(_metadata);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* The directory and its existing entries are not readable. */
+	ASSERT_EQ(EACCES, test_open(dir_s2d1, O_RDONLY | O_DIRECTORY));
+	ASSERT_EQ(EACCES, test_open(file1_s2d1, O_RDONLY));
+	ASSERT_EQ(EACCES, test_open(dir_s2d2, O_RDONLY | O_DIRECTORY));
+
+	/* New entries are usable. */
+	ASSERT_EQ(0, create_file_at(new_file));
+	ASSERT_EQ(0, test_open(new_file, O_RDWR));
+	ASSERT_EQ(0, mkdir(new_dir, 0700));
+	ASSERT_EQ(0, test_open(new_dir, O_RDONLY | O_DIRECTORY));
+	ASSERT_EQ(0, create_file_at(new_nested));
+	ASSERT_EQ(0, test_open(new_nested, O_RDWR));
+
+	/*
+	 * Only direct children of the directory are covered.  The file is
+	 * created before open(2) fails, and the layout teardown expects
+	 * dir_s2d2 to contain only its own files.
+	 */
+	ASSERT_EQ(EACCES, create_file_at(TMP_DIR "/s2d1/s2d2/new"));
+	ASSERT_EQ(0, unlink(TMP_DIR "/s2d1/s2d2/new"));
+}
+
+/*
+ * Checks that a file renamed into a LANDLOCK_ADD_RULE_CREATED directory keeps
+ * the access rights it had at its source and gains none it did not have, by
+ * moving one file from a readable source and one from an unreadable source.
+ */
+TEST_F_FORK(layout1, created_rule_refer)
+{
+	const char *const moved_rw = TMP_DIR "/s2d1/moved_rw";
+	const char *const moved_ro = TMP_DIR "/s2d1/moved_ro";
+	const int ruleset_fd = create_created_ruleset(_metadata);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* Keeps the access rights a file already had at its source. */
+	ASSERT_EQ(0, rename(file1_s1d1, moved_rw));
+	ASSERT_EQ(0, test_open(moved_rw, O_RDWR));
+
+	/* Does not gain access rights a file did not have at its source. */
+	ASSERT_EQ(EACCES, test_open(file1_s3d1, O_RDONLY));
+	ASSERT_EQ(0, rename(file1_s3d1, moved_ro));
+	ASSERT_EQ(EACCES, test_open(moved_ro, O_RDONLY));
+}
+
+/*
+ * Checks that the layer holding the LANDLOCK_ADD_RULE_CREATED rule grants
+ * access to files created by a nested domain, by stacking a permissive layer
+ * and creating the file from within it.
+ */
+TEST_F_FORK(layout1, created_rule_nested_domain)
+{
+	const char *const new_file = TMP_DIR "/s2d1/new";
+	const struct rule rules[] = {
+		{
+			.path = TMP_DIR,
+			.access = ACCESS_CREATED,
+		},
+		{},
+	};
+	const int ruleset_fd = create_created_ruleset(_metadata);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+	enforce_fs(_metadata, ACCESS_CREATED, rules);
+
+	/* The parent layer grants access to what nested domains create. */
+	ASSERT_EQ(0, create_file_at(new_file));
+	ASSERT_EQ(0, test_open(new_file, O_RDWR));
+	ASSERT_EQ(EACCES, test_open(file1_s2d1, O_RDONLY));
+}
+
+/*
+ * Checks that an unrelated domain with the same policy cannot use a file
+ * another domain created, by keeping the creating child alive while the
+ * parent enforces its own domain and tries to read the file.
+ */
+TEST_F_FORK(layout1, created_rule_other_domain)
+{
+	const char *const new_file = TMP_DIR "/s2d1/new";
+	int pipe_created[2], pipe_done[2], status;
+	pid_t child;
+	char buf;
+
+	ASSERT_EQ(0, pipe2(pipe_created, O_CLOEXEC));
+	ASSERT_EQ(0, pipe2(pipe_done, O_CLOEXEC));
+
+	child = fork();
+	ASSERT_LE(0, child);
+	if (child == 0) {
+		const int ruleset_fd = create_created_ruleset(_metadata);
+
+		ASSERT_EQ(0, close(pipe_created[0]));
+		ASSERT_EQ(0, close(pipe_done[1]));
+
+		enforce_ruleset(_metadata, ruleset_fd);
+		ASSERT_EQ(0, create_file_at(new_file));
+		ASSERT_EQ(1, write(pipe_created[1], ".", 1));
+		ASSERT_EQ(1, read(pipe_done[0], &buf, 1));
+		_exit(_metadata->exit_code);
+	}
+
+	/*
+	 * Closing the unused ends makes a read return if the other process
+	 * fails before writing.
+	 */
+	ASSERT_EQ(0, close(pipe_created[1]));
+	ASSERT_EQ(0, close(pipe_done[0]));
+
+	/* A domain with the same policy can't use what another one created. */
+	ASSERT_EQ(1, read(pipe_created[0], &buf, 1));
+	enforce_ruleset(_metadata, create_created_ruleset(_metadata));
+	ASSERT_EQ(EACCES, test_open(new_file, O_RDONLY));
+	ASSERT_EQ(1, write(pipe_done[1], ".", 1));
+
+	ASSERT_EQ(child, waitpid(child, &status, 0));
+	ASSERT_EQ(1, WIFEXITED(status));
+	ASSERT_EQ(EXIT_SUCCESS, WEXITSTATUS(status));
+}
+
+/*
+ * Checks that LANDLOCK_ADD_RULE_CREATED is rejected on a file, combined with
+ * LANDLOCK_ADD_RULE_QUIET, or with an empty allowed_access.
+ */
+TEST_F_FORK(layout1, created_rule_invalid)
+{
+	const int ruleset_fd = create_ruleset(_metadata, ACCESS_CREATED, NULL);
+	struct landlock_path_beneath_attr path_beneath = {
+		.allowed_access = ACCESS_RW,
+	};
+
+	path_beneath.parent_fd = open(file1_s1d1, O_PATH | O_CLOEXEC);
+	ASSERT_LE(0, path_beneath.parent_fd);
+	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+					&path_beneath,
+					LANDLOCK_ADD_RULE_CREATED));
+	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(0, close(path_beneath.parent_fd));
+
+	path_beneath.parent_fd = open(dir_s1d1, O_PATH | O_CLOEXEC);
+	ASSERT_LE(0, path_beneath.parent_fd);
+	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+					&path_beneath,
+					LANDLOCK_ADD_RULE_CREATED |
+						LANDLOCK_ADD_RULE_QUIET));
+	ASSERT_EQ(EINVAL, errno);
+
+	path_beneath.allowed_access = 0;
+	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
+					&path_beneath,
+					LANDLOCK_ADD_RULE_CREATED));
+	ASSERT_EQ(ENOMSG, errno);
+	ASSERT_EQ(0, close(path_beneath.parent_fd));
+	ASSERT_EQ(0, close(ruleset_fd));
+}
+
 TEST_F_FORK(layout1, refer_part_mount_tree_is_allowed)
 {
 	const struct rule layer1[] = {

-- 
2.55.0

Re: [PATCH RFC 3/3] selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] selftests/landlock: tautological EINVAL checks in created_rule_invalid()
--

commit 0bd817b616774ee644b06b2a2368b4843876b252
Author: Farid Zakaria <farid.m.zakaria@gmail.com>

selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED

This commit adds Landlock filesystem selftests covering the
LANDLOCK_ADD_RULE_CREATED flag, including newly created entries, renamed
files, nested domains, independent domains, and invalid rule arguments.

> diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
> index 7e25a6f6073c0..84980d426436b 100644
> --- a/tools/testing/selftests/landlock/fs_test.c
> +++ b/tools/testing/selftests/landlock/fs_test.c
> @@ -2640,6 +2640,233 @@ TEST_F_FORK(layout1, refer_mount_root_deny)

[ ... ]

> +/*
> + * Checks that LANDLOCK_ADD_RULE_CREATED is rejected on a file, combined with
> + * LANDLOCK_ADD_RULE_QUIET, or with an empty allowed_access.
> + */
> +TEST_F_FORK(layout1, created_rule_invalid)
> +{
> +	const int ruleset_fd = create_ruleset(_metadata, ACCESS_CREATED, NULL);
> +	struct landlock_path_beneath_attr path_beneath = {
> +		.allowed_access = ACCESS_RW,
> +	};
> +
> +	path_beneath.parent_fd = open(file1_s1d1, O_PATH | O_CLOEXEC);
> +	ASSERT_LE(0, path_beneath.parent_fd);
> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
> +					&path_beneath,
> +					LANDLOCK_ADD_RULE_CREATED));
> +	ASSERT_EQ(EINVAL, errno);
> +	ASSERT_EQ(0, close(path_beneath.parent_fd));
> +
> +	path_beneath.parent_fd = open(dir_s1d1, O_PATH | O_CLOEXEC);
> +	ASSERT_LE(0, path_beneath.parent_fd);
> +	ASSERT_EQ(-1, landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
> +					&path_beneath,
> +					LANDLOCK_ADD_RULE_CREATED |
> +						LANDLOCK_ADD_RULE_QUIET));
> +	ASSERT_EQ(EINVAL, errno);

[Severity: Medium]
Do these two checks in created_rule_invalid() actually exercise the
LANDLOCK_ADD_RULE_CREATED validation paths?

In the first check on file1_s1d1, path_beneath.allowed_access is set to
ACCESS_RW, which includes the directory-only right
LANDLOCK_ACCESS_FS_READ_DIR:

tools/testing/selftests/landlock/fs_test.c:
    #define ACCESS_RO ( \
        LANDLOCK_ACCESS_FS_READ_FILE | \
        LANDLOCK_ACCESS_FS_READ_DIR)

    #define ACCESS_RW ( \
        ACCESS_RO | \
        LANDLOCK_ACCESS_FS_WRITE_FILE)

In security/landlock/fs.c:landlock_append_fs_rule(), a rule on a regular
file is rejected with -EINVAL whenever access_rights is not a subset of
ACCESS_FILE:

    if (!d_is_dir(path->dentry) &&
        ((flags & LANDLOCK_ADD_RULE_CREATED) ||
         !access_mask_subset(access_rights, ACCESS_FILE)))
            return -EINVAL;

Because ACCESS_RW is not a subset of ACCESS_FILE, landlock_add_rule() on
file1_s1d1 will return -1 with errno set to EINVAL even without
LANDLOCK_ADD_RULE_CREATED.

In the second check on dir_s1d1, ruleset_fd is created via
create_ruleset(), which leaves quiet_access_fs as 0. In
security/landlock/syscalls.c:add_rule_path_beneath(), passing
LANDLOCK_ADD_RULE_QUIET on a ruleset with quiet_access.fs == 0 also
returns -EINVAL:

    if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_access.fs)
            return -EINVAL;

Would it make sense to use file-only access rights (such as
LANDLOCK_ACCESS_FS_READ_FILE | LANDLOCK_ACCESS_FS_WRITE_FILE) for the
file1_s1d1 check and a ruleset with quiet_access_fs configured for the
LANDLOCK_ADD_RULE_QUIET check so that both calls would succeed if not for
LANDLOCK_ADD_RULE_CREATED?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261024-landlock-created-rights-v1-0-77c8db3f6bd2@gmail.com?part=3

Re: [PATCH RFC 0/3] landlock: Add access rights on created files

[Justin Suess]

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.
Hi,

I'll chime in since I run nix and have a little bit of familiarity.

Looking at my (admittedly bloated) nix store.

[justin@zenbox ~]$ ls /nix/store | wc -l
259356

That's a lot of inodes. And a nix derivation creates hundreds if not
thousands potentially throughout it's build.

My main concern is that rulesets are generally considered
immutable after enforcement. This allows for well-defined semantics on
inheritence. This from my understanding would mutate the ruleset after
it's already enforced, so changes wouldn't be copied to inherited
domains automatically.

My other concern is with the overhead of automatically generating an inserting
a rule for every new inode that gets created. It seems like a complex
way to tackle this problem.

But beyond that, let's go back to your usecase.

....

Basically from what I'm seeing you want to grant some access to certain
subdirectories (including newly created ones) but exclude others.

Presumably in the nix case, you want to grant /nix/store/{abc,efg,hij} and NOT
/nix/store/{xyz}, but still allow /nix/store/{foo} (the derivation
output) to be created and written to. This would allow the nix build to only
have access to certain inputs, while being able to write to the /nix/store
and produce an output. This is a valid usecase that Landlock cannot today
accomplish.

There was this NO_INHERIT series I proposed a little while back.
[1] It's sort of the inverse of this (removes accesses inherited from
parents rather than granting new access) but would accomplish this
goal.

With NO_INHERIT, you could grant whatever access rights you want by default on
the parent directory, and place NO_INHERIT on any specific entries you
don't want to have inherit those rights. 

So for nix use case, you could grant no_inherit on everything except the
inputs you need, then add READ/WRITE/MKDIR etc on /nix/store. The new
directories would inherit this by default, while the ones you added
NO_INHERIT on would be restricted.

Additionally, you should also examine the mutable domains proposal by
Tingmao Wang, which could also facilitate this usecase as well [2] (i.e by
dynamically adding and removing rules with some fanotify/inotify glue).

That being said, neither of these two patchsets / proposals are merged,
and the playing field is still open on this.

So the problem you are pointing to is legitamate, but the solution still
needs to be considered carefully to avoid complexity.

Justin

[1] https://lore.kernel.org/linux-security-module/20260621035223.2651547-1-utilityemal77@gmail.com/
[2] https://lore.kernel.org/linux-security-module/cover.1741047969.git.m@maowtm.org/
> 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
> 
> 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.
> 
> 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?
> 
> - 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?
> 
> - 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()?
> 
> - A file renamed or linked in keeps only the rights it had at its source.
>   Should it get none instead?
> 
> ---
> Farid Zakaria (3):
>       lsm: Call inode_init_security hooks without xattrs
>       landlock: Add access rights on created files
>       selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED
> 
>  Documentation/userspace-api/landlock.rst     |  42 +++++
>  include/uapi/linux/landlock.h                |  18 +++
>  security/landlock/domain.c                   | 182 ++++++++++++++++++++-
>  security/landlock/domain.h                   |  28 ++++
>  security/landlock/fs.c                       | 176 +++++++++++++++++++--
>  security/landlock/object.h                   |   6 +
>  security/landlock/ruleset.c                  |   6 +
>  security/landlock/ruleset.h                  |  14 ++
>  security/landlock/syscalls.c                 |  20 ++-
>  security/security.c                          |   9 +-
>  tools/testing/selftests/landlock/base_test.c |   2 +-
>  tools/testing/selftests/landlock/fs_test.c   | 227 +++++++++++++++++++++++++++
>  12 files changed, 703 insertions(+), 27 deletions(-)
> ---
> base-commit: 3e271a27c46fd591d1011c7de5a644a41e2010f1
> change-id: 20261024-landlock-created-rights-9340b00a8c4d
> 
> Best regards,
> --  
> Farid Zakaria <farid.m.zakaria@gmail.com>
> 
> 

Re: [PATCH RFC 0/3] landlock: Add access rights on created files

[Justin Suess]

On Sat, Oct 24, 2026 at 05:43:26PM -0400, Justin Suess wrote:
> 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.
> Hi,
> 
> I'll chime in since I run nix and have a little bit of familiarity.
> 
> Looking at my (admittedly bloated) nix store.
> 
> [justin@zenbox ~]$ ls /nix/store | wc -l
> 259356
> 
> That's a lot of inodes. And a nix derivation creates hundreds if not
> thousands potentially throughout it's build.
> 
> My main concern is that rulesets are generally considered
> immutable after enforcement. This allows for well-defined semantics on
> inheritence. This from my understanding would mutate the ruleset after
> it's already enforced, so changes wouldn't be copied to inherited
> domains automatically.

* nvm on this, it looks like you are adding them to a different
heirarchy. The point still stands though on the inode pinning, which you
pointed out.


> 
> My other concern is with the overhead of automatically generating an inserting
> a rule for every new inode that gets created. It seems like a complex
> way to tackle this problem.
> 
> But beyond that, let's go back to your usecase.
> 
> ....
> 
> Basically from what I'm seeing you want to grant some access to certain
> subdirectories (including newly created ones) but exclude others.
> 
> Presumably in the nix case, you want to grant /nix/store/{abc,efg,hij} and NOT
> /nix/store/{xyz}, but still allow /nix/store/{foo} (the derivation
> output) to be created and written to. This would allow the nix build to only
> have access to certain inputs, while being able to write to the /nix/store
> and produce an output. This is a valid usecase that Landlock cannot today
> accomplish.
> 
> There was this NO_INHERIT series I proposed a little while back.
> [1] It's sort of the inverse of this (removes accesses inherited from
> parents rather than granting new access) but would accomplish this
> goal.
> 
> With NO_INHERIT, you could grant whatever access rights you want by default on
> the parent directory, and place NO_INHERIT on any specific entries you
> don't want to have inherit those rights. 
>
It seems like NO_INHERIT too might scale poorly for the usecase. Since then
you are trading O(store) for O(inputs) so not much better (strictly
worse).

Thoughts?

Justin
> So for nix use case, you could grant no_inherit on everything except the
> inputs you need, then add READ/WRITE/MKDIR etc on /nix/store. The new
> directories would inherit this by default, while the ones you added
> NO_INHERIT on would be restricted.
> 
> Additionally, you should also examine the mutable domains proposal by
> Tingmao Wang, which could also facilitate this usecase as well [2] (i.e by
> dynamically adding and removing rules with some fanotify/inotify glue).
> 
> That being said, neither of these two patchsets / proposals are merged,
> and the playing field is still open on this.
> 
> So the problem you are pointing to is legitamate, but the solution still
> needs to be considered carefully to avoid complexity.
> 
> Justin
> 
> [1] https://lore.kernel.org/linux-security-module/20260621035223.2651547-1-utilityemal77@gmail.com/
> [2] https://lore.kernel.org/linux-security-module/cover.1741047969.git.m@maowtm.org/
> > 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
> > 
> > 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.
> > 
> > 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?
> > 
> > - 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?
> > 
> > - 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()?
> > 
> > - A file renamed or linked in keeps only the rights it had at its source.
> >   Should it get none instead?
> > 
> > ---
> > Farid Zakaria (3):
> >       lsm: Call inode_init_security hooks without xattrs
> >       landlock: Add access rights on created files
> >       selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED
> > 
> >  Documentation/userspace-api/landlock.rst     |  42 +++++
> >  include/uapi/linux/landlock.h                |  18 +++
> >  security/landlock/domain.c                   | 182 ++++++++++++++++++++-
> >  security/landlock/domain.h                   |  28 ++++
> >  security/landlock/fs.c                       | 176 +++++++++++++++++++--
> >  security/landlock/object.h                   |   6 +
> >  security/landlock/ruleset.c                  |   6 +
> >  security/landlock/ruleset.h                  |  14 ++
> >  security/landlock/syscalls.c                 |  20 ++-
> >  security/security.c                          |   9 +-
> >  tools/testing/selftests/landlock/base_test.c |   2 +-
> >  tools/testing/selftests/landlock/fs_test.c   | 227 +++++++++++++++++++++++++++
> >  12 files changed, 703 insertions(+), 27 deletions(-)
> > ---
> > base-commit: 3e271a27c46fd591d1011c7de5a644a41e2010f1
> > change-id: 20261024-landlock-created-rights-9340b00a8c4d
> > 
> > Best regards,
> > --  
> > Farid Zakaria <farid.m.zakaria@gmail.com>
> > 
> > 

Re: [PATCH RFC 0/3] landlock: Add access rights on created files

[Farid Zakaria]

Hi Justin,

Thank you for engaging me on this RFC. Admittedly, I have not followed
the landlock discussions closely. This was an idea I had concocted while
thinking how we can make Nix sandboxing faster.
(I'm also exploring the space with AI - so I'm a landlock newbie but not
a Nix newbie.)

Mickael, et al: I see that sashiko found some legitimate concerns.
I can fix that up in a V2 if we like the general idea and want to
continue exploring it.

On Sat Oct 24, 2026 at 3:03 PM PDT, Justin Suess wrote:
> On Sat, Oct 24, 2026 at 05:43:26PM -0400, Justin Suess wrote:
>> 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.
>> Hi,
>> 
>> I'll chime in since I run nix and have a little bit of familiarity.
>> 
>> Looking at my (admittedly bloated) nix store.
>> 
>> [justin@zenbox ~]$ ls /nix/store | wc -l
>> 259356
>> 
>> That's a lot of inodes. And a nix derivation creates hundreds if not
>> thousands potentially throughout it's build.
>> 
>> My main concern is that rulesets are generally considered
>> immutable after enforcement. This allows for well-defined semantics on
>> inheritence. This from my understanding would mutate the ruleset after
>> it's already enforced, so changes wouldn't be copied to inherited
>> domains automatically.
>
> * nvm on this, it looks like you are adding them to a different
> heirarchy. The point still stands though on the inode pinning, which you
> pointed out.
>
>

On the cost:

only entries created directly in the directory that has
the rule get recorded. For Nix that is /nix/store, so a build records
one entry per output ($out, $dev, ...). Nothing is recorded for
$out/foo or anything else under $out.

Access there is allowed because the path walk at access time reaches
$out and finds it in the recorded set, the same way a rule on a directory
covers what is beneath it. Existing store paths are never recorded, so the
store's size does not matter... right?

A bad actor could still create many entries in the store
(touch /nix/store/junk1 ... junk1000000), and each one stays pinned
until the domain is freed. I can add a per-domain limit on the number
of created entries. Nix only needs as many as the derivation has
outputs ($out, $dev etc..).

>> 
>> My other concern is with the overhead of automatically generating an inserting
>> a rule for every new inode that gets created. It seems like a complex
>> way to tackle this problem.
>> 
>> But beyond that, let's go back to your usecase.
>> 
>> ....
>> 
>> Basically from what I'm seeing you want to grant some access to certain
>> subdirectories (including newly created ones) but exclude others.
>> 
>> Presumably in the nix case, you want to grant /nix/store/{abc,efg,hij} and NOT
>> /nix/store/{xyz}, but still allow /nix/store/{foo} (the derivation
>> output) to be created and written to. This would allow the nix build to only
>> have access to certain inputs, while being able to write to the /nix/store
>> and produce an output. This is a valid usecase that Landlock cannot today
>> accomplish.
>> 
>> There was this NO_INHERIT series I proposed a little while back.
>> [1] It's sort of the inverse of this (removes accesses inherited from
>> parents rather than granting new access) but would accomplish this
>> goal.
>> 
>> With NO_INHERIT, you could grant whatever access rights you want by default on
>> the parent directory, and place NO_INHERIT on any specific entries you
>> don't want to have inherit those rights. 
>>
> It seems like NO_INHERIT too might scale poorly for the usecase. Since then
> you are trading O(store) for O(inputs) so not much better (strictly
> worse).
>
> Thoughts?
>
> Justin

On NO_INHERIT: besides the O(store) cost, other builds keep adding
paths to the store while this one runs. Those new paths have no
NO_INHERIT rule, so they would inherit the store-wide access and be
readable by the build. With the CREATED flag, nothing is readable
unless a rule covers it.

>> So for nix use case, you could grant no_inherit on everything except the
>> inputs you need, then add READ/WRITE/MKDIR etc on /nix/store. The new
>> directories would inherit this by default, while the ones you added
>> NO_INHERIT on would be restricted.
>> 
>> Additionally, you should also examine the mutable domains proposal by
>> Tingmao Wang, which could also facilitate this usecase as well [2] (i.e by
>> dynamically adding and removing rules with some fanotify/inotify glue).
>> 
>> That being said, neither of these two patchsets / proposals are merged,
>> and the playing field is still open on this.
>> 
>> So the problem you are pointing to is legitamate, but the solution still
>> needs to be considered carefully to avoid complexity.
>> 
>> Justin
>> 
>> [1] https://lore.kernel.org/linux-security-module/20260621035223.2651547-1-utilityemal77@gmail.com/
>> [2] https://lore.kernel.org/linux-security-module/cover.1741047969.git.m@maowtm.org/
>> > 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
>> > 
>> > 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.
>> > 
>> > 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?
>> > 
>> > - 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?
>> > 
>> > - 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()?
>> > 
>> > - A file renamed or linked in keeps only the rights it had at its source.
>> >   Should it get none instead?
>> > 
>> > ---
>> > Farid Zakaria (3):
>> >       lsm: Call inode_init_security hooks without xattrs
>> >       landlock: Add access rights on created files
>> >       selftests/landlock: Test LANDLOCK_ADD_RULE_CREATED
>> > 
>> >  Documentation/userspace-api/landlock.rst     |  42 +++++
>> >  include/uapi/linux/landlock.h                |  18 +++
>> >  security/landlock/domain.c                   | 182 ++++++++++++++++++++-
>> >  security/landlock/domain.h                   |  28 ++++
>> >  security/landlock/fs.c                       | 176 +++++++++++++++++++--
>> >  security/landlock/object.h                   |   6 +
>> >  security/landlock/ruleset.c                  |   6 +
>> >  security/landlock/ruleset.h                  |  14 ++
>> >  security/landlock/syscalls.c                 |  20 ++-
>> >  security/security.c                          |   9 +-
>> >  tools/testing/selftests/landlock/base_test.c |   2 +-
>> >  tools/testing/selftests/landlock/fs_test.c   | 227 +++++++++++++++++++++++++++
>> >  12 files changed, 703 insertions(+), 27 deletions(-)
>> > ---
>> > base-commit: 3e271a27c46fd591d1011c7de5a644a41e2010f1
>> > change-id: 20261024-landlock-created-rights-9340b00a8c4d
>> > 
>> > Best regards,
>> > --  
>> > Farid Zakaria <farid.m.zakaria@gmail.com>
>> > 
>> > 

Re: [PATCH RFC 0/3] landlock: Add access rights on created files

[Günther Noack]

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?


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


> 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?


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

Thanks,
–Günther