Inter-revision diff: patch 2

Comparing v20 (message) to v30 (message)

--- v20
+++ v30
@@ -1,23 +1,84 @@
+From: Mickaël Salaün <mic@linux.microsoft.com>
+
 A Landlock ruleset is mainly a red-black tree with Landlock rules as
-nodes.  This enables quick update and lookup to match a requested access
-e.g., to a file.  A ruleset is usable through a dedicated file
-descriptor (cf. following commit implementing the syscall) which enables
-a process to create and populate a ruleset with new rules.
+nodes.  This enables quick update and lookup to match a requested
+access, e.g. to a file.  A ruleset is usable through a dedicated file
+descriptor (cf. following commit implementing syscalls) which enables a
+process to create and populate a ruleset with new rules.
 
 A domain is a ruleset tied to a set of processes.  This group of rules
-define the security policy enforced on these processes and their future
+defines the security policy enforced on these processes and their future
 children.  A domain can transition to a new domain which is the
 intersection of all its constraints and those of a ruleset provided by
 the current process.  This modification only impact the current process.
 This means that a process can only gain more constraints (i.e. lose
 accesses) over time.
 
-Signed-off-by: Mickaël Salaün <mic@digikod.net>
 Cc: James Morris <jmorris@namei.org>
 Cc: Jann Horn <jannh@google.com>
 Cc: Kees Cook <keescook@chromium.org>
-Cc: Serge E. Hallyn <serge@hallyn.com>
+Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
+Acked-by: Serge Hallyn <serge@hallyn.com>
+Link: https://lore.kernel.org/r/20210316204252.427806-3-mic@digikod.net
 ---
+
+Changes since v28:
+* Add Acked-by Serge Hallyn.
+* Clean up comment.
+
+Changes since v27:
+* Fix domains with layers of non-overlapping access rights.
+* Add stricter limit checks (same semantic).
+* Change the grow direction of a rule layer stack to make it the same as
+  the new ruleset fs_access_masks stack (cosmetic change).
+* Cosmetic fix for a comment block.
+
+Changes since v26:
+* Fix spelling.
+
+Changes since v25:
+* Add build-time checks for the num_layers and num_rules variables
+  according to LANDLOCK_MAX_NUM_LAYERS and LANDLOCK_MAX_NUM_RULES, and
+  move these limits to a dedicated file.
+* Cosmetic variable renames.
+
+Changes since v24:
+* Update struct landlock_rule with a layer stack.  This reverts "Always
+  intersect access rights" from v24 and also adds the ability to tie
+  access rights with their policy layer.  As noted by Jann Horn, always
+  intersecting access rights made some use cases uselessly more
+  difficult to handle in user space.  Thanks to this new stack, we still
+  have a deterministic policy behavior whatever their level in the stack
+  of policies, while using a "union" of accesses when building a
+  ruleset.  The implementation use a FAM to keep the access checks quick
+  and memory efficient (4 bytes per layer per inode).  Update
+  insert_rule() accordingly.
+
+Changes since v23:
+* Always intersect access rights.  Following the filesystem change
+  logic, make ruleset updates more consistent by always intersecting
+  access rights (boolean AND) instead of combining them (boolean OR) for
+  the same layer.  This defensive approach could also help avoid user
+  space to inadvertently allow multiple access rights for the same
+  object (e.g.  write and execute access on a path hierarchy) instead of
+  dealing with such inconsistency.  This can happen when there is no
+  deduplication of objects (e.g. paths and underlying inodes) whereas
+  they get different access rights with landlock_add_rule(2).
+* Add extra checks to make sure that:
+  - there is always an (allocated) object in each used rules;
+  - when updating a ruleset with a new rule (i.e. not merging two
+    rulesets), the ruleset doesn't contain multiple layers.
+* Hide merge parameter from the public landlock_insert_rule() API.  This
+  helps avoid misuse of this function.
+* Replace a remaining hardcoded 1 with SINGLE_DEPTH_NESTING.
+
+Changes since v22:
+* Explicitely use RB_ROOT and SINGLE_DEPTH_NESTING (suggested by Jann
+  Horn).
+* Improve comments and fix spelling (suggested by Jann Horn).
+
+Changes since v21:
+* Add and clean up comments.
 
 Changes since v18:
 * Account rulesets to kmemcg.
