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
This commit is contained in:
Xin LI 2009-10-11 07:03:56 +00:00
parent 0b4b0b0fee
commit 82cf92d483
3 changed files with 115 additions and 8 deletions

View file

@ -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
/* --------------------------------------------------------------------- */
/*

View file

@ -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] = '.';

View file

@ -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;