[RFC PATCH v1 0/5] Landlock tracepoints

[RFC PATCH v1 0/5] Landlock tracepoints

[Mickaël Salaün]

Hi,

This series adds two tracepoints to Landlock, one tied to rule addition,
and another to rule checking.  With these new tracepoints, we can see
all steps leading to an access decision.  They can be directly used with
/sys/kernel/tracing/events/landlock/* or attached by eBPF programs to
get a more complete view of Landlock internals.

This new feature is useful to trouble shoot policy issues, and it should
also limit the need for custom debugging kernel code when developing new
Landlock features.

Landlock already has audit support, which enables us to log denied
access requests.  Audit is useful to identify security issues or sandbox
misconfiguration.  However, it might not be enough to debug Landlock
policies.  The main differences with audit events is that traces are
disabled by default, can be very verbose, and can be filtered according
to process and Landlock properties (e.g. domain ID).

As for audit, this tracing feature may expose sensitive information and
must then only be accessible to the system administrator.

This RFC only fully supports filesystem rules but the next series will
also support network rules.  Tests are also missing for now.

Regards,

Mickaël Salaün (5):
  landlock: Rename landlock_id to landlock_rule_ref
  landlock: Merge landlock_find_rule() into landlock_unmask_layers()
  tracing: Add __print_untrusted_str()
  landlock: Add landlock_add_rule_fs tracepoint
  landlock: Add landlock_check_rule tracepoint

 MAINTAINERS                                |   1 +
 include/linux/trace_events.h               |   3 +
 include/trace/events/landlock.h            | 124 ++++++++++++++
 include/trace/stages/stage3_trace_output.h |   4 +
 include/trace/stages/stage7_class_define.h |   1 +
 kernel/trace/trace_output.c                |  40 +++++
 security/landlock/Makefile                 |  11 +-
 security/landlock/fs.c                     | 178 +++++++++++++--------
 security/landlock/fs.h                     |   3 +
 security/landlock/net.c                    |  18 +--
 security/landlock/ruleset.c                |  65 ++++----
 security/landlock/ruleset.h                |  15 +-
 security/landlock/trace.c                  |  15 ++
 13 files changed, 365 insertions(+), 113 deletions(-)
 create mode 100644 include/trace/events/landlock.h
 create mode 100644 security/landlock/trace.c


base-commit: a5806cd506af5a7c19bcd596e4708b5c464bfd21
-- 
2.49.0

[RFC PATCH v1 1/5] landlock: Rename landlock_id to landlock_rule_ref

[Mickaël Salaün]

This avoids confusion with the new Landlock IDs.

TODO: Split in several commits to ease potential backports according to
stable branches

Cc: Günther Noack <gnoack@google.com>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 security/landlock/fs.c      | 18 ++++++-------
 security/landlock/net.c     | 14 +++++-----
 security/landlock/ruleset.c | 52 ++++++++++++++++++-------------------
 security/landlock/ruleset.h |  8 +++---
 4 files changed, 46 insertions(+), 46 deletions(-)

diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 6fee7c20f64d..f5087688190a 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -325,7 +325,7 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 			    access_mask_t access_rights)
 {
 	int err;
-	struct landlock_id id = {
+	struct landlock_rule_ref ref = {
 		.type = LANDLOCK_KEY_INODE,
 	};
 
@@ -339,17 +339,17 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 	/* Transforms relative access rights to absolute ones. */
 	access_rights |= LANDLOCK_MASK_ACCESS_FS &
 			 ~landlock_get_fs_access_mask(ruleset, 0);
-	id.key.object = get_inode_object(d_backing_inode(path->dentry));
-	if (IS_ERR(id.key.object))
-		return PTR_ERR(id.key.object);
+	ref.key.object = get_inode_object(d_backing_inode(path->dentry));
+	if (IS_ERR(ref.key.object))
+		return PTR_ERR(ref.key.object);
 	mutex_lock(&ruleset->lock);
-	err = landlock_insert_rule(ruleset, id, access_rights);
+	err = landlock_insert_rule(ruleset, ref, access_rights);
 	mutex_unlock(&ruleset->lock);
 	/*
 	 * 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);
+	landlock_put_object(ref.key.object);
 	return err;
 }
 
@@ -366,7 +366,7 @@ find_rule(const struct landlock_ruleset *const domain,
 {
 	const struct landlock_rule *rule;
 	const struct inode *inode;
-	struct landlock_id id = {
+	struct landlock_rule_ref ref = {
 		.type = LANDLOCK_KEY_INODE,
 	};
 
@@ -376,8 +376,8 @@ find_rule(const struct landlock_ruleset *const domain,
 
 	inode = d_backing_inode(dentry);
 	rcu_read_lock();
-	id.key.object = rcu_dereference(landlock_inode(inode)->object);
-	rule = landlock_find_rule(domain, id);
+	ref.key.object = rcu_dereference(landlock_inode(inode)->object);
+	rule = landlock_find_rule(domain, ref);
 	rcu_read_unlock();
 	return rule;
 }
diff --git a/security/landlock/net.c b/security/landlock/net.c
index 1f3915a90a80..cd7327b5f43e 100644
--- a/security/landlock/net.c
+++ b/security/landlock/net.c
@@ -23,19 +23,19 @@ int landlock_append_net_rule(struct landlock_ruleset *const ruleset,
 			     const u16 port, access_mask_t access_rights)
 {
 	int err;
-	const struct landlock_id id = {
+	const struct landlock_rule_ref ref = {
 		.key.data = (__force uintptr_t)htons(port),
 		.type = LANDLOCK_KEY_NET_PORT,
 	};
 
-	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
+	BUILD_BUG_ON(sizeof(port) > sizeof(ref.key.data));
 
 	/* Transforms relative access rights to absolute ones. */
 	access_rights |= LANDLOCK_MASK_ACCESS_NET &
 			 ~landlock_get_net_access_mask(ruleset, 0);
 
 	mutex_lock(&ruleset->lock);
-	err = landlock_insert_rule(ruleset, id, access_rights);
+	err = landlock_insert_rule(ruleset, ref, access_rights);
 	mutex_unlock(&ruleset->lock);
 
 	return err;