@@ -99,26 +160,15 @@
 Previous changes:
 https://lore.kernel.org/lkml/20190721213116.23476-7-mic@digikod.net/
 ---
- MAINTAINERS                 |   1 +
  security/landlock/Makefile  |   2 +-
- security/landlock/ruleset.c | 342 ++++++++++++++++++++++++++++++++++++
- security/landlock/ruleset.h | 157 +++++++++++++++++
- 4 files changed, 501 insertions(+), 1 deletion(-)
+ security/landlock/limits.h  |  17 ++
+ security/landlock/ruleset.c | 469 ++++++++++++++++++++++++++++++++++++
+ security/landlock/ruleset.h | 165 +++++++++++++
+ 4 files changed, 652 insertions(+), 1 deletion(-)
+ create mode 100644 security/landlock/limits.h
  create mode 100644 security/landlock/ruleset.c
  create mode 100644 security/landlock/ruleset.h
 
-diff --git a/MAINTAINERS b/MAINTAINERS
-index 4c229c961d0d..f2c2480d8590 100644
---- a/MAINTAINERS
-+++ b/MAINTAINERS
-@@ -9636,6 +9636,7 @@ L:	linux-security-module@vger.kernel.org
- S:	Supported
- W:	https://landlock.io
- T:	git https://github.com/landlock-lsm/linux.git
-+F:	include/uapi/linux/landlock.h
- F:	security/landlock/
- K:	landlock
- K:	LANDLOCK
 diff --git a/security/landlock/Makefile b/security/landlock/Makefile
 index cb6deefbf4c0..d846eba445bb 100644
 --- a/security/landlock/Makefile
@@ -128,12 +178,35 @@
  
 -landlock-y := object.o
 +landlock-y := object.o ruleset.o
+diff --git a/security/landlock/limits.h b/security/landlock/limits.h
+new file mode 100644
+index 000000000000..b734f597bb0e
+--- /dev/null
++++ b/security/landlock/limits.h
+@@ -0,0 +1,17 @@
++/* SPDX-License-Identifier: GPL-2.0-only */
++/*
++ * Landlock LSM - Limits for different components
++ *
++ * Copyright © 2016-2020 Mickaël Salaün <mic@digikod.net>
++ * Copyright © 2018-2020 ANSSI
++ */
++
++#ifndef _SECURITY_LANDLOCK_LIMITS_H
++#define _SECURITY_LANDLOCK_LIMITS_H
++
++#include <linux/limits.h>
++
++#define LANDLOCK_MAX_NUM_LAYERS		64
++#define LANDLOCK_MAX_NUM_RULES		U32_MAX
++
++#endif /* _SECURITY_LANDLOCK_LIMITS_H */
 diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
 new file mode 100644
-index 000000000000..f9ef8a6793e2
+index 000000000000..59c86126ea1c
 --- /dev/null
 +++ b/security/landlock/ruleset.c
-@@ -0,0 +1,342 @@
+@@ -0,0 +1,469 @@
 +// SPDX-License-Identifier: GPL-2.0-only
 +/*
 + * Landlock LSM - Ruleset management
@@ -148,31 +221,34 @@
 +#include <linux/err.h>
 +#include <linux/errno.h>
 +#include <linux/kernel.h>
-+#include <linux/limits.h>
++#include <linux/lockdep.h>
++#include <linux/overflow.h>
 +#include <linux/rbtree.h>
 +#include <linux/refcount.h>
 +#include <linux/slab.h>
 +#include <linux/spinlock.h>
 +#include <linux/workqueue.h>
 +
++#include "limits.h"
 +#include "object.h"
 +#include "ruleset.h"
 +
-+static struct landlock_ruleset *create_ruleset(void)
++static struct landlock_ruleset *create_ruleset(const u32 num_layers)
 +{
 +	struct landlock_ruleset *new_ruleset;
 +
-+	new_ruleset = kzalloc(sizeof(*new_ruleset), GFP_KERNEL_ACCOUNT);
++	new_ruleset = kzalloc(struct_size(new_ruleset, fs_access_masks,
++				num_layers), GFP_KERNEL_ACCOUNT);
 +	if (!new_ruleset)
 +		return ERR_PTR(-ENOMEM);
 +	refcount_set(&new_ruleset->usage, 1);
 +	mutex_init(&new_ruleset->lock);
++	new_ruleset->root = RB_ROOT;
++	new_ruleset->num_layers = num_layers;
 +	/*
-+	 * root = RB_ROOT
 +	 * hierarchy = NULL
-+	 * nb_rules = 0
-+	 * nb_layers = 0
-+	 * fs_access_mask = 0
++	 * num_rules = 0
++	 * fs_access_masks[] = 0
 +	 */
 +	return new_ruleset;
 +}
