[RFC PATCH v1 2/5] landlock: Merge landlock_find_rule() into landlock_unmask_layers()

Mickaël Salaün mic at digikod.net
Fri May 23 16:57:38 UTC 2025


To be able to have useful traces, let's consolidate rule finding into
unmask checking.  landlock_unmask_layers() now gets a landlock_rule_ref
instead of a rule pointer.

This enables us to not deal with Landlock rule pointers outside of
ruleset.c, to avoid two calls, and to get all required information
available to landlock_unmask_layers().

We could make struct landlock_rule private because it is now only used
in the ruleset.c file.

Cc: Günther Noack <gnoack at google.com>
Signed-off-by: Mickaël Salaün <mic at digikod.net>
---
 security/landlock/fs.c      | 144 ++++++++++++++++++++++--------------
 security/landlock/net.c     |   6 +-
 security/landlock/ruleset.c |  12 ++-
 security/landlock/ruleset.h |   9 +--
 4 files changed, 100 insertions(+), 71 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index f5087688190a..73a20a501c3c 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -356,30 +356,27 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 /* Access-control management */
 
 /*
- * The lifetime of the returned rule is tied to @domain.
- *
- * Returns NULL if no rule is found or if @dentry is negative.
+ * Returns true if an object is tied to @dentry, and updates @ref accordingly.
  */
