On Fri, 30 Jul 2021, Al Viro wrote:
> On Wed, Jul 28, 2021 at 08:37:45AM +1000, NeilBrown wrote:
>
> > diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
> > index baa12ac36ece..22523e1cd478 100644
> > --- a/fs/nfsd/vfs.c
> > +++ b/fs/nfsd/vfs.c
> > @@ -64,7 +64,7 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct path *path_parent,
> > .dentry = dget(path_parent->dentry)};
> > int err = 0;
> >
> > - err = follow_down(&path, 0);
> > + err = follow_down(&path, LOOKUP_AUTOMOUNT);
> > if (err < 0)
> > goto out;
> > if (path.mnt == path_parent->mnt && path.dentry == path_parent->dentry &&
> > @@ -73,6 +73,13 @@ nfsd_cross_mnt(struct svc_rqst *rqstp, struct path *path_parent,
> > path_put(&path);
> > goto out;
> > }
> > + if (mount_is_internal(path.mnt)) {
> > + /* Use the new path, but don't look for a new export */
> > + /* FIXME should I check NOHIDE in this case?? */
> > + path_put(path_parent);
> > + *path_parent = path;
> > + goto out;
> > + }
>
> ... IOW, mount_is_internal() is called with no exclusion whatsoever. What's there
> to
> * keep its return value valid?
> * prevent fetching ->mnt_mountpoint, getting preempted away, having
> the mount moved *and* what used to be ->mnt_mountpoint evicted from dcache,
> now that it's no longer pinned, then mount_is_internal() regaining CPU and
> dereferencing ->mnt_mountpoint, which now points to hell knows what?
>
Yes, mount_is_internal needs to same mount_lock protection that
lookup_mnt() has. Thanks.
I don't think it matter how long the result remains valid. The only
realistic transtion is from True to False, but the fact that it *was*
True means that it is acceptable for the lookup to have succeeded.
i.e. If the mountpoint was moved which a request was being processed it
will either cause the same result as if it happened before the request
started, or after it finished. Either seems OK.
Thanks,
NeilBrown
