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 <gno...@google.com>
Cc: Masami Hiramatsu <mhira...@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoy...@efficios.com>
Cc: Steven Rostedt <rost...@goodmis.org>
Cc: Tingmao Wang <m...@maowtm.org>
Signed-off-by: Mickaël Salaün <m...@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>


Reply via email to