-static const struct landlock_rule *
-find_rule(const struct landlock_ruleset *const domain,
-	  const struct dentry *const dentry)
+static bool find_rule_ref(const struct dentry *const dentry,
+			  struct landlock_rule_ref *ref)
 {
-	const struct landlock_rule *rule;
 	const struct inode *inode;
-	struct landlock_rule_ref ref = {
-		.type = LANDLOCK_KEY_INODE,
-	};
+
+	/*
+	 * We do not strictly need an RCU read-side critical section if
+	 * ref->key.object is not dereferenced or if a domain's rule own a reference
+	 * to it, but it is simpler and safer to always require one.
+	 */
+	lockdep_assert_in_rcu_read_lock();
 
 	/* Ignores nonexistent leafs. */
-	if (d_is_negative(dentry))
-		return NULL;
+	if (!dentry || d_is_negative(dentry))
+		return false;
 
 	inode = d_backing_inode(dentry);
-	rcu_read_lock();
-	ref.key.object = rcu_dereference(landlock_inode(inode)->object);
-	rule = landlock_find_rule(domain, ref);
-	rcu_read_unlock();
-	return rule;
+	ref->key.object = rcu_dereference(landlock_inode(inode)->object);
+	return true;
 }
 
 /*
@@ -809,25 +806,36 @@ static bool is_access_to_paths_allowed(
 		is_dom_check = false;
 	}
 
-	if (unlikely(dentry_child1)) {
-		landlock_unmask_layers(
-			find_rule(domain, dentry_child1),
-			landlock_init_layer_masks(
-				domain, LANDLOCK_MASK_ACCESS_FS,
-				&_layer_masks_child1, LANDLOCK_KEY_INODE),
-			&_layer_masks_child1, ARRAY_SIZE(_layer_masks_child1));
-		layer_masks_child1 = &_layer_masks_child1;
-		child1_is_directory = d_is_dir(dentry_child1);
-	}
-	if (unlikely(dentry_child2)) {
-		landlock_unmask_layers(
-			find_rule(domain, dentry_child2),
-			landlock_init_layer_masks(
-				domain, LANDLOCK_MASK_ACCESS_FS,
-				&_layer_masks_child2, LANDLOCK_KEY_INODE),
-			&_layer_masks_child2, ARRAY_SIZE(_layer_masks_child2));
-		layer_masks_child2 = &_layer_masks_child2;
-		child2_is_directory = d_is_dir(dentry_child2);
+	scoped_guard(rcu)
+	{
+		struct landlock_rule_ref ref = {
+			.type = LANDLOCK_KEY_INODE,
+		};
+
+		if (unlikely(find_rule_ref(dentry_child1, &ref))) {
+			landlock_unmask_layers(domain, ref,
+					       landlock_init_layer_masks(
+						       domain,
+						       LANDLOCK_MASK_ACCESS_FS,
+						       &_layer_masks_child1,
+						       LANDLOCK_KEY_INODE),
+					       &_layer_masks_child1,
+					       ARRAY_SIZE(_layer_masks_child1));
+			layer_masks_child1 = &_layer_masks_child1;
+			child1_is_directory = d_is_dir(dentry_child1);
+		}
+		if (unlikely(find_rule_ref(dentry_child2, &ref))) {
+			landlock_unmask_layers(domain, ref,
+					       landlock_init_layer_masks(
+						       domain,
+						       LANDLOCK_MASK_ACCESS_FS,
+						       &_layer_masks_child2,
+						       LANDLOCK_KEY_INODE),
+					       &_layer_masks_child2,
+					       ARRAY_SIZE(_layer_masks_child2));
+			layer_masks_child2 = &_layer_masks_child2;
+			child2_is_directory = d_is_dir(dentry_child2);
+		}
 	}
 
 	walker_path = *path;
@@ -838,7 +846,6 @@ static bool is_access_to_paths_allowed(
 	 */
 	while (true) {
 		struct dentry *parent_dentry;
-		const struct landlock_rule *rule;
 
 		/*
 		 * If at least all accesses allowed on the destination are
@@ -880,17 +887,32 @@ static bool is_access_to_paths_allowed(
 				break;
 		}
 
-		rule = find_rule(domain, walker_path.dentry);
-		allowed_parent1 = allowed_parent1 ||
-				  landlock_unmask_layers(
-					  rule, access_masked_parent1,
-					  layer_masks_parent1,
-					  ARRAY_SIZE(*layer_masks_parent1));
-		allowed_parent2 = allowed_parent2 ||
-				  landlock_unmask_layers(
-					  rule, access_masked_parent2,
-					  layer_masks_parent2,
-					  ARRAY_SIZE(*layer_masks_parent2));
+		scoped_guard(rcu)
+		{
+			struct landlock_rule_ref ref = {
+				.type = LANDLOCK_KEY_INODE,
+			};
+
+			if (find_rule_ref(walker_path.dentry, &ref)) {
+				allowed_parent1 =
+					allowed_parent1 ||
+					landlock_unmask_layers(
+						domain, ref,
+						access_masked_parent1,
+						layer_masks_parent1,
+						ARRAY_SIZE(
+							*layer_masks_parent1));
+
+				allowed_parent2 =
+					allowed_parent2 ||
+					landlock_unmask_layers(
+						domain, ref,
+						access_masked_parent2,
+						layer_masks_parent2,
+						ARRAY_SIZE(
+							*layer_masks_parent2));
+			}
+		}
 
 		/* Stops when a rule from each layer grants access. */
 		if (allowed_parent1 && allowed_parent2)
@@ -1050,15 +1072,23 @@ static bool collect_domain_accesses(
 		struct dentry *parent_dentry;
 
 		/* Gets all layers allowing all domain accesses. */
-		if (landlock_unmask_layers(find_rule(domain, dir), access_dom,
-					   layer_masks_dom,
-					   ARRAY_SIZE(*layer_masks_dom))) {
-			/*
-			 * Stops when all handled accesses are allowed by at
-			 * least one rule in each layer.
-			 */
-			ret = true;
-			break;
+		scoped_guard(rcu)
+		{
+			struct landlock_rule_ref ref = {
+				.type = LANDLOCK_KEY_INODE,
+			};
+
+			if (find_rule_ref(dir, &ref) &&
+			    landlock_unmask_layers(
+				    domain, ref, access_dom, layer_masks_dom,
+				    ARRAY_SIZE(*layer_masks_dom))) {
+				/*
+				* Stops when all handled accesses are allowed by at
+				* least one rule in each layer.
+				*/
+				ret = true;
+				break;
+			}
 		}
 
 		/* We should not reach a root other than @mnt_root. */
diff --git a/security/landlock/net.c b/security/landlock/net.c
index cd7327b5f43e..44489760e8ec 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -48,7 +48,6 @@ static int current_check_access_socket(struct socket *const sock,
 {
 	__be16 port;
 	layer_mask_t layer_masks[LANDLOCK_NUM_ACCESS_NET] = {};
-	const struct landlock_rule *rule;
 	struct landlock_rule_ref ref = {
 		.type = LANDLOCK_KEY_NET_PORT,
 	};
@@ -174,12 +173,11 @@ static int current_check_access_socket(struct socket *const sock,
 	ref.key.data = (__force uintptr_t)port;
 	BUILD_BUG_ON(sizeof(port) > sizeof(ref.key.data));
 
-	rule = landlock_find_rule(subject->domain, ref);
 	access_request = landlock_init_layer_masks(subject->domain,
 						   access_request, &layer_masks,
 						   LANDLOCK_KEY_NET_PORT);
-	if (landlock_unmask_layers(rule, access_request, &layer_masks,
-				   ARRAY_SIZE(layer_masks)))
+	if (landlock_unmask_layers(subject->domain, ref, access_request,
+				   &layer_masks, ARRAY_SIZE(layer_masks)))
 		return 0;
 
 	audit_net.family = address->sa_family;
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 647ee570475b..20a4bbb2526f 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -579,9 +579,9 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
 /*
  * The returned access has the same lifetime as @ruleset.
  */
-const struct landlock_rule *
-landlock_find_rule(const struct landlock_ruleset *const ruleset,
-		   const struct landlock_rule_ref ref)
+static const struct landlock_rule *
+find_rule(const struct landlock_ruleset *const ruleset,
+	  const struct landlock_rule_ref ref)
 {
 	const struct rb_root *root;
 	const struct rb_node *node;
@@ -613,15 +613,19 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
  * Returns true if the request is allowed (i.e. relevant layer masks for the
  * request are empty).
  */
-bool landlock_unmask_layers(const struct landlock_rule *const rule,
+bool landlock_unmask_layers(const struct landlock_ruleset *const domain,
+			    const struct landlock_rule_ref ref,
 			    const access_mask_t access_request,
 			    layer_mask_t (*const layer_masks)[],
 			    const size_t masks_array_size)
 {
 	size_t layer_level;
+	const struct landlock_rule *rule;
 
 	if (!access_request || !layer_masks)
 		return true;
+
+	rule = find_rule(domain, ref);
 	if (!rule)
 		return false;
 
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 967d0123cb73..9f25dbd3022a 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -44,7 +44,7 @@ union landlock_key {
 	/**
 	 * @object: Pointer to identify a kernel object (e.g. an inode).
 	 */
-	struct landlock_object *object;
+	struct landlock_object __rcu *object;
 	/**
 	 * @data: Raw data to identify an arbitrary 32-bit value
 	 * (e.g. a TCP port).
@@ -208,10 +208,6 @@ struct landlock_ruleset *
 landlock_merge_ruleset(struct landlock_ruleset *const parent,
 		       struct landlock_ruleset *const ruleset);
 
-const struct landlock_rule *
-landlock_find_rule(const struct landlock_ruleset *const ruleset,
-		   const struct landlock_rule_ref ref);
-
 static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
 {
 	if (ruleset)
@@ -301,7 +297,8 @@ landlock_get_scope_mask(const struct landlock_ruleset *const ruleset,
 	return ruleset->access_masks[layer_level].scope;
 }
 
-bool landlock_unmask_layers(const struct landlock_rule *const rule,
+bool landlock_unmask_layers(const struct landlock_ruleset *const domain,
+			    const struct landlock_rule_ref ref,
 			    const access_mask_t access_request,
 			    layer_mask_t (*const layer_masks)[],
 			    const size_t masks_array_size);
-- 
2.49.0




More information about the Linux-security-module-archive mailing list