From owner-svn-src-all@FreeBSD.ORG Sun Oct 11 07:03:57 2009 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 1FBF1106566B; Sun, 11 Oct 2009 07:03:57 +0000 (UTC) (envelope-from delphij@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id 0E04F8FC15; Sun, 11 Oct 2009 07:03:57 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id n9B73u3G003726; Sun, 11 Oct 2009 07:03:56 GMT (envelope-from delphij@svn.freebsd.org) Received: (from delphij@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id n9B73uSs003722; Sun, 11 Oct 2009 07:03:56 GMT (envelope-from delphij@svn.freebsd.org) Message-Id: <200910110703.n9B73uSs003722@svn.freebsd.org> From: Xin LI Date: Sun, 11 Oct 2009 07:03:56 +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: r197953 - head/sys/fs/tmpfs 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: Sun, 11 Oct 2009 07:03:57 -0000 Author: delphij Date: Sun Oct 11 07:03:56 2009 New Revision: 197953 URL: http://svn.freebsd.org/changeset/base/197953 Log: Add locking around access to parent node, and bail out when the parent node is already freed rather than panicking the system. PR: kern/122038 Submitted by: gk Tested by: pho MFC after: 1 week Modified: head/sys/fs/tmpfs/tmpfs.h head/sys/fs/tmpfs/tmpfs_subr.c head/sys/fs/tmpfs/tmpfs_vnops.c Modified: head/sys/fs/tmpfs/tmpfs.h ============================================================================== --- head/sys/fs/tmpfs/tmpfs.h Sun Oct 11 05:59:43 2009 (r197952) +++ head/sys/fs/tmpfs/tmpfs.h Sun Oct 11 07:03:56 2009 (r197953) @@ -303,10 +303,30 @@ LIST_HEAD(tmpfs_node_list, tmpfs_node); #define TMPFS_NODE_LOCK(node) mtx_lock(&(node)->tn_interlock) #define TMPFS_NODE_UNLOCK(node) mtx_unlock(&(node)->tn_interlock) -#define TMPFS_NODE_MTX(node) (&(node)->tn_interlock) +#define TMPFS_NODE_MTX(node) (&(node)->tn_interlock) + +#ifdef INVARIANTS +#define TMPFS_ASSERT_LOCKED(node) do { \ + MPASS(node != NULL); \ + MPASS(node->tn_vnode != NULL); \ + if (!VOP_ISLOCKED(node->tn_vnode) && \ + !mtx_owned(TMPFS_NODE_MTX(node))) \ + panic("tmpfs: node is not locked: %p", node); \ + } while (0) +#define TMPFS_ASSERT_ELOCKED(node) do { \ + MPASS((node) != NULL); \ + MPASS((node)->tn_vnode != NULL); \ + mtx_assert(TMPFS_NODE_MTX(node), MA_OWNED); \ + ASSERT_VOP_LOCKED((node)->tn_vnode, "tmpfs"); \ + } while (0) +#else +#define TMPFS_ASSERT_LOCKED(node) (void)0 +#define TMPFS_ASSERT_ELOCKED(node) (void)0 +#endif #define TMPFS_VNODE_ALLOCATING 1 #define TMPFS_VNODE_WANT 2 +#define TMPFS_VNODE_DOOMED 4 /* --------------------------------------------------------------------- */ /* Modified: head/sys/fs/tmpfs/tmpfs_subr.c ============================================================================== --- head/sys/fs/tmpfs/tmpfs_subr.c Sun Oct 11 05:59:43 2009 (r197952) +++ head/sys/fs/tmpfs/tmpfs_subr.c Sun Oct 11 07:03:56 2009 (r197953) @@ -124,7 +124,9 @@ tmpfs_alloc_node(struct tmpfs_mount *tmp nnode->tn_dir.tn_readdir_lastn = 0; nnode->tn_dir.tn_readdir_lastp = NULL; nnode->tn_links++; + TMPFS_NODE_LOCK(nnode->tn_dir.tn_parent); nnode->tn_dir.tn_parent->tn_links++; + TMPFS_NODE_UNLOCK(nnode->tn_dir.tn_parent); break; case VFIFO: @@ -187,6 +189,7 @@ tmpfs_free_node(struct tmpfs_mount *tmp, #ifdef INVARIANTS TMPFS_NODE_LOCK(node); MPASS(node->tn_vnode == NULL); + MPASS((node->tn_vpstate & TMPFS_VNODE_ALLOCATING) == 0); TMPFS_NODE_UNLOCK(node); #endif @@ -312,6 +315,7 @@ tmpfs_alloc_vp(struct mount *mp, struct loop: TMPFS_NODE_LOCK(node); if ((vp = node->tn_vnode) != NULL) { + MPASS((node->tn_vpstate & TMPFS_VNODE_DOOMED) == 0); VI_LOCK(vp); TMPFS_NODE_UNLOCK(node); vholdl(vp); @@ -330,6 +334,14 @@ loop: goto out; } + if ((node->tn_vpstate & TMPFS_VNODE_DOOMED) || + (node->tn_type == VDIR && node->tn_dir.tn_parent == NULL)) { + TMPFS_NODE_UNLOCK(node); + error = ENOENT; + vp = NULL; + goto out; + } + /* * otherwise lock the vp list while we call getnewvnode * since that can block. @@ -375,6 +387,7 @@ loop: vp->v_op = &tmpfs_fifoop_entries; break; case VDIR: + MPASS(node->tn_dir.tn_parent != NULL); if (node->tn_dir.tn_parent == node) vp->v_vflag |= VV_ROOT; break; @@ -428,10 +441,9 @@ tmpfs_free_vp(struct vnode *vp) node = VP_TO_TMPFS_NODE(vp); - TMPFS_NODE_LOCK(node); + mtx_assert(TMPFS_NODE_MTX(node), MA_OWNED); node->tn_vnode = NULL; vp->v_data = NULL; - TMPFS_NODE_UNLOCK(node); } /* --------------------------------------------------------------------- */ @@ -653,7 +665,18 @@ tmpfs_dir_getdotdotdent(struct tmpfs_nod TMPFS_VALIDATE_DIR(node); MPASS(uio->uio_offset == TMPFS_DIRCOOKIE_DOTDOT); + /* + * Return ENOENT if the current node is already removed. + */ + TMPFS_ASSERT_LOCKED(node); + if (node->tn_dir.tn_parent == NULL) { + return (ENOENT); + } + + TMPFS_NODE_LOCK(node->tn_dir.tn_parent); dent.d_fileno = node->tn_dir.tn_parent->tn_id; + TMPFS_NODE_UNLOCK(node->tn_dir.tn_parent); + dent.d_type = DT_DIR; dent.d_namlen = 2; dent.d_name[0] = '.'; Modified: head/sys/fs/tmpfs/tmpfs_vnops.c ============================================================================== --- head/sys/fs/tmpfs/tmpfs_vnops.c Sun Oct 11 05:59:43 2009 (r197952) +++ head/sys/fs/tmpfs/tmpfs_vnops.c Sun Oct 11 07:03:56 2009 (r197953) @@ -87,6 +87,11 @@ tmpfs_lookup(struct vop_cachedlookup_arg dnode->tn_dir.tn_parent == dnode, !(cnp->cn_flags & ISDOTDOT))); + TMPFS_ASSERT_LOCKED(dnode); + if (dnode->tn_dir.tn_parent == NULL) { + error = ENOENT; + goto out; + } if (cnp->cn_flags & ISDOTDOT) { int ltype = 0; @@ -914,6 +919,7 @@ tmpfs_rename(struct vop_rename_args *v) char *newname; int error; struct tmpfs_dirent *de; + struct tmpfs_mount *tmp; struct tmpfs_node *fdnode; struct tmpfs_node *fnode; struct tmpfs_node *tnode; @@ -934,6 +940,7 @@ tmpfs_rename(struct vop_rename_args *v) goto out; } + tmp = VFS_TO_TMPFS(tdvp->v_mount); tdnode = VP_TO_TMPFS_DIR(tdvp); /* If source and target are the same file, there is nothing to do. */ @@ -1018,25 +1025,63 @@ tmpfs_rename(struct vop_rename_args *v) * directory being moved. Otherwise, we'd end up * with stale nodes. */ n = tdnode; + /* TMPFS_LOCK garanties that no nodes are freed while + * traversing the list. Nodes can only be marked as + * removed: tn_parent == NULL. */ + TMPFS_LOCK(tmp); + TMPFS_NODE_LOCK(n); while (n != n->tn_dir.tn_parent) { + struct tmpfs_node *parent; + if (n == fnode) { + TMPFS_NODE_UNLOCK(n); + TMPFS_UNLOCK(tmp); error = EINVAL; if (newname != NULL) free(newname, M_TMPFSNAME); goto out_locked; } - n = n->tn_dir.tn_parent; + parent = n->tn_dir.tn_parent; + TMPFS_NODE_UNLOCK(n); + if (parent == NULL) { + n = NULL; + break; + } + TMPFS_NODE_LOCK(parent); + if (parent->tn_dir.tn_parent == NULL) { + TMPFS_NODE_UNLOCK(parent); + n = NULL; + break; + } + n = parent; + } + TMPFS_UNLOCK(tmp); + if (n == NULL) { + error = EINVAL; + if (newname != NULL) + free(newname, M_TMPFSNAME); + goto out_locked; } + TMPFS_NODE_UNLOCK(n); /* Adjust the parent pointer. */ TMPFS_VALIDATE_DIR(fnode); + TMPFS_NODE_LOCK(de->td_node); de->td_node->tn_dir.tn_parent = tdnode; + TMPFS_NODE_UNLOCK(de->td_node); /* As a result of changing the target of the '..' * entry, the link count of the source and target * directories has to be adjusted. */ - fdnode->tn_links--; + TMPFS_NODE_LOCK(tdnode); + TMPFS_ASSERT_LOCKED(tdnode); tdnode->tn_links++; + TMPFS_NODE_UNLOCK(tdnode); + + TMPFS_NODE_LOCK(fdnode); + TMPFS_ASSERT_LOCKED(fdnode); + fdnode->tn_links--; + TMPFS_NODE_UNLOCK(fdnode); } /* Do the move: just remove the entry from the source directory @@ -1163,15 +1208,26 @@ tmpfs_rmdir(struct vop_rmdir_args *v) goto out; } + /* Detach the directory entry from the directory (dnode). */ tmpfs_dir_detach(dvp, de); + /* No vnode should be allocated for this entry from this point */ + TMPFS_NODE_LOCK(node); + TMPFS_ASSERT_ELOCKED(node); node->tn_links--; + node->tn_dir.tn_parent = NULL; node->tn_status |= TMPFS_NODE_ACCESSED | TMPFS_NODE_CHANGED | \ TMPFS_NODE_MODIFIED; - node->tn_dir.tn_parent->tn_links--; - node->tn_dir.tn_parent->tn_status |= TMPFS_NODE_ACCESSED | \ + + TMPFS_NODE_UNLOCK(node); + + TMPFS_NODE_LOCK(dnode); + TMPFS_ASSERT_ELOCKED(dnode); + dnode->tn_links--; + dnode->tn_status |= TMPFS_NODE_ACCESSED | \ TMPFS_NODE_CHANGED | TMPFS_NODE_MODIFIED; + TMPFS_NODE_UNLOCK(dnode); cache_purge(dvp); cache_purge(vp); @@ -1359,13 +1415,21 @@ tmpfs_reclaim(struct vop_reclaim_args *v vnode_destroy_vobject(vp); cache_purge(vp); + + TMPFS_NODE_LOCK(node); + TMPFS_ASSERT_ELOCKED(node); tmpfs_free_vp(vp); /* If the node referenced by this vnode was deleted by the user, * we must free its associated data structures (now that the vnode * is being reclaimed). */ - if (node->tn_links == 0) + if (node->tn_links == 0 && + (node->tn_vpstate & TMPFS_VNODE_ALLOCATING) == 0) { + node->tn_vpstate = TMPFS_VNODE_DOOMED; + TMPFS_NODE_UNLOCK(node); tmpfs_free_node(tmp, node); + } else + TMPFS_NODE_UNLOCK(node); MPASS(vp->v_data == NULL); return 0;