Inter-revision diff: patch 2

Comparing v33 (message) to v16 (message)

--- v33
+++ v16
@@ -1,101 +1,23 @@
-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 syscalls) 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 the syscall) 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
-defines the security policy enforced on these processes and their future
+define 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>
-Signed-off-by: Mickaël Salaün <mic@linux.microsoft.com>
-Acked-by: Serge Hallyn <serge@hallyn.com>
-Reviewed-by: Kees Cook <keescook@chromium.org>
-Reviewed-by: Jann Horn <jannh@google.com>
-Link: https://lore.kernel.org/r/20210407160726.542794-3-mic@digikod.net
+Cc: Jann Horn <jannh@google.com>
+Cc: Kees Cook <keescook@chromium.org>
+Cc: Serge E. Hallyn <serge@hallyn.com>
 ---
-
-Changes since v30:
-* Rename put_rule() to free_rule() to be more consistent.
-* Add Reviewed-by Kees Cook.
-* Add Reviewed-by Jann Horn.
-
-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.
-* Remove struct holes.
-* Cosmetic changes.
-
-Changes since v17:
-* Move include/uapi/linux/landlock.h and _LANDLOCK_ACCESS_FS_* to a
-  following patch.
-
-Changes since v16:
-* Allow enforcement of empty ruleset, which enables deny-all policies.
 
 Changes since v15:
 * Replace layer_levels and layer_depth with a bitfield of layers, cf.
@@ -165,15 +87,112 @@
 Previous changes:
 https://lore.kernel.org/lkml/20190721213116.23476-7-mic@digikod.net/
 ---
- security/landlock/Makefile  |   2 +-
- 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
+ MAINTAINERS                   |   1 +
+ include/uapi/linux/landlock.h |  78 ++++++++
+ security/landlock/Makefile    |   2 +-
+ security/landlock/ruleset.c   | 344 ++++++++++++++++++++++++++++++++++
+ security/landlock/ruleset.h   | 161 ++++++++++++++++
+ 5 files changed, 585 insertions(+), 1 deletion(-)
+ create mode 100644 include/uapi/linux/landlock.h
  create mode 100644 security/landlock/ruleset.c
  create mode 100644 security/landlock/ruleset.h
 
