Re: [PATCH bpf-next 3/4] bpf: Introduce path iterator

[Al Viro <viro@zeniv.linux.org.uk>]

On Thu, May 29, 2025 at 11:00:51AM -0700, Song Liu wrote:
> On Thu, May 29, 2025 at 10:38 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> > On Thu, May 29, 2025 at 09:53:21AM -0700, Song Liu wrote:
> >
> > > Current version of path iterator only supports walking towards the root,
> > > with helper path_parent. But the path iterator API can be extended
> > > to cover other use cases.
> >
> > Clarify the last part, please - call me paranoid, but that sounds like
> > a beginning of something that really should be discussed upfront.
> 
> We don't have any plan with future use cases yet. The only example
> I mentioned in the original version of the commit log is "walk the
> mount tree". IOW, it is similar to the current iterator, but skips non
> mount point iterations.
> 
> Since we call it "path iterator", it might make sense to add ways to
> iterate the VFS tree in different patterns. For example, we may
> have an iterator that iterates all files within a directory. Again, we
> don't see urgent use cases other than the current "walk to root"
> iterator.

What kinds of locking environments can that end up used in?

The reason why I'm getting more and more unhappy with this thing is
that it sounds like a massive headache for any correctness analysis in
VFS work.

Going straight to the root starting at a point you already have pinned
is relatively mild - you can't do path_put() in any blocking contexts,
obviously, and you'd better be careful with what you are doing on
mountpoint traversal (e.g. combined with "now let's open that directory
and read it" it's an instant "hell, no" - you could easily bypass MNT_LOCKED
restrictions that way), but if there's a threat of that getting augmented
with other things (iterating through all files in directory would be
a very different beast from the locking POV, if nothing else)... ouch.

Basically, you are creating a spot we will need to watch very carefully
from now on.  And the rationale appears to include "so that we could
expose that to random out-of-tree code that decided to call itself LSM",
so pardon me for being rather suspicious about the details.

PS: one general observation: "some LSM does it" does not imply even
"what that LSM is doing is sane and safe", let along "what that LSM is
doing doesn't happen to avoid breakage only by accident".