Re: [PATCH][RFC] ->mnt_devname is never NULL

[Christian Brauner <brauner@kernel.org>]

On Mon, Apr 21, 2025 at 06:03:19PM +0100, Al Viro wrote:
> On Mon, Apr 21, 2025 at 05:29:47PM +0100, Al Viro wrote:
> 
> > What's to prevent the 'beneath' case from getting mnt mount --move'd
> > away *AND* the ex-parent from getting unmounted while we are blocked
> > in inode_lock?  At this point we are not holding any locks whatsoever
> > (and all mount-related locks nest inside inode_lock(), so we couldn't
> > hold them there anyway).
> > 
> > Hit that race and watch a very unhappy umount...
> 
> While we are at it, in normal case inode_unlock() in unlock_mount()
> is safe since we have dentry (and associated mount) pinned by
> struct path we'd fed to matching lock_mount().  No longer true for
> the 'beneath' case, AFAICS...

I'm not following. Please explain the issue in detail. Both mount and
dentry are pinned via struct path independent of whether its beneath or
not beneath.

What we pass to unlock_mount() is the mountpoint which pins the relevant
dentry separately. do_lock_mount() keeps @dentry for the mountpoint
pinned until it has taken a separate reference. We only put the
reference to the mountpoint's dentry if we know that the for (;;) will
continue aka not break or when get_mountpoint() has taken it's own
reference.

So really, I'm very confused atm.

Also if this were the case all invasive move mount beneath tests I added
should cause endless splats under any sort of KASAN which they are
constantly run under in a tight loop in my local testing and by syzbot.
For the latter I explicitly added support for it in:

https://github.com/google/syzkaller/commit/058b3a5a6a945a55767811552eb7b9f4a20307f8