From owner-svn-src-all@FreeBSD.ORG Mon Dec 29 12:07:18 2008 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9AD51106564A; Mon, 29 Dec 2008 12:07:18 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 814B98FC18; Mon, 29 Dec 2008 12:07:18 +0000 (UTC) (envelope-from kib@FreeBSD.org) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id mBTC7IxH056728; Mon, 29 Dec 2008 12:07:18 GMT (envelope-from kib@svn.freebsd.org) Received: (from kib@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id mBTC7IOg056727; Mon, 29 Dec 2008 12:07:18 GMT (envelope-from kib@svn.freebsd.org) Message-Id: <200812291207.mBTC7IOg056727@svn.freebsd.org> From: Konstantin Belousov Date: Mon, 29 Dec 2008 12:07:18 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r186560 - head/sys/fs/pseudofs X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 29 Dec 2008 12:07:18 -0000 Author: kib Date: Mon Dec 29 12:07:18 2008 New Revision: 186560 URL: http://svn.freebsd.org/changeset/base/186560 Log: After the pfs_vncache_mutex is dropped, another thread may attempt to do pfs_vncache_alloc() for the same pfs_node and pid. In this case, we could end up with two vnodes for the pair. Recheck the cache under the locked pfs_vncache_mutex after all sleeping operations are done [1]. This case mostly cannot happen now because pseudofs uses exclusive vnode locking for lookup. But it does drop the vnode lock for dotdot lookups, and Marcus' pseudofs_vptocnp implementation is vulnerable too. Do not call free() on the struct pfs_vdata after insmntque() failure, because vp->v_data points to the structure, and pseudofs_reclaim() frees it by the call to pfs_vncache_free(). Tested by: pho [1] Approved by: des MFC after: 2 weeks Modified: head/sys/fs/pseudofs/pseudofs_vncache.c Modified: head/sys/fs/pseudofs/pseudofs_vncache.c ============================================================================== --- head/sys/fs/pseudofs/pseudofs_vncache.c Mon Dec 29 10:26:02 2008 (r186559) +++ head/sys/fs/pseudofs/pseudofs_vncache.c Mon Dec 29 12:07:18 2008 (r186560) @@ -111,7 +111,7 @@ int pfs_vncache_alloc(struct mount *mp, struct vnode **vpp, struct pfs_node *pn, pid_t pid) { - struct pfs_vdata *pvd; + struct pfs_vdata *pvd, *pvd2; struct vnode *vp; int error; @@ -146,19 +146,11 @@ retry: } } mtx_unlock(&pfs_vncache_mutex); - ++pfs_vncache_misses; /* nope, get a new one */ pvd = malloc(sizeof *pvd, M_PFSVNCACHE, M_WAITOK); - mtx_lock(&pfs_vncache_mutex); - if (++pfs_vncache_entries > pfs_vncache_maxentries) - pfs_vncache_maxentries = pfs_vncache_entries; - mtx_unlock(&pfs_vncache_mutex); error = getnewvnode("pseudofs", mp, &pfs_vnodeops, vpp); if (error) { - mtx_lock(&pfs_vncache_mutex); - --pfs_vncache_entries; - mtx_unlock(&pfs_vncache_mutex); free(pvd, M_PFSVNCACHE); return (error); } @@ -200,14 +192,35 @@ retry: vn_lock(*vpp, LK_EXCLUSIVE | LK_RETRY); error = insmntque(*vpp, mp); if (error != 0) { - mtx_lock(&pfs_vncache_mutex); - --pfs_vncache_entries; - mtx_unlock(&pfs_vncache_mutex); - free(pvd, M_PFSVNCACHE); *vpp = NULLVP; return (error); } +retry2: mtx_lock(&pfs_vncache_mutex); + /* + * Other thread may race with us, creating the entry we are + * going to insert into the cache. Recheck after + * pfs_vncache_mutex is reacquired. + */ + for (pvd2 = pfs_vncache; pvd2; pvd2 = pvd2->pvd_next) { + if (pvd2->pvd_pn == pn && pvd2->pvd_pid == pid && + pvd2->pvd_vnode->v_mount == mp) { + vp = pvd2->pvd_vnode; + VI_LOCK(vp); + mtx_unlock(&pfs_vncache_mutex); + if (vget(vp, LK_EXCLUSIVE | LK_INTERLOCK, curthread) == 0) { + ++pfs_vncache_hits; + vgone(*vpp); + *vpp = vp; + cache_purge(vp); + return (0); + } + goto retry2; + } + } + ++pfs_vncache_misses; + if (++pfs_vncache_entries > pfs_vncache_maxentries) + pfs_vncache_maxentries = pfs_vncache_entries; pvd->pvd_prev = NULL; pvd->pvd_next = pfs_vncache; if (pvd->pvd_next)