Date: Fri, 05 Aug 2011 10:43:32 +0200 From: Martin Matuska <mm@FreeBSD.org> To: Kostik Belousov <kostikbel@gmail.com> Cc: Xin Li <delphij@freebsd.org>, current@freebsd.org Subject: Re: panic: share -> excl @r224632 Message-ID: <4E3BAD34.2030103@FreeBSD.org> In-Reply-To: <20110805082640.GQ17489@deviant.kiev.zoral.com.ua> References: <20110804125454.GI29364@albert.catwhisker.org> <20110804135932.GG17489@deviant.kiev.zoral.com.ua> <4E3BA769.3090408@FreeBSD.org> <20110805082640.GQ17489@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format. --------------080606080909080607070704 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Patch updated. On 05.08.2011 10:26, Kostik Belousov wrote: > On Fri, Aug 05, 2011 at 10:18:49AM +0200, Martin Matuska wrote: >> I agree to Kostik's approach, but I suggest implementing it in a >> separate function and also use for the unmount() part. >> >> Please review attached patch. > Since you are moving the fragment to a function, you may somewhat reduce > the code duplication by moving at least free() and return to the end > of function and jumping to it. > > Also, after more looking at this, I think now that the check for VI_DOOMED > is not needed, due to relookup and comparision of vp and vp1. -- Martin Matuska FreeBSD committer http://blog.vx.sk --------------080606080909080607070704 Content-Type: text/x-patch; name="vfs_mount.c.patch" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="vfs_mount.c.patch" Index: sys/kern/vfs_mount.c =================================================================== --- sys/kern/vfs_mount.c (revision 224654) +++ sys/kern/vfs_mount.c (working copy) @@ -362,6 +362,58 @@ } /* + * Verify vnode's global path + */ +static int +vfs_verify_global_path(struct thread *td, struct vnode *vp, char *fspath) +{ + struct nameidata nd; + struct vnode *vp1; + char *rpath, *fbuf; + int error; + + ASSERT_VOP_ELOCKED(vp, __func__); + + /* Construct global filesystem path from vp. */ + VOP_UNLOCK(vp, 0); + error = vn_fullpath_global(td, vp, &rpath, &fbuf); + if (error != 0) { + vrele(vp); + return (error); + } + if (strlen(rpath) >= MNAMELEN) { + vrele(vp); + error = ENAMETOOLONG; + goto out; + } + + /* + * Re-lookup the vnode by path. As a side effect, the vnode is + * relocked. If vnode was renamed, return ENOENT. + */ + NDINIT(&nd, LOOKUP, FOLLOW | LOCKLEAF | MPSAFE | AUDITVNODE1, + UIO_SYSSPACE, fspath, td); + error = namei(&nd); + if (error != 0) { + vrele(vp); + goto out; + } + NDFREE(&nd, NDF_ONLY_PNBUF); + vp1 = nd.ni_vp; + vrele(vp); + if (vp1 != vp) { + vput(vp1); + error = ENOENT; + goto out; + } + + strlcpy(fspath,rpath,MNAMELEN); +out: + free(fbuf, M_TEMP); + return (error); +} + +/* * Mount a filesystem. */ int @@ -745,6 +797,7 @@ vfs_domount_first( struct thread *td, /* Calling thread. */ struct vfsconf *vfsp, /* File system type. */ + char *fspath, /* Mount path. */ struct vnode *vp, /* Vnode to be covered. */ int fsflags, /* Flags common to all filesystems. */ struct vfsoptlist **optlist /* Options local to the filesystem. */ @@ -753,25 +806,12 @@ struct vattr va; struct mount *mp; struct vnode *newdp; - char *fspath, *fbuf; int error; mtx_assert(&Giant, MA_OWNED); ASSERT_VOP_ELOCKED(vp, __func__); KASSERT((fsflags & MNT_UPDATE) == 0, ("MNT_UPDATE shouldn't be here")); - /* Construct global filesystem path from vp. */ - error = vn_fullpath_global(td, vp, &fspath, &fbuf); - if (error != 0) { - vput(vp); - return (error); - } - if (strlen(fspath) >= MNAMELEN) { - vput(vp); - free(fbuf, M_TEMP); - return (ENAMETOOLONG); - } - /* * If the user is not root, ensure that they own the directory * onto which we are attempting to mount. @@ -793,14 +833,12 @@ } if (error != 0) { vput(vp); - free(fbuf, M_TEMP); return (error); } VOP_UNLOCK(vp, 0); /* Allocate and initialize the filesystem. */ mp = vfs_mount_alloc(vp, vfsp, fspath, td->td_ucred); - free(fbuf, M_TEMP); /* XXXMAC: pass to vfs_mount_alloc? */ mp->mnt_optnew = *optlist; /* Set the mount level flags. */ @@ -1083,15 +1121,15 @@ mtx_lock(&Giant); NDFREE(&nd, NDF_ONLY_PNBUF); vp = nd.ni_vp; - if ((fsflags & MNT_UPDATE) == 0) - error = vfs_domount_first(td, vfsp, vp, fsflags, optlist); - else + if ((fsflags & MNT_UPDATE) == 0) { + error = vfs_verify_global_path(td, vp, fspath); + if (error == 0) + error = vfs_domount_first(td, vfsp, fspath, vp, + fsflags, optlist); + } else error = vfs_domount_update(td, vp, fsflags, optlist); mtx_unlock(&Giant); - ASSERT_VI_UNLOCKED(vp, __func__); - ASSERT_VOP_UNLOCKED(vp, __func__); - return (error); } @@ -1118,7 +1156,7 @@ { struct mount *mp; struct nameidata nd; - char *pathbuf, *rpathbuf, *fbuf; + char *pathbuf; int error, id0, id1; AUDIT_ARG_VALUE(uap->flags); @@ -1164,15 +1202,10 @@ UIO_SYSSPACE, pathbuf, td); if (namei(&nd) == 0) { NDFREE(&nd, NDF_ONLY_PNBUF); - if (vn_fullpath_global(td, nd.ni_vp, &rpathbuf, - &fbuf) == 0) { - if (strlen(rpathbuf) < MNAMELEN) { - strlcpy(pathbuf, rpathbuf, - MNAMELEN); - } - free(fbuf, M_TEMP); - } - vput(nd.ni_vp); + error = vfs_verify_global_path(td, nd.ni_vp, + pathbuf); + if (error == 0) + vput(nd.ni_vp); } } mtx_lock(&mountlist_mtx); --------------080606080909080607070704--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4E3BAD34.2030103>