+diff --git a/MAINTAINERS b/MAINTAINERS
+index 527062ba139b..b8b726a0a0f0 100644
+--- a/MAINTAINERS
++++ b/MAINTAINERS
+@@ -9489,6 +9489,7 @@ L:	linux-security-module@vger.kernel.org
+ W:	https://landlock.io
+ T:	git https://github.com/landlock-lsm/linux.git
+ S:	Supported
++F:	include/uapi/linux/landlock.h
+ F:	security/landlock/
+ K:	landlock
+ K:	LANDLOCK
+diff --git a/include/uapi/linux/landlock.h b/include/uapi/linux/landlock.h
+new file mode 100644
+index 000000000000..4b7e69a8806b
+--- /dev/null
++++ b/include/uapi/linux/landlock.h
+@@ -0,0 +1,78 @@
++/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
++/*
++ * Landlock - UAPI headers
++ *
++ * Copyright © 2017-2020 Mickaël Salaün <mic@digikod.net>
++ * Copyright © 2018-2020 ANSSI
++ */
++
++#ifndef _UAPI__LINUX_LANDLOCK_H__
++#define _UAPI__LINUX_LANDLOCK_H__
++
++/**
++ * DOC: fs_access
++ *
++ * A set of actions on kernel objects may be defined by an attribute (e.g.
++ * &struct landlock_attr_path_beneath) and a bitmask of access.
++ *
++ * Filesystem flags
++ * ~~~~~~~~~~~~~~~~
++ *
++ * These flags enable to restrict a sandbox process to a set of actions on
++ * files and directories.  Files or directories opened before the sandboxing
++ * are not subject to these restrictions.
++ *
++ * A file can only receive these access rights:
++ *
++ * - %LANDLOCK_ACCESS_FS_EXECUTE: Execute a file.
++ * - %LANDLOCK_ACCESS_FS_WRITE_FILE: Write to a file.
++ * - %LANDLOCK_ACCESS_FS_READ_FILE: Open a file with read access.
++ *
++ * A directory can receive access rights related to files or directories.  This
++ * set of access rights is applied to the directory itself, and the directories
++ * beneath it:
++ *
++ * - %LANDLOCK_ACCESS_FS_READ_DIR: Open a directory or list its content.
++ * - %LANDLOCK_ACCESS_FS_CHROOT: Change the root directory of the current
++ *   process.
++ *
++ * However, the following access rights only apply to the content of a
++ * directory, not the directory itself:
++ *
++ * - %LANDLOCK_ACCESS_FS_REMOVE_DIR: Remove an empty directory or rename one.
++ * - %LANDLOCK_ACCESS_FS_REMOVE_FILE: Unlink (or rename) a file.
++ * - %LANDLOCK_ACCESS_FS_MAKE_CHAR: Create (or rename or link) a character
++ *   device.
++ * - %LANDLOCK_ACCESS_FS_MAKE_DIR: Create (or rename or link) a directory.
++ * - %LANDLOCK_ACCESS_FS_MAKE_REG: Create (or rename or link) a regular file.
++ * - %LANDLOCK_ACCESS_FS_MAKE_SOCK: Create (or rename or link) a UNIX domain
++ *   socket.
++ * - %LANDLOCK_ACCESS_FS_MAKE_FIFO: Create (or rename or link) a named pipe.
++ * - %LANDLOCK_ACCESS_FS_MAKE_BLOCK: Create (or rename or link) a block device.
++ * - %LANDLOCK_ACCESS_FS_MAKE_SYM: Create (or rename or link) a symbolic link.
++ *
++ * .. warning::
++ *
++ *   It is currently not possible to restrict some file-related actions
++ *   accessible through these syscall families: :manpage:`chdir(2)`,
++ *   :manpage:`truncate(2)`, :manpage:`stat(2)`, :manpage:`flock(2)`,
++ *   :manpage:`chmod(2)`, :manpage:`chown(2)`, :manpage:`setxattr(2)`,
++ *   :manpage:`ioctl(2)`, :manpage:`fcntl(2)`.
++ *   Future Landlock evolutions will enable to restrict them.
++ */
++#define LANDLOCK_ACCESS_FS_EXECUTE		(1ULL << 0)
++#define LANDLOCK_ACCESS_FS_WRITE_FILE		(1ULL << 1)
++#define LANDLOCK_ACCESS_FS_READ_FILE		(1ULL << 2)
++#define LANDLOCK_ACCESS_FS_READ_DIR		(1ULL << 3)
++#define LANDLOCK_ACCESS_FS_CHROOT		(1ULL << 4)
++#define LANDLOCK_ACCESS_FS_REMOVE_DIR		(1ULL << 5)
++#define LANDLOCK_ACCESS_FS_REMOVE_FILE		(1ULL << 6)
++#define LANDLOCK_ACCESS_FS_MAKE_CHAR		(1ULL << 7)
++#define LANDLOCK_ACCESS_FS_MAKE_DIR		(1ULL << 8)
++#define LANDLOCK_ACCESS_FS_MAKE_REG		(1ULL << 9)
++#define LANDLOCK_ACCESS_FS_MAKE_SOCK		(1ULL << 10)
++#define LANDLOCK_ACCESS_FS_MAKE_FIFO		(1ULL << 11)
++#define LANDLOCK_ACCESS_FS_MAKE_BLOCK		(1ULL << 12)
++#define LANDLOCK_ACCESS_FS_MAKE_SYM		(1ULL << 13)
++
++#endif /* _UAPI__LINUX_LANDLOCK_H__ */
 diff --git a/security/landlock/Makefile b/security/landlock/Makefile
 index cb6deefbf4c0..d846eba445bb 100644
 --- a/security/landlock/Makefile
@@ -183,35 +202,12 @@
  
 -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..2e616f6d5274
+index 000000000000..6d22a06e24e2
 --- /dev/null
 +++ b/security/landlock/ruleset.c