@@ -49,7 +49,7 @@ 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_id id = {
+	struct landlock_rule_ref ref = {
 		.type = LANDLOCK_KEY_NET_PORT,
 	};
 	const struct access_masks masks = {
@@ -171,10 +171,10 @@ static int current_check_access_socket(struct socket *const sock,
 			return -EINVAL;
 	}
 
-	id.key.data = (__force uintptr_t)port;
-	BUILD_BUG_ON(sizeof(port) > sizeof(id.key.data));
+	ref.key.data = (__force uintptr_t)port;
+	BUILD_BUG_ON(sizeof(port) > sizeof(ref.key.data));
 
-	rule = landlock_find_rule(subject->domain, id);
+	rule = landlock_find_rule(subject->domain, ref);
 	access_request = landlock_init_layer_masks(subject->domain,
 						   access_request, &layer_masks,
 						   LANDLOCK_KEY_NET_PORT);
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index ce7940efea51..647ee570475b 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -104,7 +104,7 @@ static bool is_object_pointer(const enum landlock_key_type key_type)
 }
 
 static struct landlock_rule *
-create_rule(const struct landlock_id id,
+create_rule(const struct landlock_rule_ref ref,
 	    const struct landlock_layer (*const layers)[], const u32 num_layers,
 	    const struct landlock_layer *const new_layer)
 {
@@ -125,13 +125,13 @@ create_rule(const struct landlock_id id,
 	if (!new_rule)
 		return ERR_PTR(-ENOMEM);
 	RB_CLEAR_NODE(&new_rule->node);
-	if (is_object_pointer(id.type)) {
+	if (is_object_pointer(ref.type)) {
 		/* This should have been caught by insert_rule(). */
-		WARN_ON_ONCE(!id.key.object);
-		landlock_get_object(id.key.object);
+		WARN_ON_ONCE(!ref.key.object);
+		landlock_get_object(ref.key.object);
 	}
 
-	new_rule->key = id.key;
+	new_rule->key = ref.key;
 	new_rule->num_layers = new_num_layers;
 	/* Copies the original layer stack. */
 	memcpy(new_rule->layers, layers,
@@ -186,8 +186,8 @@ static void build_check_ruleset(void)
  * insert_rule - Create and insert a rule in a ruleset
  *
  * @ruleset: The ruleset to be updated.
- * @id: The ID to build the new rule with.  The underlying kernel object, if
- *      any, must be held by the caller.
+ * @ref: The reference to build the new rule with.  The underlying kernel
+ *       object, if any, 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.
  *
@@ -201,7 +201,7 @@ static void build_check_ruleset(void)
  * access rights.
  */
 static int insert_rule(struct landlock_ruleset *const ruleset,
-		       const struct landlock_id id,
+		       const struct landlock_rule_ref ref,
 		       const struct landlock_layer (*const layers)[],
 		       const size_t num_layers)
 {
@@ -215,10 +215,10 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
 	if (WARN_ON_ONCE(!layers))
 		return -ENOENT;
 
-	if (is_object_pointer(id.type) && WARN_ON_ONCE(!id.key.object))
+	if (is_object_pointer(ref.type) && WARN_ON_ONCE(!ref.key.object))
 		return -ENOENT;
 
-	root = get_root(ruleset, id.type);
+	root = get_root(ruleset, ref.type);
 	if (IS_ERR(root))
 		return PTR_ERR(root);
 
@@ -227,9 +227,9 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
 		struct landlock_rule *const this =
 			rb_entry(*walker_node, struct landlock_rule, node);
 
-		if (this->key.data != id.key.data) {
+		if (this->key.data != ref.key.data) {
 			parent_node = *walker_node;
-			if (this->key.data < id.key.data)
+			if (this->key.data < ref.key.data)
 				walker_node = &((*walker_node)->rb_right);
 			else
 				walker_node = &((*walker_node)->rb_left);
@@ -261,20 +261,20 @@ static int insert_rule(struct landlock_ruleset *const ruleset,
 		 * Intersects access rights when it is a merge between a
 		 * ruleset and a domain.
 		 */
-		new_rule = create_rule(id, &this->layers, this->num_layers,
+		new_rule = create_rule(ref, &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, root);
-		free_rule(this, id.type);
+		free_rule(this, ref.type);
 		return 0;
 	}
 
-	/* There is no match for @id. */
+	/* There is no match for @ref. */
 	build_check_ruleset();
 	if (ruleset->num_rules >= LANDLOCK_MAX_NUM_RULES)
 		return -E2BIG;
-	new_rule = create_rule(id, layers, num_layers, NULL);
+	new_rule = create_rule(ref, layers, num_layers, NULL);
 	if (IS_ERR(new_rule))
 		return PTR_ERR(new_rule);
 	rb_link_node(&new_rule->node, parent_node, walker_node);
@@ -296,7 +296,7 @@ static void build_check_layer(void)
 
 /* @ruleset must be locked by the caller. */
 int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-			 const struct landlock_id id,
+			 const struct landlock_rule_ref ref,
 			 const access_mask_t access)
 {
 	struct landlock_layer layers[] = { {
@@ -306,7 +306,7 @@ int landlock_insert_rule(struct landlock_ruleset *const ruleset,
 	} };
 
 	build_check_layer();
-	return insert_rule(ruleset, id, &layers, ARRAY_SIZE(layers));
+	return insert_rule(ruleset, ref, &layers, ARRAY_SIZE(layers));
 }
 
 static int merge_tree(struct landlock_ruleset *const dst,
@@ -331,7 +331,7 @@ static int merge_tree(struct landlock_ruleset *const dst,
 		struct landlock_layer layers[] = { {
 			.level = dst->num_layers,
 		} };
-		const struct landlock_id id = {
+		const struct landlock_rule_ref ref = {
 			.key = walker_rule->key,
 			.type = key_type,
 		};
@@ -344,7 +344,7 @@ static int merge_tree(struct landlock_ruleset *const dst,
 
 		layers[0].access = walker_rule->layers[0].access;
 
-		err = insert_rule(dst, id, &layers, ARRAY_SIZE(layers));
+		err = insert_rule(dst, ref, &layers, ARRAY_SIZE(layers));
 		if (err)
 			return err;
 	}
@@ -413,12 +413,12 @@ static int inherit_tree(struct landlock_ruleset *const parent,
 	/* Copies the @parent inode or network tree. */
 	rbtree_postorder_for_each_entry_safe(walker_rule, next_rule,
 					     parent_root, node) {
-		const struct landlock_id id = {
+		const struct landlock_rule_ref ref = {
 			.key = walker_rule->key,
 			.type = key_type,
 		};
 
-		err = insert_rule(child, id, &walker_rule->layers,
+		err = insert_rule(child, ref, &walker_rule->layers,
 				  walker_rule->num_layers);
 		if (err)
 			return err;
@@ -581,12 +581,12 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
  */
 const struct landlock_rule *
 landlock_find_rule(const struct landlock_ruleset *const ruleset,
-		   const struct landlock_id id)
+		   const struct landlock_rule_ref ref)
 {
 	const struct rb_root *root;
 	const struct rb_node *node;
 
-	root = get_root((struct landlock_ruleset *)ruleset, id.type);
+	root = get_root((struct landlock_ruleset *)ruleset, ref.type);
 	if (IS_ERR(root))
 		return NULL;
 	node = root->rb_node;
@@ -595,9 +595,9 @@ landlock_find_rule(const struct landlock_ruleset *const ruleset,
 		struct landlock_rule *this =
 			rb_entry(node, struct landlock_rule, node);
 
-		if (this->key.data == id.key.data)
+		if (this->key.data == ref.key.data)
 			return this;
-		if (this->key.data < id.key.data)
+		if (this->key.data < ref.key.data)
 			node = node->rb_right;
 		else
 			node = node->rb_left;
diff --git a/security/landlock/ruleset.h b/security/landlock/ruleset.h
index 5da9a64f5af7..967d0123cb73 100644
--- a/security/landlock/ruleset.h
+++ b/security/landlock/ruleset.h
@@ -69,9 +69,9 @@ enum landlock_key_type {
 };
 
 /**
- * struct landlock_id - Unique rule identifier for a ruleset
+ * struct landlock_rule_ref - Rule reference for a ruleset
  */
-struct landlock_id {
+struct landlock_rule_ref {
 	/**
 	 * @key: Identifies either a kernel object (e.g. an inode) or
 	 * a raw value (e.g. a TCP port).
@@ -201,7 +201,7 @@ DEFINE_FREE(landlock_put_ruleset, struct landlock_ruleset *,
 	    if (!IS_ERR_OR_NULL(_T)) landlock_put_ruleset(_T))
 
 int landlock_insert_rule(struct landlock_ruleset *const ruleset,
-			 const struct landlock_id id,
+			 const struct landlock_rule_ref ref,
 			 const access_mask_t access);
 
 struct landlock_ruleset *
@@ -210,7 +210,7 @@ landlock_merge_ruleset(struct landlock_ruleset *const parent,
 
 const struct landlock_rule *
 landlock_find_rule(const struct landlock_ruleset *const ruleset,
-		   const struct landlock_id id);
+		   const struct landlock_rule_ref ref);
 
 static inline void landlock_get_ruleset(struct landlock_ruleset *const ruleset)
 {
-- 
2.49.0

Re: [RFC PATCH v1 1/5] landlock: Rename landlock_id to landlock_rule_ref

[Tingmao Wang]

On 5/23/25 17:57, Mickaël Salaün wrote:
> [RFC PATCH v1 1/5] landlock: Rename landlock_id to landlock_rule_ref
> 
> This avoids confusion with the new Landlock IDs.

A very very minor suggestion, but I think to someone new, 
landlock_rule_ref would sound like a reference to a specific rule (like 
a *struct landlock_rule), but really it represents the "name", or in 
fact, target of a rule... Maybe we should call it "landlock_rule_target"?

(Or maybe the confusion is resolved quickly when they look at the 
definition so maybe it doesn't matter)

> 
> TODO: Split in several commits to ease potential backports according to
> stable branches
> 
> Cc: Günther Noack <gnoack@google.com>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
[...]

Re: [RFC PATCH v1 1/5] landlock: Rename landlock_id to landlock_rule_ref

[Mickaël Salaün]

On Mon, May 26, 2025 at 07:38:00PM +0100, Tingmao Wang wrote:
> On 5/23/25 17:57, Mickaël Salaün wrote:
> > [RFC PATCH v1 1/5] landlock: Rename landlock_id to landlock_rule_ref
> > 
> > This avoids confusion with the new Landlock IDs.
> 
> A very very minor suggestion, but I think to someone new, landlock_rule_ref
> would sound like a reference to a specific rule (like a *struct
> landlock_rule), but really it represents the "name", or in fact, target of a
> rule... Maybe we should call it "landlock_rule_target"?
> 
> (Or maybe the confusion is resolved quickly when they look at the definition
> so maybe it doesn't matter)

You're right that the name is confusing.  What about just struct
landlock_reference?  Such structure do reference an element (an object
or a raw value), which might be in a ruleset, a domain, or none of them.

We should also probably use "ref=" instead of "object=" in the trace
event.

> 
> > 
> > TODO: Split in several commits to ease potential backports according to
> > stable branches
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> [...]
> 

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

[Mickaël Salaün]

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@google.com>
Signed-off-by: Mickaël Salaün <mic@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

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

[Tingmao Wang]

On 5/23/25 17:57, Mickaël Salaün wrote:
> 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@google.com>
> Signed-off-by: Mickaël Salaün <mic@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)

I think a better name would be something like "get_rule_ref"? Since it's 
not really _finding_ anything (like doing a search in a rbtree).

(If you take the rename suggestion, then it would be "get_rule_target")

> [...]

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

[Mickaël Salaün]

On Mon, May 26, 2025 at 07:38:07PM +0100, Tingmao Wang wrote:
> On 5/23/25 17:57, Mickaël Salaün wrote:
> > 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@google.com>
> > Signed-off-by: Mickaël Salaün <mic@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)
> 
> I think a better name would be something like "get_rule_ref"? Since it's not
> really _finding_ anything (like doing a search in a rbtree).

Correct.

> 
> (If you take the rename suggestion, then it would be "get_rule_target")

What about get_inode_ref()?

[RFC PATCH v1 3/5] tracing: Add __print_untrusted_str()

[Mickaël Salaün]

Add a new __print_untrusted_str() helper to safely print strings after escaping
all special characters, including common separators (space, equal sign),
quotes, and backslashes.  This transforms a string from an untrusted source
(e.g. user space) to make it:
- safe to parse,
- easy to read (for simple strings),
- easy to get back the original.

Cc: Günther Noack <gnoack@google.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 include/linux/trace_events.h               |  3 ++
 include/trace/stages/stage3_trace_output.h |  4 +++
 include/trace/stages/stage7_class_define.h |  1 +
 kernel/trace/trace_output.c                | 40 ++++++++++++++++++++++
 4 files changed, 48 insertions(+)

diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
index fa9cf4292dff..78f543bb7558 100644
--- a/include/linux/trace_events.h
+++ b/include/linux/trace_events.h
@@ -54,6 +54,9 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str,
 			 int prefix_type, int rowsize, int groupsize,
 			 const void *buf, size_t len, bool ascii);
 
+const char *trace_print_untrusted_str_seq(struct trace_seq *s, const char *str);
+
+
 struct trace_iterator;
 struct trace_event;
 
diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
index 1e7b0bef95f5..36947ca2abcb 100644
--- a/include/trace/stages/stage3_trace_output.h
+++ b/include/trace/stages/stage3_trace_output.h
@@ -133,6 +133,10 @@
 	trace_print_hex_dump_seq(p, prefix_str, prefix_type,		\
 				 rowsize, groupsize, buf, len, ascii)
 
+#undef __print_untrusted_str
+#define __print_untrusted_str(str)							\
+		trace_print_untrusted_str_seq(p, __get_str(str))
+
 #undef __print_ns_to_secs
 #define __print_ns_to_secs(value)			\
 	({						\
diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
index fcd564a590f4..bc10b69b755d 100644
--- a/include/trace/stages/stage7_class_define.h
+++ b/include/trace/stages/stage7_class_define.h
@@ -24,6 +24,7 @@
 #undef __print_array
 #undef __print_dynamic_array
 #undef __print_hex_dump
+#undef __print_untrusted_string
 #undef __get_buf
 
 /*
diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
index b9ab06c99543..17d576941147 100644
--- a/kernel/trace/trace_output.c
+++ b/kernel/trace/trace_output.c
@@ -16,6 +16,7 @@
 #include <linux/btf.h>
 #include <linux/bpf.h>
 #include <linux/hashtable.h>
+#include <linux/string_helpers.h>
 
 #include "trace_output.h"
 #include "trace_btf.h"
@@ -297,6 +298,45 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str,
 }
 EXPORT_SYMBOL(trace_print_hex_dump_seq);
 
+/**
+ * trace_print_untrusted_str_seq - print a string after escaping characters
+ * @s: trace seq struct to write to
+ * @src: The string to print
+ *
+ * Prints a string to a trace seq after escaping all special characters,
+ * including common separators (space, equal sign), quotes, and backslashes.
+ * This transforms a string from an untrusted source (e.g. user space) to make
+ * it:
+ * - safe to parse,
+ * - easy to read (for simple strings),
+ * - easy to get back the original.
+ */
+const char *trace_print_untrusted_str_seq(struct trace_seq *s,
+                                          const char *src)
+{
+	int escaped_size;
+	char *buf;
+	size_t buf_size = seq_buf_get_buf(&s->seq, &buf);
+	const char *ret = trace_seq_buffer_ptr(s);
+
+	if (!src || WARN_ON(buf_size == 0))
+		return NULL;
+
+	escaped_size = string_escape_mem(src, strlen(src), buf, buf_size,
+		ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_NAP | ESCAPE_APPEND |
+		ESCAPE_OCTAL, " ='\"\\");
+	if (unlikely(escaped_size >= buf_size)) {
+		/* We need some room for the final '\0'. */
+		seq_buf_set_overflow(&s->seq);
+		s->full = 1;
+		return NULL;
+	}
+	seq_buf_commit(&s->seq, escaped_size);
+	trace_seq_putc(s, 0);
+	return ret;
+}
+EXPORT_SYMBOL(trace_print_untrusted_str_seq);
+
 int trace_raw_output_prep(struct trace_iterator *iter,
 			  struct trace_event *trace_event)
 {
-- 
2.49.0

Re: [RFC PATCH v1 3/5] tracing: Add __print_untrusted_str()

[Steven Rostedt]

On Fri, 23 May 2025 18:57:39 +0200
Mickaël Salaün <mic@digikod.net> wrote:

> Add a new __print_untrusted_str() helper to safely print strings after escaping
> all special characters, including common separators (space, equal sign),
> quotes, and backslashes.  This transforms a string from an untrusted source
> (e.g. user space) to make it:
> - safe to parse,
> - easy to read (for simple strings),
> - easy to get back the original.

Hmm, so this can be an issue if this is printed out via seq_file()?

I'm curious to what exactly can be "unsafe" about a string being printed
via "%s"?

I'm not against this change, I just want to understand more about what the
issue is.

> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>  include/linux/trace_events.h               |  3 ++
>  include/trace/stages/stage3_trace_output.h |  4 +++
>  include/trace/stages/stage7_class_define.h |  1 +
>  kernel/trace/trace_output.c                | 40 ++++++++++++++++++++++
>  4 files changed, 48 insertions(+)
> 
> diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> index fa9cf4292dff..78f543bb7558 100644
> --- a/include/linux/trace_events.h
> +++ b/include/linux/trace_events.h
> @@ -54,6 +54,9 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str,
>  			 int prefix_type, int rowsize, int groupsize,
>  			 const void *buf, size_t len, bool ascii);
>  
> +const char *trace_print_untrusted_str_seq(struct trace_seq *s, const char *str);
> +
> +
>  struct trace_iterator;
>  struct trace_event;
>  
> diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
> index 1e7b0bef95f5..36947ca2abcb 100644
> --- a/include/trace/stages/stage3_trace_output.h
> +++ b/include/trace/stages/stage3_trace_output.h
> @@ -133,6 +133,10 @@
>  	trace_print_hex_dump_seq(p, prefix_str, prefix_type,		\
>  				 rowsize, groupsize, buf, len, ascii)
>  
> +#undef __print_untrusted_str
> +#define __print_untrusted_str(str)							\
> +		trace_print_untrusted_str_seq(p, __get_str(str))
> +
>  #undef __print_ns_to_secs
>  #define __print_ns_to_secs(value)			\
>  	({						\
> diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
> index fcd564a590f4..bc10b69b755d 100644
> --- a/include/trace/stages/stage7_class_define.h
> +++ b/include/trace/stages/stage7_class_define.h
> @@ -24,6 +24,7 @@
>  #undef __print_array
>  #undef __print_dynamic_array
>  #undef __print_hex_dump
> +#undef __print_untrusted_string
>  #undef __get_buf
>  
>  /*
> diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> index b9ab06c99543..17d576941147 100644
> --- a/kernel/trace/trace_output.c
> +++ b/kernel/trace/trace_output.c
> @@ -16,6 +16,7 @@
>  #include <linux/btf.h>
>  #include <linux/bpf.h>
>  #include <linux/hashtable.h>
> +#include <linux/string_helpers.h>
>  
>  #include "trace_output.h"
>  #include "trace_btf.h"
> @@ -297,6 +298,45 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str,
>  }
>  EXPORT_SYMBOL(trace_print_hex_dump_seq);
>  
> +/**
> + * trace_print_untrusted_str_seq - print a string after escaping characters
> + * @s: trace seq struct to write to
> + * @src: The string to print
> + *
> + * Prints a string to a trace seq after escaping all special characters,
> + * including common separators (space, equal sign), quotes, and backslashes.
> + * This transforms a string from an untrusted source (e.g. user space) to make
> + * it:
> + * - safe to parse,
> + * - easy to read (for simple strings),
> + * - easy to get back the original.
> + */
> +const char *trace_print_untrusted_str_seq(struct trace_seq *s,
> +                                          const char *src)
> +{
> +	int escaped_size;
> +	char *buf;
> +	size_t buf_size = seq_buf_get_buf(&s->seq, &buf);
> +	const char *ret = trace_seq_buffer_ptr(s);
> +
> +	if (!src || WARN_ON(buf_size == 0))

		    WARN_ON_ONCE() please.

-- Steve

> +		return NULL;
> +
> +	escaped_size = string_escape_mem(src, strlen(src), buf, buf_size,
> +		ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_NAP | ESCAPE_APPEND |
> +		ESCAPE_OCTAL, " ='\"\\");
> +	if (unlikely(escaped_size >= buf_size)) {
> +		/* We need some room for the final '\0'. */
> +		seq_buf_set_overflow(&s->seq);
> +		s->full = 1;
> +		return NULL;
> +	}
> +	seq_buf_commit(&s->seq, escaped_size);
> +	trace_seq_putc(s, 0);
> +	return ret;
> +}
> +EXPORT_SYMBOL(trace_print_untrusted_str_seq);
> +
>  int trace_raw_output_prep(struct trace_iterator *iter,
>  			  struct trace_event *trace_event)
>  {

Re: [RFC PATCH v1 3/5] tracing: Add __print_untrusted_str()

[Mickaël Salaün]

On Fri, May 23, 2025 at 02:22:42PM -0400, Steven Rostedt wrote:
> On Fri, 23 May 2025 18:57:39 +0200
> Mickaël Salaün <mic@digikod.net> wrote:
> 
> > Add a new __print_untrusted_str() helper to safely print strings after escaping
> > all special characters, including common separators (space, equal sign),
> > quotes, and backslashes.  This transforms a string from an untrusted source
> > (e.g. user space) to make it:
> > - safe to parse,
> > - easy to read (for simple strings),
> > - easy to get back the original.
> 
> Hmm, so this can be an issue if this is printed out via seq_file()?
> 
> I'm curious to what exactly can be "unsafe" about a string being printed
> via "%s"?

There is no issue for the kernel, only for users and user space. :)

> 
> I'm not against this change, I just want to understand more about what the
> issue is.

The issue is about a malicious process triggering a trace event with an
arbitrary string.  If such string is printed to the root's terminal, it
can print escape sequences and do nasty things.  For instance, the
terminal can beep, the window's title can be updated, a path name can be
"hidden" with specific colors, the screen can be completely cleared and
rewritten to trick peoples, and other "original" terminal features can
be triggered by custom escape sequences.

This is definitely not something new but still relevant.  There are a
lot of articles about this kind of issues:
https://phrack.org/issues/25/5
https://marc.info/?l=bugtraq&m=104612710031920
https://www.cyberark.com/resources/threat-research-blog/dont-trust-this-title-abusing-terminal-emulators-with-ansi-escape-characters
https://blog.trailofbits.com/2025/04/29/deceiving-users-with-ansi-terminal-codes-in-mcp/

In a less malicious environment, this helper would also be useful to
just sanitize arbitrary text.  For instance, because '=' and ' ' are
escaped, it's easy to write a key=value parser in shell (without bug),
or to say it another way, it's more difficult for a parser to fail. ;)

Anyway, this sanitization should not be visible in most cases.

> 
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >  include/linux/trace_events.h               |  3 ++
> >  include/trace/stages/stage3_trace_output.h |  4 +++
> >  include/trace/stages/stage7_class_define.h |  1 +
> >  kernel/trace/trace_output.c                | 40 ++++++++++++++++++++++
> >  4 files changed, 48 insertions(+)
> > 
> > diff --git a/include/linux/trace_events.h b/include/linux/trace_events.h
> > index fa9cf4292dff..78f543bb7558 100644
> > --- a/include/linux/trace_events.h
> > +++ b/include/linux/trace_events.h
> > @@ -54,6 +54,9 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str,
> >  			 int prefix_type, int rowsize, int groupsize,
> >  			 const void *buf, size_t len, bool ascii);
> >  
> > +const char *trace_print_untrusted_str_seq(struct trace_seq *s, const char *str);
> > +
> > +
> >  struct trace_iterator;
> >  struct trace_event;
> >  
> > diff --git a/include/trace/stages/stage3_trace_output.h b/include/trace/stages/stage3_trace_output.h
> > index 1e7b0bef95f5..36947ca2abcb 100644
> > --- a/include/trace/stages/stage3_trace_output.h
> > +++ b/include/trace/stages/stage3_trace_output.h
> > @@ -133,6 +133,10 @@
> >  	trace_print_hex_dump_seq(p, prefix_str, prefix_type,		\
> >  				 rowsize, groupsize, buf, len, ascii)
> >  
> > +#undef __print_untrusted_str
> > +#define __print_untrusted_str(str)							\
> > +		trace_print_untrusted_str_seq(p, __get_str(str))
> > +
> >  #undef __print_ns_to_secs
> >  #define __print_ns_to_secs(value)			\
> >  	({						\
> > diff --git a/include/trace/stages/stage7_class_define.h b/include/trace/stages/stage7_class_define.h
> > index fcd564a590f4..bc10b69b755d 100644
> > --- a/include/trace/stages/stage7_class_define.h
> > +++ b/include/trace/stages/stage7_class_define.h
> > @@ -24,6 +24,7 @@
> >  #undef __print_array
> >  #undef __print_dynamic_array
> >  #undef __print_hex_dump
> > +#undef __print_untrusted_string
> >  #undef __get_buf
> >  
> >  /*
> > diff --git a/kernel/trace/trace_output.c b/kernel/trace/trace_output.c
> > index b9ab06c99543..17d576941147 100644
> > --- a/kernel/trace/trace_output.c
> > +++ b/kernel/trace/trace_output.c
> > @@ -16,6 +16,7 @@
> >  #include <linux/btf.h>
> >  #include <linux/bpf.h>
> >  #include <linux/hashtable.h>
> > +#include <linux/string_helpers.h>
> >  
> >  #include "trace_output.h"
> >  #include "trace_btf.h"
> > @@ -297,6 +298,45 @@ trace_print_hex_dump_seq(struct trace_seq *p, const char *prefix_str,
> >  }
> >  EXPORT_SYMBOL(trace_print_hex_dump_seq);
> >  
> > +/**
> > + * trace_print_untrusted_str_seq - print a string after escaping characters
> > + * @s: trace seq struct to write to
> > + * @src: The string to print
> > + *
> > + * Prints a string to a trace seq after escaping all special characters,
> > + * including common separators (space, equal sign), quotes, and backslashes.
> > + * This transforms a string from an untrusted source (e.g. user space) to make
> > + * it:
> > + * - safe to parse,
> > + * - easy to read (for simple strings),
> > + * - easy to get back the original.
> > + */
> > +const char *trace_print_untrusted_str_seq(struct trace_seq *s,
> > +                                          const char *src)
> > +{
> > +	int escaped_size;
> > +	char *buf;
> > +	size_t buf_size = seq_buf_get_buf(&s->seq, &buf);
> > +	const char *ret = trace_seq_buffer_ptr(s);
> > +
> > +	if (!src || WARN_ON(buf_size == 0))
> 
> 		    WARN_ON_ONCE() please.

I mimicked nearby code but WARN_ON_ONCE() is indeed better.

Thanks.

> 
> -- Steve
> 
> > +		return NULL;
> > +
> > +	escaped_size = string_escape_mem(src, strlen(src), buf, buf_size,
> > +		ESCAPE_SPACE | ESCAPE_SPECIAL | ESCAPE_NAP | ESCAPE_APPEND |
> > +		ESCAPE_OCTAL, " ='\"\\");
> > +	if (unlikely(escaped_size >= buf_size)) {
> > +		/* We need some room for the final '\0'. */
> > +		seq_buf_set_overflow(&s->seq);
> > +		s->full = 1;
> > +		return NULL;
> > +	}
> > +	seq_buf_commit(&s->seq, escaped_size);
> > +	trace_seq_putc(s, 0);
> > +	return ret;
> > +}
> > +EXPORT_SYMBOL(trace_print_untrusted_str_seq);
> > +
> >  int trace_raw_output_prep(struct trace_iterator *iter,
> >  			  struct trace_event *trace_event)
> >  {
> 
> 

[RFC PATCH v1 4/5] landlock: Add landlock_add_rule_fs tracepoint

[Mickaël Salaün]

Add a tracepoint for Landlock path_beneath rule addition.  This is
useful to tie a Landlock object with a file for debug purpose.

Allocate the absolute path names when adding new rules.  This is OK
because landlock_add_rule(2) is not a performance critical code.

Here is an example of landlock_add_rule_fs traces:
  ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59260 allowed=0xd dev=0:16 ino=306 path=/usr
  ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59240 allowed=0xffff dev=0:16 ino=346 path=/root

TODO: Use Landlock IDs instead of kernel addresses to identify Landlock
objects (e.g. inode).

Cc: Günther Noack <gnoack@google.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 MAINTAINERS                     |  1 +
 include/trace/events/landlock.h | 68 +++++++++++++++++++++++++++++++++
 security/landlock/Makefile      | 11 +++++-
 security/landlock/fs.c          | 22 +++++++++++
 security/landlock/fs.h          |  3 ++
 security/landlock/trace.c       | 14 +++++++
 6 files changed, 117 insertions(+), 2 deletions(-)
 create mode 100644 include/trace/events/landlock.h
 create mode 100644 security/landlock/trace.c

diff --git a/MAINTAINERS b/MAINTAINERS
index d48dd6726fe6..f75c21a935c1 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -13393,6 +13393,7 @@ F:	Documentation/admin-guide/LSM/landlock.rst
 F:	Documentation/security/landlock.rst
 F:	Documentation/userspace-api/landlock.rst
 F:	fs/ioctl.c
+F:	include/trace/events/landlock.h
 F:	include/uapi/linux/landlock.h
 F:	samples/landlock/
 F:	security/landlock/
diff --git a/include/trace/events/landlock.h b/include/trace/events/landlock.h
new file mode 100644
index 000000000000..41e10965ba7b
--- /dev/null
+++ b/include/trace/events/landlock.h
@@ -0,0 +1,68 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/*
+ * Copyright © 2025 Microsoft Corporation
+ */
+
+#undef TRACE_SYSTEM
+#define TRACE_SYSTEM landlock
+
+#if !defined(_TRACE_LANDLOCK_H) || defined(TRACE_HEADER_MULTI_READ)
+#define _TRACE_LANDLOCK_H
+
+#include <linux/tracepoint.h>
+
+struct landlock_rule_ref;
+struct landlock_ruleset;
+struct path;
+typedef u16 access_mask_t;
+
+TRACE_EVENT(landlock_add_rule_fs,
+
+	TP_PROTO(
+		const struct landlock_ruleset *ruleset,
+		const struct landlock_rule_ref *ref,
+		access_mask_t access_rights,
+		const struct path *path,
+		const char *pathname
+	),
+
+	TP_ARGS(ruleset, ref, access_rights, path, pathname),
+
+	TP_STRUCT__entry(
+		__field(const struct landlock_ruleset *, ruleset)
+		__field(uintptr_t, ref_key)
+		__field(access_mask_t, allowed)
+		__field(dev_t, dev)
+		__field(ino_t, ino)
+		__string(pathname, pathname)
+	),
+
+	TP_fast_assign(
+		__entry->ruleset = ruleset;
+		__entry->ref_key = ref->key.data;
+		__entry->allowed = access_rights;
+		__entry->dev = path->dentry->d_sb->s_dev;
+		__entry->ino = path->dentry->d_inode->i_ino;
+		__assign_str(pathname);
+	),
+
+	/*
+	 * The inode number may not be the user-visible one, but it will be the same
+	 * used by audit.
+	 */
+	TP_printk(
+		"ruleset=0x%p key=inode:0x%lx allowed=0x%x dev=%u:%u ino=%lu path=%s",
+		__entry->ruleset,
+		__entry->ref_key,
+		__entry->allowed,
+		MAJOR(__entry->dev),
+		MINOR(__entry->dev),
+		__entry->ino,
+		__print_untrusted_str(pathname)
+	)
+);
+
+#endif /* _TRACE_LANDLOCK_H */
+
+/* This part must be outside protection */
+#include <trace/define_trace.h>
diff --git a/security/landlock/Makefile b/security/landlock/Makefile
index 3160c2bdac1d..c19b406a6c67 100644
--- a/security/landlock/Makefile
+++ b/security/landlock/Makefile
@@ -1,7 +1,14 @@
 obj-$(CONFIG_SECURITY_LANDLOCK) := landlock.o
 
-landlock-y := setup.o syscalls.o object.o ruleset.o \
-	cred.o task.o fs.o
+landlock-y := \
+	setup.o \
+	syscalls.o \
+	object.o \
+	ruleset.o \
+	cred.o \
+	task.o \
+	fs.o \
+	trace.o
 
 landlock-$(CONFIG_INET) += net.o
 
diff --git a/security/landlock/fs.c b/security/landlock/fs.c
index 73a20a501c3c..e5d673240882 100644
--- a/security/landlock/fs.c
+++ b/security/landlock/fs.c
@@ -36,6 +36,7 @@
 #include <linux/types.h>
 #include <linux/wait_bit.h>
 #include <linux/workqueue.h>
+#include <trace/events/landlock.h>
 #include <uapi/linux/fiemap.h>
 #include <uapi/linux/landlock.h>
 
@@ -345,6 +346,27 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 	mutex_lock(&ruleset->lock);
 	err = landlock_insert_rule(ruleset, ref, access_rights);
 	mutex_unlock(&ruleset->lock);
+
+	if (!err && trace_landlock_add_rule_fs_enabled()) {
+		const char *pathname;
+		/* Does not handle deleted files. */
+		char *buffer __free(__putname) = __getname();
+
+		if (buffer) {
+			const char *absolute_path =
+				d_absolute_path(path, buffer, PATH_MAX);
+			if (!IS_ERR_OR_NULL(absolute_path))
+				pathname = absolute_path;
+			else
+				pathname = "<too_long>";
+		} else {
+			/* Same format as audit_log_d_path(). */
+			pathname = "<no_memory>";
+		}
+		trace_landlock_add_rule_fs(ruleset, &ref, access_rights, path,
+					   pathname);
+	}
+
 	/*
 	 * No need to check for an error because landlock_insert_rule()
 	 * increments the refcount for the new object if needed.
diff --git a/security/landlock/fs.h b/security/landlock/fs.h
index bf9948941f2f..60be95ebfb0b 100644
--- a/security/landlock/fs.h
+++ b/security/landlock/fs.h
@@ -11,6 +11,7 @@
 #define _SECURITY_LANDLOCK_FS_H
 
 #include <linux/build_bug.h>
+#include <linux/cleanup.h>
 #include <linux/fs.h>
 #include <linux/init.h>
 #include <linux/rcupdate.h>
@@ -128,4 +129,6 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
 			    const struct path *const path,
 			    access_mask_t access_hierarchy);
 
+DEFINE_FREE(__putname, char *, if (_T) __putname(_T))
+
 #endif /* _SECURITY_LANDLOCK_FS_H */
diff --git a/security/landlock/trace.c b/security/landlock/trace.c
new file mode 100644
index 000000000000..98874cda473b
--- /dev/null
+++ b/security/landlock/trace.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Landlock - Tracepoints
+ *
+ * Copyright © 2025 Microsoft Corporation
+ */
+
+#include <linux/path.h>
+
+#include "access.h"
+#include "ruleset.h"
+
+#define CREATE_TRACE_POINTS
+#include <trace/events/landlock.h>
-- 
2.49.0

Re: [RFC PATCH v1 4/5] landlock: Add landlock_add_rule_fs tracepoint

[Tingmao Wang]

On 5/23/25 17:57, Mickaël Salaün wrote:
> Add a tracepoint for Landlock path_beneath rule addition.  This is
> useful to tie a Landlock object with a file for debug purpose.
> 
> Allocate the absolute path names when adding new rules.  This is OK
> because landlock_add_rule(2) is not a performance critical code.
> 
> Here is an example of landlock_add_rule_fs traces:
>    ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59260 allowed=0xd dev=0:16 ino=306 path=/usr
>    ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59240 allowed=0xffff dev=0:16 ino=346 path=/root
> 
> TODO: Use Landlock IDs instead of kernel addresses to identify Landlock
> objects (e.g. inode).

Do you mean like the u64 domain ID for audit, but for objects?  Since 
there currently isn't a u64, non-pointer ID, I guess we would have to 
create one for the object just for tracing?

My understanding is that kernel pointers are not hidden from the root 
user / someone who can read traces, so maybe just printing the pointer 
is good enough?

> 
> Cc: Günther Noack <gnoack@google.com>
> Cc: Masami Hiramatsu <mhiramat@kernel.org>
> Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Tingmao Wang <m@maowtm.org>
> Signed-off-by: Mickaël Salaün <mic@digikod.net>
> ---
>   MAINTAINERS                     |  1 +
>   include/trace/events/landlock.h | 68 +++++++++++++++++++++++++++++++++
>   security/landlock/Makefile      | 11 +++++-
>   security/landlock/fs.c          | 22 +++++++++++
>   security/landlock/fs.h          |  3 ++
>   security/landlock/trace.c       | 14 +++++++
>   6 files changed, 117 insertions(+), 2 deletions(-)
>   create mode 100644 include/trace/events/landlock.h
>   create mode 100644 security/landlock/trace.c
> 
 > > [...]
 > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> index 73a20a501c3c..e5d673240882 100644
> --- a/security/landlock/fs.c
> +++ b/security/landlock/fs.c
> @@ -36,6 +36,7 @@
>   #include <linux/types.h>
>   #include <linux/wait_bit.h>
>   #include <linux/workqueue.h>
> +#include <trace/events/landlock.h>
>   #include <uapi/linux/fiemap.h>
>   #include <uapi/linux/landlock.h>
>   
> @@ -345,6 +346,27 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>   	mutex_lock(&ruleset->lock);
>   	err = landlock_insert_rule(ruleset, ref, access_rights);
>   	mutex_unlock(&ruleset->lock);
> +
> +	if (!err && trace_landlock_add_rule_fs_enabled()) {
> +		const char *pathname;
> +		/* Does not handle deleted files. */
> +		char *buffer __free(__putname) = __getname();
> +
> +		if (buffer) {
> +			const char *absolute_path =
> +				d_absolute_path(path, buffer, PATH_MAX);
> +			if (!IS_ERR_OR_NULL(absolute_path))
> +				pathname = absolute_path;
> +			else
> +				pathname = "<too_long>";

Not sure if it's necessary to go that far, but I think d_absolute_path 
returns -ENAMETOOLONG in the too long case, and -EINVAL in the "not 
possible to construct a path" case (I guess e.g. if it's an anonymous 
file or detached mount).  We could add an else if branch to check which 
case it is and use different strings.

> +		} else {
> +			/* Same format as audit_log_d_path(). */
> +			pathname = "<no_memory>";
> +		}
> +		trace_landlock_add_rule_fs(ruleset, &ref, access_rights, path,
> +					   pathname);
> +	}
> +
>   	/*
>   	 * No need to check for an error because landlock_insert_rule()
>   	 * increments the refcount for the new object if needed.
> diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> index bf9948941f2f..60be95ebfb0b 100644
> --- a/security/landlock/fs.h
> +++ b/security/landlock/fs.h
> @@ -11,6 +11,7 @@
>   #define _SECURITY_LANDLOCK_FS_H
>   
>   #include <linux/build_bug.h>
> +#include <linux/cleanup.h>
>   #include <linux/fs.h>
>   #include <linux/init.h>
>   #include <linux/rcupdate.h>
> @@ -128,4 +129,6 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
>   			    const struct path *const path,
>   			    access_mask_t access_hierarchy);
>   
> +DEFINE_FREE(__putname, char *, if (_T) __putname(_T))

Out of curiosity why not put this in include/linux/fs.h (seems to 
compile for me when added there)?

> +
>   #endif /* _SECURITY_LANDLOCK_FS_H */
> diff --git a/security/landlock/trace.c b/security/landlock/trace.c
> new file mode 100644
> index 000000000000..98874cda473b
> --- /dev/null
> +++ b/security/landlock/trace.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Landlock - Tracepoints
> + *
> + * Copyright © 2025 Microsoft Corporation
> + */
> +
> +#include <linux/path.h>
> +
> +#include "access.h"
> +#include "ruleset.h"
> +
> +#define CREATE_TRACE_POINTS
> +#include <trace/events/landlock.h>

Re: [RFC PATCH v1 4/5] landlock: Add landlock_add_rule_fs tracepoint

[Mickaël Salaün]

On Mon, May 26, 2025 at 07:37:52PM +0100, Tingmao Wang wrote:
> On 5/23/25 17:57, Mickaël Salaün wrote:
> > Add a tracepoint for Landlock path_beneath rule addition.  This is
> > useful to tie a Landlock object with a file for debug purpose.
> > 
> > Allocate the absolute path names when adding new rules.  This is OK
> > because landlock_add_rule(2) is not a performance critical code.
> > 
> > Here is an example of landlock_add_rule_fs traces:
> >    ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59260 allowed=0xd dev=0:16 ino=306 path=/usr
> >    ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59240 allowed=0xffff dev=0:16 ino=346 path=/root
> > 
> > TODO: Use Landlock IDs instead of kernel addresses to identify Landlock
> > objects (e.g. inode).
> 
> Do you mean like the u64 domain ID for audit, but for objects?  Since there
> currently isn't a u64, non-pointer ID, I guess we would have to create one
> for the object just for tracing?

Yes, this is the idea.  Landlock objects are scarce so it should not
change much.  Another advantage of using a Landlock ID is that there is
no risk for confusion (i.e. they are not recycled).  I'm not sure if
it's worth it though.

> 
> My understanding is that kernel pointers are not hidden from the root user /
> someone who can read traces, so maybe just printing the pointer is good
> enough?

In theory printed kernel pointers should be hashed, but I guess in
practice and especially with eBPF, kernel addresses may not be really
hidden.

On the other hand, providing a pointer to an eBPF program enables us to
inspect the related data structure.  However, such pointer cannot be
dereferenced in TP_printk() because it is called after the tracepoint.

In fact, I guess we should probably provide kernel addresses (to the
trace context), but I'm not sure if we should print IDs or just kernel
addresses.  It might be handy to easily map a Landlock domain pointer to
its ID in the audit log, but that would require to also copy IDs in the
trace context...  It looks like this is how works sock tracepoints (e.g.
skaddr, but I'm not sure if void * is only there to avoid dereferencing
this pointer in TP_printk).

My understanding is that with eBPF we can read a kernel address from the
trace context without race condition wrt TP_print() which may be called
when the address was already recycled (which could lead to misleading
concurrent traces).

Steven, is it correct? Any advice?

> 
> > 
> > Cc: Günther Noack <gnoack@google.com>
> > Cc: Masami Hiramatsu <mhiramat@kernel.org>
> > Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Cc: Tingmao Wang <m@maowtm.org>
> > Signed-off-by: Mickaël Salaün <mic@digikod.net>
> > ---
> >   MAINTAINERS                     |  1 +
> >   include/trace/events/landlock.h | 68 +++++++++++++++++++++++++++++++++
> >   security/landlock/Makefile      | 11 +++++-
> >   security/landlock/fs.c          | 22 +++++++++++
> >   security/landlock/fs.h          |  3 ++
> >   security/landlock/trace.c       | 14 +++++++
> >   6 files changed, 117 insertions(+), 2 deletions(-)
> >   create mode 100644 include/trace/events/landlock.h
> >   create mode 100644 security/landlock/trace.c
> > 
> > > [...]
> > > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
> > index 73a20a501c3c..e5d673240882 100644
> > --- a/security/landlock/fs.c
> > +++ b/security/landlock/fs.c
> > @@ -36,6 +36,7 @@
> >   #include <linux/types.h>
> >   #include <linux/wait_bit.h>
> >   #include <linux/workqueue.h>
> > +#include <trace/events/landlock.h>
> >   #include <uapi/linux/fiemap.h>
> >   #include <uapi/linux/landlock.h>
> > @@ -345,6 +346,27 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> >   	mutex_lock(&ruleset->lock);
> >   	err = landlock_insert_rule(ruleset, ref, access_rights);
> >   	mutex_unlock(&ruleset->lock);
> > +
> > +	if (!err && trace_landlock_add_rule_fs_enabled()) {
> > +		const char *pathname;
> > +		/* Does not handle deleted files. */
> > +		char *buffer __free(__putname) = __getname();
> > +
> > +		if (buffer) {
> > +			const char *absolute_path =
> > +				d_absolute_path(path, buffer, PATH_MAX);
> > +			if (!IS_ERR_OR_NULL(absolute_path))
> > +				pathname = absolute_path;
> > +			else
> > +				pathname = "<too_long>";
> 
> Not sure if it's necessary to go that far, but I think d_absolute_path
> returns -ENAMETOOLONG in the too long case, and -EINVAL in the "not possible
> to construct a path" case (I guess e.g. if it's an anonymous file or
> detached mount).  We could add an else if branch to check which case it is
> and use different strings.

I mimicked the audit behavior but we can indeed add another case.

> 
> > +		} else {
> > +			/* Same format as audit_log_d_path(). */
> > +			pathname = "<no_memory>";
> > +		}
> > +		trace_landlock_add_rule_fs(ruleset, &ref, access_rights, path,
> > +					   pathname);
> > +	}
> > +
> >   	/*
> >   	 * No need to check for an error because landlock_insert_rule()
> >   	 * increments the refcount for the new object if needed.
> > diff --git a/security/landlock/fs.h b/security/landlock/fs.h
> > index bf9948941f2f..60be95ebfb0b 100644
> > --- a/security/landlock/fs.h
> > +++ b/security/landlock/fs.h
> > @@ -11,6 +11,7 @@
> >   #define _SECURITY_LANDLOCK_FS_H
> >   #include <linux/build_bug.h>
> > +#include <linux/cleanup.h>
> >   #include <linux/fs.h>
> >   #include <linux/init.h>
> >   #include <linux/rcupdate.h>
> > @@ -128,4 +129,6 @@ int landlock_append_fs_rule(struct landlock_ruleset *const ruleset,
> >   			    const struct path *const path,
> >   			    access_mask_t access_hierarchy);
> > +DEFINE_FREE(__putname, char *, if (_T) __putname(_T))
> 
> Out of curiosity why not put this in include/linux/fs.h (seems to compile
> for me when added there)?

I moved it here for this RFC but the next patch series will put it in
linux/fs.h

> 
> > +
> >   #endif /* _SECURITY_LANDLOCK_FS_H */
> > diff --git a/security/landlock/trace.c b/security/landlock/trace.c
> > new file mode 100644
> > index 000000000000..98874cda473b
> > --- /dev/null
> > +++ b/security/landlock/trace.c
> > @@ -0,0 +1,14 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +/*
> > + * Landlock - Tracepoints
> > + *
> > + * Copyright © 2025 Microsoft Corporation
> > + */
> > +
> > +#include <linux/path.h>
> > +
> > +#include "access.h"
> > +#include "ruleset.h"
> > +
> > +#define CREATE_TRACE_POINTS
> > +#include <trace/events/landlock.h>
> 
> 

[RFC PATCH v1 5/5] landlock: Add landlock_check_rule tracepoint

[Mickaël Salaün]

Add a tracepoint called by landlock_unmask_layers() when a rule matches
the current Landlock domain.  This enables us to observe each matching
rule for a specific access request.

The main difference with audit events is that traces are disabled by
default, can be very verbose, and can be filtered according to Landlock
properties (e.g. domain ID).  This event show all steps leading to an
access decision.  We can also attach eBPF programs to this tracepoint
and the landlock_add_rule_fs one to get a more standalone view of
Landlock.

In the followning example, we create a directory with special
characters, then create a initial sandbox allowing reading the parent
directory, and then a nested sandbox not allowing reading of the parent
directory:
  mkdir "$(printf 'a\\\n\047\042 \a\e')"
  cd "$_"
  LL_FS_RO=/usr LL_FS_RW=.. ../sandboxer sh -c \
          "LL_FS_RO=/usr LL_FS_RW=. ../sandboxer ls .."

The landlock_add_rule_fs traces show the initial sandbox's rules:
  ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59260 allowed=0xd dev=0:16 ino=306 path=/usr
  ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59240 allowed=0xffff dev=0:16 ino=346 path=/root

0xd maps to FS_EXECUTE | FS_READ_FILE | FS_READ_DIR, and 0xffff maps to
all supported access rights.

We can then see some access requests with the landlock_check_rule
traces. The first 0x4005 maps to a read-execute (with optional truncate)
request, and the second 0x4004 maps to a read (with optional truncate)
request:
  domain=1362e78a1 key=inode:0xffff888004f59240 request=0x4005 allowed={0xffff}
  domain=1362e78a1 key=inode:0xffff888004f59260 request=0x4004 allowed={0xd}

The landlock_add_rule_fs traces show the nested sandbox's rules:
  ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59260 allowed=0xd dev=0:16 ino=306 path=/usr
  ruleset=0x000000007e3b1c4a key=inode:0xffff888004f59280 allowed=0xffff dev=0:16 ino=210608 path=/root/a\\\n\047\"\040\a\e

We can finally see some access requests with the landlock_check_rule
traces, listing access rights per layers.  Because the second layer does
not allow access (0x0) to /root (inode:0xffff888004f59240), this request
is denied:
  domain=1362e78a7 key=inode:0xffff888004f59240 request=0x4008 allowed={0xffff,0x0}
  domain=1362e78a7 key=inode:0xffff888004f59260 request=0x4004 allowed={0xd,0xd}

Cc: Günther Noack <gnoack@google.com>
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Tingmao Wang <m@maowtm.org>
Signed-off-by: Mickaël Salaün <mic@digikod.net>
---
 include/trace/events/landlock.h | 56 +++++++++++++++++++++++++++++++++
 security/landlock/ruleset.c     |  3 ++
 security/landlock/trace.c       |  1 +
 3 files changed, 60 insertions(+)

diff --git a/include/trace/events/landlock.h b/include/trace/events/landlock.h
index 41e10965ba7b..c88c51aaf22b 100644
--- a/include/trace/events/landlock.h
+++ b/include/trace/events/landlock.h
@@ -11,6 +11,7 @@
 
 #include <linux/tracepoint.h>
 
+struct landlock_rule;
 struct landlock_rule_ref;
 struct landlock_ruleset;
 struct path;
@@ -62,6 +63,61 @@ TRACE_EVENT(landlock_add_rule_fs,
 	)
 );
 
+TRACE_EVENT(landlock_check_rule,
+
+	TP_PROTO(
+		const struct landlock_ruleset *domain,
+		const struct landlock_rule_ref *ref,
+		access_mask_t access_request,
+		const struct landlock_rule *rule
+	),
+
+	TP_ARGS(domain, ref, access_request, rule),
+
+	TP_STRUCT__entry(
+		__field(__u64, domain_id)
+		__field(access_mask_t, access_request)
+		__field(int, ref_type)
+		__field(uintptr_t, ref_key)
+		__dynamic_array(access_mask_t, layers, domain->num_layers)
+	),
+
+	TP_fast_assign(
+		__entry->domain_id = domain->hierarchy->id;
+		__entry->access_request = access_request;
+		__entry->ref_type = ref->type;
+		/* TODO: Use an object's Landlock ID instead of a kernel address. */
+		__entry->ref_key = ref->key.data;
+
+		for (size_t level = 1, i = 0;
+			level <= __get_dynamic_array_len(layers) / sizeof(access_mask_t);
+			level++) {
+			access_mask_t allowed;
+
+			if (i < rule->num_layers &&
+			    level == rule->layers[i].level) {
+				allowed = rule->layers[i].access;
+				i++;
+			} else {
+				allowed = 0;
+			}
+			((access_mask_t *)__get_dynamic_array( layers))[level - 1] = allowed;
+		}
+	),
+
+	/* TODO: Do not print network ports as big endian. */
+	TP_printk("domain=%llx key=%s:0x%lx request=0x%x allowed=%s",
+		__entry->domain_id,
+		__print_symbolic(__entry->ref_type,
+			{ LANDLOCK_KEY_INODE, "inode" },
+			{ LANDLOCK_KEY_NET_PORT, "net_port" }
+		),
+		__entry->ref_key,
+		__entry->access_request,
+		__print_dynamic_array(layers, sizeof(access_mask_t))
+	)
+);
+
 #endif /* _TRACE_LANDLOCK_H */
 
 /* This part must be outside protection */
diff --git a/security/landlock/ruleset.c b/security/landlock/ruleset.c
index 20a4bbb2526f..f9e407e4038c 100644
--- a/security/landlock/ruleset.c
+++ b/security/landlock/ruleset.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
+#include <trace/events/landlock.h>
 
 #include "access.h"
 #include "audit.h"
@@ -629,6 +630,8 @@ bool landlock_unmask_layers(const struct landlock_ruleset *const domain,
 	if (!rule)
 		return false;
 
+	trace_landlock_check_rule(domain, &ref, access_request, rule);
+
 	/*
 	 * An access is granted if, for each policy layer, at least one rule
 	 * encountered on the pathwalk grants the requested access,
diff --git a/security/landlock/trace.c b/security/landlock/trace.c
index 98874cda473b..c0c450536be9 100644
--- a/security/landlock/trace.c
+++ b/security/landlock/trace.c
@@ -8,6 +8,7 @@
 #include <linux/path.h>
 
 #include "access.h"
+#include "domain.h"
 #include "ruleset.h"
 
 #define CREATE_TRACE_POINTS
-- 
2.49.0

Re: [RFC PATCH v1 0/5] Landlock tracepoints

[Tingmao Wang]

On 5/23/25 17:57, Mickaël Salaün wrote:
> Hi,
> 
> This series adds two tracepoints to Landlock, one tied to rule addition,
> and another to rule checking.  With these new tracepoints, we can see
> all steps leading to an access decision.  They can be directly used with
> /sys/kernel/tracing/events/landlock/* or attached by eBPF programs to
> get a more complete view of Landlock internals.
> 
> This new feature is useful to trouble shoot policy issues, and it should
> also limit the need for custom debugging kernel code when developing new
> Landlock features.
> 
> Landlock already has audit support, which enables us to log denied
> access requests.  Audit is useful to identify security issues or sandbox
> misconfiguration.  However, it might not be enough to debug Landlock
> policies.  The main differences with audit events is that traces are
> disabled by default, can be very verbose, and can be filtered according
> to process and Landlock properties (e.g. domain ID).
> 
> As for audit, this tracing feature may expose sensitive information and
> must then only be accessible to the system administrator.
> 
> This RFC only fully supports filesystem rules but the next series will
> also support network rules.  Tests are also missing for now.
> 
> Regards,
> 
> Mickaël Salaün (5):
>    landlock: Rename landlock_id to landlock_rule_ref
>    landlock: Merge landlock_find_rule() into landlock_unmask_layers()
>    tracing: Add __print_untrusted_str()
>    landlock: Add landlock_add_rule_fs tracepoint
>    landlock: Add landlock_check_rule tracepoint
> 
>   MAINTAINERS                                |   1 +
>   include/linux/trace_events.h               |   3 +
>   include/trace/events/landlock.h            | 124 ++++++++++++++
>   include/trace/stages/stage3_trace_output.h |   4 +
>   include/trace/stages/stage7_class_define.h |   1 +
>   kernel/trace/trace_output.c                |  40 +++++
>   security/landlock/Makefile                 |  11 +-
>   security/landlock/fs.c                     | 178 +++++++++++++--------
>   security/landlock/fs.h                     |   3 +
>   security/landlock/net.c                    |  18 +--
>   security/landlock/ruleset.c                |  65 ++++----
>   security/landlock/ruleset.h                |  15 +-
>   security/landlock/trace.c                  |  15 ++
>   13 files changed, 365 insertions(+), 113 deletions(-)
>   create mode 100644 include/trace/events/landlock.h
>   create mode 100644 security/landlock/trace.c
> 
> 
> base-commit: a5806cd506af5a7c19bcd596e4708b5c464bfd21

Tested-by: Tingmao Wang <m@maowtm.org>

Here is an example output I got with `trace-cmd record -e landlock`:

landlock-add-ru-776   [001] 75134.554776: landlock_add_rule_fs: [FAILED 
TO PARSE] ruleset=0xffff88811a459200 ref_key=18446612686420679296 
allowed=57343 dev=21 ino=53 pathname=/tmp/test
landlock-add-ru-776   [001] 75134.555336: landlock_check_rule:  [FAILED 
TO PARSE] domain_id=7838764077 access_request=4 ref_type=1 
ref_key=18446612686420679296 layers=ARRAY[ff, df]
I suggest adding some more events which I think shouldn't be too 
difficult to implement and I can see them helping a lot with tracing / 
debugging especially by BPF programs or someone new to landlock:

- landlock_restrict_self: Currently we trace out the ruleset pointer in 
add rule, and the domain ID in check rule.  However there is no way for 
someone just looking at this trace (or a BPF program for landlock) to 
relate which rulesets are applied to which domains. I think a simple 
event we could add that will help with this is, on 
landlock_restrict_self, prints out the ruleset passed in as well as the 
domain ID newly created.  Maybe also num_rules and num_layers on the new 
ruleset since it's trivial to do so, and could be informative to someone 
analyzing a Landlock thing.

- landlock_check_fs: Distinct from landlock_check_rule, this will happen 
once the outcome of the access check is determined (maybe at the end of 
is_access_to_paths_allowed and collect_domain_accesses?). It would 
include the pathname of the target file (only allocated if this event is 
enabled of course), so something like:

   landlock_check_fs: domain_id=7838764077 access_request=4 
pathname=/tmp/test allowed=true
   landlock_check_fs: domain_id=7838764077 access_request=4 
pathname=/tmp/test2 allowed=false

We already produces audit logs for denied requests so it is a little bit 
duplicating that, but I think this trace event shouldn't be too costly 
to include.  It has the benefit that

1. If an access is denied because no rules matched, we don't get any 
landlock_check_rule traces, and so there's no way for someone looking at 
the trace log to find out landlock denied something.

2. Having an event that represents an access check makes it possible for 
BPF programs to find out about all landlock access checks (most 
interestingly denied ones but we could expose the allowed ones too), and 
potentially relate the various `landlock_check_rule` events to an access 
check.

Actually, maybe it's worth having a "landlock_check_fs_enter", emitted 
at the beginning of is_access_to_paths_allowed and 
collect_domain_accesses? This could be useful for performance 
measurements, and also makes it more explicit which landlock_check_rule 
resulted from which access check.  Maybe the pathname and domain_id 
could be printed on the enter event, and the exit event would just have 
the outcome?  (Just an idea, I feel less certain about this. 
"Enter"/"exit" naming taken from sys_enter_*/sys_exit_* but start/end 
also works)

Re: [RFC PATCH v1 0/5] Landlock tracepoints

[Mickaël Salaün]

On Mon, May 26, 2025 at 07:37:41PM +0100, Tingmao Wang wrote:
> On 5/23/25 17:57, Mickaël Salaün wrote:
> > Hi,
> > 
> > This series adds two tracepoints to Landlock, one tied to rule addition,
> > and another to rule checking.  With these new tracepoints, we can see
> > all steps leading to an access decision.  They can be directly used with
> > /sys/kernel/tracing/events/landlock/* or attached by eBPF programs to
> > get a more complete view of Landlock internals.
> > 
> > This new feature is useful to trouble shoot policy issues, and it should
> > also limit the need for custom debugging kernel code when developing new
> > Landlock features.
> > 
> > Landlock already has audit support, which enables us to log denied
> > access requests.  Audit is useful to identify security issues or sandbox
> > misconfiguration.  However, it might not be enough to debug Landlock
> > policies.  The main differences with audit events is that traces are
> > disabled by default, can be very verbose, and can be filtered according
> > to process and Landlock properties (e.g. domain ID).
> > 
> > As for audit, this tracing feature may expose sensitive information and
> > must then only be accessible to the system administrator.
> > 
> > This RFC only fully supports filesystem rules but the next series will
> > also support network rules.  Tests are also missing for now.
> > 
> > Regards,
> > 
> > Mickaël Salaün (5):
> >    landlock: Rename landlock_id to landlock_rule_ref
> >    landlock: Merge landlock_find_rule() into landlock_unmask_layers()
> >    tracing: Add __print_untrusted_str()
> >    landlock: Add landlock_add_rule_fs tracepoint
> >    landlock: Add landlock_check_rule tracepoint
> > 
> >   MAINTAINERS                                |   1 +
> >   include/linux/trace_events.h               |   3 +
> >   include/trace/events/landlock.h            | 124 ++++++++++++++
> >   include/trace/stages/stage3_trace_output.h |   4 +
> >   include/trace/stages/stage7_class_define.h |   1 +
> >   kernel/trace/trace_output.c                |  40 +++++
> >   security/landlock/Makefile                 |  11 +-
> >   security/landlock/fs.c                     | 178 +++++++++++++--------
> >   security/landlock/fs.h                     |   3 +
> >   security/landlock/net.c                    |  18 +--
> >   security/landlock/ruleset.c                |  65 ++++----
> >   security/landlock/ruleset.h                |  15 +-
> >   security/landlock/trace.c                  |  15 ++
> >   13 files changed, 365 insertions(+), 113 deletions(-)
> >   create mode 100644 include/trace/events/landlock.h
> >   create mode 100644 security/landlock/trace.c
> > 
> > 
> > base-commit: a5806cd506af5a7c19bcd596e4708b5c464bfd21
> 
> Tested-by: Tingmao Wang <m@maowtm.org>
> 
> Here is an example output I got with `trace-cmd record -e landlock`:
> 
> landlock-add-ru-776   [001] 75134.554776: landlock_add_rule_fs: [FAILED TO
> PARSE] ruleset=0xffff88811a459200 ref_key=18446612686420679296 allowed=57343
> dev=21 ino=53 pathname=/tmp/test
> landlock-add-ru-776   [001] 75134.555336: landlock_check_rule:  [FAILED TO
> PARSE] domain_id=7838764077 access_request=4 ref_type=1
> ref_key=18446612686420679296 layers=ARRAY[ff, df]
> I suggest adding some more events which I think shouldn't be too difficult
> to implement and I can see them helping a lot with tracing / debugging
> especially by BPF programs or someone new to landlock:
> 
> - landlock_restrict_self: Currently we trace out the ruleset pointer in add
> rule, and the domain ID in check rule.  However there is no way for someone
> just looking at this trace (or a BPF program for landlock) to relate which
> rulesets are applied to which domains. I think a simple event we could add
> that will help with this is, on landlock_restrict_self, prints out the
> ruleset passed in as well as the domain ID newly created.  Maybe also
> num_rules and num_layers on the new ruleset since it's trivial to do so, and
> could be informative to someone analyzing a Landlock thing.

Yep, that was my plan for v2. :)

I was also wondering about adding a tracepoint for ruleset creation.
This one would print the ruleset's attributes as an array.

> 
> - landlock_check_fs: Distinct from landlock_check_rule, this will happen
> once the outcome of the access check is determined (maybe at the end of
> is_access_to_paths_allowed and collect_domain_accesses?). It would include
> the pathname of the target file (only allocated if this event is enabled of
> course), so something like:
> 
>   landlock_check_fs: domain_id=7838764077 access_request=4
> pathname=/tmp/test allowed=true
>   landlock_check_fs: domain_id=7838764077 access_request=4
> pathname=/tmp/test2 allowed=false
> 
> We already produces audit logs for denied requests so it is a little bit
> duplicating that, but I think this trace event shouldn't be too costly to
> include.  It has the benefit that
> 
> 1. If an access is denied because no rules matched, we don't get any
> landlock_check_rule traces, and so there's no way for someone looking at the
> trace log to find out landlock denied something.

That would indeed be useful to see final access decisions.  It might be
a bit tricky to generically handle all the allowed access but I'll try
something.

> 
> 2. Having an event that represents an access check makes it possible for BPF
> programs to find out about all landlock access checks (most interestingly
> denied ones but we could expose the allowed ones too), and potentially
> relate the various `landlock_check_rule` events to an access check.
> 
> Actually, maybe it's worth having a "landlock_check_fs_enter", emitted at
> the beginning of is_access_to_paths_allowed and collect_domain_accesses?
> This could be useful for performance measurements, and also makes it more

Aren't kprobes/kretprobes enough for performance measurement?  They can
also be used with current kernels.

> explicit which landlock_check_rule resulted from which access check.  Maybe
> the pathname and domain_id could be printed on the enter event, and the exit
> event would just have the outcome?  (Just an idea, I feel less certain about
> this. "Enter"/"exit" naming taken from sys_enter_*/sys_exit_* but start/end
> also works)

This landlock_check_fs_enter event would only be called after the
landlock_get_applicable_subject() check though.