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

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

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()?