@@ -184,24 +260,53 @@
 +	/* Informs about useless ruleset. */
 +	if (!fs_access_mask)
 +		return ERR_PTR(-ENOMSG);
-+	new_ruleset = create_ruleset();
++	new_ruleset = create_ruleset(1);
 +	if (!IS_ERR(new_ruleset))
-+		new_ruleset->fs_access_mask = fs_access_mask;
++		new_ruleset->fs_access_masks[0] = fs_access_mask;
 +	return new_ruleset;
 +}
 +
-+static struct landlock_rule *duplicate_rule(struct landlock_rule *const src)
++static void build_check_rule(void)
++{
++	const struct landlock_rule rule = {
++		.num_layers = ~0,
++	};
++
++	BUILD_BUG_ON(rule.num_layers < LANDLOCK_MAX_NUM_LAYERS);
++}
++
++static struct landlock_rule *create_rule(
++		struct landlock_object *const object,
++		const struct landlock_layer (*const layers)[],
++		const u32 num_layers,
++		const struct landlock_layer *const new_layer)
 +{
 +	struct landlock_rule *new_rule;
-+
-+	new_rule = kzalloc(sizeof(*new_rule), GFP_KERNEL_ACCOUNT);
++	u32 new_num_layers;
++
++	build_check_rule();
++	if (new_layer) {
++		/* Should already be checked by landlock_merge_ruleset(). */
++		if (WARN_ON_ONCE(num_layers >= LANDLOCK_MAX_NUM_LAYERS))
++			return ERR_PTR(-E2BIG);
++		new_num_layers = num_layers + 1;
++	} else {
++		new_num_layers = num_layers;
++	}
++	new_rule = kzalloc(struct_size(new_rule, layers, new_num_layers),
++			GFP_KERNEL_ACCOUNT);
 +	if (!new_rule)
 +		return ERR_PTR(-ENOMEM);
 +	RB_CLEAR_NODE(&new_rule->node);
-+	landlock_get_object(src->object);
-+	new_rule->object = src->object;
-+	new_rule->access = src->access;
-+	new_rule->layers = src->layers;
++	landlock_get_object(object);
++	new_rule->object = object;
++	new_rule->num_layers = new_num_layers;
++	/* Copies the original layer stack. */
++	memcpy(new_rule->layers, layers,
++			flex_array_size(new_rule, layers, num_layers));
++	if (new_layer)
++		/* Adds a copy of @new_layer on the layer stack. */
++		new_rule->layers[new_rule->num_layers - 1] = *new_layer;
 +	return new_rule;
 +}
 +
@@ -214,18 +319,39 @@
 +	kfree(rule);
 +}
 +