-@@ -0,0 +1,469 @@
+@@ -0,0 +1,344 @@
 +// SPDX-License-Identifier: GPL-2.0-only
 +/*
 + * Landlock LSM - Ruleset management
@@ -226,96 +222,64 @@
 +#include <linux/err.h>
 +#include <linux/errno.h>
 +#include <linux/kernel.h>
-+#include <linux/lockdep.h>
-+#include <linux/overflow.h>
++#include <linux/limits.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(const u32 num_layers)
-+{
-+	struct landlock_ruleset *new_ruleset;
-+
-+	new_ruleset = kzalloc(struct_size(new_ruleset, fs_access_masks,
-+				num_layers), GFP_KERNEL_ACCOUNT);
-+	if (!new_ruleset)
++static struct landlock_ruleset *create_ruleset(void)
++{
++	struct landlock_ruleset *ruleset;
++
++	ruleset = kzalloc(sizeof(*ruleset), GFP_KERNEL);
++	if (!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;
++	refcount_set(&ruleset->usage, 1);
++	mutex_init(&ruleset->lock);
 +	/*
++	 * root = RB_ROOT
 +	 * hierarchy = NULL
-+	 * num_rules = 0
-+	 * fs_access_masks[] = 0
-+	 */
-+	return new_ruleset;
++	 * nb_rules = 0
++	 * nb_layers = 0
++	 * fs_access_mask = 0
++	 */
++	return ruleset;
 +}
 +
 +struct landlock_ruleset *landlock_create_ruleset(const u32 fs_access_mask)
 +{
-+	struct landlock_ruleset *new_ruleset;
++	struct landlock_ruleset *ruleset;
 +
 +	/* Informs about useless ruleset. */
 +	if (!fs_access_mask)
 +		return ERR_PTR(-ENOMSG);
-+	new_ruleset = create_ruleset(1);
-+	if (!IS_ERR(new_ruleset))
-+		new_ruleset->fs_access_masks[0] = fs_access_mask;
-+	return new_ruleset;
-+}
-+
-+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)
++	ruleset = create_ruleset();
++	if (!IS_ERR(ruleset))
++		ruleset->fs_access_mask = fs_access_mask;
++	return ruleset;
++}
++
++static struct landlock_rule *duplicate_rule(struct landlock_rule *const src)
 +{
 +	struct landlock_rule *new_rule;
-+	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);
++
++	new_rule = kzalloc(sizeof(*new_rule), GFP_KERNEL);
 +	if (!new_rule)
 +		return ERR_PTR(-ENOMEM);
 +	RB_CLEAR_NODE(&new_rule->node);
-+	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;
++	landlock_get_object(src->object);
++	new_rule->object = src->object;
++	new_rule->access = src->access;
++	new_rule->layers = src->layers;
 +	return new_rule;
 +}
 +
-+static void free_rule(struct landlock_rule *const rule)
++static void put_rule(struct landlock_rule *const rule)
 +{
 +	might_sleep();
 +	if (!rule)
@@ -324,39 +288,18 @@
 +	kfree(rule);
 +}
 +
-+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
-+ *
-+ * @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)
++/*
++ * Assumptions:
++ * - An inserted rule can not be removed.
++ * - The underlying kernel object must be held by the caller.
++ *
++ * @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)
 +{
 +	struct rb_node **walker_node;
 +	struct rb_node *parent_node = NULL;
@@ -364,90 +307,47 @@
 +
 +	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 != object) {
++		if (this->object != rule->object) {
 +			parent_node = *walker_node;
-+			if (this->object < object)
++			if (this->object < rule->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 ((*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 (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 (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);
-+		free_rule(this);
 +		return 0;
 +	}
 +
-+	/* There is no match for @object. */
-+	build_check_ruleset();
-+	if (ruleset->num_rules >= LANDLOCK_MAX_NUM_RULES)
++	/* There is no match for @rule->object. */
++	if (ruleset->nb_rules == U32_MAX)
 +		return -E2BIG;
-+	new_rule = create_rule(object, layers, num_layers, NULL);
++	new_rule = duplicate_rule(rule);
 +	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->num_rules++;
++	ruleset->nb_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)
@@ -473,45 +373,32 @@
 +	int err = 0;
 +
 +	might_sleep();
-+	/* Should already be checked by landlock_merge_ruleset() */
-+	if (WARN_ON_ONCE(!src))
++	if (!src)
 +		return 0;
 +	/* Only merge into a domain. */
 +	if (WARN_ON_ONCE(!dst || !dst->hierarchy))
-+		return -EINVAL;
-+
-+	/* Locks @dst first because we are its only owner. */
++		return -EFAULT;
++
 +	mutex_lock(&dst->lock);
