[PATCH v3 0/5] Implement LANDLOCK_ADD_RULE_NO_INHERIT

[PATCH v3 0/5] Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Hi,

This is version 3 of the LANDLOCK_ADD_RULE_NO_INHERIT series, which
implements a new flag to suppress inheritance of access rights and
flags from parent objects.

This series is rebased on v5 of Tingmao Wang's "quiet flag" series.

The new flag enables policies where a parent directory needs broader
access than its children. For example, a sandbox may permit read-write
access to /home/user but still prohibit writes to ~/.bashrc or
~/.ssh, even though they are nested beneath the parent. Today this is
not possible because access rights always propagate from parent to
child within a layer.

When a rule is added with LANDLOCK_ADD_RULE_NO_INHERIT:

  * access rights on parent inodes are ignored for that inode and its
    descendants; and
  * operations that change the direct parent subtree of such objects
    (rename, rmdir, link) are denied up to the mountpoint; and
  * parent flags do not propagate below a NO_INHERIT rule (new in v3).

These parent-directory restrictions help mitigate sandbox-restart
attacks: a sandboxed process could otherwise move a protected
directory before exit, causing the next sandbox instance to apply its
policy to the wrong path.

Changes since v2:

  1. Add six new selftests for the new flag.
  2. Add an optimization to stop permission harvesting when all
     relevant layers are tagged with NO_INHERIT.
  3. Suppress inheritance of parent flags.
  4. Rebase onto v5 of the quiet-flag series.
  5. Remove the xarray structure used for flag tracking in favor of
     blank rule insertion, simplifying the implementation.
  6. Fix edge cases involving flag inheritance across multiple
     NO_INHERIT layers.
  7. Add documenting comments to new functions.

Links:

v1:
  https://lore.kernel.org/linux-security-module/20251105180019.1432367-1-utilityemal77@gmail.com/T/#t
v2:
  https://lore.kernel.org/linux-security-module/20251120222346.1157004-1-utilityemal77@gmail.com/T/#t
quiet-flag v5:
  https://lore.kernel.org/linux-security-module/cover.1763931318.git.m@maowtm.org/T/#t

Example usage:

  # LL_FS_RO="" LL_FS_RW="/" LL_FS_RO_NO_INHERIT="/a/b/c" landlock-sandboxer sh
  # touch /a/b/c/fi                    # denied; / RW does not inherit
  # rmdir /a/b/c                       # denied due to parent-directory protections
  # mv /a /bad                         # denied
  # mkdir /a/good; touch /a/good/fi    # allowed; unrelated path

If preferred, I'm happy to split the selftests into multiple commits.

I am particularly interested in feedback on:

  * The soundness of inserting blank rules in ensure_rule_for_dentry.
    A zero-access rule is lazily inserted into parent directories on
    first access to enforce topology-change protections. This replaces
    the prior xarray tracking, and should reduce complexity and improve
    performance.

  * Additional edge cases that should be covered by new tests.

  * Performance implications of the current design.

All existing Landlock selftests and KUnit tests, as well as the new
tests added in this series, are passing.

Thank you for your time and review.

Regards,
Justin Suess

Justin Suess (5):
  landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT
  landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api
  samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to
    landlock-sandboxer
  selftests/landlock: Implement selftests for
    LANDLOCK_ADD_RULE_NO_INHERIT
  landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT

 include/uapi/linux/landlock.h              |  29 +
 samples/landlock/sandboxer.c               |  37 +-
 security/landlock/audit.c                  |   4 +-
 security/landlock/domain.c                 |   4 +-
 security/landlock/fs.c                     | 592 ++++++++++++++++++++-
 security/landlock/ruleset.c                | 116 +++-
 security/landlock/ruleset.h                |  36 +-
 security/landlock/syscalls.c               |  14 +-
 tools/testing/selftests/landlock/fs_test.c | 459 +++++++++++++++-
 9 files changed, 1249 insertions(+), 42 deletions(-)


base-commit: 91d200c5385c926c8d1f2df33a8a4160924fa977
-- 
2.51.0

[PATCH v3 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Implements a flag to prevent access grant inheritance within the filesystem hierarchy
for landlock rules.

If a landlock rule on an inode has this flag, any access grants on parent inodes will be
ignored. Moreover, operations that involve altering the direct parent tree of the subject with
LANDLOCK_ADD_RULE_NO_INHERIT will be denied up to the mountpoint.

Additionally (new in v3) parent flag inheritance is blocked by this flag, allowing fine
grained access control over LANDLOCK_ADD_RULE_QUIET.

For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and / = read write, writes to
files in /a/b/c will be denied. Moreover, moving /a to /bad, removing /a/b/c, or creating links to
/a will be prohibited.

And if / has LANDLOCK_ADD_RULE_QUIET, /a/b/c will still audit (handled)
accesses. This is because LANDLOCK_ADD_RULE_NO_INHERIT also
suppresses flag inheritance from parent objects.

The parent directory restrictions mitigate sandbox-restart attacks. For example, if a sandboxed program
is able to move a LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upon sandbox restart, the policy
applied naively on the same filenames would be invalid. Preventing these operations mitigates these attacks.

v2..v3 changes:

  * Parent directory topology protections now work by lazily
    inserting blank rules on parent inodes if they do not
    exist. This replaces the previous xarray implementation
    with simplified logic.
  * Added an optimization to skip further processing if all layers collected
    no inherit.
  * Added support to block flag inheritance.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 security/landlock/audit.c   |   4 +-
 security/landlock/domain.c  |   4 +-
 security/landlock/fs.c      | 592 +++++++++++++++++++++++++++++++++++-
 security/landlock/ruleset.c |  27 +-
 security/landlock/ruleset.h |  36 ++-
 5 files changed, 645 insertions(+), 18 deletions(-)

diff --git a/security/landlock/audit.c b/security/landlock/audit.c
index d51563712325..4da97dd6985c 100644
--- a/security/landlock/audit.c
+++ b/security/landlock/audit.c
@@ -588,7 +588,9 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
 				subject->domain, &missing, request->layer_masks,
 				request->layer_masks_size);
 			object_quiet_flag = !!(request->rule_flags.quiet_masks &
-					       BIT(youngest_layer));
+				       BIT(youngest_layer)) &&
+				!(request->rule_flags.blocked_flag_masks &
+				  BIT(youngest_layer));
 		} else {
 			youngest_layer = get_layer_from_deny_masks(
 				&missing, request->all_existing_optional_access,
diff --git a/security/landlock/domain.c b/security/landlock/domain.c
index 8caf07250328..5bd83865c87d 100644
--- a/security/landlock/domain.c
+++ b/security/landlock/domain.c
@@ -236,7 +236,9 @@ optional_access_t landlock_get_quiet_optional_accesses(
 			 BITS_PER_TYPE(access_mask_t)) {
 		const u8 layer = (deny_masks >> (access_index * 4)) &
 				 (LANDLOCK_MAX_NUM_LAYERS - 1);
-		const bool is_quiet = !!(rule_flags.quiet_masks & BIT(layer));
+		const layer_mask_t layer_bit = BIT(layer);
+		const bool is_quiet = !!(rule_flags.quiet_masks & layer_bit) &&
+				  !(rule_flags.blocked_flag_masks & layer_bit);
 
 		if (is_quiet)
 			quiet_optional_accesses |= BIT(access_index);
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 29f10da32141..0a5c73f18f26 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -317,6 +317,206 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
 	LANDLOCK_ACCESS_FS_IOCTL_DEV)
 /* clang-format on */
 
+static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
+					     const struct dentry *const dentry);
+
+/**
+ * landlock_domain_layers_mask - Build a mask covering all layers of a domain
+ * @domain: The ruleset (domain) to inspect.
+ *
+ * Return a layer mask with a 1 bit for each existing layer of @domain.
+ * If @domain has no layers 0 is returned.  If the number of layers is
+ * greater than or equal to the number of bits in layer_mask_t, all bits
+ * are set.
+ */
+static layer_mask_t landlock_domain_layers_mask(const struct landlock_ruleset
+						*const domain)
+{
+	if (!domain || !domain->num_layers)
+		return 0;
+
+	if (domain->num_layers >= sizeof(layer_mask_t) * BITS_PER_BYTE)
+		return (layer_mask_t)~0ULL;
+
+	return GENMASK_ULL(domain->num_layers - 1, 0);
+}
+
+/**
+ * rule_blocks_all_layers_no_inherit - check whether a rule disables inheritance
+ * @domain_layers_mask: Mask describing the domain's active layers.
+ * @rule: Rule to inspect.
+ *
+ * Return true if every layer present in @rule has its no_inherit flag set
+ * and the set of layers covered by the rule equals @domain_layers_mask.
+ * This indicates that the rule prevents inheritance on all layers of the
+ * domain and thus further walking for inheritance checks can stop.
+ */
+static bool rule_blocks_all_layers_no_inherit(const layer_mask_t domain_layers_mask,
+					      const struct landlock_rule *const rule)
+{
+	layer_mask_t rule_layers = 0;
+	u32 layer_index;
+
+	if (!domain_layers_mask || !rule)
+		return false;
+
+	for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+		const struct landlock_layer *const layer =
+			&rule->layers[layer_index];
+		const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
+
+		if (!layer->flags.no_inherit)
+			return false;
+
+		rule_layers |= layer_bit;
+	}
+
+	return rule_layers && rule_layers == domain_layers_mask;
+}
+
+/**
+ * landlock_collect_no_inherit_layers - Collects layers with no_inherit flags
+ */
+/**
+ * landlock_collect_no_inherit_layers - collect effective no_inherit layers
+ * @ruleset: Ruleset to consult.
+ * @dentry: Dentry used as a starting point for the upward walk.
+ *
+ * Walk upwards from @dentry and return a layer mask containing the layers
+ * for which either a rule on the visited dentry has the no_inherit flag set
+ * or where an ancestor was previously marked as having a descendant with
+ * a no_inherit rule.  The search prefers the closest matching dentry and
+ * stops once any relevant layer bits are found or the root is reached.
+ *
+ * Returns a layer_mask_t where each set bit corresponds to a layer with an
+ * effective no_inherit influence for @dentry.  Returns 0 if none apply or if
+ * inputs are invalid.
+ */
+static layer_mask_t landlock_collect_no_inherit_layers(const struct landlock_ruleset
+						       *const ruleset,
+						       struct dentry *const dentry)
+{
+	struct dentry *cursor, *parent;
+	layer_mask_t layers = 0;
+	bool include_descendants = true;
+
+	if (!ruleset || !dentry || d_is_negative(dentry))
+		return 0;
+
+	cursor = dget(dentry);
+	while (true) {
+		const struct landlock_rule *rule;
+		u32 layer_index;
+
+		rule = find_rule(ruleset, cursor);
+		if (rule) {
+			for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+				const struct landlock_layer *layer = &rule->layers[layer_index];
+
+				if (layer->flags.no_inherit ||
+				    (include_descendants &&
+				     layer->flags.has_no_inherit_descendant))
+					layers |= BIT_ULL((layer->level ?
+						layer->level : layer_index + 1) - 1);
+			}
+		}
+
+		if (layers) {
+			dput(cursor);
+			return layers;
+		}
+
+		if (IS_ROOT(cursor)) {
+			dput(cursor);
+			break;
+		}
+
+		parent = dget_parent(cursor);
+		dput(cursor);
+		if (!parent)
+			break;
+
+		cursor = parent;
+		include_descendants = false;
+	}
+	return 0;
+}
+
+static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
+				     struct dentry *dentry,
+				     layer_mask_t descendant_layers);
+
+/**
+ * mask_no_inherit_descendant_layers - apply descendant no_inherit masking
+ * @domain: The ruleset (domain) to consult.
+ * @dentry: The dentry whose descendants are considered.
+ * @child_layers: Layers present on the child that may be subject to masking.
+ * @access_request: Accesses being requested (bitmask).
+ * @layer_masks: Per-access layer masks to be modified in-place.
+ * @rule_flags: Collected flags which will be updated accordingly.
+ *
+ * If descendant dentries have no_inherit, clear that
+ * layer's bit from @layer_masks. Also updates @rule_flags to reflect
+ * which layers were blocked.  Returns true if any of the @layer_masks were
+ * modified, false otherwise.
+ */
+static bool mask_no_inherit_descendant_layers(const struct landlock_ruleset
+					      *const domain,
+					      struct dentry *const dentry,
+					      layer_mask_t child_layers,
+					      const access_mask_t access_request,
+					      layer_mask_t
+					      (*const layer_masks)
+					      [LANDLOCK_NUM_ACCESS_FS],
+					      struct collected_rule_flags
+					      *const rule_flags)
+{
+	layer_mask_t descendant_layers;
+	const unsigned long access_req = access_request;
+	unsigned long access_bit;
+	bool changed = false;
+
+	if (!access_request || !layer_masks || !rule_flags || !dentry)
+		return false;
+	if (d_is_negative(dentry))
+		return false;
+
+	descendant_layers = landlock_collect_no_inherit_layers(domain, dentry);
+	{
+		layer_mask_t shared_layers = descendant_layers & child_layers;
+
+		if (shared_layers) {
+			rule_flags->no_inherit_masks |= shared_layers;
+			rule_flags->no_inherit_desc_masks |= shared_layers;
+			rule_flags->blocked_flag_masks |= shared_layers;
+		}
+	}
+	descendant_layers &= ~child_layers;
+	descendant_layers &= ~rule_flags->no_inherit_masks;
+	if (!descendant_layers)
+		return false;
+
+	rule_flags->blocked_flag_masks |= descendant_layers;
+
+	for_each_set_bit(access_bit, &access_req,
+			 ARRAY_SIZE(*layer_masks)) {
+		layer_mask_t *const layer_mask = &(*layer_masks)[access_bit];
+
+		if (*layer_mask & descendant_layers) {
+			*layer_mask &= ~descendant_layers;
+			changed = true;
+		}
+	}
+
+	if (!changed)
+		return false;
+
+	rule_flags->no_inherit_masks |= descendant_layers;
+	rule_flags->no_inherit_desc_masks |= descendant_layers;
+
+	return true;
+}
+
 /*
  * @path: Should have been checked by get_path_from_fd().
  */
@@ -325,12 +525,13 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 			    access_mask_t access_rights, const int flags)
 {
 	int err;
+	const bool is_dir = d_is_dir(path->dentry);
 	struct landlock_id id = {
 		.type = LANDLOCK_KEY_INODE,
 	};
 
 	/* Files only get access rights that make sense. */
-	if (!d_is_dir(path->dentry) &&
+	if (!is_dir &&
 	    (access_rights | ACCESS_FILE) != ACCESS_FILE)
 		return -EINVAL;
 	if (WARN_ON_ONCE(ruleset->num_layers != 1))
@@ -344,13 +545,43 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 		return PTR_ERR(id.key.object);
 	mutex_lock(&ruleset->lock);
 	err = landlock_insert_rule(ruleset, id, access_rights, flags);
+	if (!err && (flags & LANDLOCK_ADD_RULE_NO_INHERIT)) {
+		const struct landlock_rule *rule;
+		layer_mask_t descendant_layers = 0;
+		u32 layer_index;
+
+		rule = find_rule(ruleset, path->dentry);
+		if (rule) {
+			for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
+				const struct landlock_layer *layer =
+					&rule->layers[layer_index];
+
+				if (layer->flags.no_inherit ||
+				    layer->flags.has_no_inherit_descendant)
+					descendant_layers |=
+						BIT_ULL((layer->level ?
+							 layer->level : layer_index + 1) - 1);
+			}
+			if (descendant_layers) {
+				err = mark_no_inherit_ancestors(ruleset, path->dentry,
+								descendant_layers);
+				if (err)
+					goto out_unlock;
+			}
+		}
+	}
 	mutex_unlock(&ruleset->lock);
+out:
 	/*
 	 * No need to check for an error because landlock_insert_rule()
 	 * increments the refcount for the new object if needed.
 	 */
 	landlock_put_object(id.key.object);
 	return err;
+
+out_unlock:
+	mutex_unlock(&ruleset->lock);
+	goto out;
 }
 
 /* Access-control management */
@@ -382,6 +613,134 @@ find_rule(const struct landlock_ruleset *const domain,
 	return rule;
 }
 
+/**
+ * ensure_rule_for_dentry - ensure a ruleset contains a rule entry for dentry,
+ * inserting a blank rule if needed.
+ * @ruleset: Ruleset to modify/inspect.  Caller must hold @ruleset->lock.
+ * @dentry: Dentry to ensure a rule exists for.
+ *
+ * If no rule is currently associated with @dentry, insert an empty rule
+ * (with zero access) tied to the backing inode.  Returns a pointer to the
+ * rule associated with @dentry on success, NULL when @dentry is negative, or
+ * an ERR_PTR()-encoded error if the rule cannot be created.
+ *
+ * This is useful for LANDLOCK_ADD_RULE_NO_INHERIT processing, where a rule
+ * may need to be created for an ancestor dentry that does not yet have one
+ * to properly track no_inherit flags.
+ *
+ * The flags are set to zero if a rule is newly created, and the caller
+ * is responsible for setting them appropriately.
+ *
+ * The returned rule pointer's lifetime is tied to @ruleset.
+ */
+static const struct landlock_rule *
+ensure_rule_for_dentry(struct landlock_ruleset *const ruleset,
+		       struct dentry *const dentry)
+{
+	struct landlock_id id = {
+		.type = LANDLOCK_KEY_INODE,
+	};
+	const struct landlock_rule *rule;
+	int err;
+
+	if (!ruleset || !dentry || d_is_negative(dentry))
+		return NULL;
+
+	lockdep_assert_held(&ruleset->lock);
+
+	rule = find_rule(ruleset, dentry);
+	if (rule)
+		return rule;
+
+	id.key.object = get_inode_object(d_backing_inode(dentry));
+	if (IS_ERR(id.key.object))
+		return ERR_CAST(id.key.object);
+
+	err = landlock_insert_rule(ruleset, id, 0, 0);
+	landlock_put_object(id.key.object);
+	if (err)
+		return ERR_PTR(err);
+
+	rule = find_rule(ruleset, dentry);
+	return rule ? rule : ERR_PTR(-ENOENT);
+}
+
+/**
+ * mark_no_inherit_ancestors - mark ancestors as having no_inherit descendants
+ * @ruleset: Ruleset to modify.  Caller must hold @ruleset->lock.
+ * @dentry: Dentry representing the descendant that carries no_inherit bits.
+ * @descendant_layers: Mask of layers from the descendant that should be
+ *                     advertised to ancestors via has_no_inherit_descendant.
+ *
+ * Walks upward from @dentry and ensures that any ancestor rule contains the
+ * has_no_inherit_descendant marker for the specified @descendant_layers so
+ * parent lookups can quickly detect descendant no_inherit influence.
+ *
+ * Returns 0 on success or a negative errno if ancestor bookkeeping fails.
+ */
+static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
+				     struct dentry *dentry,
+				     layer_mask_t descendant_layers)
+{
+	struct dentry *cursor;
+	u32 layer_index;
+	int err = 0;
+
+	if (!ruleset || !dentry || !descendant_layers)
+		return -EINVAL;
+
+	lockdep_assert_held(&ruleset->lock);
+
+	cursor = dget(dentry);
+	while (cursor) {
+		struct dentry *parent;
+
+		if (IS_ROOT(cursor)) {
+			dput(cursor);
+			break;
+		}
+
+		parent = dget_parent(cursor);
+		dput(cursor);
+		if (!parent)
+			break;
+
+		if (!d_is_negative(parent)) {
+			const struct landlock_rule *rule;
+			/* Ensures a rule exists for the parent dentry,
+			 * inserting a blank one if needed
+			 */
+			rule = ensure_rule_for_dentry(ruleset, parent);
+			if (IS_ERR(rule)) {
+				err = PTR_ERR(rule);
+				dput(parent);
+				cursor = NULL;
+				break;
+			}
+			if (rule) {
+				struct landlock_rule *mutable_rule =
+					(struct landlock_rule *)rule;
+
+				for (layer_index = 0;
+				     layer_index < mutable_rule->num_layers;
+				     layer_index++) {
+					struct landlock_layer *layer =
+						&mutable_rule->layers[layer_index];
+					layer_mask_t layer_bit =
+						BIT_ULL((layer->level ?
+							layer->level : layer_index + 1) - 1);
+
+					if (descendant_layers & layer_bit)
+						layer->flags.has_no_inherit_descendant = true;
+				}
+			}
+		}
+
+		cursor = parent;
+	}
+	return err;
+}
+
 /*
  * Allows access to pseudo filesystems that will never be mountable (e.g.
  * sockfs, pipefs), but can still be reachable through
@@ -764,6 +1123,8 @@ static bool is_access_to_paths_allowed(
 	struct landlock_request *const log_request_parent2,
 	struct dentry *const dentry_child2)
 {
+	const layer_mask_t domain_layers_mask =
+		landlock_domain_layers_mask(domain);
 	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
 	     is_dom_check_bkp, child1_is_directory = true,
 	     child2_is_directory = true;
@@ -778,6 +1139,13 @@ static bool is_access_to_paths_allowed(
 	struct collected_rule_flags *rule_flags_parent1 = &log_request_parent1->rule_flags;
 	struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
 	struct collected_rule_flags _rule_flag_parent1_bkp, _rule_flag_parent2_bkp;
+	layer_mask_t child1_layers = 0;
+	layer_mask_t child2_layers = 0;
+
+	if (dentry_child1)
+		child1_layers = landlock_collect_no_inherit_layers(domain, dentry_child1);
+	if (dentry_child2)
+		child2_layers = landlock_collect_no_inherit_layers(domain, dentry_child2);
 
 	if (!access_request_parent1 && !access_request_parent2)
 		return true;
@@ -931,6 +1299,10 @@ static bool is_access_to_paths_allowed(
 					       ARRAY_SIZE(*layer_masks_parent2),
 					       rule_flags_parent2);
 
+		if (rule &&
+		    rule_blocks_all_layers_no_inherit(domain_layers_mask, rule))
+			break;
+
 		/* Stops when a rule from each layer grants access. */
 		if (allowed_parent1 && allowed_parent2) {
 			/*
@@ -976,8 +1348,13 @@ static bool is_access_to_paths_allowed(
 					memcpy(&_rule_flag_parent2_bkp,
 					       rule_flags_parent2,
 					       sizeof(_rule_flag_parent2_bkp));
-					is_dom_check_bkp = is_dom_check;
 				}
+				is_dom_check_bkp = is_dom_check;
+				child1_layers = landlock_collect_no_inherit_layers(domain,
+										   walker_path
+										   .dentry);
+				if (layer_masks_parent2)
+					child2_layers = child1_layers;
 
 				/* Ignores hidden mount points. */
 				goto jump_up;
@@ -1001,15 +1378,50 @@ static bool is_access_to_paths_allowed(
 				break;
 			}
 
-			/*
-			 * We reached a disconnected root directory from a bind mount, and
-			 * we need to reset the walk to the current mount root.
-			 */
-			goto reset_to_mount_root;
-		}
-		parent_dentry = dget_parent(walker_path.dentry);
-		dput(walker_path.dentry);
-		walker_path.dentry = parent_dentry;
+		/*
+		 * We reached a disconnected root directory from a bind mount, and
+		 * we need to reset the walk to the current mount root.
+		 */
+		goto reset_to_mount_root;
+	}
+	if (likely(!d_is_negative(walker_path.dentry))) {
+		child1_layers = landlock_collect_no_inherit_layers(domain,
+								   walker_path.dentry);
+		if (layer_masks_parent2)
+			child2_layers = child1_layers;
+	} else {
+		child1_layers = 0;
+		if (layer_masks_parent2)
+			child2_layers = 0;
+	}
+	parent_dentry = dget_parent(walker_path.dentry);
+	dput(walker_path.dentry);
+	walker_path.dentry = parent_dentry;
+	/*
+	 * Apply descendant no-inherit masking now that we've moved to the
+	 * parent. This ensures the parent respects any no-inherit rules from
+	 * the child we just left. Only applies to refer operations (rename/link).
+	 */
+	if (unlikely(layer_masks_parent2)) {
+		if (mask_no_inherit_descendant_layers(domain, walker_path.dentry,
+						      child1_layers,
+						      access_masked_parent1,
+						      layer_masks_parent1,
+						      rule_flags_parent1))
+			allowed_parent1 =
+				allowed_parent1 ||
+				is_layer_masks_allowed(layer_masks_parent1);
+
+		if (rule_flags_parent2 &&
+		    mask_no_inherit_descendant_layers(domain, walker_path.dentry,
+						      child2_layers,
+						      access_masked_parent2,
+						      layer_masks_parent2,
+						      rule_flags_parent2))
+			allowed_parent2 =
+				allowed_parent2 ||
+				is_layer_masks_allowed(layer_masks_parent2);
+	}
 		continue;
 
 reset_to_mount_root:
