From owner-freebsd-current@FreeBSD.ORG Fri Aug 5 08:43:38 2011 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id F2A531065670; Fri, 5 Aug 2011 08:43:38 +0000 (UTC) (envelope-from mm@FreeBSD.org) Received: from mail.vx.sk (mail.vx.sk [IPv6:2a01:4f8:100:1043::3]) by mx1.freebsd.org (Postfix) with ESMTP id 791598FC0C; Fri, 5 Aug 2011 08:43:38 +0000 (UTC) Received: from core.vx.sk (localhost [127.0.0.1]) by mail.vx.sk (Postfix) with ESMTP id 92FAB18D6EF; Fri, 5 Aug 2011 10:43:37 +0200 (CEST) X-Virus-Scanned: amavisd-new at mail.vx.sk Received: from mail.vx.sk ([127.0.0.1]) by core.vx.sk (mail.vx.sk [127.0.0.1]) (amavisd-new, port 10024) with LMTP id 8e+YbMkr+1Bv; Fri, 5 Aug 2011 10:43:32 +0200 (CEST) Received: from [10.0.3.3] (188-167-66-148.dynamic.chello.sk [188.167.66.148]) by mail.vx.sk (Postfix) with ESMTPSA id 9D21318D6E3; Fri, 5 Aug 2011 10:43:32 +0200 (CEST) Message-ID: <4E3BAD34.2030103@FreeBSD.org> Date: Fri, 05 Aug 2011 10:43:32 +0200 From: Martin Matuska User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:5.0) Gecko/20110627 Thunderbird/5.0 MIME-Version: 1.0 To: Kostik Belousov References: <20110804125454.GI29364@albert.catwhisker.org> <20110804135932.GG17489@deviant.kiev.zoral.com.ua> <4E3BA769.3090408@FreeBSD.org> <20110805082640.GQ17489@deviant.kiev.zoral.com.ua> In-Reply-To: <20110805082640.GQ17489@deviant.kiev.zoral.com.ua> X-Enigmail-Version: 1.2pre Content-Type: multipart/mixed; boundary="------------080606080909080607070704" Cc: Xin Li , current@freebsd.org Subject: Re: panic: share -> excl @r224632 X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Aug 2011 08:43:39 -0000 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--