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

3 files changed, 115 insertions, 8 deletions

diff --git a/sys/fs/tmpfs/tmpfs.h b/sys/fs/tmpfs/tmpfs.h
index ffd705f..1edbc41 100644
--- a/sys/fs/tmpfs/tmpfs.h
+++ b/sys/fs/tmpfs/tmpfs.h
@@ -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
 /* --------------------------------------------------------------------- */
 
 /*
diff --git a/sys/fs/tmpfs/tmpfs_subr.c b/sys/fs/tmpfs/tmpfs_subr.c
index dad634e..8dc8338 100644
--- a/sys/fs/tmpfs/tmpfs_subr.c
+++ b/sys/fs/tmpfs/tmpfs_subr.c
@@ -124,7 +124,9 @@ tmpfs_alloc_node(struct tmpfs_mount *tmp, enum vtype type,
 		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, struct tmpfs_node *node)
 #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 tmpfs_node *node, int lkflag,
 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_node *node, struct uio *uio)
 	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] = '.';
diff --git a/sys/fs/tmpfs/tmpfs_vnops.c b/sys/fs/tmpfs/tmpfs_vnops.c
index 49a7197..c17efff 100644
--- a/sys/fs/tmpfs/tmpfs_vnops.c
+++ b/sys/fs/tmpfs/tmpfs_vnops.c
@@ -87,6 +87,11 @@ tmpfs_lookup(struct vop_cachedlookup_args *v)
 	    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;