@@ -1057,6 +1469,10 @@ static bool is_access_to_paths_allowed(
 		dput(walker_path.dentry);
 		walker_path.dentry = walker_path.mnt->mnt_root;
 		dget(walker_path.dentry);
+		child1_layers = landlock_collect_no_inherit_layers(domain,
+								   walker_path.dentry);
+		if (layer_masks_parent2)
+			child2_layers = child1_layers;
 	}
 	path_put(&walker_path);
 
@@ -1172,6 +1588,8 @@ static bool collect_domain_accesses(
 	struct collected_rule_flags *const rule_flags)
 {
 	access_mask_t access_dom;
+	const layer_mask_t domain_layers_mask =
+		landlock_domain_layers_mask(domain);
 	bool ret = false;
 
 	if (WARN_ON_ONCE(!domain || !mnt_dir || !dir || !layer_masks_dom))
@@ -1187,9 +1605,11 @@ static bool collect_domain_accesses(
 	while (true) {
 		struct dentry *parent_dentry;
 
+		const struct landlock_rule *rule = find_rule(domain, dir);
+
 		/* Gets all layers allowing all domain accesses. */
 		if (landlock_unmask_layers(
-			    find_rule(domain, dir), access_dom, layer_masks_dom,
+			    rule, access_dom, layer_masks_dom,
 			    ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
 			/*
 			 * Before allowing this side of the access request, checks that the
@@ -1206,6 +1626,10 @@ static bool collect_domain_accesses(
 			break;
 		}
 
+		if (rule &&
+		    rule_blocks_all_layers_no_inherit(domain_layers_mask, rule))
+			break;
+
 		/* Stops at the mount point. */
 		if (dir == mnt_dir->dentry)
 			break;
@@ -1232,6 +1656,121 @@ static bool collect_domain_accesses(
 	return ret;
 }
 
+/**
+ * collect_topology_sealed_layers - collect layers sealed against topology changes
+ * @domain: Ruleset to consult.
+ * @dentry: Starting dentry for the upward walk.
+ * @override_layers: Optional out parameter filled with layers that are
+ *                   present on ancestors but considered overrides (not
+ *                   sealing the topology for descendants).
+ *
+ * Walk upwards from @dentry and return a mask of layers where either the
+ * visited dentry contains a no_inherit rule or ancestors were previously
+ * marked as having a descendant with no_inherit.  @override_layers, if not
+ * NULL, is filled with layers that would normally be overridden by more
+ * specific descendant rules.
+ *
+ * Returns a layer mask where set bits indicate layers that are "sealed"
+ * (topology changes like rename/rmdir are denied) for the subtree rooted at
+ * @dentry.
+ *
+ * Useful for LANDLOCK_ADD_RULE_NO_INHERIT parent directory enforcement to ensure
+ * that topology changes do not violate the no_inherit constraints.
+ */
+static layer_mask_t
+collect_topology_sealed_layers(const struct landlock_ruleset *const domain,
+			       struct dentry *dentry,
+			       layer_mask_t *const override_layers)
+{
+	struct dentry *cursor, *parent;
+	bool include_descendants = true;
+	layer_mask_t sealed_layers = 0;
+
+	if (override_layers)
+		*override_layers = 0;
+
+	if (!domain || !dentry || d_is_negative(dentry))
+		return 0;
+
+	cursor = dget(dentry);
+	while (cursor) {
+		const struct landlock_rule *rule;
+		u32 layer_index;
+
+		rule = find_rule(domain, cursor);
+		if (rule) {
+			for (layer_index = 0; layer_index < rule->num_layers;
+			     layer_index++) {
+				const struct landlock_layer *layer =
+					&rule->layers[layer_index];
+				const int level = layer->level ? layer->level :
+								 layer_index + 1;
+				layer_mask_t layer_bit = BIT_ULL(level - 1);
+
+				if (include_descendants &&
+				    (layer->flags.no_inherit ||
+				     layer->flags.has_no_inherit_descendant)) {
+					sealed_layers |= layer_bit;
+				} else if (override_layers) {
+					*override_layers |= layer_bit;
+				}
+			}
+		}
+
+		if (sealed_layers || IS_ROOT(cursor))
+			break;
+
+		parent = dget_parent(cursor);
+		dput(cursor);
+		if (!parent)
+			return sealed_layers;
+
+		cursor = parent;
+		include_descendants = false;
+	}
+	dput(cursor);
+	return sealed_layers;
+}
+
+/**
+ * deny_no_inherit_topology_change - deny topology changes on sealed layers
+ * @subject: Subject performing the operation (contains the domain).
+ * @dentry: Dentry that is the target of the topology modification.
+ *
+ * Checks whether any domain layers are sealed against topology changes at
+ * @dentry (via collect_topology_sealed_layers).  If so, emit an audit record
+ * and return -EACCES.  Otherwise return 0.
+ */
+static int deny_no_inherit_topology_change(const struct landlock_cred_security
+					   *subject,
+					   struct dentry *dentry)
+{
+	layer_mask_t sealed_layers;
+	layer_mask_t override_layers;
+	unsigned long layer_index;
+
+	if (!subject || !dentry || d_is_negative(dentry))
+		return 0;
+	sealed_layers = collect_topology_sealed_layers(subject->domain,
+						       dentry, &override_layers);
+	sealed_layers &= ~override_layers;
+
+	if (!sealed_layers)
+		return 0;
+
+	layer_index = __ffs((unsigned long)sealed_layers);
+	landlock_log_denial(subject, &(struct landlock_request) {
+		.type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
+		.audit = {
+			.type = LSM_AUDIT_DATA_DENTRY,
+			.u.dentry = dentry,
+		},
+		.layer_plus_one = layer_index + 1,
+	});
+
+	return -EACCES;
+}
+
 /**
  * current_check_refer_path - Check if a rename or link action is allowed
  *
@@ -1316,6 +1855,16 @@ static int current_check_refer_path(struct dentry *const old_dentry,
 	access_request_parent2 =
 		get_mode_access(d_backing_inode(old_dentry)->i_mode);
 	if (removable) {
+		int err;
+
+		err = deny_no_inherit_topology_change(subject, old_dentry);
+		if (err)
+			return err;
+		if (exchange) {
+			err = deny_no_inherit_topology_change(subject, new_dentry);
+			if (err)
+				return err;
+		}
 		access_request_parent1 |= maybe_remove(old_dentry);
 		access_request_parent2 |= maybe_remove(new_dentry);
 	}
@@ -1707,12 +2256,31 @@ static int hook_path_symlink(const struct path *const dir,
 static int hook_path_unlink(const struct path *const dir,
 			    struct dentry *const dentry)
 {
+	const struct landlock_cred_security *const subject =
+		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+	int err;
+
+	if (subject) {
+		err = deny_no_inherit_topology_change(subject, dentry);
+		if (err)
+			return err;
+	}
 	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
 }
 
 static int hook_path_rmdir(const struct path *const dir,
 			   struct dentry *const dentry)
 {
+	const struct landlock_cred_security *const subject =
+		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
+	int err;
+
+	if (subject) {
+		err = deny_no_inherit_topology_change(subject, dentry);
+		if (err)
+			return err;
+	}
+
 	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
 }
 
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 750a444e1983..f7b6a48bbf39 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -255,8 +255,13 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
 				return -EINVAL;
 			if (WARN_ON_ONCE(this->layers[0].level != 0))
 				return -EINVAL;
+			/* Merge the flags into the rules */
 			this->layers[0].access |= (*layers)[0].access;
 			this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
+			this->layers[0].flags.no_inherit |=
+				(*layers)[0].flags.no_inherit;
+			this->layers[0].flags.has_no_inherit_descendant |=
+				(*layers)[0].flags.has_no_inherit_descendant;
 			return 0;
 		}
 
@@ -315,7 +320,10 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
 		.level = 0,
 		.flags = {
 			.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
-		},
+			.no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+			.has_no_inherit_descendant =
+				!!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
+		}
 	} };
 
 	build_check_layer();
@@ -662,9 +670,22 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
 		unsigned long access_bit;
 		bool is_empty;
 
-		/* Collect rule flags for each layer. */
-		if (rule_flags && layer->flags.quiet)
+		/* Skip layers that already have no inherit flags. */
+		if (rule_flags &&
+		    (rule_flags->no_inherit_masks & layer_bit))
+			continue;
+
+		/* Collect rule flags for each layer.
+		 * We block flag inheritance if needed
+		 * because of a no_inherit rule.
+		 */
+		if (rule_flags && layer->flags.quiet &&
+		    !(rule_flags->blocked_flag_masks & layer_bit))
 			rule_flags->quiet_masks |= layer_bit;
+		if (rule_flags && layer->flags.no_inherit)
+			rule_flags->no_inherit_masks |= layer_bit;
+		if (rule_flags && layer->flags.has_no_inherit_descendant)
+			rule_flags->no_inherit_desc_masks |= layer_bit;
 
 		/*
 		 * Records in @layer_masks which layer grants access to each requested
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index eb60db646422..8b46ab14e995 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -40,6 +40,21 @@ struct landlock_layer {
 		 * down the file hierarchy.
 		 */
 		bool quiet:1;
+		/**
+		 * @no_inherit: Prevents this rule from being inherited by
+		 * descendant directories in the filesystem layer.  Only used
+		 * for filesystem rules.
+		 */
+		bool no_inherit:1;
+		/**
+		 * @has_no_inherit_descendant: Marker to indicate that this layer
+		 * has at least one descendant directory with a rule having the
+		 * no_inherit flag.  Only used for filesystem rules.
+		 * This "flag" is not set by the user, but by Landlock on
+		 * parent directories of rules when the child rule has
+		 * a rule with the no_inherit flag.
+		 */
+		bool has_no_inherit_descendant:1;
 	} flags;
 	/**
 	 * @access: Bitfield of allowed actions on the kernel object.  They are
@@ -49,13 +64,32 @@ struct landlock_layer {
 };
 
 /**
- * struct collected_rule_flags - Hold accumulated flags for each layer.
+ * struct collected_rule_flags - Hold accumulated flags and their markers for each layer.
  */
 struct collected_rule_flags {
 	/**
 	 * @quiet_masks: Layers for which the quiet flag is effective.
 	 */
 	layer_mask_t quiet_masks;
+	/**
+	 * @no_inherit_masks: Layers for which the no_inherit flag is effective.
+	 */
+	layer_mask_t no_inherit_masks;
+	/**
+	 * @no_inherit_desc_masks: Layers for which the
+	 * has_no_inherit_descendant tag is effective.
+	 * This is not a flag itself, but a marker set on ancestors
+	 * of rules with the no_inherit flag to deny topology changes
+	 * in the direct parent path.
+	 */
+	layer_mask_t no_inherit_desc_masks;
+	/**
+	 * @blocked_flag_masks: Layers where flag inheritance must be blocked
+	 * because of a no_inherit rule. This is not a flag itself, but a marker
+	 * for layers that have their flags blocked due to no_inherit rule
+	 * propagation.
+	 */
+	layer_mask_t blocked_flag_masks;
 };
 
 /**
-- 
2.51.0

[PATCH v3 2/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT userspace api

[Justin Suess]

Implements the syscall side flag handling and kernel api headers for the
LANDLOCK_ADD_RULE_NO_INHERIT flag.

v2..v3 changes:

  * Extended documentation for flag inheritance suppression on
    LANDLOCK_ADD_RULE_NO_INHERIT.
  * Extended the flag validation rules in the syscall.
  * Added mention of no inherit in empty rules in add_rule_path_beneath
    as per Tingmao Wang's suggestion.
  * Added check for useless no-inherit flag in networking rules.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 include/uapi/linux/landlock.h | 29 +++++++++++++++++++++++++++++
 security/landlock/syscalls.c  | 14 +++++++++++---
 2 files changed, 40 insertions(+), 3 deletions(-)

diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
index d4f47d20361a..cf5c8068f513 100644
--- a/include/uapi/linux/landlock.h
+++ b/include/uapi/linux/landlock.h
@@ -127,10 +127,39 @@ 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_NO_INHERIT
+ *     When set on a rule being added to a ruleset, this flag disables the
+ *     inheritance of access rights and flags from parent objects.
+ *
+ *     This flag currently applies only to filesystem rules.  Adding it to
+ *     non-filesystem rules wil return -EINVAL, unless future extensions
+ *     of Landlock define other hierarchical object types.
+ *
+ *     By default, Landlock filesystem rules inherit allowed accesses from
+ *     ancestor directories: if a parent directory grants certain rights,
+ *     those rights also apply to its children.  A rule marked with
+ *     LANDLOCK_ADD_RULE_NO_INHERIT stops this propagation at the directory
+ *     covered by the rule.  Descendants of that directory continue to inherit
+ *     normally unless they also have rules using this flag.
+ *
+ *     If a regular file is marked with this flag, it will not inherit any
+ *     access rights from its parent directories; only the accesses explicitly
+ *     allowed by the rule will apply to that file.
+ *
+ *     This flag also enforces parent-directory restrictions: rename, rmdir,
+ *     link, and other operations that would change the directory's immediate
+ *     parent subtree are denied up to the mount point.  This prevents
+ *     sandboxed processes from manipulating the filesystem hierarchy to evade
+ *     restrictions (e.g., via sandbox-restart attacks).
+ *
+ *     In addition, this flag blocks the inheritance of rule-layer flags
+ *     (such as the quiet flag) from parent directories to the object covered
+ *     by this rule.
  */
 
 /* clang-format off */
 #define LANDLOCK_ADD_RULE_QUIET			(1U << 0)
+#define LANDLOCK_ADD_RULE_NO_INHERIT		(1U << 1)
 /* clang-format on */
 
 /**
diff --git a/security/landlock/syscalls.c b/security/landlock/syscalls.c
index 93396bfc1500..1ea9bf95ef61 100644
--- a/security/landlock/syscalls.c
+++ b/security/landlock/syscalls.c
@@ -352,7 +352,7 @@ static int add_rule_path_beneath(struct landlock_ruleset *const ruleset,
 	/*
 	 * Informs about useless rule: empty allowed_access (i.e. deny rules)
 	 * are ignored in path walks.  However, the rule is not useless if it
-	 * is there to hold a quiet flag
+	 * is there to hold a quiet or no inherit flag.
 	 */
 	if (!flags && !path_beneath_attr.allowed_access)
 		return -ENOMSG;
@@ -407,6 +407,10 @@ static int add_rule_net_port(struct landlock_ruleset *ruleset,
 	if (flags & LANDLOCK_ADD_RULE_QUIET && !ruleset->quiet_masks.net)
 		return -EINVAL;
 
+	/* No inherit is always useless for this scope */
+	if (flags & LANDLOCK_ADD_RULE_NO_INHERIT)
+		return -EINVAL;
+
 	/* Denies inserting a rule with port greater than 65535. */
 	if (net_port_attr.port > U16_MAX)
 		return -EINVAL;
@@ -462,8 +466,12 @@ SYSCALL_DEFINE4(landlock_add_rule, const int, ruleset_fd,
 
 	if (!is_initialized())
 		return -EOPNOTSUPP;
-
-	if (flags && flags != LANDLOCK_ADD_RULE_QUIET)
+	/* Checks flag existence */
+	if (flags && flags & ~(LANDLOCK_ADD_RULE_QUIET | LANDLOCK_ADD_RULE_NO_INHERIT))
+		return -EINVAL;
+	/* No inherit may only apply on path_beneath rules. */
+	if ((flags & LANDLOCK_ADD_RULE_NO_INHERIT) &&
+	    rule_type != LANDLOCK_RULE_PATH_BENEATH)
 		return -EINVAL;
 
 	/* Gets and checks the ruleset. */
-- 
2.51.0

[PATCH v3 3/5] samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to landlock-sandboxer

[Justin Suess]

Adds support to landlock-sandboxer with environment variables LL_FS_RO_NO_INHERIT
and LL_FS_RW_NO_INHERIT. These create the same rulesets as their non-no inherit variants,
plus the LANDLOCK_ADD_RULE_NO_INHERIT flag.

v2..v3 changes:

  * Minor formatting fixes

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 samples/landlock/sandboxer.c | 37 +++++++++++++++++++++++++++---------
 1 file changed, 28 insertions(+), 9 deletions(-)

diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
index 2d8e3e94b77b..6f6bfc4e5110 100644
--- a/samples/landlock/sandboxer.c
+++ b/samples/landlock/sandboxer.c
@@ -58,6 +58,8 @@ static inline int landlock_restrict_self(const int ruleset_fd,
 
 #define ENV_FS_RO_NAME "LL_FS_RO"
 #define ENV_FS_RW_NAME "LL_FS_RW"
+#define ENV_FS_RO_NO_INHERIT_NAME "LL_FS_RO_NO_INHERIT"
+#define ENV_FS_RW_NO_INHERIT_NAME "LL_FS_RW_NO_INHERIT"
 #define ENV_FS_QUIET_NAME "LL_FS_QUIET"
 #define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
 #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
@@ -121,7 +123,8 @@ static int parse_path(char *env_path, const char ***const path_list)
 /* clang-format on */
 
 static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
-			       const __u64 allowed_access, bool quiet)
+		       const __u64 allowed_access,
+		       __u32 add_rule_flags, bool mandatory)
 {
 	int num_paths, i, ret = 1;
 	char *env_path_name;
@@ -132,9 +135,13 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
 
 	env_path_name = getenv(env_var);
 	if (!env_path_name) {
-		/* Prevents users to forget a setting. */
-		fprintf(stderr, "Missing environment variable %s\n", env_var);
-		return 1;
+		if (mandatory) {
+			/* Prevents from forgetting to set necessary env vars. */
+			fprintf(stderr, "Missing environment variable %s\n",
+				env_var);
+			return 1;
+		}
+		return 0;
 	}
 	env_path_name = strdup(env_path_name);
 	unsetenv(env_var);
@@ -171,8 +178,7 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
 		if (!S_ISDIR(statbuf.st_mode))
 			path_beneath.allowed_access &= ACCESS_FILE;
 		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
-				      &path_beneath,
-				      quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
+			      &path_beneath, add_rule_flags)) {
 			fprintf(stderr,
 				"Failed to update the ruleset with \"%s\": %s\n",
 				path_list[i], strerror(errno));
@@ -375,6 +381,8 @@ static const char help[] =
 	"Optional settings (when not set, their associated access check "
 	"is always allowed, which is different from an empty string which "
 	"means an empty list):\n"
+	"* " ENV_FS_RO_NO_INHERIT_NAME ": read-only paths without rule inheritance\n"
+	"* " ENV_FS_RW_NO_INHERIT_NAME ": read-write paths without rule inheritance\n"
 	"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
 	"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
 	"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
@@ -596,17 +604,28 @@ int main(const int argc, char *const argv[], char *const *const envp)
 	}
 
 	if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
