Message-ID: <20250625.Ee2Ci6chae8h@digikod.net>
Date: Wed, 25 Jun 2025 15:14:08 +0200
From: Mickaël Salaün <mic@...ikod.net>
To: NeilBrown <neil@...wn.name>
Cc: Song Liu <song@...nel.org>, bpf@...r.kernel.org, 
	linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	linux-security-module@...r.kernel.org, brauner@...nel.org, kernel-team@...a.com, andrii@...nel.org, 
	eddyz87@...il.com, ast@...nel.org, daniel@...earbox.net, martin.lau@...ux.dev, 
	viro@...iv.linux.org.uk, jack@...e.cz, kpsingh@...nel.org, mattbobrowski@...gle.com, 
	Tingmao Wang <m@...wtm.org>, Günther Noack <gnoack@...gle.com>
Subject: Re: [PATCH v5 bpf-next 0/5] bpf path iterator

On Wed, Jun 25, 2025 at 07:38:53AM +1000, NeilBrown wrote:
> On Wed, 25 Jun 2025, Mickaël Salaün wrote:
> > On Fri, Jun 20, 2025 at 02:59:17PM -0700, Song Liu wrote:
> > > Hi Christian, Mickaël, and folks,
> > > 
> > > Could you please share your comments on this version? Does this
> > > look sane?
> > 
> > This looks good to me but we need to know what is the acceptable next
> > step to support RCU.  If we can go with another _rcu helper, I'm good
> > with the current approach, otherwise we need to figure out a way to
> > leverage the current helper to make it compatible with callers being in
> > a RCU read-side critical section while leveraging safe path walk (i.e.
> > several calls to path_walk_parent).
> 
> Can you spell out the minimum that you need?

Sure.  We'd like to call this new helper in a RCU
read-side critical section and leverage this capability to speed up path
walk when there is no concurrent hierarchy modification.  This use case
is similar to handle_dots() with LOOKUP_RCU calling follow_dotdot_rcu().

The main issue with this approach is to keep some state of the path walk
to know if the next call to "path_walk_parent_rcu()" would be valid
(i.e. something like a very light version of nameidata, mainly sequence
integers), and to get back to the non-RCU version otherwise.

> 
> My vague impression is that you want to search up from a given strut path,
> no further then some other given path, looking for a dentry that matches
> some rule.  Is that correct?

Yes

> 
> In general, the original dentry could be moved away from under the
> dentry you find moments after the match is reported.  What mechanisms do
> you have in place to ensure this doesn't happen, or that it doesn't
> matter?

In the case of Landlock, by default, a set of access rights are denied
and can only be allowed by an element in the file hierarchy.  The goal
is to only allow access to files under a specific directory (or directly
a specific file).  That's why we only care of the file hierarchy at the
time of access check.  It's not an issue if the file/directory was
moved or is being moved as long as we can walk its "current" hierarchy.
Furthermore, a sandboxed process is restricted from doing arbitrary
mounts (and renames/links are controlled with the
LANDLOCK_ACCESS_FS_REFER right).

However, we need to get a valid "snapshot" of the set of dentries that
(could) lead to the evaluated file/directory.

> 
> Would it be sufficient to have an iterator which reported successive
> ancestors in turn, or reported that you need to restart because something
> changed?  Would you need to know that a restart happened or would it be
> acceptable to transparently start again at the parent of the starting
> point?

If the path walk is being invalidated, we need to reset the collected
access right and start again the path walk to get all the access rights
from a consistent/real file hierarchy.

> 
> Or do you really need a "one step at a time" interface?

We need to check each component of the path walk, so either we call an
helper to get each of them and we do our check after that (we should be
able to do that in RCU), or we provide a callback function which is
called by the path walk helper.

> 
> Do you need more complex movements around the tree, or is just walking
> up sufficient?

Just walking up.

> 
> If this has been discussed or documented elsewhere I'd be happy for you
> just to provide a reference, and I can come back with follow-up
> questions if needed.

Tingmao initially described the goal here:
https://lore.kernel.org/all/afe77383-fe56-4029-848e-1401e3297139@maowtm.org/

and she sent an RFC to illustrate that:
https://lore.kernel.org/all/cover.1748997840.git.m@maowtm.org/

The discussion mainly raised two questions:
- Should we have one or two APIs?
- How to store the state of the walk without exposing VFS internals to
  the rest of the kernel?

Thanks

> 
> Thanks,
> NeilBrown
> 
> 
> > 
> > > 
> > > Thanks,
> > > Song
> > > 
> > > On Mon, Jun 16, 2025 at 11:11 PM Song Liu <song@...nel.org> wrote:
> > > >
> > > > In security use cases, it is common to apply rules to VFS subtrees.
> > > > However, filtering files in a subtree is not straightforward [1].
> > > >
> > > > One solution to this problem is to start from a path and walk up the VFS
> > > > tree (towards the root). Among in-tree LSMs, Landlock uses this solution.
> > > >
> > > 
> > > [...]
> > > 
> > 
> 
> 

Powered by blists - more mailing lists