-+	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;
++	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;
 +		goto out_unlock;
 +	}
-+	dst->fs_access_masks[dst->num_layers - 1] = src->fs_access_masks[0];
++	dst->fs_access_mask |= src->fs_access_mask;
 +
 +	/* Merges the @src tree. */
 +	rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
 +			&src->root, node) {
-+		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));
++		err = landlock_insert_rule(dst, walker_rule, true);
 +		if (err)
 +			goto out_unlock;
 +	}
++	dst->nb_layers++;
 +
 +out_unlock:
 +	mutex_unlock(&src->lock);
@@ -519,48 +406,54 @@
 +	return err;
 +}
 +
-+static int inherit_ruleset(struct landlock_ruleset *const parent,
-+		struct landlock_ruleset *const child)
++static struct landlock_ruleset *inherit_ruleset(
++		struct landlock_ruleset *const parent)
 +{
 +	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);
++	if (!new_ruleset->hierarchy) {
++		err = -ENOMEM;
++		goto out_put_ruleset;
++	}
++	refcount_set(&new_ruleset->hierarchy->usage, 1);
 +	if (!parent)
-+		return 0;
-+
-+	/* Locks @child first because we are its only owner. */
-+	mutex_lock(&child->lock);
-+	mutex_lock_nested(&parent->lock, SINGLE_DEPTH_NESTING);
++		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;
 +
 +	/* Copies the @parent tree. */
 +	rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
 +			&parent->root, node) {
-+		err = insert_rule(child, walker_rule->object,
-+				&walker_rule->layers, walker_rule->num_layers);
++		err = landlock_insert_rule(new_ruleset, walker_rule, false);
 +		if (err)
 +			goto out_unlock;
 +	}
-+
-+	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;
++	mutex_unlock(&parent->lock);
++	mutex_unlock(&new_ruleset->lock);
++	return new_ruleset;
 +
 +out_unlock:
 +	mutex_unlock(&parent->lock);
-+	mutex_unlock(&child->lock);
-+	return err;
++	mutex_unlock(&new_ruleset->lock);
++
++out_put_ruleset:
++	landlock_put_ruleset(new_ruleset);
++	return ERR_PTR(err);
 +}
 +
 +static void free_ruleset(struct landlock_ruleset *const ruleset)
@@ -570,7 +463,7 @@
 +	might_sleep();
 +	rbtree_postorder_for_each_entry_safe(freeme, next, &ruleset->root,
 +			node)
-+		free_rule(freeme);
++		put_rule(freeme);
 +	put_hierarchy(ruleset->hierarchy);
 +	kfree(ruleset);
 +}
@@ -598,62 +491,40 @@
 +	}
 +}
 +
-+/**
-+ * 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.
++/*
++ * 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.
 + */
 +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();
