Message-ID: <CAPhsuW53QiS8Aa5c4VLFjojShmgibftVe=py-RuL+ZyHBY5Pbg@mail.gmail.com>
Date: Wed, 25 Jun 2025 16:17:20 -0700
From: Song Liu <song@...nel.org>
To: NeilBrown <neil@...wn.name>
Cc: Mickaël Salaün <mic@...ikod.net>, 
	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 4:05 PM NeilBrown <neil@...wn.name> wrote:
>
> On Wed, 25 Jun 2025, Mickaël Salaün wrote:
> > On Wed, Jun 25, 2025 at 07:38:53AM +1000, NeilBrown wrote:
> > >
> > > 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.
>
> A "snapshot" is an interesting idea - though looking at the landlock
> code you one need inodes, not dentries.
> I imagine an interface where you give it a starting path, a root, and
> and array of inode pointers, and it fills in the pointers with the path
> - all under rcu so no references are needed.
> But you would need some fallback if the array isn't big enough, so maybe
> that isn't a good idea.
>
> Based on the comments by Al and Christian, I think the only viable
> approach is to pass a callback to some vfs function that does the
> walking.
>
>    vfs_walk_ancestors(struct path *path, struct path *root,
>                       int (*walk_cb)(struct path *ancestor, void *data),
>                       void *data)

I like this idea.

Maybe we want "struct path *ancestor" of walk_cb to be const.
walk_cb should only change "data", so that we can undo all the
changes when the rcu walk fails.

Thanks,
Song

Powered by blists - more mailing lists