On Thu, Jun 01, 2023 at 06:13:53PM +0200, Jan Kara wrote: > On Thu 01-06-23 15:37:32, David Laight wrote: > > ... > > > > > + * Lock any non-NULL argument. The caller must make sure that if he is passing > > > > > + * in two directories, one is not ancestor of the other > > > > Not directly relevant to this change but is the 'not an ancestor' > > check actually robust? > > > > I found a condition in which the kernel 'pwd' code (which follows > > the inode chain) failed to stop at the base of a chroot. > > > > I suspect that the ancestor check would fail the same way. > > Honestly, I'm not sure how this could be the case but I'm not a dcache > expert. d_ancestor() works on dentries and the whole dcache code pretty > much relies on the fact that there always is at most one dentry for any > directory. Also in case we call d_ancestor() from this code, we have the > whole filesystem locked from any other directory moves so the ancestor > relationship of two dirs cannot change (which is different from pwd code > AFAIK). So IMHO no failure is possible in our case. Yes, this is a red herring. What matters is that the tree topology can't change which is up to the caller to guarantee. And where it's called we're under s_vfs_rename_mutex. It's also literally mentioned in the directory locking documentation.
