On Fri, Dec 29, 2023 at 5:22 PM Trond Myklebust <trondmy@xxxxxxxxxxxxxxx> wrote: > > On Fri, 2023-12-29 at 07:46 +0200, Amir Goldstein wrote: > > [CC: fsdevel, viro] > > > > On Thu, Dec 28, 2023 at 10:22 PM <trondmy@xxxxxxxxxx> wrote: > > > > > > From: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx> > > > > > > The fallback implementation for the get_name export operation uses > > > readdir() to try to match the inode number to a filename. That > > > filename > > > is then used together with lookup_one() to produce a dentry. > > > A problem arises when we match the '.' or '..' entries, since that > > > causes lookup_one() to fail. This has sometimes been seen to occur > > > for > > > filesystems that violate POSIX requirements around uniqueness of > > > inode > > > numbers, something that is common for snapshot directories. > > > > Ouch. Nasty. > > > > Looks to me like the root cause is "filesystems that violate POSIX > > requirements around uniqueness of inode numbers". > > This violation can cause any of the parent's children to wrongly > > match > > get_name() not only '.' and '..' and fail the d_inode sanity check > > after > > lookup_one(). > > > > I understand why this would be common with parent of snapshot dir, > > but the only fs that support snapshots that I know of (btrfs, > > bcachefs) > > do implement ->get_name(), so which filesystem did you encounter > > this behavior with? can it be fixed by implementing a snapshot > > aware ->get_name()? > > NFS (i.e. re-exporting NFS). > Ah. > Why do you not want a fix in the generic code? > I do not object to your fix at all. I only objected to the Fixes tag. I was just pointing out that this is not a complete solution. A decode of an NFS (re-exported) file handle could fail if get_name() iterates the parent of a snapshot root dir and finds a false match (which is not "." nor "..") before it finds the snapshot subdir name. It may be solved by nfs_get_name() which does not stop when if finds an ino match but checks further, but I don't know nfs re-export to know what else could be checked. Anyway, for this patch, without the Fixes tag, feel free to add: Acked-by: Amir Goldstein <amir73il@xxxxxxxxx> I'd prefer the use of is_dot_dotdot(), but I do not insist. Thanks and a happy new year! Amir.