-				false)) {
+			0, true)) {
 		goto err_close_ruleset;
 	}
 	if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
+			0, true)) {
+		goto err_close_ruleset;
+	}
+	/* Optional no-inherit rules mirror the regular read-only/read-write sets. */
+	if (populate_ruleset_fs(ENV_FS_RO_NO_INHERIT_NAME, ruleset_fd,
+				access_fs_ro, LANDLOCK_ADD_RULE_NO_INHERIT,
+				false)) {
+		goto err_close_ruleset;
+	}
+	if (populate_ruleset_fs(ENV_FS_RW_NO_INHERIT_NAME, ruleset_fd,
+				access_fs_rw, LANDLOCK_ADD_RULE_NO_INHERIT,
 				false)) {
 		goto err_close_ruleset;
 	}
 	/* Don't require this env to be present. */
-	if (quiet_supported && getenv(ENV_FS_QUIET_NAME)) {
+	if (quiet_supported) {
 		if (populate_ruleset_fs(ENV_FS_QUIET_NAME, ruleset_fd, 0,
-					true)) {
+				LANDLOCK_ADD_RULE_QUIET, false)) {
 			goto err_close_ruleset;
 		}
 	}
-- 
2.51.0

[PATCH v3 4/5] selftests/landlock: Implement selftests for LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Implements 11 selftests for the flag, covering allowed and disallowed operations on parent
and child directories when this flag is set, as well as multi-layer configurations
and flag inheritance / audit logging.

v2..v3 changes:

  * Also covers flag inheritance, audit logging and LANDLOCK_ADD_RULE_QUIET suppression.
  * Increases number of selftests from 5 -> 11.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 tools/testing/selftests/landlock/fs_test.c | 459 ++++++++++++++++++++-
 1 file changed, 447 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/landlock/fs_test.c b/tools/testing/selftests/landlock/fs_test.c
index 6aa65d344c72..87b66ad7a0b8 100644
--- a/tools/testing/selftests/landlock/fs_test.c
+++ b/tools/testing/selftests/landlock/fs_test.c
@@ -717,16 +717,12 @@ TEST_F_FORK(layout1, rule_with_unhandled_access)
 }
 
 static void add_path_beneath(struct __test_metadata *const _metadata,
-			     const int ruleset_fd, const __u64 allowed_access,
-			     const char *const path, bool quiet)
+			 const int ruleset_fd, const __u64 allowed_access,
+			 const char *const path, __u32 flags)
 {
 	struct landlock_path_beneath_attr path_beneath = {
 		.allowed_access = allowed_access,
 	};
-	__u32 flags = 0;
-
-	if (quiet)
-		flags |= LANDLOCK_ADD_RULE_QUIET;
 
 	path_beneath.parent_fd = open(path, O_PATH | O_CLOEXEC);
 	ASSERT_LE(0, path_beneath.parent_fd)
@@ -790,7 +786,7 @@ static int create_ruleset(struct __test_metadata *const _metadata,
 			continue;
 
 		add_path_beneath(_metadata, ruleset_fd, rules[i].access,
-				 rules[i].path, false);
+				 rules[i].path, 0);
 	}
 	return ruleset_fd;
 }
@@ -1368,7 +1364,7 @@ TEST_F_FORK(layout1, inherit_subset)
 	 * ANDed with the previous ones.
 	 */
 	add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
-			 dir_s1d2, false);
+			 dir_s1d2, 0);
 	/*
 	 * According to ruleset_fd, dir_s1d2 should now have the
 	 * LANDLOCK_ACCESS_FS_READ_FILE and LANDLOCK_ACCESS_FS_WRITE_FILE
@@ -1400,7 +1396,7 @@ TEST_F_FORK(layout1, inherit_subset)
 	 * Try to get more privileges by adding new access rights to the parent
 	 * directory: dir_s1d1.
 	 */
-	add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, false);
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
 	enforce_ruleset(_metadata, ruleset_fd);
 
 	/* Same tests and results as above. */
@@ -1423,7 +1419,7 @@ TEST_F_FORK(layout1, inherit_subset)
 	 * that there was no rule tied to it before.
 	 */
 	add_path_beneath(_metadata, ruleset_fd, LANDLOCK_ACCESS_FS_WRITE_FILE,
-			 dir_s1d3, false);
+			 dir_s1d3, 0);
 	enforce_ruleset(_metadata, ruleset_fd);
 	ASSERT_EQ(0, close(ruleset_fd));
 
@@ -1476,7 +1472,7 @@ TEST_F_FORK(layout1, inherit_superset)
 	add_path_beneath(_metadata, ruleset_fd,
 			 LANDLOCK_ACCESS_FS_READ_FILE |
 				 LANDLOCK_ACCESS_FS_READ_DIR,
-			 dir_s1d2, false);
+			 dir_s1d2, 0);
 	enforce_ruleset(_metadata, ruleset_fd);
 	ASSERT_EQ(0, close(ruleset_fd));
 
@@ -1488,6 +1484,111 @@ TEST_F_FORK(layout1, inherit_superset)
 	ASSERT_EQ(0, test_open(file1_s1d3, O_RDONLY));
 }
 
+TEST_F_FORK(layout1, inherit_no_inherit_flag)
+{
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW,
+	};
+	int ruleset_fd;
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RW, dir_s1d1, 0);
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* Parent directory still grants write access to its direct children. */
+	EXPECT_EQ(0, test_open(dir_s1d1, O_RDONLY | O_DIRECTORY));
+	EXPECT_EQ(0, test_open(file1_s1d1, O_WRONLY));
+
+	/* dir_s1d2 gets only its explicit read-only access rights. */
+	EXPECT_EQ(0, test_open(dir_s1d2, O_RDONLY | O_DIRECTORY));
+	EXPECT_EQ(0, test_open(file1_s1d2, O_RDONLY));
+	EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+
+	/* Descendants of dir_s1d2 inherit the reduced access mask. */
+	EXPECT_EQ(0, test_open(dir_s1d3, O_RDONLY | O_DIRECTORY));
+	EXPECT_EQ(0, test_open(file1_s1d3, O_RDONLY));
+	EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_nested_levels)
+{
+	int ruleset_fd;
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+				     LANDLOCK_ACCESS_FS_REMOVE_FILE |
+				     LANDLOCK_ACCESS_FS_REMOVE_DIR,
+	};
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Level 1: s1d1 (RW + REFER + REMOVE + NO_INHERIT) */
+	add_path_beneath(_metadata, ruleset_fd,
+			 ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+				 LANDLOCK_ACCESS_FS_REMOVE_FILE |
+				 LANDLOCK_ACCESS_FS_REMOVE_DIR,
+			 dir_s1d1, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	/* Level 2: s1d2 (RO + NO_INHERIT) */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	/* Level 3: s1d3 (RW + REFER + REMOVE + NO_INHERIT) */
+	add_path_beneath(_metadata, ruleset_fd,
+			 ACCESS_RW | LANDLOCK_ACCESS_FS_REFER |
+				 LANDLOCK_ACCESS_FS_REMOVE_FILE |
+				 LANDLOCK_ACCESS_FS_REMOVE_DIR,
+			 dir_s1d3, LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/*
+	 * Level 3: s1d3
+	 * - RW allowed (unlink file)
+	 * - REFER allowed (rename file)
+	 * - REMOVE_DIR denied (parent s1d2 is part of direct parent tree)
+	 */
+	ASSERT_EQ(0, unlink(file1_s1d3));
+	ASSERT_EQ(0, rename(file2_s1d3, file1_s1d3));
+	ASSERT_EQ(0, rename(file1_s1d3, file2_s1d3));
+	ASSERT_EQ(-1, rmdir(dir_s1d3));
+	ASSERT_EQ(EACCES, errno);
+
+	/*
+	 * Level 2: s1d2
+	 * - RW denied (unlink file), layer is RO
+	 * - REFER denied (rename file)
+	 * - REMOVE_DIR of s1d2 not allowed (parent s1d1 is part of direct parent tree)
+	 */
+	ASSERT_EQ(-1, unlink(file1_s1d2));
+	ASSERT_EQ(EACCES, errno);
+	ASSERT_EQ(-1, rename(file2_s1d2, file1_s1d2));
+	ASSERT_EQ(EACCES, errno);
+	ASSERT_EQ(-1, rmdir(dir_s1d2));
+	ASSERT_EQ(EACCES, errno);
+
+	/*
+	 * Level 1: s1d1
+	 * - RW allowed
+	 * - Rename allowed (except for direct parent tree s1d2)
+	 * - REMOVE_DIR denied (parent tmp is denied)
+	 */
+	ASSERT_EQ(0, unlink(file1_s1d1));
+	ASSERT_EQ(0, rename(file2_s1d1, file1_s1d1));
+	ASSERT_EQ(0, rename(file1_s1d1, file2_s1d1));
+	ASSERT_EQ(-1, rmdir(dir_s1d1));
+	ASSERT_EQ(EACCES, errno);
+}
+
 TEST_F_FORK(layout0, max_layers)
 {
 	int i, err;
@@ -4412,6 +4513,246 @@ TEST_F_FORK(layout1, named_unix_domain_socket_ioctl)
 	ASSERT_EQ(0, close(cli_fd));
 }
 
+TEST_F_FORK(layout1, inherit_no_inherit_topology_dir)
+{
+	const struct rule rules[] = {
+		{
+			.path = TMP_DIR,
+			.access = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		},
+		{},
+	};
+	int ruleset_fd;
+
+	ruleset_fd = create_ruleset(_metadata,
+				    ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+				    rules);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Adds a no-inherit rule on a leaf directory. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/*
+	 * Topology modifications of the rule path and its parents are denied.
+	 */
+
+	/* Target directory s1d3 */
+	ASSERT_EQ(-1, rmdir(dir_s1d3));
+	ASSERT_EQ(EACCES, errno);
+	ASSERT_EQ(-1, rename(dir_s1d3, dir_s2d3));
+	ASSERT_EQ(EACCES, errno);
+
+	/* Parent directory s1d2 */
+	ASSERT_EQ(-1, rmdir(dir_s1d2));
+	ASSERT_EQ(EACCES, errno);
+	ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+	ASSERT_EQ(EACCES, errno);
+
+	/* Grandparent directory s1d1 */
+	ASSERT_EQ(-1, rmdir(dir_s1d1));
+	ASSERT_EQ(EACCES, errno);
+	ASSERT_EQ(-1, rename(dir_s1d1, dir_s2d1));
+	ASSERT_EQ(EACCES, errno);
+
+	/*
+	 * Sibling operations are allowed.
+	 */
+	/* Sibling of s1d3 */
+	ASSERT_EQ(0, unlink(file1_s1d2));
+	/* Sibling of s1d2 */
+	ASSERT_EQ(0, unlink(file1_s1d1));
+
+	/*
+	 * Content of the no-inherit directory is restricted by the rule (RO).
+	 */
+	ASSERT_EQ(-1, unlink(file1_s1d3));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, no_inherit_allow_inner_removal)
+{
+	int ruleset_fd;
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+	};
+
+	ruleset_fd =
+		landlock_create_ruleset(&ruleset_attr, sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	add_path_beneath(_metadata, ruleset_fd,
+			 ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE, dir_s1d2,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/*
+	 * Content of the no-inherit directory is mutable (RW).
+	 * This checks that the no-inherit flag does not seal the content.
+	 */
+	ASSERT_EQ(0, unlink(file1_s1d2));
+
+	/*
+	 * Topology modifications of the rule path are denied.
+	 */
+	ASSERT_EQ(-1, rmdir(dir_s1d2));
+	ASSERT_EQ(EACCES, errno);
+	ASSERT_EQ(-1, rename(dir_s1d2, dir_s2d2));
+	ASSERT_EQ(EACCES, errno);
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_unrelated)
+{
+	const struct rule rules[] = {
+		{
+			.path = TMP_DIR,
+			.access = ACCESS_RW,
+		},
+		{},
+	};
+	static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+	static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+	int ruleset_fd;
+
+	ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Adds a no-inherit rule on a leaf directory unrelated to s2. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* Ensure we can still create and delete files outside the sealed branch. */
+	ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+	ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+	ASSERT_EQ(0, unlink(unrelated_file));
+	ASSERT_EQ(0, rmdir(unrelated_dir));
+
+	/* Existing siblings in s2 remain modifiable. */
+	ASSERT_EQ(0, unlink(file1_s2d1));
+	ASSERT_EQ(0, mknod(file1_s2d1, S_IFREG | 0700, 0));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_descendant_rw)
+{
+	const struct rule rules[] = {
+		{
+			.path = TMP_DIR,
+			.access = ACCESS_RO,
+		},
+		{},
+	};
+	const __u64 handled_access = ACCESS_RW | LANDLOCK_ACCESS_FS_MAKE_REG |
+				     LANDLOCK_ACCESS_FS_REMOVE_FILE;
+	static const char child_file[] =
+		TMP_DIR "/s1d1/s1d2/s1d3/rw_descendant";
+	int ruleset_fd;
+
+	ruleset_fd = create_ruleset(_metadata, handled_access, rules);
+	ASSERT_LE(0, ruleset_fd);
+
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+	add_path_beneath(_metadata, ruleset_fd,
+			 ACCESS_RW | LANDLOCK_ACCESS_FS_MAKE_REG |
+				 LANDLOCK_ACCESS_FS_REMOVE_FILE,
+			 dir_s1d3, 0);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	ASSERT_EQ(0, mknod(child_file, S_IFREG | 0600, 0));
+	ASSERT_EQ(0, unlink(child_file));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_topology_file)
+{
+	const struct rule rules[] = {
+		{
+			.path = TMP_DIR,
+			.access = ACCESS_RW,
+		},
+		{},
+	};
+	int ruleset_fd;
+	struct landlock_path_beneath_attr path_beneath = {
+		.allowed_access = ACCESS_RO,
+	};
+
+	ruleset_fd = create_ruleset(_metadata, ACCESS_RW, rules);
+	ASSERT_LE(0, ruleset_fd);
+
+	path_beneath.parent_fd = open(file1_s1d2, 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_NO_INHERIT));
+	ASSERT_EQ(EINVAL, errno);
+	ASSERT_EQ(0, close(path_beneath.parent_fd));
+	ASSERT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F_FORK(layout1, inherit_no_inherit_layered)
+{
+	const struct rule layer1[] = {
+		{
+			.path = TMP_DIR,
+			.access = ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+		},
+		{},
+	};
+	int ruleset_fd;
+	static const char unrelated_dir[] = TMP_DIR "/s2d1/unrelated";
+	static const char unrelated_file[] = TMP_DIR "/s2d1/unrelated/f1";
+
+	/* Layer 1: RW on TMP_DIR */
+	ruleset_fd = create_ruleset(_metadata,
+				    ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+				    layer1);
+	ASSERT_LE(0, ruleset_fd);
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* Layer 2: Add no-inherit RO rule on s1d2 */
+	ruleset_fd = create_ruleset(_metadata,
+				    ACCESS_RW | LANDLOCK_ACCESS_FS_REMOVE_FILE,
+				    layer1);
+	ASSERT_LE(0, ruleset_fd);
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d2,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+	enforce_ruleset(_metadata, ruleset_fd);
+	ASSERT_EQ(0, close(ruleset_fd));
+
+	/* Operations in unrelated areas should still work */
+	ASSERT_EQ(0, mkdir(unrelated_dir, 0700));
+	ASSERT_EQ(0, mknod(unrelated_file, S_IFREG | 0600, 0));
+	ASSERT_EQ(0, unlink(unrelated_file));
+	ASSERT_EQ(0, rmdir(unrelated_dir));
+
+	/* Creating in s1d1 should be allowed (parent still has RW) */
+	ASSERT_EQ(0, mknod(TMP_DIR "/s1d1/newfile", S_IFREG | 0600, 0));
+	ASSERT_EQ(0, unlink(TMP_DIR "/s1d1/newfile"));
+
+	/* Content of s1d2 should be read-only */
+	ASSERT_EQ(-1, unlink(file1_s1d2));
+	ASSERT_EQ(EACCES, errno);
+
+	/* Topology changes to s1d2 should be denied */
+	ASSERT_EQ(-1, rename(dir_s1d2, TMP_DIR "/s2d1/renamed"));
+	ASSERT_EQ(EACCES, errno);
+
+	/* Renaming s1d1 should also be denied (it's an ancestor) */
+	ASSERT_EQ(-1, rename(dir_s1d1, TMP_DIR "/s2d1/renamed"));
+	ASSERT_EQ(EACCES, errno);
+}
+
 /* clang-format off */
 FIXTURE(ioctl) {};
 
@@ -7088,6 +7429,100 @@ TEST_F(audit_layout1, write_file)
 	EXPECT_EQ(1, records.domain);
 }
 
+TEST_F(audit_layout1, no_inherit_parent_is_logged)
+{
+	struct audit_records records;
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW,
+	};
+	int ruleset_fd;
+
+	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+					     sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Base read-only rule at s1d1. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1, 0);
+	/* Descendant s1d1/s1d2/s1d3 forbids inheritance but should still log. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+
+	EXPECT_EQ(EACCES, test_open(file1_s1d2, O_WRONLY));
+	EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+				    "fs\\.write_file", file1_s1d2));
+	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+	EXPECT_EQ(0, records.access);
+	EXPECT_EQ(1, records.domain);
+
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(audit_layout1, no_inherit_blocks_quiet_flag_inheritence)
+{
+	struct audit_records records;
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW,
+		.quiet_access_fs = ACCESS_RW,
+	};
+	int ruleset_fd;
+
+	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+					     sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Base read-only rule at tmp/s1d1 with quiet flag. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1,
+			 LANDLOCK_ADD_RULE_QUIET);
+	/* Descendant tmp/s1d1/s1d2/s1d3 forbids inheritance of quiet flag and should still log. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+
+	EXPECT_EQ(EACCES, test_open(file1_s1d3, O_WRONLY));
+	EXPECT_EQ(0, matches_log_fs(_metadata, self->audit_fd,
+				    "fs\\.write_file", file1_s1d3));
+	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+	EXPECT_EQ(0, records.access);
+	EXPECT_EQ(1, records.domain);
+
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
+TEST_F(audit_layout1, no_inherit_quiet_parent)
+{
+	struct audit_records records;
+	struct landlock_ruleset_attr ruleset_attr = {
+		.handled_access_fs = ACCESS_RW,
+		.quiet_access_fs = ACCESS_RW,
+	};
+	int ruleset_fd;
+
+	ruleset_fd = landlock_create_ruleset(&ruleset_attr,
+					     sizeof(ruleset_attr), 0);
+	ASSERT_LE(0, ruleset_fd);
+
+	/* Base read-only rule at tmp/s1d1 with quiet flag. */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d1,
+			 LANDLOCK_ADD_RULE_QUIET);
+	/* Access to dir_s1d1 shouldn't log */
+	add_path_beneath(_metadata, ruleset_fd, ACCESS_RO, dir_s1d3,
+			 LANDLOCK_ADD_RULE_NO_INHERIT);
+
+	enforce_ruleset(_metadata, ruleset_fd);
+
+	EXPECT_EQ(EACCES, test_open(file1_s1d1, O_WRONLY));
+	EXPECT_NE(0, matches_log_fs(_metadata, self->audit_fd,
+				    "fs\\.write_file", file1_s1d1));
+	EXPECT_EQ(0, audit_count_records(self->audit_fd, &records));
+	EXPECT_EQ(0, records.access);
+	EXPECT_EQ(0, records.domain);
+
+	EXPECT_EQ(0, close(ruleset_fd));
+}
+
 TEST_F(audit_layout1, read_file)
 {
 	struct audit_records records;
@@ -7647,7 +8082,7 @@ static int apply_a_layer(struct __test_metadata *const _metadata,
 			continue;
 
 		add_path_beneath(_metadata, rs_fd, r->access, r->path,
-				 r->quiet);
+				 r->quiet ? LANDLOCK_ADD_RULE_QUIET : 0);
 	}
 
 	ASSERT_EQ(0, prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0));
-- 
2.51.0

[PATCH v3 5/5] landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Add a unit test for rule_flag collection, ensuring that access masks
are properly propagated with the flags.

changes v2..v3:

   * Removing erroneously misplaced code and placed in the proper
     patch.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>

	build.log
---
 security/landlock/ruleset.c | 89 +++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index f7b6a48bbf39..0e0de8b20dc4 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -22,6 +22,7 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <uapi/linux/landlock.h>
