Re: [PATCH 1/7] autofs4: Save autofs trigger's vfsmount in super block info

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

On Mon, Jan 18, 2010 at 12:21:09PM +0800, Ian Kent wrote:

> In that case we may find an autofs mount that has something mounted on
> top of it and user space wants to know the super of the covering mount.
> 
> If there is something mounted on top of it user space needs to know if
> it is another autofs file system or some other type of file system. So
> if the nameidata path, located by autofs_dev_ioctl_find_super(), is not
> the top (or bottom, depending on the terminology you prefer) then we
> need to follow the mount and return the magic of the thing mounted on
> top of it.

IDGI.  What you are doing there is
                if (path.mnt->mnt_mountpoint != path.mnt->mnt_root) {
                        if (follow_down(&path))
                                magic = path.mnt->mnt_sb->s_magic;
                }
and I don't think it means what you think it means.  Just what is that
mnt_mountpoint check about?  Before that point we'd found the autofs
vfsmount M that
	1) M is mounted on <name>
	2) M->mnt_sb has the right s_dev
	3) M is the closest one to root in mount tree out of vfsmounts
satisfying (1) and (2)
Now we check that
	4) the mountpoint M is attached to has dentry different from
M->mnt_root.  That's an interesting thing to check, seeing that the
only way to get it false is to have mount --bind name name, with name
not being the mountpoint before that.  And M being the result of
that mount --bind.
	5) something is mounted on top of root of M.

Then we proceed to return the s_magic of that something.  For one thing,
if there *are* several vfsmounts satisfying (1,2), which one do we really
want?  For another, what's the intent of (4)?  It looks very odd; what's
really being checked there?

In another branch we have
                if (path.dentry->d_inode &&
                    path.mnt->mnt_root == path.dentry) {
                        err = 1;
                        magic = path.dentry->d_inode->i_sb->s_magic;
                }
and AFAICT, path.dentry->d_inode == NULL is impossible there.  Besides,
path.mnt->mnt_sb->s_magic would be simpler anyway (and evaluate to
the same thing).