Re: [PATCH 27/32] xfs: Add parent pointer ioctls

Darrick J. Wong
2024-04-16T16:50:57Z

On Tue, Apr 16, 2024 at 06:47:16AM +0200, Christoph Hellwig wrote:
> On Mon, Apr 15, 2024 at 12:40:36PM -0700, Darrick J. Wong wrote:
> > True, libhandle is a very nice wrapper for the kernel ioctls. I wish
> > Linux projects did that more often. But suppose you're calling the
> > ioctls directly without libhandle and mess it up?
>
> The you get different inodes back. Not really any different from
> pointing your path name based code to the wrong fs or directory,
> is it?

I suppose not. But why bother setting the fsid at all, then?

> > > In theory no one cares as all operations work just fine with disconnected
> > > dentries, and exportfs_decode_fh doesn't do these checks unless the
> > > accpetable parameter is passed to it. The real question is why we (which
> > > in this case means 15 years younger me) decided back then we want this
> > > checking for XFS handle operations? I can't really think of one
> > > right now..
> >
> > Me neither. Though at this point there are a lot of filesystems that
> > implement ->get_parent, so I think removing XFS's will need a discussion
> > at least on linux-xfs, if not fsdevel. In the meantime, getparents can
> > do minimal validation + iget for now and if it makes sense to port it
> > back to xfs_khandle_to_dentry, I can do that easily.
>
> Uhh, I'm not advocating for removing ->get_parent at all. We actually
> do need that for security on NFS, where the file handles are used
> undernath pathname based operations.

Ahh, I wasn't aware of that, beyond a sense that "a lot of
NFS-exportable fses do this, so there's likely a general desire for this
to be wired up."

> And it turns out my previous analysis wasn't quite sport on. The
> exportfs code always reconnects directories, because we basically
> have to, not connecting them would make the VFS locking scheme
> not work.

Noted.

> But as we never generate the file handles that encode the parent
> we already never connect files to their parent directory anyway.

I pondered whether or not we should encode parent info in a regular
file's handle. Would that result in an invalid handle if the file gets
moved to another directory? That doesn't seem to fit with the behavior
that fds remain attached to the file even if it gets moved/deleted.

> OTOH we should be able to optimize ->get_parent a bit with parent
> pointers, as we can find the name in the parent directory for
> a directory instead of doing linear scans in the parent directory.
> (for non-directory files we currenty don't fully connect anwyay)

<nod> But does exportfs actually want parent info for a nondirectory?
There aren't any stubs or XXX/FIXME comments, and I've never heard any
calls (at least on fsdevel) for that functionality.

--D