+#include <kunit/test.h>
 
 #include "access.h"
 #include "audit.h"
@@ -774,3 +775,91 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
 	}
 	return handled_accesses;
 }
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+/**
+ * test_unmask_layers_no_inherit - Test landlock_unmask_layers() with no_inherit
+ * @rule_flags: Pointer to collected_rule_flags structure to track flags.
+ */
+static void test_unmask_layers_no_inherit(struct kunit *const test)
+{
+	struct landlock_rule *rule;
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS];
+	struct collected_rule_flags rule_flags;
+	const access_mask_t access_request = BIT_ULL(0) | BIT_ULL(1);
+	const layer_mask_t layers_initialized = BIT_ULL(0) | BIT_ULL(1);
+	size_t i;
+
+	rule = kzalloc(struct_size(rule, layers, 2), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, rule);
+
+	rule->num_layers = 2;
+
+	/* Layer 1: allows access 0, no_inherit */
+	rule->layers[0].level = 1;
+	rule->layers[0].access = BIT_ULL(0);
+	rule->layers[0].flags.no_inherit = 1;
+
+	/* Layer 2: allows access 1 */
+	rule->layers[1].level = 2;
+	rule->layers[1].access = BIT_ULL(1);
+
+	/* Case 1: No rule_flags provided (should behave normally) */
+	for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+		layer_masks[i] = layers_initialized;
+
+	landlock_unmask_layers(rule, access_request, &layer_masks,
+			       ARRAY_SIZE(layer_masks), NULL);
+
+	/* Access 0 should be unmasked by layer 1 */
+	KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+	/* Access 1 should be unmasked by layer 2 */
+	KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+	/* Case 2: rule_flags provided, no existing no_inherit_masks */
+	for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+		layer_masks[i] = layers_initialized;
+	memset(&rule_flags, 0, sizeof(rule_flags));
+
+	landlock_unmask_layers(rule, access_request, &layer_masks,
+			       ARRAY_SIZE(layer_masks), &rule_flags);
+
+	/* Access 0 should be unmasked by layer 1 */
+	KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+	/* Access 1 should be unmasked by layer 2 */
+	KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+	/* rule_flags should collect no_inherit from layer 1 */
+	KUNIT_EXPECT_EQ(test, rule_flags.no_inherit_masks, (layer_mask_t)BIT_ULL(0));
+
+	/* Case 3: rule_flags provided, layer 1 is masked by no_inherit_masks */
+	for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+		layer_masks[i] = layers_initialized;
+	memset(&rule_flags, 0, sizeof(rule_flags));
+	rule_flags.no_inherit_masks = BIT_ULL(0); /* Mask layer 1 */
+
+	landlock_unmask_layers(rule, access_request, &layer_masks,
+			       ARRAY_SIZE(layer_masks), &rule_flags);
+
+	/* Access 0 should NOT be unmasked by layer 1 because it is skipped */
+	KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized);
+	/* Access 1 should be unmasked by layer 2 */
+	KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+	kfree(rule);
+}
+
+static struct kunit_case ruleset_test_cases[] = {
+	KUNIT_CASE(test_unmask_layers_no_inherit),
+	{}
+};
+
+static struct kunit_suite ruleset_test_suite = {
+	.name = "landlock_ruleset",
+	.test_cases = ruleset_test_cases,
+};
+
+kunit_test_suite(ruleset_test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
-- 
2.51.0

[PATCH v3 5/5] landlock: landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Add a unit test for rule_flag collection, ensuring that access masks
are properly propagated with the flags.

changes v2..v3:

   * Removing erroneously misplaced code and placed in the proper
     patch.

Signed-off-by: Justin Suess <utilityemal77@gmail.com>
---
 security/landlock/ruleset.c | 89 +++++++++++++++++++++++++++++++++++++
 1 file changed, 89 insertions(+)

diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index f7b6a48bbf39..0e0de8b20dc4 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -22,6 +22,7 @@
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <uapi/linux/landlock.h>
+#include <kunit/test.h>
 
 #include "access.h"
 #include "audit.h"
@@ -774,3 +775,91 @@ landlock_init_layer_masks(const struct landlock_ruleset *const domain,
 	}
 	return handled_accesses;
 }
+
+#ifdef CONFIG_SECURITY_LANDLOCK_KUNIT_TEST
+
+/**
+ * test_unmask_layers_no_inherit - Test landlock_unmask_layers() with no_inherit
+ * @rule_flags: Pointer to collected_rule_flags structure to track flags.
+ */
+static void test_unmask_layers_no_inherit(struct kunit *const test)
+{
+	struct landlock_rule *rule;
+	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_FS];
+	struct collected_rule_flags rule_flags;
+	const access_mask_t access_request = BIT_ULL(0) | BIT_ULL(1);
+	const layer_mask_t layers_initialized = BIT_ULL(0) | BIT_ULL(1);
+	size_t i;
+
+	rule = kzalloc(struct_size(rule, layers, 2), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, rule);
+
+	rule->num_layers = 2;
+
+	/* Layer 1: allows access 0, no_inherit */
+	rule->layers[0].level = 1;
+	rule->layers[0].access = BIT_ULL(0);
+	rule->layers[0].flags.no_inherit = 1;
+
+	/* Layer 2: allows access 1 */
+	rule->layers[1].level = 2;
+	rule->layers[1].access = BIT_ULL(1);
+
+	/* Case 1: No rule_flags provided (should behave normally) */
+	for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+		layer_masks[i] = layers_initialized;
+
+	landlock_unmask_layers(rule, access_request, &layer_masks,
+			       ARRAY_SIZE(layer_masks), NULL);
+
+	/* Access 0 should be unmasked by layer 1 */
+	KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+	/* Access 1 should be unmasked by layer 2 */
+	KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+	/* Case 2: rule_flags provided, no existing no_inherit_masks */
+	for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+		layer_masks[i] = layers_initialized;
+	memset(&rule_flags, 0, sizeof(rule_flags));
+
+	landlock_unmask_layers(rule, access_request, &layer_masks,
+			       ARRAY_SIZE(layer_masks), &rule_flags);
+
+	/* Access 0 should be unmasked by layer 1 */
+	KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized & ~BIT_ULL(0));
+	/* Access 1 should be unmasked by layer 2 */
+	KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+	/* rule_flags should collect no_inherit from layer 1 */
+	KUNIT_EXPECT_EQ(test, rule_flags.no_inherit_masks, (layer_mask_t)BIT_ULL(0));
+
+	/* Case 3: rule_flags provided, layer 1 is masked by no_inherit_masks */
+	for (i = 0; i < ARRAY_SIZE(layer_masks); i++)
+		layer_masks[i] = layers_initialized;
+	memset(&rule_flags, 0, sizeof(rule_flags));
+	rule_flags.no_inherit_masks = BIT_ULL(0); /* Mask layer 1 */
+
+	landlock_unmask_layers(rule, access_request, &layer_masks,
+			       ARRAY_SIZE(layer_masks), &rule_flags);
+
+	/* Access 0 should NOT be unmasked by layer 1 because it is skipped */
+	KUNIT_EXPECT_EQ(test, layer_masks[0], layers_initialized);
+	/* Access 1 should be unmasked by layer 2 */
+	KUNIT_EXPECT_EQ(test, layer_masks[1], layers_initialized & ~BIT_ULL(1));
+
+	kfree(rule);
+}
+
+static struct kunit_case ruleset_test_cases[] = {
+	KUNIT_CASE(test_unmask_layers_no_inherit),
+	{}
+};
+
+static struct kunit_suite ruleset_test_suite = {
+	.name = "landlock_ruleset",
+	.test_cases = ruleset_test_cases,
+};
+
+kunit_test_suite(ruleset_test_suite);
+
+#endif /* CONFIG_SECURITY_LANDLOCK_KUNIT_TEST */
-- 
2.51.0

Re: [PATCH v3 5/5] landlock: landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Hi,

Apologies. Thi is a duplicate of the above patch
v3 5/5 I sent accidentally.

I forgot to clear the old .patch file from my directory after I changed the
commit message, and didn't catch it when doing git send-email.

The contents are identical, I just changed the commit message from
a typo.

"landlock: landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT"
(typo)
to 
"landlock: Implement KUnit test for LANDLOCK_ADD_RULE_NO_INHERIT"
(correct commit message)

Please ignore this duplicate. I promise to be more careful in the
future.

Regards,
Justin Suess

Re: [PATCH v3 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

This is the core part of the patch series I was unsure about.

Here when a rule tagged with LANDLOCK_ADD_RULE_NO_INHERIT is 
inserted, "blank" dentry rules (meaning rules with no access grants), 
are inserted along with the original rule on each parent of the 
original rule up to the root (but only for inodes without existing 
rules). These are then (by the mark_no_inherit_ancestors call) tagged 
with the has no inherit descendent marker (has_no_inherit_desc). This 
is used to provide the parent directory protections. 

The purpose of these blank rules is to ensure when we do a 
find_rule() on any of the LANDLOCK_ADD_RULE_NO_INHERIT tagged rule's 
parents, we will immediately know we have to disallow topology 
changes on that inode to enforce the parent directory protections 
described in the cover letter.

This lets us perform the check for these protections in O(log(n)) 
consistent with the red black tree's insertion time. The insertion 
penalty for this is O(depth * log(n)), but this is done only at 
ruleset creation time. That is somewhat more acceptable in my 
opinion. 

Additionally I suspect keeping all the rule tracking logic in one 
data structure helps with cache locality and decreases bugs from not 
keeping the rules in sync with a seperate structure. It also reduces 
the LOC and complexity somewhat. 

The previous v2 of this patch used a complex (and buggy I found after 
running the v3 test suite on it) xarray to track this. The check for 
parent directory protections was O(n) and insertion was O(n * depth). 

My questions for reviewers:

  * Is it acceptable in this case for landlock to automatically and 
silently insert rules that the user didn't explicity declare?

  * Should these protections instead be implemented in a seperate 
data structure?

  * Is the performance cost for the current  implementation 
acceptable? 

Normal landlock insertion and checking is O(log(n)). For rules with 
this tag, checking is still O(log(n)) but insertion is O(depth * 
log(n)). 

Kind Regards,

Justin Suess

Re: [PATCH v3 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Tingmao Wang]

Hi!

I've done a "partial" review of this series (with most of the attention on
this patch).  Aside from the comments below, I think we might need to
think a bit more about the implications of things like hard links and bind
mounts, which makes the notion of "parent" non-trivial (as Mickaël also
pointed out on the GitHub thread).  However, I think it should mostly be
good.  My first pass of reasoning for bind mounts are:

A deny rule does not prevent access through a bind mount if the bind mount
points deep into the denied hierarchy (not to the denied dentry itself),
but does protect it if the bind mount points to the denied dentry itself
or its parent.  Therefore, to properly protect a directory that contains
children that might possibly be bind mounted to, a sandboxer just has to
attach deny rules to that directory, plus any bind mounts pointing toward
anything in it, which seems like a reasonable ask given that the sandboxed
application cannot make mounts itself.

Hopefully this review turns out to be useful :)

On 11/26/25 12:20, Justin Suess wrote:
> Implements a flag to prevent access grant inheritance within the filesystem hierarchy
> for landlock rules.
>
> If a landlock rule on an inode has this flag, any access grants on parent inodes will be
> ignored. Moreover, operations that involve altering the direct parent tree of the subject with

I feel like the wording "direct parent tree" is a bit unclear - I think
what you meant is "altering the parent of the subject or its ancestors",
right?

> LANDLOCK_ADD_RULE_NO_INHERIT will be denied up to the mountpoint.

Tbh I'm not entirely clear on why we only deny rename of parents up to the
mountpoint.

>
> Additionally (new in v3) parent flag inheritance is blocked by this flag, allowing fine
> grained access control over LANDLOCK_ADD_RULE_QUIET.
>
> For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and / = read write, writes to
> files in /a/b/c will be denied. Moreover, moving /a to /bad, removing /a/b/c, or creating links to
> /a will be prohibited.
>
> And if / has LANDLOCK_ADD_RULE_QUIET, /a/b/c will still audit (handled)
> accesses. This is because LANDLOCK_ADD_RULE_NO_INHERIT also
> suppresses flag inheritance from parent objects.
>
> The parent directory restrictions mitigate sandbox-restart attacks. For example, if a sandboxed program
> is able to move a LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upon sandbox restart, the policy
> applied naively on the same filenames would be invalid. Preventing these operations mitigates these attacks.
>
> v2..v3 changes:
>
>   * Parent directory topology protections now work by lazily
>     inserting blank rules on parent inodes if they do not
>     exist. This replaces the previous xarray implementation
>     with simplified logic.
>   * Added an optimization to skip further processing if all layers collected
>     no inherit.
>   * Added support to block flag inheritance.
>
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
>  security/landlock/audit.c   |   4 +-
>  security/landlock/domain.c  |   4 +-
>  security/landlock/fs.c      | 592 +++++++++++++++++++++++++++++++++++-
>  security/landlock/ruleset.c |  27 +-
>  security/landlock/ruleset.h |  36 ++-
>  5 files changed, 645 insertions(+), 18 deletions(-)
>
> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
> index d51563712325..4da97dd6985c 100644
> --- a/security/landlock/audit.c
> +++ b/security/landlock/audit.c
> @@ -588,7 +588,9 @@ void landlock_log_denial(const struct landlock_cred_security *const subject,
>  				subject->domain, &missing, request->layer_masks,
>  				request->layer_masks_size);
>  			object_quiet_flag = !!(request->rule_flags.quiet_masks &
> -					       BIT(youngest_layer));
> +				       BIT(youngest_layer)) &&
> +				!(request->rule_flags.blocked_flag_masks &
> +				  BIT(youngest_layer));
>  		} else {
>  			youngest_layer = get_layer_from_deny_masks(
>  				&missing, request->all_existing_optional_access,
> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
> index 8caf07250328..5bd83865c87d 100644
> --- a/security/landlock/domain.c
> +++ b/security/landlock/domain.c
> @@ -236,7 +236,9 @@ optional_access_t landlock_get_quiet_optional_accesses(
>  			 BITS_PER_TYPE(access_mask_t)) {
>  		const u8 layer = (deny_masks >> (access_index * 4)) &
>  				 (LANDLOCK_MAX_NUM_LAYERS - 1);
> -		const bool is_quiet = !!(rule_flags.quiet_masks & BIT(layer));
> +		const layer_mask_t layer_bit = BIT(layer);
> +		const bool is_quiet = !!(rule_flags.quiet_masks & layer_bit) &&
> +				  !(rule_flags.blocked_flag_masks & layer_bit);
>
>  		if (is_quiet)
>  			quiet_optional_accesses |= BIT(access_index);
> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 29f10da32141..0a5c73f18f26 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -317,6 +317,206 @@ static struct landlock_object *get_inode_object(struct inode *const inode)
>  	LANDLOCK_ACCESS_FS_IOCTL_DEV)
>  /* clang-format on */
>
> +static const struct landlock_rule *find_rule(const struct landlock_ruleset *const domain,
> +					     const struct dentry *const dentry);
> +
> +/**
> + * landlock_domain_layers_mask - Build a mask covering all layers of a domain
> + * @domain: The ruleset (domain) to inspect.
> + *
> + * Return a layer mask with a 1 bit for each existing layer of @domain.
> + * If @domain has no layers 0 is returned.  If the number of layers is
> + * greater than or equal to the number of bits in layer_mask_t, all bits
> + * are set.
> + */
> +static layer_mask_t landlock_domain_layers_mask(const struct landlock_ruleset
> +						*const domain)
> +{
> +	if (!domain || !domain->num_layers)
> +		return 0;
> +
> +	if (domain->num_layers >= sizeof(layer_mask_t) * BITS_PER_BYTE)
> +		return (layer_mask_t)~0ULL;
> +
> +	return GENMASK_ULL(domain->num_layers - 1, 0);
> +}
> +
> +/**
> + * rule_blocks_all_layers_no_inherit - check whether a rule disables inheritance
> + * @domain_layers_mask: Mask describing the domain's active layers.
> + * @rule: Rule to inspect.
> + *
> + * Return true if every layer present in @rule has its no_inherit flag set
> + * and the set of layers covered by the rule equals @domain_layers_mask.
> + * This indicates that the rule prevents inheritance on all layers of the
> + * domain and thus further walking for inheritance checks can stop.
> + */
> +static bool rule_blocks_all_layers_no_inherit(const layer_mask_t domain_layers_mask,
> +					      const struct landlock_rule *const rule)
> +{
> +	layer_mask_t rule_layers = 0;
> +	u32 layer_index;
> +
> +	if (!domain_layers_mask || !rule)
> +		return false;
> +
> +	for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
> +		const struct landlock_layer *const layer =
> +			&rule->layers[layer_index];
> +		const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
> +
> +		if (!layer->flags.no_inherit)
> +			return false;
> +
> +		rule_layers |= layer_bit;
> +	}
> +
> +	return rule_layers && rule_layers == domain_layers_mask;
> +}
> +
> +/**
> + * landlock_collect_no_inherit_layers - Collects layers with no_inherit flags
> + */

Note likely misplaced comment here

> +/**
> + * landlock_collect_no_inherit_layers - collect effective no_inherit layers
> + * @ruleset: Ruleset to consult.
> + * @dentry: Dentry used as a starting point for the upward walk.
> + *
> + * Walk upwards from @dentry and return a layer mask containing the layers
> + * for which either a rule on the visited dentry has the no_inherit flag set
> + * or where an ancestor was previously marked as having a descendant with
> + * a no_inherit rule.  The search prefers the closest matching dentry and
> + * stops once any relevant layer bits are found or the root is reached.
> + *
> + * Returns a layer_mask_t where each set bit corresponds to a layer with an
> + * effective no_inherit influence for @dentry.  Returns 0 if none apply or if
> + * inputs are invalid.
> + */
> +static layer_mask_t landlock_collect_no_inherit_layers(const struct landlock_ruleset
> +						       *const ruleset,
> +						       struct dentry *const dentry)
> +{
> +	struct dentry *cursor, *parent;
> +	layer_mask_t layers = 0;
> +	bool include_descendants = true;
> +
> +	if (!ruleset || !dentry || d_is_negative(dentry))
> +		return 0;
> +
> +	cursor = dget(dentry);
> +	while (true) {
> +		const struct landlock_rule *rule;
> +		u32 layer_index;
> +
> +		rule = find_rule(ruleset, cursor);
> +		if (rule) {
> +			for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {
> +				const struct landlock_layer *layer = &rule->layers[layer_index];
> +
> +				if (layer->flags.no_inherit ||
> +				    (include_descendants &&
> +				     layer->flags.has_no_inherit_descendant))
> +					layers |= BIT_ULL((layer->level ?
> +						layer->level : layer_index + 1) - 1);
> +			}
> +		}
> +
> +		if (layers) {
> +			dput(cursor);
> +			return layers;
> +		}
> +
> +		if (IS_ROOT(cursor)) {
> +			dput(cursor);
> +			break;
> +		}
> +
> +		parent = dget_parent(cursor);
> +		dput(cursor);
> +		if (!parent)
> +			break;
> +
> +		cursor = parent;
> +		include_descendants = false;
> +	}
> +	return 0;
> +}
> +
> +static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
> +				     struct dentry *dentry,
> +				     layer_mask_t descendant_layers);

Wouldn't you be able to avoid this declaration by moving the definition
for ensure_rule_for_dentry and mark_no_inherit_ancestors up a bit, before
mask_no_inherit_descendant_layers?