-+/*
-+ * Assumptions:
-+ * - An inserted rule can not be removed.
-+ * - The underlying kernel object must be held by the caller.
++static void build_check_ruleset(void)
++{
++	const struct landlock_ruleset ruleset = {
++		.num_rules = ~0,
++		.num_layers = ~0,
++	};
++
++	BUILD_BUG_ON(ruleset.num_rules < LANDLOCK_MAX_NUM_RULES);
++	BUILD_BUG_ON(ruleset.num_layers < LANDLOCK_MAX_NUM_LAYERS);
++}
++
++/**
++ * insert_rule - Create and insert a rule in a ruleset
 + *
-+ * @rule: Read-only payload to be inserted (not own by this function).
-+ * @is_merge: If true, intersects access rights and updates the rule's layers
-+ * (e.g. merge two rulesets), else do a union of access rights and keep the
-+ * rule's layers (e.g. extend a ruleset)
-+ */
-+int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-+		struct landlock_rule *const rule, const bool is_merge)
++ * @ruleset: The ruleset to be updated.
++ * @object: The object to build the new rule with.  The underlying kernel
++ *          object must be held by the caller.
++ * @layers: One or multiple layers to be copied into the new rule.
++ * @num_layers: The number of @layers entries.
++ *
++ * When user space requests to add a new rule to a ruleset, @layers only
++ * contains one entry and this entry is not assigned to any level.  In this
++ * case, the new rule will extend @ruleset, similarly to a boolean OR between
++ * access rights.
++ *
++ * When merging a ruleset in a domain, or copying a domain, @layers will be
++ * added to @ruleset as new constraints, similarly to a boolean AND between
++ * access rights.
++ */
++static int insert_rule(struct landlock_ruleset *const ruleset,
++		struct landlock_object *const object,
++		const struct landlock_layer (*const layers)[],
++		size_t num_layers)
 +{
 +	struct rb_node **walker_node;
 +	struct rb_node *parent_node = NULL;
@@ -233,47 +359,90 @@
 +
 +	might_sleep();
 +	lockdep_assert_held(&ruleset->lock);
++	if (WARN_ON_ONCE(!object || !layers))
++		return -ENOENT;
 +	walker_node = &(ruleset->root.rb_node);
 +	while (*walker_node) {
 +		struct landlock_rule *const this = rb_entry(*walker_node,
 +				struct landlock_rule, node);
 +
-+		if (this->object != rule->object) {
++		if (this->object != object) {
 +			parent_node = *walker_node;
-+			if (this->object < rule->object)
++			if (this->object < object)
 +				walker_node = &((*walker_node)->rb_right);
 +			else
 +				walker_node = &((*walker_node)->rb_left);
 +			continue;
 +		}
 +
++		/* Only a single-level layer should match an existing rule. */
++		if (WARN_ON_ONCE(num_layers != 1))
++			return -EINVAL;
++
 +		/* If there is a matching rule, updates it. */
-+		if (is_merge) {
-+			/* Intersects access rights. */
-+			this->access &= rule->access;
-+
-+			/* Updates the rule layers with the next one. */
-+			this->layers |= BIT_ULL(ruleset->nb_layers);
-+		} else {
-+			/* Extends access rights. */
-+			this->access |= rule->access;
++		if ((*layers)[0].level == 0) {
++			/*
++			 * Extends access rights when the request comes from
++			 * landlock_add_rule(2), i.e. @ruleset is not a domain.
++			 */
++			if (WARN_ON_ONCE(this->num_layers != 1))
++				return -EINVAL;
++			if (WARN_ON_ONCE(this->layers[0].level != 0))
++				return -EINVAL;
++			this->layers[0].access |= (*layers)[0].access;
++			return 0;
 +		}
++
++		if (WARN_ON_ONCE(this->layers[0].level == 0))
++			return -EINVAL;
++
++		/*
++		 * Intersects access rights when it is a merge between a
++		 * ruleset and a domain.
++		 */
++		new_rule = create_rule(object, &this->layers, this->num_layers,
++				&(*layers)[0]);
++		if (IS_ERR(new_rule))
++			return PTR_ERR(new_rule);
++		rb_replace_node(&this->node, &new_rule->node, &ruleset->root);
++		put_rule(this);
 +		return 0;
 +	}
 +
-+	/* There is no match for @rule->object. */
-+	if (ruleset->nb_rules == U32_MAX)
++	/* There is no match for @object. */
++	build_check_ruleset();
++	if (ruleset->num_rules >= LANDLOCK_MAX_NUM_RULES)
 +		return -E2BIG;
-+	new_rule = duplicate_rule(rule);
++	new_rule = create_rule(object, layers, num_layers, NULL);
 +	if (IS_ERR(new_rule))
 +		return PTR_ERR(new_rule);
-+	if (is_merge)
-+		/* Sets the rule layer to the next one. */
-+		new_rule->layers = BIT_ULL(ruleset->nb_layers);
 +	rb_link_node(&new_rule->node, parent_node, walker_node);
 +	rb_insert_color(&new_rule->node, &ruleset->root);
-+	ruleset->nb_rules++;
++	ruleset->num_rules++;
 +	return 0;
++}
++
++static void build_check_layer(void)
++{
++	const struct landlock_layer layer = {
++		.level = ~0,
++	};
++
++	BUILD_BUG_ON(layer.level < LANDLOCK_MAX_NUM_LAYERS);
++}
++
++/* @ruleset must be locked by the caller. */
++int landlock_insert_rule(struct landlock_ruleset *const ruleset,
++		struct landlock_object *const object, const u32 access)
++{
++	struct landlock_layer layers[] = {{
++		.access = access,
++		/* When @level is zero, insert_rule() extends @ruleset. */
++		.level = 0,
++	}};
++
++	build_check_layer();
++	return insert_rule(ruleset, object, &layers, ARRAY_SIZE(layers));
 +}
 +
 +static inline void get_hierarchy(struct landlock_hierarchy *const hierarchy)
@@ -299,32 +468,45 @@
 +	int err = 0;
 +
 +	might_sleep();
-+	if (!src)
++	/* Should already be checked by landlock_merge_ruleset() */
++	if (WARN_ON_ONCE(!src))
 +		return 0;
 +	/* Only merge into a domain. */
 +	if (WARN_ON_ONCE(!dst || !dst->hierarchy))
-+		return -EFAULT;
-+
++		return -EINVAL;
++
++	/* Locks @dst first because we are its only owner. */
 +	mutex_lock(&dst->lock);
-+	mutex_lock_nested(&src->lock, 1);
-+	/*
-+	 * Makes a new layer, but only increments the number of layers after
-+	 * the rules are inserted.
-+	 */
-+	if (dst->nb_layers == sizeof(walker_rule->layers) * BITS_PER_BYTE) {
-+		err = -E2BIG;
++	mutex_lock_nested(&src->lock, SINGLE_DEPTH_NESTING);
++
++	/* Stacks the new layer. */
++	if (WARN_ON_ONCE(src->num_layers != 1 || dst->num_layers < 1)) {
++		err = -EINVAL;
 +		goto out_unlock;
 +	}
-+	dst->fs_access_mask |= src->fs_access_mask;
++	dst->fs_access_masks[dst->num_layers - 1] = src->fs_access_masks[0];
 +
 +	/* Merges the @src tree. */
 +	rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
 +			&src->root, node) {
-+		err = landlock_insert_rule(dst, walker_rule, true);
++		struct landlock_layer layers[] = {{
++			.level = dst->num_layers,
++		}};
++
++		if (WARN_ON_ONCE(walker_rule->num_layers != 1)) {
++			err = -EINVAL;
++			goto out_unlock;
++		}
++		if (WARN_ON_ONCE(walker_rule->layers[0].level != 0)) {
++			err = -EINVAL;
++			goto out_unlock;
++		}
++		layers[0].access = walker_rule->layers[0].access;
++		err = insert_rule(dst, walker_rule->object, &layers,
++				ARRAY_SIZE(layers));
 +		if (err)
 +			goto out_unlock;
 +	}
-+	dst->nb_layers++;
 +
 +out_unlock:
 +	mutex_unlock(&src->lock);
@@ -332,54 +514,48 @@
 +	return err;
 +}
 +
-+static struct landlock_ruleset *inherit_ruleset(
-+		struct landlock_ruleset *const parent)
++static int inherit_ruleset(struct landlock_ruleset *const parent,
++		struct landlock_ruleset *const child)
 +{
 +	struct landlock_rule *walker_rule, *next_rule;
-+	struct landlock_ruleset *new_ruleset;
 +	int err = 0;
 +
 +	might_sleep();
-+	new_ruleset = create_ruleset();
-+	if (IS_ERR(new_ruleset))
-+		return new_ruleset;
-+
-+	new_ruleset->hierarchy = kzalloc(sizeof(*new_ruleset->hierarchy),
-+			GFP_KERNEL_ACCOUNT);
-+	if (!new_ruleset->hierarchy) {
-+		err = -ENOMEM;
-+		goto out_put_ruleset;
-+	}
-+	refcount_set(&new_ruleset->hierarchy->usage, 1);
 +	if (!parent)
-+		return new_ruleset;
-+
-+	mutex_lock(&new_ruleset->lock);
-+	mutex_lock_nested(&parent->lock, 1);
-+	new_ruleset->nb_layers = parent->nb_layers;
-+	new_ruleset->fs_access_mask = parent->fs_access_mask;
-+	WARN_ON_ONCE(!parent->hierarchy);
-+	get_hierarchy(parent->hierarchy);
-+	new_ruleset->hierarchy->parent = parent->hierarchy;
++		return 0;
++
++	/* Locks @child first because we are its only owner. */
++	mutex_lock(&child->lock);
++	mutex_lock_nested(&parent->lock, SINGLE_DEPTH_NESTING);
 +
 +	/* Copies the @parent tree. */
 +	rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
 +			&parent->root, node) {
-+		err = landlock_insert_rule(new_ruleset, walker_rule, false);
++		err = insert_rule(child, walker_rule->object,
++				&walker_rule->layers, walker_rule->num_layers);
 +		if (err)
 +			goto out_unlock;
 +	}
-+	mutex_unlock(&parent->lock);
-+	mutex_unlock(&new_ruleset->lock);
-+	return new_ruleset;
++
++	if (WARN_ON_ONCE(child->num_layers <= parent->num_layers)) {
++		err = -EINVAL;
++		goto out_unlock;
++	}
++	/* Copies the parent layer stack and leaves a space for the new layer. */
++	memcpy(child->fs_access_masks, parent->fs_access_masks,
++			flex_array_size(parent, fs_access_masks, parent->num_layers));
++
++	if (WARN_ON_ONCE(!parent->hierarchy)) {
++		err = -EINVAL;
++		goto out_unlock;
++	}
++	get_hierarchy(parent->hierarchy);
++	child->hierarchy->parent = parent->hierarchy;
 +
 +out_unlock:
 +	mutex_unlock(&parent->lock);
-+	mutex_unlock(&new_ruleset->lock);
-+
-+out_put_ruleset:
-+	landlock_put_ruleset(new_ruleset);
-+	return ERR_PTR(err);
++	mutex_unlock(&child->lock);
++	return err;
 +}
 +
 +static void free_ruleset(struct landlock_ruleset *const ruleset)
