Quoting Trond Myklebust ([email protected]):
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
>
> fs/nfs/super.c | 24 ++++++++++++++++++++++++
> 1 files changed, 24 insertions(+), 0 deletions(-)
>
>
> diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> index 8da7e59..daecbad 100644
> --- a/fs/nfs/super.c
> +++ b/fs/nfs/super.c
> @@ -2548,6 +2548,27 @@ static struct vfsmount *nfs_do_root_mount(struct file_system_type *fs_type,
> return root_mnt;
> }
>
> +static void nfs_fix_devname(const struct path *path, struct vfsmount *mnt)
> +{
> + char *page = (char *) __get_free_page(GFP_KERNEL);
> + char *devname, *tmp;
> +
> + if (page == NULL)
> + return;
> + devname = nfs_path(path->mnt->mnt_devname,
> + path->mnt->mnt_root, path->dentry,
> + page, PAGE_SIZE);
> + if (devname == NULL)
> + goto out_freepage;
> + tmp = kstrdup(devname, GFP_KERNEL);
> + if (tmp == NULL)
> + goto out_freepage;
> + kfree(mnt->mnt_devname);
> + mnt->mnt_devname = tmp;
(looking through patch 3 a bit) is this expected to be safe because all
callers will send in a mnt which was privately mounted as nfs root_mnt through
vfs_kern_mount? So that at this point noone else can have a ref to
mnt?
If that isn't the intent, then this seems problematic... (If it is, it
seems worth commenting both so that every reader doesn't feel compelled
to verify, and so that no new callers will naively violate that
expectation)
thanks,
-serge
> +out_freepage:
> + free_page((unsigned long)page);
> +}
> +
> static int nfs_follow_remote_path(struct vfsmount *root_mnt,
> const char *export_path, struct vfsmount *mnt_target)
> {
> @@ -2574,6 +2595,9 @@ static int nfs_follow_remote_path(struct vfsmount *root_mnt,
> mnt_target->mnt_sb = s;
> mnt_target->mnt_root = dget(nd.path.dentry);
>
> + /* Correct the device pathname */
> + nfs_fix_devname(&nd.path, mnt_target);
> +
> path_put(&nd.path);
> down_write(&s->s_umount);
> return 0;
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, 2009-06-23 at 15:42 -0500, Serge E. Hallyn wrote:
> Quoting Trond Myklebust ([email protected]):
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> >
> > fs/nfs/super.c | 24 ++++++++++++++++++++++++
> > 1 files changed, 24 insertions(+), 0 deletions(-)
> >
> >
> > diff --git a/fs/nfs/super.c b/fs/nfs/super.c
> > index 8da7e59..daecbad 100644
> > --- a/fs/nfs/super.c
> > +++ b/fs/nfs/super.c
> > @@ -2548,6 +2548,27 @@ static struct vfsmount *nfs_do_root_mount(struct file_system_type *fs_type,
> > return root_mnt;
> > }
> >
> > +static void nfs_fix_devname(const struct path *path, struct vfsmount *mnt)
> > +{
> > + char *page = (char *) __get_free_page(GFP_KERNEL);
> > + char *devname, *tmp;
> > +
> > + if (page == NULL)
> > + return;
> > + devname = nfs_path(path->mnt->mnt_devname,
> > + path->mnt->mnt_root, path->dentry,
> > + page, PAGE_SIZE);
> > + if (devname == NULL)
> > + goto out_freepage;
> > + tmp = kstrdup(devname, GFP_KERNEL);
> > + if (tmp == NULL)
> > + goto out_freepage;
> > + kfree(mnt->mnt_devname);
> > + mnt->mnt_devname = tmp;
>
> (looking through patch 3 a bit) is this expected to be safe because all
> callers will send in a mnt which was privately mounted as nfs root_mnt through
> vfs_kern_mount? So that at this point noone else can have a ref to
> mnt?
>
> If that isn't the intent, then this seems problematic... (If it is, it
> seems worth commenting both so that every reader doesn't feel compelled
> to verify, and so that no new callers will naively violate that
> expectation)
The call to nfs_fix_devname() is only applied to the 'mnt_target'
vfsmount, which is the one that was passed down directly from
do_kern_mount() to the ->get_sb() method. It is entirely unreferenced by
any other process since we haven't yet called 'do_add_mount()' to
publish it.
Cheers
Trond
> thanks,
> -serge
>
> > +out_freepage:
> > + free_page((unsigned long)page);
> > +}
> > +
> > static int nfs_follow_remote_path(struct vfsmount *root_mnt,
> > const char *export_path, struct vfsmount *mnt_target)
> > {
> > @@ -2574,6 +2595,9 @@ static int nfs_follow_remote_path(struct vfsmount *root_mnt,
> > mnt_target->mnt_sb = s;
> > mnt_target->mnt_root = dget(nd.path.dentry);
> >
> > + /* Correct the device pathname */
> > + nfs_fix_devname(&nd.path, mnt_target);
> > +
> > path_put(&nd.path);
> > down_write(&s->s_umount);
> > return 0;
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> > the body of a message to [email protected]
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Trond Myklebust
Linux NFS client maintainer
NetApp
[email protected]
http://www.netapp.com