Re: [RFC PATCH 2/9] Refactor per-layer information in rulesets and rules

["Mickaël Salaün" <mic@digikod.net>]

On Thu, Mar 06, 2025 at 02:58:01AM +0000, Tingmao Wang wrote:
> On 3/4/25 19:49, Mickaël Salaün wrote:
> 
> > We could indeed have a pointer in the  landlock_hierarchy and have a
> > dedicated bit in each layer's access_masks to indicate that this layer
> > is supervised.  This should simplify the whole patch series.
> 
> That seems sensible.  I did consider using the landlock_hierarchy, but chose
> the current way as it initially seemed more sensible, but on second thought
> this means that we have to carefully increment all the refcounts on domain
> merge etc.  On the other hand storing the supervisor pointer in the
> hierarchy, if we have an extra bit in struct access_masks then we can
> quickly determine if supervisors are involved without effectively walking a
> linked list, which is nice.

Right

> 
> Actually, just to check, is the reason why we have the access_masks FAM in
> the ruleset purely for performance? Initially I wasn't sure if each layer
> correspond 1-to-1 with landlock_hierarchy, since otherwise it seemed to me
> you could just put the access mask in the hierarchy too.

Yes, we could put the access rights in the hierarchy, but that would
involve walking through the hierarchy to know if Landlock should
actually handle (i.e. allow or potentially deny) an access request.
Landlock is designed in a way that makes legitimate/allowed access as
fast as possible (there is still room for improvement though).  In the
case of the supervisor feature, it should mainly be used to dynamically
allow access which are statically denied for one layer.  And because it
will require a round trip to user space anyway, the performance impact
of putting the supervisor pointer in landlock_hierarchy is negligible.

Initially the purpose of landlock_hierarchy was to be able to compare
domains (for ptrace and later scope restrictions), whereas the
landlock_ruleset is to store immutable data (without references) when
used as a domain.  With the audit feature, the landlock_hierarchy will
also contain domain's shared/mutable states and pointers that should
only be rarely accessed (i.e. only for denials).  So, in a nutshell
landlock_ruleset as a domain should stay minimal and improve data
locality to speed up allowed access requests.

We could decorrelate the current content of landlock_hierarchy from the
shared data, but I think it would only be meaningful if this data is
significant (see the landlock_details pointer in the audit patch series).

> In other words, is it right to assume that, if a domain has 3 layers, for
> example, then domain->hierarchy correspond to the third layer,
> domain->hierarchy->parent correspond to the second, and
> d->h->parent->parent would be the first layer's hierarchy?

Yes, that is always the case for a domain.

If we create the supervisor FD with landlock_restrict_self(2), then
we'll not have to add a new pointer to landlock_ruleset, but only
directly to landlock_hierarchy.