> +
> +/**
> + * mask_no_inherit_descendant_layers - apply descendant no_inherit masking
> + * @domain: The ruleset (domain) to consult.
> + * @dentry: The dentry whose descendants are considered.
> + * @child_layers: Layers present on the child that may be subject to masking.
> + * @access_request: Accesses being requested (bitmask).
> + * @layer_masks: Per-access layer masks to be modified in-place.
> + * @rule_flags: Collected flags which will be updated accordingly.
> + *
> + * If descendant dentries have no_inherit, clear that
> + * layer's bit from @layer_masks. Also updates @rule_flags to reflect
> + * which layers were blocked.  Returns true if any of the @layer_masks were
> + * modified, false otherwise.
> + */
> +static bool mask_no_inherit_descendant_layers(const struct landlock_ruleset
> +					      *const domain,
> +					      struct dentry *const dentry,
> +					      layer_mask_t child_layers,
> +					      const access_mask_t access_request,
> +					      layer_mask_t
> +					      (*const layer_masks)
> +					      [LANDLOCK_NUM_ACCESS_FS],
> +					      struct collected_rule_flags
> +					      *const rule_flags)
> +{
> +	layer_mask_t descendant_layers;
> +	const unsigned long access_req = access_request;
> +	unsigned long access_bit;
> +	bool changed = false;
> +
> +	if (!access_request || !layer_masks || !rule_flags || !dentry)
> +		return false;
> +	if (d_is_negative(dentry))
> +		return false;
> +
> +	descendant_layers = landlock_collect_no_inherit_layers(domain, dentry);
> +	{
> +		layer_mask_t shared_layers = descendant_layers & child_layers;
> +
> +		if (shared_layers) {
> +			rule_flags->no_inherit_masks |= shared_layers;
> +			rule_flags->no_inherit_desc_masks |= shared_layers;
> +			rule_flags->blocked_flag_masks |= shared_layers;
> +		}
> +	}
> +	descendant_layers &= ~child_layers;
> +	descendant_layers &= ~rule_flags->no_inherit_masks;
> +	if (!descendant_layers)
> +		return false;
> +
> +	rule_flags->blocked_flag_masks |= descendant_layers;
> +
> +	for_each_set_bit(access_bit, &access_req,
> +			 ARRAY_SIZE(*layer_masks)) {
> +		layer_mask_t *const layer_mask = &(*layer_masks)[access_bit];
> +
> +		if (*layer_mask & descendant_layers) {
> +			*layer_mask &= ~descendant_layers;
> +			changed = true;
> +		}
> +	}
> +
> +	if (!changed)
> +		return false;
> +
> +	rule_flags->no_inherit_masks |= descendant_layers;
> +	rule_flags->no_inherit_desc_masks |= descendant_layers;
> +
> +	return true;
> +}
> +
>  /*
>   * @path: Should have been checked by get_path_from_fd().
>   */
> @@ -325,12 +525,13 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>  			    access_mask_t access_rights, const int flags)
>  {
>  	int err;
> +	const bool is_dir = d_is_dir(path->dentry);
>  	struct landlock_id id = {
>  		.type = LANDLOCK_KEY_INODE,
>  	};
>
>  	/* Files only get access rights that make sense. */
> -	if (!d_is_dir(path->dentry) &&
> +	if (!is_dir &&
>  	    (access_rights | ACCESS_FILE) != ACCESS_FILE)
>  		return -EINVAL;
>  	if (WARN_ON_ONCE(ruleset->num_layers != 1))
> @@ -344,13 +545,43 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>  		return PTR_ERR(id.key.object);
>  	mutex_lock(&ruleset->lock);
>  	err = landlock_insert_rule(ruleset, id, access_rights, flags);
> +	if (!err && (flags & LANDLOCK_ADD_RULE_NO_INHERIT)) {
> +		const struct landlock_rule *rule;
> +		layer_mask_t descendant_layers = 0;
> +		u32 layer_index;
> +
> +		rule = find_rule(ruleset, path->dentry);
> +		if (rule) {
> +			for (layer_index = 0; layer_index < rule->num_layers; layer_index++) {

This function is only called to add rules to an "unmerged" ruleset, which
will always only have one layer.  So probably this can just be a

	err = landlock_insert_rule(ruleset, id, access_rights, flags);
	if (err)
		goto out_unlock;
	if (flags & LANDLOCK_ADD_RULE_NO_INHERIT) {
		err = mark_no_inherit_ancestors(ruleset, path->dentry);
		if (err)
			goto out_unlock;
	}

And for a similar reason, you don't have to do a for(layer_index) in
mark_no_inherit_ancestors either.  (Basically I think you would just set
rule->layers[0].flags.has_no_inherit_descendant to true)

It might be helpful to validate / document this reasoning by adding
WARN_ONCE(rule->num_layers != 1, "unmerged rulesets should only have one
layer") either here or in mark_no_inherit_ancestors.

> +				const struct landlock_layer *layer =
> +					&rule->layers[layer_index];
> +
> +				if (layer->flags.no_inherit ||
> +				    layer->flags.has_no_inherit_descendant)
> +					descendant_layers |=
> +						BIT_ULL((layer->level ?
> +							 layer->level : layer_index + 1) - 1);
> +			}
> +			if (descendant_layers) {
> +				err = mark_no_inherit_ancestors(ruleset, path->dentry,
> +								descendant_layers);
> +				if (err)
> +					goto out_unlock;
> +			}
> +		}
> +	}
>  	mutex_unlock(&ruleset->lock);
> +out:
>  	/*
>  	 * No need to check for an error because landlock_insert_rule()
>  	 * increments the refcount for the new object if needed.
>  	 */
>  	landlock_put_object(id.key.object);
>  	return err;
> +
> +out_unlock:
> +	mutex_unlock(&ruleset->lock);
> +	goto out;
>  }
>
>  /* Access-control management */
> @@ -382,6 +613,134 @@ find_rule(const struct landlock_ruleset *const domain,
>  	return rule;
>  }
>
> +/**
> + * ensure_rule_for_dentry - ensure a ruleset contains a rule entry for dentry,
> + * inserting a blank rule if needed.
> + * @ruleset: Ruleset to modify/inspect.  Caller must hold @ruleset->lock.
> + * @dentry: Dentry to ensure a rule exists for.
> + *
> + * If no rule is currently associated with @dentry, insert an empty rule
> + * (with zero access) tied to the backing inode.  Returns a pointer to the
> + * rule associated with @dentry on success, NULL when @dentry is negative, or
> + * an ERR_PTR()-encoded error if the rule cannot be created.
> + *
> + * This is useful for LANDLOCK_ADD_RULE_NO_INHERIT processing, where a rule
> + * may need to be created for an ancestor dentry that does not yet have one
> + * to properly track no_inherit flags.
> + *
> + * The flags are set to zero if a rule is newly created, and the caller
> + * is responsible for setting them appropriately.
> + *
> + * The returned rule pointer's lifetime is tied to @ruleset.
> + */
> +static const struct landlock_rule *
> +ensure_rule_for_dentry(struct landlock_ruleset *const ruleset,
> +		       struct dentry *const dentry)
> +{
> +	struct landlock_id id = {
> +		.type = LANDLOCK_KEY_INODE,
> +	};
> +	const struct landlock_rule *rule;
> +	int err;
> +
> +	if (!ruleset || !dentry || d_is_negative(dentry))
> +		return NULL;
> +
> +	lockdep_assert_held(&ruleset->lock);
> +
> +	rule = find_rule(ruleset, dentry);
> +	if (rule)
> +		return rule;
> +
> +	id.key.object = get_inode_object(d_backing_inode(dentry));
> +	if (IS_ERR(id.key.object))
> +		return ERR_CAST(id.key.object);
> +
> +	err = landlock_insert_rule(ruleset, id, 0, 0);
> +	landlock_put_object(id.key.object);
> +	if (err)
> +		return ERR_PTR(err);
> +
> +	rule = find_rule(ruleset, dentry);
> +	return rule ? rule : ERR_PTR(-ENOENT);

I feel like this deserves a WARN_ON_ONCE(!rule) before this line - we
don't really expect to not find a rule right after adding it.

On the other hand, the only reason why we need to re-lookup the rule seems
to be because landlock_insert_rule() does not return the newly added rule.
We could change it to do so, and not have to do this extra lookup.

This also nicely solves the constness issue - landlock_insert_rule could
return the mutable pointer.

> +}
> +
> +/**
> + * mark_no_inherit_ancestors - mark ancestors as having no_inherit descendants
> + * @ruleset: Ruleset to modify.  Caller must hold @ruleset->lock.
> + * @dentry: Dentry representing the descendant that carries no_inherit bits.
> + * @descendant_layers: Mask of layers from the descendant that should be
> + *                     advertised to ancestors via has_no_inherit_descendant.
> + *
> + * Walks upward from @dentry and ensures that any ancestor rule contains the
> + * has_no_inherit_descendant marker for the specified @descendant_layers so
> + * parent lookups can quickly detect descendant no_inherit influence.
> + *
> + * Returns 0 on success or a negative errno if ancestor bookkeeping fails.
> + */
> +static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
> +				     struct dentry *dentry,
> +				     layer_mask_t descendant_layers)
> +{
> +	struct dentry *cursor;
> +	u32 layer_index;
> +	int err = 0;
> +
> +	if (!ruleset || !dentry || !descendant_layers)
> +		return -EINVAL;
> +
> +	lockdep_assert_held(&ruleset->lock);
> +
> +	cursor = dget(dentry);
> +	while (cursor) {
> +		struct dentry *parent;
> +
> +		if (IS_ROOT(cursor)) {
> +			dput(cursor);
> +			break;
> +		}
> +
> +		parent = dget_parent(cursor);
> +		dput(cursor);
> +		if (!parent)
> +			break;
> +
> +		if (!d_is_negative(parent)) {

My understanding is that if the child is not negative (which is required
for us to actually get here), as long as we always have a reference to it,
none of its parents should be negative as well.  This should probably be a
WARN_ON_ONCE(d_is_negative(parent)).

I think some of the other d_is_negative() checks in this patch also have a
similar argument (i.e. turn into WARN_ON_ONCE or be removed if not
necessary), but I've not looked at them all.

> +			const struct landlock_rule *rule;
> +			/* Ensures a rule exists for the parent dentry,
> +			 * inserting a blank one if needed
> +			 */
> +			rule = ensure_rule_for_dentry(ruleset, parent);
> +			if (IS_ERR(rule)) {
> +				err = PTR_ERR(rule);
> +				dput(parent);
> +				cursor = NULL;
> +				break;
> +			}
> +			if (rule) {
> +				struct landlock_rule *mutable_rule =
> +					(struct landlock_rule *)rule;
> +
> +				for (layer_index = 0;
> +				     layer_index < mutable_rule->num_layers;
> +				     layer_index++) {
> +					struct landlock_layer *layer =
> +						&mutable_rule->layers[layer_index];
> +					layer_mask_t layer_bit =
> +						BIT_ULL((layer->level ?
> +							layer->level : layer_index + 1) - 1);
> +
> +					if (descendant_layers & layer_bit)
> +						layer->flags.has_no_inherit_descendant = true;
> +				}
> +			}
> +		}
> +
> +		cursor = parent;
> +	}
> +	return err;
> +}
> +
>  /*
>   * Allows access to pseudo filesystems that will never be mountable (e.g.
>   * sockfs, pipefs), but can still be reachable through
> @@ -764,6 +1123,8 @@ static bool is_access_to_paths_allowed(
>  	struct landlock_request *const log_request_parent2,
>  	struct dentry *const dentry_child2)
>  {
> +	const layer_mask_t domain_layers_mask =
> +		landlock_domain_layers_mask(domain);
>  	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check,
>  	     is_dom_check_bkp, child1_is_directory = true,
>  	     child2_is_directory = true;
> @@ -778,6 +1139,13 @@ static bool is_access_to_paths_allowed(
>  	struct collected_rule_flags *rule_flags_parent1 = &log_request_parent1->rule_flags;
>  	struct collected_rule_flags *rule_flags_parent2 = &log_request_parent2->rule_flags;
>  	struct collected_rule_flags _rule_flag_parent1_bkp, _rule_flag_parent2_bkp;
> +	layer_mask_t child1_layers = 0;
> +	layer_mask_t child2_layers = 0;
> +
> +	if (dentry_child1)
> +		child1_layers = landlock_collect_no_inherit_layers(domain, dentry_child1);
> +	if (dentry_child2)
> +		child2_layers = landlock_collect_no_inherit_layers(domain, dentry_child2);
>
>  	if (!access_request_parent1 && !access_request_parent2)
>  		return true;
> @@ -931,6 +1299,10 @@ static bool is_access_to_paths_allowed(
>  					       ARRAY_SIZE(*layer_masks_parent2),
>  					       rule_flags_parent2);
>
> +		if (rule &&
> +		    rule_blocks_all_layers_no_inherit(domain_layers_mask, rule))
> +			break;
> +
>  		/* Stops when a rule from each layer grants access. */
>  		if (allowed_parent1 && allowed_parent2) {
>  			/*
> @@ -976,8 +1348,13 @@ static bool is_access_to_paths_allowed(
>  					memcpy(&_rule_flag_parent2_bkp,
>  					       rule_flags_parent2,
>  					       sizeof(_rule_flag_parent2_bkp));
> -					is_dom_check_bkp = is_dom_check;
>  				}
> +				is_dom_check_bkp = is_dom_check;
> +				child1_layers = landlock_collect_no_inherit_layers(domain,
> +										   walker_path
> +										   .dentry);
> +				if (layer_masks_parent2)
> +					child2_layers = child1_layers;
>
>  				/* Ignores hidden mount points. */
>  				goto jump_up;
> @@ -1001,15 +1378,50 @@ static bool is_access_to_paths_allowed(
>  				break;
>  			}
>
> -			/*
> -			 * We reached a disconnected root directory from a bind mount, and
> -			 * we need to reset the walk to the current mount root.
> -			 */
> -			goto reset_to_mount_root;
> -		}
> -		parent_dentry = dget_parent(walker_path.dentry);
> -		dput(walker_path.dentry);
> -		walker_path.dentry = parent_dentry;
> +		/*
> +		 * We reached a disconnected root directory from a bind mount, and
> +		 * we need to reset the walk to the current mount root.
> +		 */

Accidental change of indentation?

> +		goto reset_to_mount_root;
> +	}
> +	if (likely(!d_is_negative(walker_path.dentry))) {
> +		child1_layers = landlock_collect_no_inherit_layers(domain,
> +								   walker_path.dentry);
> +		if (layer_masks_parent2)
> +			child2_layers = child1_layers;
> +	} else {
> +		child1_layers = 0;
> +		if (layer_masks_parent2)
> +			child2_layers = 0;
> +	}
> +	parent_dentry = dget_parent(walker_path.dentry);
> +	dput(walker_path.dentry);
> +	walker_path.dentry = parent_dentry;
> +	/*
> +	 * Apply descendant no-inherit masking now that we've moved to the
> +	 * parent. This ensures the parent respects any no-inherit rules from
> +	 * the child we just left. Only applies to refer operations (rename/link).
> +	 */
> +	if (unlikely(layer_masks_parent2)) {
> +		if (mask_no_inherit_descendant_layers(domain, walker_path.dentry,
> +						      child1_layers,
> +						      access_masked_parent1,
> +						      layer_masks_parent1,
> +						      rule_flags_parent1))
> +			allowed_parent1 =
> +				allowed_parent1 ||
> +				is_layer_masks_allowed(layer_masks_parent1);
> +
> +		if (rule_flags_parent2 &&
> +		    mask_no_inherit_descendant_layers(domain, walker_path.dentry,
> +						      child2_layers,
> +						      access_masked_parent2,
> +						      layer_masks_parent2,
> +						      rule_flags_parent2))
> +			allowed_parent2 =
> +				allowed_parent2 ||
> +				is_layer_masks_allowed(layer_masks_parent2);
> +	}

Maybe I'm missing something, but I can't tell what's the purpose of this
block, or in fact what mask_no_inherit_descendant_layers and
landlock_collect_no_inherit_layers is for.  The doc comment for
mask_no_inherit_descendant_layers seems to suggest that it's supposed to
walk descendents, but landlock_collect_no_inherit_layers is actually
walking ancestors.  Removing these checks doesn't seem to break any tests,
and sandboxer still seems to work as expected wrt. no_inherit rules and
denial of renaming denied dentries and its parents.

Note that the special "reverting" style disconnected directory handling
has been removed in Mickaël's next branch (i.e. the "backup" logic is
removed), which should hopefully simplify reasoning about this.

