[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 at 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.