-+	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);
++	/*
++	 * Rulesets without rule must be rejected at the syscall step to inform
++	 * user space.  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 || ruleset->nb_rules == 0 ||
++				parent == ruleset)) {
++		landlock_get_ruleset(parent);
++		return parent;
++	}
++
++	new_dom = inherit_ruleset(parent);
 +	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)
-+		goto out_put_dom;
-+
++	if (err) {
++		landlock_put_ruleset(new_dom);
++		return ERR_PTR(err);
++	}
 +	return new_dom;
-+
-+out_put_dom:
-+	landlock_put_ruleset(new_dom);
-+	return ERR_PTR(err);
 +}
 +
 +/*
@@ -683,10 +554,10 @@
 +}
 diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
 new file mode 100644
-index 000000000000..2d3ed7ec5a0a
+index 000000000000..206814974525
 --- /dev/null
 +++ b/security/landlock/ruleset.h
-@@ -0,0 +1,165 @@
+@@ -0,0 +1,161 @@
 +/* SPDX-License-Identifier: GPL-2.0-only */
 +/*
 + * Landlock LSM - Ruleset management
@@ -702,48 +573,49 @@
 +#include <linux/rbtree.h>
 +#include <linux/refcount.h>
 +#include <linux/workqueue.h>
++#include <uapi/linux/landlock.h>
 +
 +#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;
-+};
++#define _LANDLOCK_ACCESS_FS_LAST	LANDLOCK_ACCESS_FS_MAKE_SYM
++#define _LANDLOCK_ACCESS_FS_MASK	((_LANDLOCK_ACCESS_FS_LAST << 1) - 1)
 +
 +/**
 + * 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 ruleset's red-black tree.
++	 * @node: Node in the 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 points to an allocated object because
-+	 * each rule increments the refcount of its object.
++	 * and never modified.  It always point to an allocated object because
++	 * each rule increment the refcount of there object.
 +	 */
 +	struct landlock_object *object;
 +	/**
-+	 * @num_layers: Number of entries in @layers.
-+	 */
-+	u32 num_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;
 +	/**
-+	 * @layers: Stack of layers, from the latest to the newest, implemented
-+	 * as a flexible array member (FAM).
-+	 */
-+	struct landlock_layer layers[];
++	 * @layers: Bitfield to identify the layers which resulted to @access
++	 * from different consecutive intersections.
++	 */
++	u64 layers;
 +};
 +
 +/**
@@ -751,8 +623,8 @@
 + */
 +struct landlock_hierarchy {
 +	/**
-+	 * @parent: Pointer to the parent node, or NULL if it is a root
-+	 * Landlock domain.
++	 * @parent: Pointer to the parent node, or NULL if it is a root Lanlock
++	 * domain.
 +	 */
 +	struct landlock_hierarchy *parent;
 +	/**
@@ -765,14 +637,13 @@
 +/**
 + * struct landlock_ruleset - Landlock ruleset
 + *
-+ * This data structure must contain unique entries, be updatable, and quick to
++ * This data structure must contains 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.  Once a ruleset is tied to a process (i.e. as a domain), this
-+	 * tree is immutable until @usage reaches zero.
++	 * nodes.
 +	 */
 +	struct rb_root root;
 +	/**
@@ -785,46 +656,42 @@
 +		 * @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, @num_rules, @num_layers and
-+		 * @fs_access_masks are then unused.
++		 * The fields @usage, @lock, @nb_layers, @nb_rules and
++		 * @fs_access_mask are then unused.
 +		 */
 +		struct work_struct work_free;
 +		struct {
-+			/**
-+			 * @lock: Protects against concurrent modifications of
-+			 * @root, if @usage is greater than zero.
-+			 */
-+			struct mutex lock;
 +			/**
 +			 * @usage: Number of processes (i.e. domains) or file
 +			 * descriptors referencing this ruleset.
 +			 */
 +			refcount_t usage;
 +			/**
-+			 * @num_rules: Number of non-overlapping (i.e. not for
++			 * @lock: Guards against concurrent modifications of
++			 * @root, if @usage is greater than zero.
++			 */
++			struct mutex lock;
++			/**
++			 * @nb_rules: Number of non-overlapping (i.e. not for
 +			 * the same object) rules in this ruleset.
 +			 */
-+			u32 num_rules;
++			u32 nb_rules;
 +			/**
-+			 * @num_layers: Number of layers that are used in this
++			 * @nb_layers: Number of layers which are used in this
 +			 * ruleset.  This enables to check that all the layers
-+			 * allow an access request.  A value of 0 identifies a
++			 * allow an access request.  A value of 0 identify a
 +			 * non-merged ruleset (i.e. not a domain).
 +			 */
-+			u32 num_layers;
++			u32 nb_layers;
 +			/**
-+			 * @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.
++			 * @fs_access_mask: Contains the subset of filesystem
++			 * actions which are restricted by a ruleset.  This is
++			 * used when merging rulesets and for userspace
++			 * backward compatibility (i.e. future-proof).  Set
++			 * once and never changed for the lifetime of the
++			 * ruleset.
 +			 */
-+			u16 fs_access_masks[];
++			u32 fs_access_mask;
 +		};
 +	};
 +};
@@ -835,7 +702,7 @@
 +void landlock_put_ruleset_deferred(struct landlock_ruleset *const ruleset);
 +
 +int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-+		struct landlock_object *const object, const u32 access);
++		struct landlock_rule *const rule, const bool is_merge);
 +
 +struct landlock_ruleset *landlock_merge_ruleset(
 +		struct landlock_ruleset *const parent,
@@ -853,5 +720,4 @@
 +
 +#endif /* _SECURITY_LANDLOCK_RULESET_H */
 -- 
-2.30.2
-
+2.26.1
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help