>  		continue;
>
>  reset_to_mount_root:
> @@ -1057,6 +1469,10 @@ static bool is_access_to_paths_allowed(
>  		dput(walker_path.dentry);
>  		walker_path.dentry = walker_path.mnt->mnt_root;
>  		dget(walker_path.dentry);
> +		child1_layers = landlock_collect_no_inherit_layers(domain,
> +								   walker_path.dentry);
> +		if (layer_masks_parent2)
> +			child2_layers = child1_layers;
>  	}
>  	path_put(&walker_path);
>
> @@ -1172,6 +1588,8 @@ static bool collect_domain_accesses(
>  	struct collected_rule_flags *const rule_flags)
>  {
>  	access_mask_t access_dom;
> +	const layer_mask_t domain_layers_mask =
> +		landlock_domain_layers_mask(domain);
>  	bool ret = false;
>
>  	if (WARN_ON_ONCE(!domain || !mnt_dir || !dir || !layer_masks_dom))
> @@ -1187,9 +1605,11 @@ static bool collect_domain_accesses(
>  	while (true) {
>  		struct dentry *parent_dentry;
>
> +		const struct landlock_rule *rule = find_rule(domain, dir);
> +
>  		/* Gets all layers allowing all domain accesses. */
>  		if (landlock_unmask_layers(
> -			    find_rule(domain, dir), access_dom, layer_masks_dom,
> +			    rule, access_dom, layer_masks_dom,
>  			    ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
>  			/*
>  			 * Before allowing this side of the access request, checks that the
> @@ -1206,6 +1626,10 @@ static bool collect_domain_accesses(
>  			break;
>  		}
>
> +		if (rule &&
> +		    rule_blocks_all_layers_no_inherit(domain_layers_mask, rule))
> +			break;
> +
>  		/* Stops at the mount point. */
>  		if (dir == mnt_dir->dentry)
>  			break;
> @@ -1232,6 +1656,121 @@ static bool collect_domain_accesses(
>  	return ret;
>  }
>
> +/**
> + * collect_topology_sealed_layers - collect layers sealed against topology changes
> + * @domain: Ruleset to consult.
> + * @dentry: Starting dentry for the upward walk.
> + * @override_layers: Optional out parameter filled with layers that are
> + *                   present on ancestors but considered overrides (not
> + *                   sealing the topology for descendants).
> + *
> + * Walk upwards from @dentry and return a mask of layers where either the
> + * visited dentry contains a no_inherit rule or ancestors were previously
> + * marked as having a descendant with no_inherit.  @override_layers, if not
> + * NULL, is filled with layers that would normally be overridden by more
> + * specific descendant rules.
> + *
> + * Returns a layer mask where set bits indicate layers that are "sealed"
> + * (topology changes like rename/rmdir are denied) for the subtree rooted at
> + * @dentry.
> + *
> + * Useful for LANDLOCK_ADD_RULE_NO_INHERIT parent directory enforcement to ensure
> + * that topology changes do not violate the no_inherit constraints.
> + */
> +static layer_mask_t
> +collect_topology_sealed_layers(const struct landlock_ruleset *const domain,
> +			       struct dentry *dentry,
> +			       layer_mask_t *const override_layers)
> +{
> +	struct dentry *cursor, *parent;
> +	bool include_descendants = true;
> +	layer_mask_t sealed_layers = 0;
> +
> +	if (override_layers)
> +		*override_layers = 0;
> +
> +	if (!domain || !dentry || d_is_negative(dentry))
> +		return 0;
> +
> +	cursor = dget(dentry);
> +	while (cursor) {
> +		const struct landlock_rule *rule;
> +		u32 layer_index;
> +
> +		rule = find_rule(domain, cursor);
> +		if (rule) {
> +			for (layer_index = 0; layer_index < rule->num_layers;
> +			     layer_index++) {
> +				const struct landlock_layer *layer =
> +					&rule->layers[layer_index];
> +				const int level = layer->level ? layer->level :
> +								 layer_index + 1;

Wouldn't layer->level always be >= 1 here?  Using layer_index doesn't make
sense since layer_index is just the index that the struct landlock_layer
happened to be in that rule's array.

> +				layer_mask_t layer_bit = BIT_ULL(level - 1);
> +
> +				if (include_descendants &&
> +				    (layer->flags.no_inherit ||
> +				     layer->flags.has_no_inherit_descendant)) {
> +					sealed_layers |= layer_bit;
> +				} else if (override_layers) {
> +					*override_layers |= layer_bit;
> +				}
> +			}
> +		}
> +
> +		if (sealed_layers || IS_ROOT(cursor))
> +			break;
> +
> +		parent = dget_parent(cursor);
> +		dput(cursor);
> +		if (!parent)
> +			return sealed_layers;
> +
> +		cursor = parent;
> +		include_descendants = false;
> +	}
> +	dput(cursor);
> +	return sealed_layers;
> +}
> +
> +/**
> + * deny_no_inherit_topology_change - deny topology changes on sealed layers
> + * @subject: Subject performing the operation (contains the domain).
> + * @dentry: Dentry that is the target of the topology modification.
> + *
> + * Checks whether any domain layers are sealed against topology changes at
> + * @dentry (via collect_topology_sealed_layers).  If so, emit an audit record
> + * and return -EACCES.  Otherwise return 0.
> + */
> +static int deny_no_inherit_topology_change(const struct landlock_cred_security
> +					   *subject,
> +					   struct dentry *dentry)
> +{
> +	layer_mask_t sealed_layers;
> +	layer_mask_t override_layers;
> +	unsigned long layer_index;
> +
> +	if (!subject || !dentry || d_is_negative(dentry))
> +		return 0;
> +	sealed_layers = collect_topology_sealed_layers(subject->domain,
> +						       dentry, &override_layers);
> +	sealed_layers &= ~override_layers;
> +
> +	if (!sealed_layers)
> +		return 0;
> +
> +	layer_index = __ffs((unsigned long)sealed_layers);
> +	landlock_log_denial(subject, &(struct landlock_request) {
> +		.type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
> +		.audit = {
> +			.type = LSM_AUDIT_DATA_DENTRY,
> +			.u.dentry = dentry,
> +		},
> +		.layer_plus_one = layer_index + 1,
> +	});
> +
> +	return -EACCES;
> +}
> +
>  /**
>   * current_check_refer_path - Check if a rename or link action is allowed
>   *
> @@ -1316,6 +1855,16 @@ static int current_check_refer_path(struct dentry *const old_dentry,
>  	access_request_parent2 =
>  		get_mode_access(d_backing_inode(old_dentry)->i_mode);
>  	if (removable) {
> +		int err;
> +
> +		err = deny_no_inherit_topology_change(subject, old_dentry);
> +		if (err)
> +			return err;
> +		if (exchange) {
> +			err = deny_no_inherit_topology_change(subject, new_dentry);
> +			if (err)
> +				return err;
> +		}
>  		access_request_parent1 |= maybe_remove(old_dentry);
>  		access_request_parent2 |= maybe_remove(new_dentry);
>  	}
> @@ -1707,12 +2256,31 @@ static int hook_path_symlink(const struct path *const dir,
>  static int hook_path_unlink(const struct path *const dir,
>  			    struct dentry *const dentry)
>  {
> +	const struct landlock_cred_security *const subject =
> +		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
> +	int err;
> +
> +	if (subject) {
> +		err = deny_no_inherit_topology_change(subject, dentry);
> +		if (err)
> +			return err;
> +	}
>  	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FILE);
>  }
>
>  static int hook_path_rmdir(const struct path *const dir,
>  			   struct dentry *const dentry)
>  {
> +	const struct landlock_cred_security *const subject =
> +		landlock_get_applicable_subject(current_cred(), any_fs, NULL);
> +	int err;
> +
> +	if (subject) {
> +		err = deny_no_inherit_topology_change(subject, dentry);
> +		if (err)
> +			return err;
> +	}
> +
>  	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR);
>  }
>
> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
> index 750a444e1983..f7b6a48bbf39 100644
> --- a/security/landlock/ruleset.c
> +++ b/security/landlock/ruleset.c
> @@ -255,8 +255,13 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
>  				return -EINVAL;
>  			if (WARN_ON_ONCE(this->layers[0].level != 0))
>  				return -EINVAL;
> +			/* Merge the flags into the rules */
>  			this->layers[0].access |= (*layers)[0].access;
>  			this->layers[0].flags.quiet |= (*layers)[0].flags.quiet;
> +			this->layers[0].flags.no_inherit |=
> +				(*layers)[0].flags.no_inherit;
> +			this->layers[0].flags.has_no_inherit_descendant |=
> +				(*layers)[0].flags.has_no_inherit_descendant;
>  			return 0;
>  		}
>
> @@ -315,7 +320,10 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
>  		.level = 0,
>  		.flags = {
>  			.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
> -		},
> +			.no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
> +			.has_no_inherit_descendant =
> +				!!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
> +		}
>  	} };
>
>  	build_check_layer();
> @@ -662,9 +670,22 @@ bool landlock_unmask_layers(const struct landlock_rule *const rule,
>  		unsigned long access_bit;
>  		bool is_empty;
>
> -		/* Collect rule flags for each layer. */
> -		if (rule_flags && layer->flags.quiet)
> +		/* Skip layers that already have no inherit flags. */
> +		if (rule_flags &&
> +		    (rule_flags->no_inherit_masks & layer_bit))
> +			continue;
> +
> +		/* Collect rule flags for each layer.
> +		 * We block flag inheritance if needed
> +		 * because of a no_inherit rule.
> +		 */
> +		if (rule_flags && layer->flags.quiet &&
> +		    !(rule_flags->blocked_flag_masks & layer_bit))

I don't quite understand the purpose of blocked_flag_masks - wouldn't the
"continue;" above naturally prevent flag inheritance?

>  			rule_flags->quiet_masks |= layer_bit;
> +		if (rule_flags && layer->flags.no_inherit)
> +			rule_flags->no_inherit_masks |= layer_bit;
> +		if (rule_flags && layer->flags.has_no_inherit_descendant)
> +			rule_flags->no_inherit_desc_masks |= layer_bit;
>
>  		/*
>  		 * Records in @layer_masks which layer grants access to each requested
> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
> index eb60db646422..8b46ab14e995 100644
> --- a/security/landlock/ruleset.h
> +++ b/security/landlock/ruleset.h
> @@ -40,6 +40,21 @@ struct landlock_layer {
>  		 * down the file hierarchy.
>  		 */
>  		bool quiet:1;
> +		/**
> +		 * @no_inherit: Prevents this rule from being inherited by
> +		 * descendant directories in the filesystem layer.  Only used
> +		 * for filesystem rules.
> +		 */
> +		bool no_inherit:1;
> +		/**
> +		 * @has_no_inherit_descendant: Marker to indicate that this layer
> +		 * has at least one descendant directory with a rule having the
> +		 * no_inherit flag.  Only used for filesystem rules.
> +		 * This "flag" is not set by the user, but by Landlock on
> +		 * parent directories of rules when the child rule has
> +		 * a rule with the no_inherit flag.
> +		 */
> +		bool has_no_inherit_descendant:1;
>  	} flags;
>  	/**
>  	 * @access: Bitfield of allowed actions on the kernel object.  They are
> @@ -49,13 +64,32 @@ struct landlock_layer {
>  };
>
>  /**
> - * struct collected_rule_flags - Hold accumulated flags for each layer.
> + * struct collected_rule_flags - Hold accumulated flags and their markers for each layer.
>   */
>  struct collected_rule_flags {
>  	/**
>  	 * @quiet_masks: Layers for which the quiet flag is effective.
>  	 */
>  	layer_mask_t quiet_masks;
> +	/**
> +	 * @no_inherit_masks: Layers for which the no_inherit flag is effective.
> +	 */
> +	layer_mask_t no_inherit_masks;
> +	/**
> +	 * @no_inherit_desc_masks: Layers for which the
> +	 * has_no_inherit_descendant tag is effective.
> +	 * This is not a flag itself, but a marker set on ancestors
> +	 * of rules with the no_inherit flag to deny topology changes
> +	 * in the direct parent path.
> +	 */
> +	layer_mask_t no_inherit_desc_masks;
> +	/**
> +	 * @blocked_flag_masks: Layers where flag inheritance must be blocked
> +	 * because of a no_inherit rule. This is not a flag itself, but a marker
> +	 * for layers that have their flags blocked due to no_inherit rule
> +	 * propagation.
> +	 */
> +	layer_mask_t blocked_flag_masks;
>  };
>
>  /**

Re: [PATCH v3 3/5] samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to landlock-sandboxer

[Tingmao Wang]

On 11/26/25 12:20, Justin Suess wrote:
> Adds support to landlock-sandboxer with environment variables LL_FS_RO_NO_INHERIT
> and LL_FS_RW_NO_INHERIT. These create the same rulesets as their non-no inherit variants,
> plus the LANDLOCK_ADD_RULE_NO_INHERIT flag.
> 
> v2..v3 changes:
> 
>   * Minor formatting fixes
> 
> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
> ---
>  samples/landlock/sandboxer.c | 37 +++++++++++++++++++++++++++---------
>  1 file changed, 28 insertions(+), 9 deletions(-)
> 
> diff --git a/samples/landlock/sandboxer.c b/samples/landlock/sandboxer.c
> index 2d8e3e94b77b..6f6bfc4e5110 100644
> --- a/samples/landlock/sandboxer.c
> +++ b/samples/landlock/sandboxer.c
> @@ -58,6 +58,8 @@ static inline int landlock_restrict_self(const int ruleset_fd,
>  
>  #define ENV_FS_RO_NAME "LL_FS_RO"
>  #define ENV_FS_RW_NAME "LL_FS_RW"
> +#define ENV_FS_RO_NO_INHERIT_NAME "LL_FS_RO_NO_INHERIT"
> +#define ENV_FS_RW_NO_INHERIT_NAME "LL_FS_RW_NO_INHERIT"
>  #define ENV_FS_QUIET_NAME "LL_FS_QUIET"
>  #define ENV_FS_QUIET_ACCESS_NAME "LL_FS_QUIET_ACCESS"
>  #define ENV_TCP_BIND_NAME "LL_TCP_BIND"
> @@ -121,7 +123,8 @@ static int parse_path(char *env_path, const char ***const path_list)
>  /* clang-format on */
>  
>  static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
> -			       const __u64 allowed_access, bool quiet)
> +		       const __u64 allowed_access,
> +		       __u32 add_rule_flags, bool mandatory)
>  {
>  	int num_paths, i, ret = 1;
>  	char *env_path_name;
> @@ -132,9 +135,13 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
>  
>  	env_path_name = getenv(env_var);
>  	if (!env_path_name) {
> -		/* Prevents users to forget a setting. */
> -		fprintf(stderr, "Missing environment variable %s\n", env_var);
> -		return 1;
> +		if (mandatory) {
> +			/* Prevents from forgetting to set necessary env vars. */
> +			fprintf(stderr, "Missing environment variable %s\n",
> +				env_var);
> +			return 1;
> +		}
> +		return 0;
>  	}
>  	env_path_name = strdup(env_path_name);
>  	unsetenv(env_var);
> @@ -171,8 +178,7 @@ static int populate_ruleset_fs(const char *const env_var, const int ruleset_fd,
>  		if (!S_ISDIR(statbuf.st_mode))
>  			path_beneath.allowed_access &= ACCESS_FILE;
>  		if (landlock_add_rule(ruleset_fd, LANDLOCK_RULE_PATH_BENEATH,
> -				      &path_beneath,
> -				      quiet ? LANDLOCK_ADD_RULE_QUIET : 0)) {
> +			      &path_beneath, add_rule_flags)) {
>  			fprintf(stderr,
>  				"Failed to update the ruleset with \"%s\": %s\n",
>  				path_list[i], strerror(errno));
> @@ -375,6 +381,8 @@ static const char help[] =
>  	"Optional settings (when not set, their associated access check "
>  	"is always allowed, which is different from an empty string which "
>  	"means an empty list):\n"
> +	"* " ENV_FS_RO_NO_INHERIT_NAME ": read-only paths without rule inheritance\n"
> +	"* " ENV_FS_RW_NO_INHERIT_NAME ": read-write paths without rule inheritance\n"

Would it make more sense to just have one LL_FS_NO_INHERIT env rule, that
will attach the "no inherit" flag without necessarily adding any access?

>  	"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
>  	"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
>  	"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
> @@ -596,17 +604,28 @@ int main(const int argc, char *const argv[], char *const *const envp)
>  	}
>  
>  	if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
> -				false)) {
> +			0, true)) {
>  		goto err_close_ruleset;
>  	}
>  	if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
> +			0, true)) {
> +		goto err_close_ruleset;
> +	}
> +	/* Optional no-inherit rules mirror the regular read-only/read-write sets. */
> +	if (populate_ruleset_fs(ENV_FS_RO_NO_INHERIT_NAME, ruleset_fd,
> +				access_fs_ro, LANDLOCK_ADD_RULE_NO_INHERIT,
> +				false)) {
> +		goto err_close_ruleset;
> +	}
> +	if (populate_ruleset_fs(ENV_FS_RW_NO_INHERIT_NAME, ruleset_fd,
> +				access_fs_rw, LANDLOCK_ADD_RULE_NO_INHERIT,

These need to be under an ABI version check like the quiet one - this
sandboxer is designed to "downgrade" what it tries to do gracefully if
running on older kernel.

However, there is an argument that maybe if deny rules aren't supported in
the current running kernel, it should just refuse to run the program at
all, otherwise by running the sandboxed program without the deny rules it
might expose the user to risks they might mitigate via some other means.
But in that case we should still have a better error message when running
on an older kernel than "Failed to update the ruleset with ...: Invalid
argument"

>  				false)) {
>  		goto err_close_ruleset;
>  	}
>  	/* Don't require this env to be present. */
> -	if (quiet_supported && getenv(ENV_FS_QUIET_NAME)) {
> +	if (quiet_supported) {
>  		if (populate_ruleset_fs(ENV_FS_QUIET_NAME, ruleset_fd, 0,
> -					true)) {
> +				LANDLOCK_ADD_RULE_QUIET, false)) {
>  			goto err_close_ruleset;
>  		}
>  	}

Re: [PATCH v3 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Justin Suess]

Thank you for the review.

I agree with the bind mount limitations for this flag. I think it's
reasonable to expect the sandboxer to provide protections for pre-
existing bind mounts as opposed to the kernel.

This limitation is a tradeoff between safety and complexity.

I looked into doing in automatically, and it ends up being sort of a
mess, and you end up having to iterate through the bind mounts, and I
suspect it would be a major performance hit, especially if we have to
account for changes outside the sandbox after the policy is already
enforced.

We can note this limitation in the docs.

> On 11/26/25 12:20, Justin Suess wrote:
>> Implements a flag to prevent access grant inheritance within the filesys
tem hierarchy
>> for landlock rules.
>>
>> If a landlock rule on an inode has this flag, any access grants on paren
t inodes will be
>> ignored. Moreover, operations that involve altering the direct parent tr
ee of the subject with
> I feel like the wording "direct parent tree" is a bit unclear - I think
> what you meant is "altering the parent of the subject or its ancestors",
> right?
>

Yes. I think that wording is more clear and use that from now on.

It just means if we have /a/b/c, the protected ancestors are /a/b and /a,
but not /a/x/ for instance.

>> LANDLOCK_ADD_RULE_NO_INHERIT will be denied up to the mountpoint.
> Tbh I'm not entirely clear on why we only deny rename of parents up to th
e
> mountpoint.

I agree.

My initial reasoning for not doing so was the fact that a mountpoint
doesn't let you move any of its ancestors.

So I thought going past the mountpoint was redundant.


    $ sudo mount --bind test test2
    $ mv test2 test3
    mv: cannot move 'test2' to 'test3': Device or resource busy

But I did not consider the fact that you can still link to a parent and
bypass restrictions that way. So the next patch version will fix this issue.

I implemented protections beyond the mountpoint in my current tree using
follow_up.

>> Additionally (new in v3) parent flag inheritance is blocked by this flag
, allowing fine
>> grained access control over LANDLOCK_ADD_RULE_QUIET.
>>
>> For example, if /a/b/c/ = read only + LANDLOCK_ADD_RULE_NO_INHERIT and /
 = read write, writes to
>> files in /a/b/c will be denied. Moreover, moving /a to /bad, removing /a
/b/c, or creating links to
>> /a will be prohibited.
>>
>> And if / has LANDLOCK_ADD_RULE_QUIET, /a/b/c will still audit (handled)
>> accesses. This is because LANDLOCK_ADD_RULE_NO_INHERIT also
>> suppresses flag inheritance from parent objects.
>>
>> The parent directory restrictions mitigate sandbox-restart attacks. For 
example, if a sandboxed program
>> is able to move a LANDLOCK_ADD_RULE_NO_INHERIT restricted directory, upo
n sandbox restart, the policy
>> applied naively on the same filenames would be invalid. Preventing these
 operations mitigates these attacks.