@@ -417,38 +593,62 @@
 +	}
 +}
 +
-+/*
-+ * Creates a new transition domain, intersection of @parent and @ruleset, or
-+ * return @parent if @ruleset is empty.  If @parent is empty, returns a
-+ * duplicate of @ruleset.
++/**
++ * landlock_merge_ruleset - Merge a ruleset with a domain
++ *
++ * @parent: Parent domain.
++ * @ruleset: New ruleset to be merged.
++ *
++ * Returns the intersection of @parent and @ruleset, or returns @parent if
++ * @ruleset is empty, or returns a duplicate of @ruleset if @parent is empty.
 + */
 +struct landlock_ruleset *landlock_merge_ruleset(
 +		struct landlock_ruleset *const parent,
 +		struct landlock_ruleset *const ruleset)
 +{
 +	struct landlock_ruleset *new_dom;
++	u32 num_layers;
 +	int err;
 +
 +	might_sleep();
-+	/*
-+	 * Merging duplicates a ruleset, so a new ruleset can't be
-+	 * the same as the parent, but they can have similar content.
-+	 */
-+	if (WARN_ON_ONCE(!ruleset || parent == ruleset)) {
-+		landlock_get_ruleset(parent);
-+		return parent;
-+	}
-+
-+	new_dom = inherit_ruleset(parent);
++	if (WARN_ON_ONCE(!ruleset || parent == ruleset))
++		return ERR_PTR(-EINVAL);
++
++	if (parent) {
++		if (parent->num_layers >= LANDLOCK_MAX_NUM_LAYERS)
++			return ERR_PTR(-E2BIG);
++		num_layers = parent->num_layers + 1;
++	} else {
++		num_layers = 1;
++	}
++
++	/* Creates a new domain... */
++	new_dom = create_ruleset(num_layers);
 +	if (IS_ERR(new_dom))
 +		return new_dom;
-+
++	new_dom->hierarchy = kzalloc(sizeof(*new_dom->hierarchy),
++			GFP_KERNEL_ACCOUNT);
++	if (!new_dom->hierarchy) {
++		err = -ENOMEM;
++		goto out_put_dom;
++	}
++	refcount_set(&new_dom->hierarchy->usage, 1);
++
++	/* ...as a child of @parent... */
++	err = inherit_ruleset(parent, new_dom);
++	if (err)
++		goto out_put_dom;
++
++	/* ...and including @ruleset. */
 +	err = merge_ruleset(new_dom, ruleset);
-+	if (err) {
-+		landlock_put_ruleset(new_dom);
-+		return ERR_PTR(err);
-+	}
++	if (err)
++		goto out_put_dom;
++
 +	return new_dom;
++
++out_put_dom:
++	landlock_put_ruleset(new_dom);
++	return ERR_PTR(err);
 +}
 +
 +/*
@@ -478,10 +678,10 @@
 +}
 diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
 new file mode 100644
-index 000000000000..d5fcec4c1a17
+index 000000000000..2d3ed7ec5a0a
 --- /dev/null
 +++ b/security/landlock/ruleset.h
-@@ -0,0 +1,157 @@
+@@ -0,0 +1,165 @@
 +/* SPDX-License-Identifier: GPL-2.0-only */
 +/*
 + * Landlock LSM - Ruleset management
@@ -501,41 +701,44 @@
 +#include "object.h"
 +
 +/**
++ * struct landlock_layer - Access rights for a given layer
++ */
++struct landlock_layer {
++	/**
++	 * @level: Position of this layer in the layer stack.
++	 */
++	u16 level;
++	/**
++	 * @access: Bitfield of allowed actions on the kernel object.  They are
++	 * relative to the object type (e.g. %LANDLOCK_ACTION_FS_READ).
++	 */
++	u16 access;
++};
++
++/**
 + * struct landlock_rule - Access rights tied to an object
-+ *
-+ * When enforcing a ruleset (i.e. merging a ruleset into the current domain),
-+ * the layer level of a new rule is the incremented top layer level (cf.
-+ * &struct landlock_ruleset).  If there is no rule (from this domain) tied to
-+ * the same object, then the depth of the new rule is 1. However, if there is
-+ * already a rule tied to the same object and if this rule's layer level is the
-+ * previous top layer level, then the depth and the layer level are both
-+ * incremented and the rule is updated with the new access rights (boolean
-+ * AND).
 + */
 +struct landlock_rule {
 +	/**
-+	 * @node: Node in the red-black tree.
++	 * @node: Node in the ruleset's red-black tree.
 +	 */
 +	struct rb_node node;
 +	/**
 +	 * @object: Pointer to identify a kernel object (e.g. an inode).  This
 +	 * is used as a key for this ruleset element.  This pointer is set once
-+	 * and never modified.  It always point to an allocated object because
-+	 * each rule increment the refcount of there object.
++	 * and never modified.  It always points to an allocated object because
++	 * each rule increments the refcount of its object.
 +	 */
 +	struct landlock_object *object;
 +	/**
-+	 * @layers: Bitfield to identify the layers which resulted to @access
-+	 * from different consecutive intersections.
-+	 */
-+	u64 layers;
-+	/**
-+	 * @access: Bitfield of allowed actions on the kernel object.  They are
-+	 * relative to the object type (e.g. %LANDLOCK_ACTION_FS_READ).  This
-+	 * may be the result of the merged access rights (boolean AND) from
-+	 * multiple layers referring to the same object.
-+	 */
-+	u32 access;
++	 * @num_layers: Number of entries in @layers.
++	 */
++	u32 num_layers;
++	/**
++	 * @layers: Stack of layers, from the latest to the newest, implemented
++	 * as a flexible array member (FAM).
++	 */
++	struct landlock_layer layers[];
 +};
 +
 +/**
@@ -543,8 +746,8 @@
 + */
 +struct landlock_hierarchy {
 +	/**
-+	 * @parent: Pointer to the parent node, or NULL if it is a root Lanlock
-+	 * domain.
++	 * @parent: Pointer to the parent node, or NULL if it is a root
++	 * Landlock domain.
 +	 */
 +	struct landlock_hierarchy *parent;
 +	/**
@@ -557,13 +760,14 @@
 +/**
 + * struct landlock_ruleset - Landlock ruleset
 + *
-+ * This data structure must contains unique entries, be updatable, and quick to
++ * This data structure must contain unique entries, be updatable, and quick to
 + * match an object.
 + */
 +struct landlock_ruleset {
 +	/**
 +	 * @root: Root of a red-black tree containing &struct landlock_rule
-+	 * nodes.
++	 * nodes.  Once a ruleset is tied to a process (i.e. as a domain), this
++	 * tree is immutable until @usage reaches zero.
 +	 */
 +	struct rb_root root;
 +	/**
@@ -576,13 +780,13 @@
 +		 * @work_free: Enables to free a ruleset within a lockless
 +		 * section.  This is only used by
 +		 * landlock_put_ruleset_deferred() when @usage reaches zero.
-+		 * The fields @lock, @usage, @nb_layers, @nb_rules and
-+		 * @fs_access_mask are then unused.
++		 * The fields @lock, @usage, @num_rules, @num_layers and
++		 * @fs_access_masks are then unused.
 +		 */
 +		struct work_struct work_free;
 +		struct {
 +			/**
-+			 * @lock: Guards against concurrent modifications of
++			 * @lock: Protects against concurrent modifications of
 +			 * @root, if @usage is greater than zero.
 +			 */
 +			struct mutex lock;
@@ -592,26 +796,30 @@
 +			 */
 +			refcount_t usage;
 +			/**
-+			 * @nb_rules: Number of non-overlapping (i.e. not for
++			 * @num_rules: Number of non-overlapping (i.e. not for
 +			 * the same object) rules in this ruleset.
 +			 */
-+			u32 nb_rules;
++			u32 num_rules;
 +			/**
-+			 * @nb_layers: Number of layers which are used in this
++			 * @num_layers: Number of layers that are used in this
 +			 * ruleset.  This enables to check that all the layers
-+			 * allow an access request.  A value of 0 identify a
++			 * allow an access request.  A value of 0 identifies a
 +			 * non-merged ruleset (i.e. not a domain).
 +			 */
-+			u32 nb_layers;
++			u32 num_layers;
 +			/**
-+			 * @fs_access_mask: Contains the subset of filesystem
-+			 * actions which are restricted by a ruleset.  This is
-+			 * used when merging rulesets and for user space
-+			 * backward compatibility (i.e. future-proof).  Set
-+			 * once and never changed for the lifetime of the
-+			 * ruleset.
++			 * @fs_access_masks: Contains the subset of filesystem
++			 * actions that are restricted by a ruleset.  A domain
++			 * saves all layers of merged rulesets in a stack
++			 * (FAM), starting from the first layer to the last
++			 * one.  These layers are used when merging rulesets,
++			 * for user space backward compatibility (i.e.
++			 * future-proof), and to properly handle merged
++			 * rulesets without overlapping access rights.  These
++			 * layers are set once and never changed for the
++			 * lifetime of the ruleset.
 +			 */
-+			u32 fs_access_mask;
++			u16 fs_access_masks[];
 +		};
 +	};
 +};
@@ -622,7 +830,7 @@
 +void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
 +
 +int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-+		struct landlock_rule *const rule, const bool is_merge);
++		struct landlock_object *const object, const u32 access);
 +
 +struct landlock_ruleset *landlock_merge_ruleset(
 +		struct landlock_ruleset *const parent,
@@ -640,5 +848,5 @@
 +
 +#endif /* _SECURITY_LANDLOCK_RULESET_H */
 -- 
-2.28.0.rc2
-
+2.30.2
+
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help