>>
>> v2..v3 changes:
>>
>>   * Parent directory topology protections now work by lazily
>>     inserting blank rules on parent inodes if they do not
>>     exist. This replaces the previous xarray implementation
>>     with simplified logic.
>>   * Added an optimization to skip further processing if all layers colle
cted
>>     no inherit.
>>   * Added support to block flag inheritance.
>>
>> Signed-off-by: Justin Suess <utilityemal77@gmail.com>
>> ---
>>  security/landlock/audit.c   |   4 +-
>>  security/landlock/domain.c  |   4 +-
>>  security/landlock/fs.c      | 592 +++++++++++++++++++++++++++++++++++-
>>  security/landlock/ruleset.c |  27 +-
>>  security/landlock/ruleset.h |  36 ++-
>>  5 files changed, 645 insertions(+), 18 deletions(-)
>>
>> diff --git a/security/landlock/audit.c b/security/landlock/audit.c
>> index d51563712325..4da97dd6985c 100644
>> --- a/security/landlock/audit.c
>> +++ b/security/landlock/audit.c
>> @@ -588,7 +588,9 @@ void landlock_log_denial(const struct landlock_cred_
security *const subject,
>>  				subject->domain, &missing, request->layer_m
asks,
>>  				request->layer_masks_size);
>>  			object_quiet_flag = !!(request->rule_flags.quiet_ma
sks &
>> -					       BIT(youngest_layer));
>> +				       BIT(youngest_layer)) &&
>> +				!(request->rule_flags.blocked_flag_masks &
>> +				  BIT(youngest_layer));
>>  		} else {
>>  			youngest_layer = get_layer_from_deny_masks(
>>  				&missing, request->all_existing_optional_ac
cess,
>> diff --git a/security/landlock/domain.c b/security/landlock/domain.c
>> index 8caf07250328..5bd83865c87d 100644
>> --- a/security/landlock/domain.c
>> +++ b/security/landlock/domain.c
>> @@ -236,7 +236,9 @@ optional_access_t landlock_get_quiet_optional_access
es(
>>  			 BITS_PER_TYPE(access_mask_t)) {
>>  		const u8 layer = (deny_masks >> (access_index * 4)) &
>>  				 (LANDLOCK_MAX_NUM_LAYERS - 1);
>> -		const bool is_quiet = !!(rule_flags.quiet_masks & BIT(layer
));
>> +		const layer_mask_t layer_bit = BIT(layer);
>> +		const bool is_quiet = !!(rule_flags.quiet_masks & layer_bit
) &&
>> +				  !(rule_flags.blocked_flag_masks & layer_b
it);
>>
>>  		if (is_quiet)
>>  			quiet_optional_accesses |= BIT(access_index);
>> diff --git a/security/landlock/fs.c b/security/landlock/fs.c
>> index 29f10da32141..0a5c73f18f26 100644
>> --- a/security/landlock/fs.c
>> +++ b/security/landlock/fs.c
>> @@ -317,6 +317,206 @@ static struct landlock_object *get_inode_object(st
ruct inode *const inode)
>>  	LANDLOCK_ACCESS_FS_IOCTL_DEV)
>>  /* clang-format on */
>>
>> +static const struct landlock_rule *find_rule(const struct landlock_rule
set *const domain,
>> +					     const struct dentry *const den
try);
>> +
>> +/**
>> + * landlock_domain_layers_mask - Build a mask covering all layers of a 
domain
>> + * @domain: The ruleset (domain) to inspect.
>> + *
>> + * Return a layer mask with a 1 bit for each existing layer of @domain.
>> + * If @domain has no layers 0 is returned.  If the number of layers is
>> + * greater than or equal to the number of bits in layer_mask_t, all bit
s
>> + * are set.
>> + */
>> +static layer_mask_t landlock_domain_layers_mask(const struct landlock_r
uleset
>> +						*const domain)
>> +{
>> +	if (!domain || !domain->num_layers)
>> +		return 0;
>> +
>> +	if (domain->num_layers >= sizeof(layer_mask_t) * BITS_PER_BYTE)
>> +		return (layer_mask_t)~0ULL;
>> +
>> +	return GENMASK_ULL(domain->num_layers - 1, 0);
>> +}
>> +
>> +/**
>> + * rule_blocks_all_layers_no_inherit - check whether a rule disables in
heritance
>> + * @domain_layers_mask: Mask describing the domain's active layers.
>> + * @rule: Rule to inspect.
>> + *
>> + * Return true if every layer present in @rule has its no_inherit flag 
set
>> + * and the set of layers covered by the rule equals @domain_layers_mask
.
>> + * This indicates that the rule prevents inheritance on all layers of t
he
>> + * domain and thus further walking for inheritance checks can stop.
>> + */
>> +static bool rule_blocks_all_layers_no_inherit(const layer_mask_t domain
_layers_mask,
>> +					      const struct landlock_rule *c
onst rule)
>> +{
>> +	layer_mask_t rule_layers = 0;
>> +	u32 layer_index;
>> +
>> +	if (!domain_layers_mask || !rule)
>> +		return false;
>> +
>> +	for (layer_index = 0; layer_index < rule->num_layers; layer_index++
) {
>> +		const struct landlock_layer *const layer =
>> +			&rule->layers[layer_index];
>> +		const layer_mask_t layer_bit = BIT_ULL(layer->level - 1);
>> +
>> +		if (!layer->flags.no_inherit)
>> +			return false;
>> +
>> +		rule_layers |= layer_bit;
>> +	}
>> +
>> +	return rule_layers && rule_layers == domain_layers_mask;
>> +}
>> +
>> +/**
>> + * landlock_collect_no_inherit_layers - Collects layers with no_inherit
 flags
>> + */
> Note likely misplaced comment here

Gotcha. Probably a mistake while rebasing.

>
>
>> +/**
>> + * landlock_collect_no_inherit_layers - collect effective no_inherit la
yers
>> + * @ruleset: Ruleset to consult.
>> + * @dentry: Dentry used as a starting point for the upward walk.
>> + *
>> + * Walk upwards from @dentry and return a layer mask containing the lay
ers
>> + * for which either a rule on the visited dentry has the no_inherit fla
g set
>> + * or where an ancestor was previously marked as having a descendant wi
th
>> + * a no_inherit rule.  The search prefers the closest matching dentry a
nd
>> + * stops once any relevant layer bits are found or the root is reached.
>> + *
>> + * Returns a layer_mask_t where each set bit corresponds to a layer wit
h an
>> + * effective no_inherit influence for @dentry.  Returns 0 if none apply
 or if
>> + * inputs are invalid.
>> + */
>> +static layer_mask_t landlock_collect_no_inherit_layers(const struct lan
dlock_ruleset
>> +						       *const ruleset,
>> +						       struct dentry *const
 dentry)
>> +{
>> +	struct dentry *cursor, *parent;
>> +	layer_mask_t layers = 0;
>> +	bool include_descendants = true;
>> +
>> +	if (!ruleset || !dentry || d_is_negative(dentry))
>> +		return 0;
>> +
>> +	cursor = dget(dentry);
>> +	while (true) {
>> +		const struct landlock_rule *rule;
>> +		u32 layer_index;
>> +
>> +		rule = find_rule(ruleset, cursor);
>> +		if (rule) {
>> +			for (layer_index = 0; layer_index < rule->num_layer
s; layer_index++) {
>> +				const struct landlock_layer *layer = &rule-
>layers[layer_index];
>> +
>> +				if (layer->flags.no_inherit ||
>> +				    (include_descendants &&
>> +				     layer->flags.has_no_inherit_descendant
))
>> +					layers |= BIT_ULL((layer->level ?
>> +						layer->level : layer_index 
+ 1) - 1);
>> +			}
>> +		}
>> +
>> +		if (layers) {
>> +			dput(cursor);
>> +			return layers;
>> +		}
>> +
>> +		if (IS_ROOT(cursor)) {
>> +			dput(cursor);
>> +			break;
>> +		}
>> +
>> +		parent = dget_parent(cursor);
>> +		dput(cursor);
>> +		if (!parent)
>> +			break;
>> +
>> +		cursor = parent;
>> +		include_descendants = false;
>> +	}
>> +	return 0;
>> +}
>> +
>> +static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
>> +				     struct dentry *dentry,
>> +				     layer_mask_t descendant_layers);
> Wouldn't you be able to avoid this declaration by moving the definition
> for ensure_rule_for_dentry and mark_no_inherit_ancestors up a bit, before
> mask_no_inherit_descendant_layers?

Good catch. I've fixed this in my working tree.

>
>
>> +
>> +/**
>> + * mask_no_inherit_descendant_layers - apply descendant no_inherit mask
ing
>> + * @domain: The ruleset (domain) to consult.
>> + * @dentry: The dentry whose descendants are considered.
>> + * @child_layers: Layers present on the child that may be subject to ma
sking.
>> + * @access_request: Accesses being requested (bitmask).
>> + * @layer_masks: Per-access layer masks to be modified in-place.
>> + * @rule_flags: Collected flags which will be updated accordingly.
>> + *
>> + * If descendant dentries have no_inherit, clear that
>> + * layer's bit from @layer_masks. Also updates @rule_flags to reflect
>> + * which layers were blocked.  Returns true if any of the @layer_masks 
were
>> + * modified, false otherwise.
>> + */
>> +static bool mask_no_inherit_descendant_layers(const struct landlock_rul
eset
>> +					      *const domain,
>> +					      struct dentry *const dentry,
>> +					      layer_mask_t child_layers,
>> +					      const access_mask_t access_re
quest,
>> +					      layer_mask_t
>> +					      (*const layer_masks)
>> +					      [LANDLOCK_NUM_ACCESS_FS],
>> +					      struct collected_rule_flags
>> +					      *const rule_flags)
>> +{
>> +	layer_mask_t descendant_layers;
>> +	const unsigned long access_req = access_request;
>> +	unsigned long access_bit;
>> +	bool changed = false;
>> +
>> +	if (!access_request || !layer_masks || !rule_flags || !dentry)
>> +		return false;
>> +	if (d_is_negative(dentry))
>> +		return false;
>> +
>> +	descendant_layers = landlock_collect_no_inherit_layers(domain, dent
ry);
>> +	{
>> +		layer_mask_t shared_layers = descendant_layers & child_laye
rs;
>> +
>> +		if (shared_layers) {
>> +			rule_flags->no_inherit_masks |= shared_layers;
>> +			rule_flags->no_inherit_desc_masks |= shared_layers;
>> +			rule_flags->blocked_flag_masks |= shared_layers;
>> +		}
>> +	}
>> +	descendant_layers &= ~child_layers;
>> +	descendant_layers &= ~rule_flags->no_inherit_masks;
>> +	if (!descendant_layers)
>> +		return false;
>> +
>> +	rule_flags->blocked_flag_masks |= descendant_layers;
>> +
>> +	for_each_set_bit(access_bit, &access_req,
>> +			 ARRAY_SIZE(*layer_masks)) {
>> +		layer_mask_t *const layer_mask = &(*layer_masks)[access_bit
];
>> +
>> +		if (*layer_mask & descendant_layers) {
>> +			*layer_mask &= ~descendant_layers;
>> +			changed = true;
>> +		}
>> +	}
>> +
>> +	if (!changed)
>> +		return false;
>> +
>> +	rule_flags->no_inherit_masks |= descendant_layers;
>> +	rule_flags->no_inherit_desc_masks |= descendant_layers;
>> +
>> +	return true;
>> +}
>> +
>>  /*
>>   * @path: Should have been checked by get_path_from_fd().
>>   */
>> @@ -325,12 +525,13 @@ int landlock_append_fs_rule(struct landlock_rulese
t *const ruleset,
>>  			    access_mask_t access_rights, const int flags)
>>  {
>>  	int err;
>> +	const bool is_dir = d_is_dir(path->dentry);
>>  	struct landlock_id id = {
>>  		.type = LANDLOCK_KEY_INODE,
>>  	};
>>
>>  	/* Files only get access rights that make sense. */
>> -	if (!d_is_dir(path->dentry) &&
>> +	if (!is_dir &&
>>  	    (access_rights | ACCESS_FILE) != ACCESS_FILE)
>>  		return -EINVAL;
>>  	if (WARN_ON_ONCE(ruleset->num_layers != 1))
>> @@ -344,13 +545,43 @@ int landlock_append_fs_rule(struct landlock_rulese
t *const ruleset,
>>  		return PTR_ERR(id.key.object);
>>  	mutex_lock(&ruleset->lock);
>>  	err = landlock_insert_rule(ruleset, id, access_rights, flags);
>> +	if (!err && (flags & LANDLOCK_ADD_RULE_NO_INHERIT)) {
>> +		const struct landlock_rule *rule;
>> +		layer_mask_t descendant_layers = 0;
>> +		u32 layer_index;
>> +
>> +		rule = find_rule(ruleset, path->dentry);
>> +		if (rule) {
>> +			for (layer_index = 0; layer_index < rule->num_layer
s; layer_index++) {
> This function is only called to add rules to an "unmerged" ruleset, which
> will always only have one layer.  So probably this can just be a
>
> 	err = landlock_insert_rule(ruleset, id, access_rights, flags);
> 	if (err)
> 		goto out_unlock;
> 	if (flags & LANDLOCK_ADD_RULE_NO_INHERIT) {
> 		err = mark_no_inherit_ancestors(ruleset, path->dentry);
> 		if (err)
> 			goto out_unlock;
> 	}
>
> And for a similar reason, you don't have to do a for(layer_index) in
> mark_no_inherit_ancestors either.  (Basically I think you would just set
> rule->layers[0].flags.has_no_inherit_descendant to true)
>
> It might be helpful to validate / document this reasoning by adding
> WARN_ONCE(rule->num_layers != 1, "unmerged rulesets should only have one
> layer") either here or in mark_no_inherit_ancestors.

Ah ok I see. I assume WARN_ONCE just is a kernel indicator of "hey this
state shouldn't happen".

And I got rid of the for loop and fixed it to use logic like the above,
and all tests passed. It will be in the next version.

>
>
>> +				const struct landlock_layer *layer =
>> +					&rule->layers[layer_index];
>> +
>> +				if (layer->flags.no_inherit ||
>> +				    layer->flags.has_no_inherit_descendant)
>> +					descendant_layers |=
>> +						BIT_ULL((layer->level ?
>> +							 layer->level : lay
er_index + 1) - 1);
>> +			}
>> +			if (descendant_layers) {
>> +				err = mark_no_inherit_ancestors(ruleset, pa
th->dentry,
>> +								descendant_
layers);
>> +				if (err)
>> +					goto out_unlock;
>> +			}
>> +		}
>> +	}
>>  	mutex_unlock(&ruleset->lock);
>> +out:
>>  	/*
>>  	 * No need to check for an error because landlock_insert_rule()
>>  	 * increments the refcount for the new object if needed.
>>  	 */
>>  	landlock_put_object(id.key.object);
>>  	return err;
>> +
>> +out_unlock:
>> +	mutex_unlock(&ruleset->lock);
>> +	goto out;
>>  }
>>
>>  /* Access-control management */
>> @@ -382,6 +613,134 @@ find_rule(const struct landlock_ruleset *const dom
ain,
>>  	return rule;
>>  }
>>
>> +/**
>> + * ensure_rule_for_dentry - ensure a ruleset contains a rule entry for 
dentry,
>> + * inserting a blank rule if needed.
>> + * @ruleset: Ruleset to modify/inspect.  Caller must hold @ruleset->loc
k.
>> + * @dentry: Dentry to ensure a rule exists for.
>> + *
>> + * If no rule is currently associated with @dentry, insert an empty rul
e
>> + * (with zero access) tied to the backing inode.  Returns a pointer to 
the
>> + * rule associated with @dentry on success, NULL when @dentry is negati
ve, or
>> + * an ERR_PTR()-encoded error if the rule cannot be created.
>> + *
>> + * This is useful for LANDLOCK_ADD_RULE_NO_INHERIT processing, where a 
rule
>> + * may need to be created for an ancestor dentry that does not yet have
 one
>> + * to properly track no_inherit flags.
>> + *
>> + * The flags are set to zero if a rule is newly created, and the caller
>> + * is responsible for setting them appropriately.
>> + *
>> + * The returned rule pointer's lifetime is tied to @ruleset.
>> + */
>> +static const struct landlock_rule *
>> +ensure_rule_for_dentry(struct landlock_ruleset *const ruleset,
>> +		       struct dentry *const dentry)
>> +{
>> +	struct landlock_id id = {
>> +		.type = LANDLOCK_KEY_INODE,
>> +	};
>> +	const struct landlock_rule *rule;
>> +	int err;
>> +
>> +	if (!ruleset || !dentry || d_is_negative(dentry))
>> +		return NULL;
>> +
>> +	lockdep_assert_held(&ruleset->lock);
>> +
>> +	rule = find_rule(ruleset, dentry);
>> +	if (rule)
>> +		return rule;
>> +
>> +	id.key.object = get_inode_object(d_backing_inode(dentry));
>> +	if (IS_ERR(id.key.object))
>> +		return ERR_CAST(id.key.object);
>> +
>> +	err = landlock_insert_rule(ruleset, id, 0, 0);
>> +	landlock_put_object(id.key.object);
>> +	if (err)
>> +		return ERR_PTR(err);
>> +
>> +	rule = find_rule(ruleset, dentry);
>> +	return rule ? rule : ERR_PTR(-ENOENT);
> I feel like this deserves a WARN_ON_ONCE(!rule) before this line - we
> don't really expect to not find a rule right after adding it.
>
> On the other hand, the only reason why we need to re-lookup the rule seem
s
> to be because landlock_insert_rule() does not return the newly added rule
.
> We could change it to do so, and not have to do this extra lookup.
>
> This also nicely solves the constness issue - landlock_insert_rule could
> return the mutable pointer.

I agree, but I'll hold off on changing landlock_insert_rule to keep the
patch scoped. The mutable cast thing seemed like a hack to me too when I
was doing it. Maybe returning a pointer or an ERR_PTR in landlock_insert_rule?

I'll add the WARN_ON_ONCE too.

>> +}
>> +
>> +/**
>> + * mark_no_inherit_ancestors - mark ancestors as having no_inherit desc
endants
>> + * @ruleset: Ruleset to modify.  Caller must hold @ruleset->lock.
>> + * @dentry: Dentry representing the descendant that carries no_inherit 
bits.
>> + * @descendant_layers: Mask of layers from the descendant that should b
e
>> + *                     advertised to ancestors via has_no_inherit_desce
ndant.
>> + *
>> + * Walks upward from @dentry and ensures that any ancestor rule contain
s the
>> + * has_no_inherit_descendant marker for the specified @descendant_layer
s so
>> + * parent lookups can quickly detect descendant no_inherit influence.
>> + *
>> + * Returns 0 on success or a negative errno if ancestor bookkeeping fai
ls.
>> + */
>> +static int mark_no_inherit_ancestors(struct landlock_ruleset *ruleset,
>> +				     struct dentry *dentry,
>> +				     layer_mask_t descendant_layers)
>> +{
>> +	struct dentry *cursor;
>> +	u32 layer_index;
>> +	int err = 0;
>> +
>> +	if (!ruleset || !dentry || !descendant_layers)
>> +		return -EINVAL;
>> +
>> +	lockdep_assert_held(&ruleset->lock);
>> +
>> +	cursor = dget(dentry);
>> +	while (cursor) {
>> +		struct dentry *parent;
>> +
>> +		if (IS_ROOT(cursor)) {
>> +			dput(cursor);
>> +			break;
>> +		}
>> +
>> +		parent = dget_parent(cursor);
>> +		dput(cursor);
>> +		if (!parent)
>> +			break;
>> +
>> +		if (!d_is_negative(parent)) {
> My understanding is that if the child is not negative (which is required
> for us to actually get here), as long as we always have a reference to it
,
> none of its parents should be negative as well.  This should probably be 
a
> WARN_ON_ONCE(d_is_negative(parent)).
>
> I think some of the other d_is_negative() checks in this patch also have 
a
> similar argument (i.e. turn into WARN_ON_ONCE or be removed if not
> necessary), but I've not looked at them all.
>

I'll do a pass and see. Again some of my understanding of the code is still
in the "I think I know what it's doing but not why" stage so missing that
is on me.

>> +			const struct landlock_rule *rule;
>> +			/* Ensures a rule exists for the parent dentry,
>> +			 * inserting a blank one if needed
>> +			 */
>> +			rule = ensure_rule_for_dentry(ruleset, parent);
>> +			if (IS_ERR(rule)) {
>> +				err = PTR_ERR(rule);
>> +				dput(parent);
>> +				cursor = NULL;
>> +				break;
>> +			}
>> +			if (rule) {
>> +				struct landlock_rule *mutable_rule =
>> +					(struct landlock_rule *)rule;
>> +
>> +				for (layer_index = 0;
>> +				     layer_index < mutable_rule->num_layers
;
>> +				     layer_index++) {
>> +					struct landlock_layer *layer =
>> +						&mutable_rule->layers[layer
_index];
>> +					layer_mask_t layer_bit =
>> +						BIT_ULL((layer->level ?
>> +							layer->level : laye
r_index + 1) - 1);
>> +
>> +					if (descendant_layers & layer_bit)
>> +						layer->flags.has_no_inherit
_descendant = true;
>> +				}
>> +			}
>> +		}
>> +
>> +		cursor = parent;
>> +	}
>> +	return err;
>> +}
>> +
>>  /*
>>   * Allows access to pseudo filesystems that will never be mountable (e.
g.
>>   * sockfs, pipefs), but can still be reachable through
>> @@ -764,6 +1123,8 @@ static bool is_access_to_paths_allowed(
>>  	struct landlock_request *const log_request_parent2,
>>  	struct dentry *const dentry_child2)
>>  {
>> +	const layer_mask_t domain_layers_mask =
>> +		landlock_domain_layers_mask(domain);
>>  	bool allowed_parent1 = false, allowed_parent2 = false, is_dom_check
,
>>  	     is_dom_check_bkp, child1_is_directory = true,
>>  	     child2_is_directory = true;
>> @@ -778,6 +1139,13 @@ static bool is_access_to_paths_allowed(
>>  	struct collected_rule_flags *rule_flags_parent1 = &log_request_pare
nt1->rule_flags;
>>  	struct collected_rule_flags *rule_flags_parent2 = &log_request_pare
nt2->rule_flags;
>>  	struct collected_rule_flags _rule_flag_parent1_bkp, _rule_flag_pare
nt2_bkp;
>> +	layer_mask_t child1_layers = 0;
>> +	layer_mask_t child2_layers = 0;
>> +
>> +	if (dentry_child1)
>> +		child1_layers = landlock_collect_no_inherit_layers(domain, 
dentry_child1);
>> +	if (dentry_child2)
>> +		child2_layers = landlock_collect_no_inherit_layers(domain, 
dentry_child2);
>>
>>  	if (!access_request_parent1 && !access_request_parent2)
>>  		return true;
>> @@ -931,6 +1299,10 @@ static bool is_access_to_paths_allowed(
>>  					       ARRAY_SIZE(*layer_masks_pare
nt2),
>>  					       rule_flags_parent2);
>>
>> +		if (rule &&
>> +		    rule_blocks_all_layers_no_inherit(domain_layers_mask, r
ule))
>> +			break;
>> +
>>  		/* Stops when a rule from each layer grants access. */
>>  		if (allowed_parent1 && allowed_parent2) {
>>  			/*
>> @@ -976,8 +1348,13 @@ static bool is_access_to_paths_allowed(
>>  					memcpy(&_rule_flag_parent2_bkp,
>>  					       rule_flags_parent2,
>>  					       sizeof(_rule_flag_parent2_bk
p));
>> -					is_dom_check_bkp = is_dom_check;
>>  				}
>> +				is_dom_check_bkp = is_dom_check;
>> +				child1_layers = landlock_collect_no_inherit
_layers(domain,
>> +									
	   walker_path
>> +									
	   .dentry);
>> +				if (layer_masks_parent2)
>> +					child2_layers = child1_layers;
>>
>>  				/* Ignores hidden mount points. */
>>  				goto jump_up;
>> @@ -1001,15 +1378,50 @@ static bool is_access_to_paths_allowed(
>>  				break;
>>  			}
>>
>> -			/*
>> -			 * We reached a disconnected root directory from a 
bind mount, and
>> -			 * we need to reset the walk to the current mount r
oot.
>> -			 */
>> -			goto reset_to_mount_root;
>> -		}
>> -		parent_dentry = dget_parent(walker_path.dentry);
>> -		dput(walker_path.dentry);
>> -		walker_path.dentry = parent_dentry;
>> +		/*
>> +		 * We reached a disconnected root directory from a bind mou
nt, and
>> +		 * we need to reset the walk to the current mount root.
>> +		 */
> Accidental change of indentation?

I'll fix that. Guess checkpatch.pl didn't catch it.

>
>
>> +		goto reset_to_mount_root;
>> +	}
>> +	if (likely(!d_is_negative(walker_path.dentry))) {
>> +		child1_layers = landlock_collect_no_inherit_layers(domain,
>> +								   walker_p
ath.dentry);
>> +		if (layer_masks_parent2)
>> +			child2_layers = child1_layers;
>> +	} else {
>> +		child1_layers = 0;
>> +		if (layer_masks_parent2)
>> +			child2_layers = 0;
>> +	}
>> +	parent_dentry = dget_parent(walker_path.dentry);
>> +	dput(walker_path.dentry);
>> +	walker_path.dentry = parent_dentry;
>> +	/*
>> +	 * Apply descendant no-inherit masking now that we've moved to the
>> +	 * parent. This ensures the parent respects any no-inherit rules fr
om
>> +	 * the child we just left. Only applies to refer operations (rename
/link).
>> +	 */
>> +	if (unlikely(layer_masks_parent2)) {
>> +		if (mask_no_inherit_descendant_layers(domain, walker_path.d
entry,
>> +						      child1_layers,
>> +						      access_masked_parent1
,
>> +						      layer_masks_parent1,
>> +						      rule_flags_parent1))
>> +			allowed_parent1 =
>> +				allowed_parent1 ||
>> +				is_layer_masks_allowed(layer_masks_parent1)
;
>> +
>> +		if (rule_flags_parent2 &&
>> +		    mask_no_inherit_descendant_layers(domain, walker_path.d
entry,
>> +						      child2_layers,
>> +						      access_masked_parent2
,
>> +						      layer_masks_parent2,
>> +						      rule_flags_parent2))
>> +			allowed_parent2 =
>> +				allowed_parent2 ||
>> +				is_layer_masks_allowed(layer_masks_parent2)
;
>> +	}
> Maybe I'm missing something, but I can't tell what's the purpose of this
> block, or in fact what mask_no_inherit_descendant_layers and
> landlock_collect_no_inherit_layers is for.  The doc comment for
> mask_no_inherit_descendant_layers seems to suggest that it's supposed to
> walk descendents, but landlock_collect_no_inherit_layers is actually
> walking ancestors.  Removing these checks doesn't seem to break any tests
,
> and sandboxer still seems to work as expected wrt. no_inherit rules and
> denial of renaming denied dentries and its parents.
>
> Note that the special "reverting" style disconnected directory handling
> has been removed in Mickaël's next branch (i.e. the "backup" logic is
> removed), which should hopefully simplify reasoning about this.
>

Gotcha. I think this came from some of my initial confusion going
through the codebase and making multiple changes before testing. So I
would add checks to be sure before doing a rebuild, and had a fear of
breaking it and having to wait around for a recompile.

I am still figuring out kernel debugging so sometimes I wouldn't know
what code was actually breaking the tests.

This is a big catch and reduces the LOC considerably so thank you! I
removed the checks and all tests passed, so it seems  like these were
indeed redundant.

>>  		continue;
>>
>>  reset_to_mount_root:
>> @@ -1057,6 +1469,10 @@ static bool is_access_to_paths_allowed(
>>  		dput(walker_path.dentry);
>>  		walker_path.dentry = walker_path.mnt->mnt_root;
>>  		dget(walker_path.dentry);
>> +		child1_layers = landlock_collect_no_inherit_layers(domain,
>> +								   walker_p
ath.dentry);
>> +		if (layer_masks_parent2)
>> +			child2_layers = child1_layers;
>>  	}
>>  	path_put(&walker_path);
>>
>> @@ -1172,6 +1588,8 @@ static bool collect_domain_accesses(
>>  	struct collected_rule_flags *const rule_flags)
>>  {
>>  	access_mask_t access_dom;
>> +	const layer_mask_t domain_layers_mask =
>> +		landlock_domain_layers_mask(domain);
>>  	bool ret = false;
>>
>>  	if (WARN_ON_ONCE(!domain || !mnt_dir || !dir || !layer_masks_dom))
>> @@ -1187,9 +1605,11 @@ static bool collect_domain_accesses(
>>  	while (true) {
>>  		struct dentry *parent_dentry;
>>
>> +		const struct landlock_rule *rule = find_rule(domain, dir);
>> +
>>  		/* Gets all layers allowing all domain accesses. */
>>  		if (landlock_unmask_layers(
>> -			    find_rule(domain, dir), access_dom, layer_masks
_dom,
>> +			    rule, access_dom, layer_masks_dom,
>>  			    ARRAY_SIZE(*layer_masks_dom), rule_flags)) {
>>  			/*
>>  			 * Before allowing this side of the access request,
 checks that the
>> @@ -1206,6 +1626,10 @@ static bool collect_domain_accesses(
>>  			break;
>>  		}
>>
>> +		if (rule &&
>> +		    rule_blocks_all_layers_no_inherit(domain_layers_mask, r
ule))
>> +			break;
>> +
>>  		/* Stops at the mount point. */
>>  		if (dir == mnt_dir->dentry)
>>  			break;
>> @@ -1232,6 +1656,121 @@ static bool collect_domain_accesses(
>>  	return ret;
>>  }
>>
>> +/**
>> + * collect_topology_sealed_layers - collect layers sealed against topol
ogy changes
>> + * @domain: Ruleset to consult.
>> + * @dentry: Starting dentry for the upward walk.
>> + * @override_layers: Optional out parameter filled with layers that are
>> + *                   present on ancestors but considered overrides (not
>> + *                   sealing the topology for descendants).
>> + *
>> + * Walk upwards from @dentry and return a mask of layers where either t
he
>> + * visited dentry contains a no_inherit rule or ancestors were previous
ly
>> + * marked as having a descendant with no_inherit.  @override_layers, if
 not
>> + * NULL, is filled with layers that would normally be overridden by mor
e
>> + * specific descendant rules.
>> + *
>> + * Returns a layer mask where set bits indicate layers that are "sealed
"
>> + * (topology changes like rename/rmdir are denied) for the subtree root
ed at
>> + * @dentry.
>> + *
>> + * Useful for LANDLOCK_ADD_RULE_NO_INHERIT parent directory enforcement
 to ensure
>> + * that topology changes do not violate the no_inherit constraints.
>> + */
>> +static layer_mask_t
>> +collect_topology_sealed_layers(const struct landlock_ruleset *const dom
ain,
>> +			       struct dentry *dentry,
>> +			       layer_mask_t *const override_layers)
>> +{
>> +	struct dentry *cursor, *parent;
>> +	bool include_descendants = true;
>> +	layer_mask_t sealed_layers = 0;
>> +
>> +	if (override_layers)
>> +		*override_layers = 0;
>> +
>> +	if (!domain || !dentry || d_is_negative(dentry))
>> +		return 0;
>> +
>> +	cursor = dget(dentry);
>> +	while (cursor) {
>> +		const struct landlock_rule *rule;
>> +		u32 layer_index;
>> +
>> +		rule = find_rule(domain, cursor);
>> +		if (rule) {
>> +			for (layer_index = 0; layer_index < rule->num_layer
s;
>> +			     layer_index++) {
>> +				const struct landlock_layer *layer =
>> +					&rule->layers[layer_index];
>> +				const int level = layer->level ? layer->lev
el :
>> +								 layer_inde
x + 1;
> Wouldn't layer->level always be >= 1 here?  Using layer_index doesn't mak
e
> sense since layer_index is just the index that the struct landlock_layer
> happened to be in that rule's array.

Hmm good catch. I was replicating logic from other places while sometimes not realizing 
exactly why the code is doing what it does and didn't make that realization. Seems to work
fine and pass after I fixed it in my working tree so it will be in the next version.

>
>> +				layer_mask_t layer_bit = BIT_ULL(level - 1)
;
>> +
>> +				if (include_descendants &&
>> +				    (layer->flags.no_inherit ||
>> +				     layer->flags.has_no_inherit_descendant
)) {
>> +					sealed_layers |= layer_bit;
>> +				} else if (override_layers) {
>> +					*override_layers |= layer_bit;
>> +				}
>> +			}
>> +		}
>> +
>> +		if (sealed_layers || IS_ROOT(cursor))
>> +			break;
>> +
>> +		parent = dget_parent(cursor);
>> +		dput(cursor);
>> +		if (!parent)
>> +			return sealed_layers;
>> +
>> +		cursor = parent;
>> +		include_descendants = false;
>> +	}
>> +	dput(cursor);
>> +	return sealed_layers;
>> +}
>> +
>> +/**
>> + * deny_no_inherit_topology_change - deny topology changes on sealed la
yers
>> + * @subject: Subject performing the operation (contains the domain).
>> + * @dentry: Dentry that is the target of the topology modification.
>> + *
>> + * Checks whether any domain layers are sealed against topology changes
 at
>> + * @dentry (via collect_topology_sealed_layers).  If so, emit an audit 
record
>> + * and return -EACCES.  Otherwise return 0.
>> + */
>> +static int deny_no_inherit_topology_change(const struct landlock_cred_s
ecurity
>> +					   *subject,
>> +					   struct dentry *dentry)
>> +{
>> +	layer_mask_t sealed_layers;
>> +	layer_mask_t override_layers;
>> +	unsigned long layer_index;
>> +
>> +	if (!subject || !dentry || d_is_negative(dentry))
>> +		return 0;
>> +	sealed_layers = collect_topology_sealed_layers(subject->domain,
>> +						       dentry, &override_la
yers);
>> +	sealed_layers &= ~override_layers;
>> +
>> +	if (!sealed_layers)
>> +		return 0;
>> +
>> +	layer_index = __ffs((unsigned long)sealed_layers);
>> +	landlock_log_denial(subject, &(struct landlock_request) {
>> +		.type = LANDLOCK_REQUEST_FS_CHANGE_TOPOLOGY,
>> +		.audit = {
>> +			.type = LSM_AUDIT_DATA_DENTRY,
>> +			.u.dentry = dentry,
>> +		},
>> +		.layer_plus_one = layer_index + 1,
>> +	});
>> +
>> +	return -EACCES;
>> +}
>> +
>>  /**
>>   * current_check_refer_path - Check if a rename or link action is allow
ed
>>   *
>> @@ -1316,6 +1855,16 @@ static int current_check_refer_path(struct dentry
 *const old_dentry,
>>  	access_request_parent2 =
>>  		get_mode_access(d_backing_inode(old_dentry)->i_mode);
>>  	if (removable) {
>> +		int err;
>> +
>> +		err = deny_no_inherit_topology_change(subject, old_dentry);
>> +		if (err)
>> +			return err;
>> +		if (exchange) {
>> +			err = deny_no_inherit_topology_change(subject, new_
dentry);
>> +			if (err)
>> +				return err;
>> +		}
>>  		access_request_parent1 |= maybe_remove(old_dentry);
>>  		access_request_parent2 |= maybe_remove(new_dentry);
>>  	}
>> @@ -1707,12 +2256,31 @@ static int hook_path_symlink(const struct path *
const dir,
>>  static int hook_path_unlink(const struct path *const dir,
>>  			    struct dentry *const dentry)
>>  {
>> +	const struct landlock_cred_security *const subject =
>> +		landlock_get_applicable_subject(current_cred(), any_fs, NUL
L);
>> +	int err;
>> +
>> +	if (subject) {
>> +		err = deny_no_inherit_topology_change(subject, dentry);
>> +		if (err)
>> +			return err;
>> +	}
>>  	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_FIL
E);
>>  }
>>
>>  static int hook_path_rmdir(const struct path *const dir,
>>  			   struct dentry *const dentry)
>>  {
>> +	const struct landlock_cred_security *const subject =
>> +		landlock_get_applicable_subject(current_cred(), any_fs, NUL
L);
>> +	int err;
>> +
>> +	if (subject) {
>> +		err = deny_no_inherit_topology_change(subject, dentry);
>> +		if (err)
>> +			return err;
>> +	}
>> +
>>  	return current_check_access_path(dir, LANDLOCK_ACCESS_FS_REMOVE_DIR
);
>>  }
>>
>> diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
>> index 750a444e1983..f7b6a48bbf39 100644
>> --- a/security/landlock/ruleset.c
>> +++ b/security/landlock/ruleset.c
>> @@ -255,8 +255,13 @@ static int insert_rule(struct landlock_ruleset *con
st ruleset,
>>  				return -EINVAL;
>>  			if (WARN_ON_ONCE(this->layers[0].level != 0))
>>  				return -EINVAL;
>> +			/* Merge the flags into the rules */
>>  			this->layers[0].access |= (*layers)[0].access;
>>  			this->layers[0].flags.quiet |= (*layers)[0].flags.q
uiet;
>> +			this->layers[0].flags.no_inherit |=
>> +				(*layers)[0].flags.no_inherit;
>> +			this->layers[0].flags.has_no_inherit_descendant |=
>> +				(*layers)[0].flags.has_no_inherit_descendan
t;
>>  			return 0;
>>  		}
>>
>> @@ -315,7 +320,10 @@ int landlock_insert_rule(struct landlock_ruleset *c
onst ruleset,
>>  		.level = 0,
>>  		.flags = {
>>  			.quiet = !!(flags & LANDLOCK_ADD_RULE_QUIET),
>> -		},
>> +			.no_inherit = !!(flags & LANDLOCK_ADD_RULE_NO_INHER
IT),
>> +			.has_no_inherit_descendant =
>> +				!!(flags & LANDLOCK_ADD_RULE_NO_INHERIT),
>> +		}
>>  	} };
>>
>>  	build_check_layer();
>> @@ -662,9 +670,22 @@ bool landlock_unmask_layers(const struct landlock_r
ule *const rule,
>>  		unsigned long access_bit;
>>  		bool is_empty;
>>
>> -		/* Collect rule flags for each layer. */
>> -		if (rule_flags && layer->flags.quiet)
>> +		/* Skip layers that already have no inherit flags. */
>> +		if (rule_flags &&
>> +		    (rule_flags->no_inherit_masks & layer_bit))
>> +			continue;
>> +
>> +		/* Collect rule flags for each layer.
>> +		 * We block flag inheritance if needed
>> +		 * because of a no_inherit rule.
>> +		 */
>> +		if (rule_flags && layer->flags.quiet &&
>> +		    !(rule_flags->blocked_flag_masks & layer_bit))
> I don't quite understand the purpose of blocked_flag_masks - wouldn't the
> "continue;" above naturally prevent flag inheritance?

Yes, initially I thought I needed to have special handling / a marker
for this. But as you pointed out the continue skips it anyways. I'll
remove that and add a comment saying it skips over the flag inheritance
as well as a side effect.

>
>
>>  			rule_flags->quiet_masks |= layer_bit;
>> +		if (rule_flags && layer->flags.no_inherit)
>> +			rule_flags->no_inherit_masks |= layer_bit;
>> +		if (rule_flags && layer->flags.has_no_inherit_descendant)
>> +			rule_flags->no_inherit_desc_masks |= layer_bit;
>>
>>  		/*
>>  		 * Records in @layer_masks which layer grants access to eac
h requested
>> diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
>> index eb60db646422..8b46ab14e995 100644
>> --- a/security/landlock/ruleset.h
>> +++ b/security/landlock/ruleset.h
>> @@ -40,6 +40,21 @@ struct landlock_layer {
>>  		 * down the file hierarchy.
>>  		 */
>>  		bool quiet:1;
>> +		/**
>> +		 * @no_inherit: Prevents this rule from being inherited by
>> +		 * descendant directories in the filesystem layer.  Only us
ed
>> +		 * for filesystem rules.
>> +		 */
>> +		bool no_inherit:1;
>> +		/**
>> +		 * @has_no_inherit_descendant: Marker to indicate that this
 layer
>> +		 * has at least one descendant directory with a rule having
 the
>> +		 * no_inherit flag.  Only used for filesystem rules.
>> +		 * This "flag" is not set by the user, but by Landlock on
>> +		 * parent directories of rules when the child rule has
>> +		 * a rule with the no_inherit flag.
>> +		 */
>> +		bool has_no_inherit_descendant:1;
>>  	} flags;
>>  	/**
>>  	 * @access: Bitfield of allowed actions on the kernel object.  They
 are
>> @@ -49,13 +64,32 @@ struct landlock_layer {
>>  };
>>
>>  /**
>> - * struct collected_rule_flags - Hold accumulated flags for each layer.
>> + * struct collected_rule_flags - Hold accumulated flags and their marke
rs for each layer.
>>   */
>>  struct collected_rule_flags {
>>  	/**
>>  	 * @quiet_masks: Layers for which the quiet flag is effective.
>>  	 */
>>  	layer_mask_t quiet_masks;
>> +	/**
>> +	 * @no_inherit_masks: Layers for which the no_inherit flag is effec
tive.
>> +	 */
>> +	layer_mask_t no_inherit_masks;
>> +	/**
>> +	 * @no_inherit_desc_masks: Layers for which the
>> +	 * has_no_inherit_descendant tag is effective.
>> +	 * This is not a flag itself, but a marker set on ancestors
>> +	 * of rules with the no_inherit flag to deny topology changes
>> +	 * in the direct parent path.
>> +	 */
>> +	layer_mask_t no_inherit_desc_masks;
>> +	/**
>> +	 * @blocked_flag_masks: Layers where flag inheritance must be block
ed
>> +	 * because of a no_inherit rule. This is not a flag itself, but a m
arker
>> +	 * for layers that have their flags blocked due to no_inherit rule
>> +	 * propagation.
>> +	 */
>> +	layer_mask_t blocked_flag_masks;
>>  };
>>
>>  /**

Thank you for the review of the patch. Trimming the code was very
satisfying. I see that the disconnected directory handling was
simplified in linux-next, which is gonna make things easier but also
make rebasing require some more careful attention. I assume your next
patch series for LANDLOCK_ADD_RULE_QUIET is going to be rebased off
those disconnected directory changes as well.

I'll implement these fixes in my working tree and be ready when you drop
your next version to do a rebase on top of your series again. If you
have a base commit SHA you could give me that you are building your next
patch series version on top of already, that would be helpful.

Kind Regards,
Justin Suess

Re: [PATCH v3 3/5] samples/landlock: Add LANDLOCK_ADD_RULE_NO_INHERIT to landlock-sandboxer

[Justin Suess]

It totally does. I think that would be more way more clear and reduce LOC.

It'll be in the next version.

>
>>  	"* " ENV_TCP_BIND_NAME ": ports allowed to bind (server)\n"
>>  	"* " ENV_TCP_CONNECT_NAME ": ports allowed to connect (client)\n"
>>  	"* " ENV_SCOPED_NAME ": actions denied on the outside of the landlock domain\n"
>> @@ -596,17 +604,28 @@ int main(const int argc, char *const argv[], char *const *const envp)
>>  	}
>>  
>>  	if (populate_ruleset_fs(ENV_FS_RO_NAME, ruleset_fd, access_fs_ro,
>> -				false)) {
>> +			0, true)) {
>>  		goto err_close_ruleset;
>>  	}
>>  	if (populate_ruleset_fs(ENV_FS_RW_NAME, ruleset_fd, access_fs_rw,
>> +			0, true)) {
>> +		goto err_close_ruleset;
>> +	}
>> +	/* Optional no-inherit rules mirror the regular read-only/read-write sets. */
>> +	if (populate_ruleset_fs(ENV_FS_RO_NO_INHERIT_NAME, ruleset_fd,
>> +				access_fs_ro, LANDLOCK_ADD_RULE_NO_INHERIT,
>> +				false)) {
>> +		goto err_close_ruleset;
>> +	}
>> +	if (populate_ruleset_fs(ENV_FS_RW_NO_INHERIT_NAME, ruleset_fd,
>> +				access_fs_rw, LANDLOCK_ADD_RULE_NO_INHERIT,
>
> These need to be under an ABI version check like the quiet one - this
> sandboxer is designed to "downgrade" what it tries to do gracefully if
> running on older kernel.
>
> However, there is an argument that maybe if deny rules aren't supported in
> the current running kernel, it should just refuse to run the program at
> all, otherwise by running the sandboxed program without the deny rules it
> might expose the user to risks they might mitigate via some other means.
> But in that case we should still have a better error message when running
> on an older kernel than "Failed to update the ruleset with ...: Invalid
> argument"

Gotcha. I'll put it under the ABI check. 

I think it'd be better to let the user decide if they wanna continue
with the policy if it's using an unsupported feature.

Kind Regards,

Justin Suess

Re: [PATCH v3 1/5] landlock: Implement LANDLOCK_ADD_RULE_NO_INHERIT

[Tingmao Wang]

On 12/6/25 15:26, Justin Suess wrote:
> Thank you for the review.
>
> I agree with the bind mount limitations for this flag. I think it's
> reasonable to expect the sandboxer to provide protections for pre-
> existing bind mounts as opposed to the kernel.
>
> This limitation is a tradeoff between safety and complexity.
>
> I looked into doing in automatically, and it ends up being sort of a
> mess, and you end up having to iterate through the bind mounts, and I
> suspect it would be a major performance hit, especially if we have to
> account for changes outside the sandbox after the policy is already
> enforced.
>
> We can note this limitation in the docs.
>
>> On 11/26/25 12:20, Justin Suess wrote:
>>> Implements a flag to prevent access grant inheritance within the filesys
> tem hierarchy

Minor nit but please try not to wrap lines this way, as when the text is
colored / indented by quote level, it makes it seem like you're adding a
reply.

> [...]
>>> @@ -1232,6 +1656,121 @@ static bool collect_domain_accesses(
>>>  	return ret;
>>>  }
>>>
>>> +/**
>>> + * collect_topology_sealed_layers - collect layers sealed against topol
> ogy changes
>>> + * @domain: Ruleset to consult.
>>> + * @dentry: Starting dentry for the upward walk.
>>> + * @override_layers: Optional out parameter filled with layers that are
>>> + *                   present on ancestors but considered overrides (not
>>> + *                   sealing the topology for descendants).
>>> + *
>>> + * Walk upwards from @dentry and return a mask of layers where either t
> he
>>> + * visited dentry contains a no_inherit rule or ancestors were previous
> ly
>>> + * marked as having a descendant with no_inherit.  @override_layers, if
>  not
>>> + * NULL, is filled with layers that would normally be overridden by mor
> e
>>> + * specific descendant rules.
>>> + *
>>> + * Returns a layer mask where set bits indicate layers that are "sealed
> "
>>> + * (topology changes like rename/rmdir are denied) for the subtree root
> ed at
>>> + * @dentry.
>>> + *
>>> + * Useful for LANDLOCK_ADD_RULE_NO_INHERIT parent directory enforcement
>  to ensure
>>> + * that topology changes do not violate the no_inherit constraints.
>>> + */
>>> +static layer_mask_t
>>> +collect_topology_sealed_layers(const struct landlock_ruleset *const dom
> ain,
>>> +			       struct dentry *dentry,
>>> +			       layer_mask_t *const override_layers)
>>> +{
>>> +	struct dentry *cursor, *parent;
>>> +	bool include_descendants = true;
>>> +	layer_mask_t sealed_layers = 0;
>>> +
>>> +	if (override_layers)
>>> +		*override_layers = 0;
>>> +
>>> +	if (!domain || !dentry || d_is_negative(dentry))
>>> +		return 0;
>>> +
>>> +	cursor = dget(dentry);
>>> +	while (cursor) {
>>> +		const struct landlock_rule *rule;
>>> +		u32 layer_index;
>>> +
>>> +		rule = find_rule(domain, cursor);
>>> +		if (rule) {
>>> +			for (layer_index = 0; layer_index < rule->num_layer
> s;
>>> +			     layer_index++) {
>>> +				const struct landlock_layer *layer =
>>> +					&rule->layers[layer_index];
>>> +				const int level = layer->level ? layer->lev
> el :
>>> +								 layer_inde
> x + 1;
>> Wouldn't layer->level always be >= 1 here?  Using layer_index doesn't mak
> e
>> sense since layer_index is just the index that the struct landlock_layer
>> happened to be in that rule's array.
>
> Hmm good catch. I was replicating logic from other places while sometimes not realizing
> exactly why the code is doing what it does and didn't make that realization. Seems to work
> fine and pass after I fixed it in my working tree so it will be in the next version.

Well actually this function would become unused after removing the "Apply
descendant no-inherit masking" code above.

> [...]
>
> Thank you for the review of the patch. Trimming the code was very
> satisfying. I see that the disconnected directory handling was
> simplified in linux-next, which is gonna make things easier but also
> make rebasing require some more careful attention. I assume your next
> patch series for LANDLOCK_ADD_RULE_QUIET is going to be rebased off
> those disconnected directory changes as well.
>
> I'll implement these fixes in my working tree and be ready when you drop
> your next version to do a rebase on top of your series again. If you
> have a base commit SHA you could give me that you are building your next
> patch series version on top of already, that would be helpful.

Will send v6 now, but you can also take it from
https://github.com/micromaomao/linux-dev/tree/landlock-quiet-flag :)