Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/fs/puffs * fix a race i introduced almost two years ago ...



details:   https://anonhg.NetBSD.org/src/rev/6487f8467d56
branches:  trunk
changeset: 747739:6487f8467d56
user:      pooka <pooka%NetBSD.org@localhost>
date:      Wed Sep 30 18:19:17 2009 +0000

description:
* fix a race i introduced almost two years ago in rev 1.116:
  operations creating a node cannot be considered outgoing operations,
  since after return from userspace they modify file system state
  by creating a new node.  if we do not protect the file system by
  holding the directory lock, a lookup operation might race us into
  the kernel and create the node earlier.
* remove pnode from hashlish before sending the reclaim faf off to
  userspace.  also, hold pmp_lock while frobbing the list.

diffstat:

 sys/fs/puffs/puffs_node.c  |  10 +++++---
 sys/fs/puffs/puffs_vnops.c |  54 +++++++++++++++++++++------------------------
 2 files changed, 31 insertions(+), 33 deletions(-)

diffs (199 lines):

diff -r eb79b0b1a7eb -r 6487f8467d56 sys/fs/puffs/puffs_node.c
--- a/sys/fs/puffs/puffs_node.c Wed Sep 30 18:17:22 2009 +0000
+++ b/sys/fs/puffs/puffs_node.c Wed Sep 30 18:19:17 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: puffs_node.c,v 1.13 2008/05/06 12:33:16 ad Exp $       */
+/*     $NetBSD: puffs_node.c,v 1.14 2009/09/30 18:19:17 pooka Exp $    */
 
 /*
  * Copyright (c) 2005, 2006, 2007  Antti Kantee.  All Rights Reserved.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: puffs_node.c,v 1.13 2008/05/06 12:33:16 ad Exp $");
+__KERNEL_RCSID(0, "$NetBSD: puffs_node.c,v 1.14 2009/09/30 18:19:17 pooka Exp $");
 
 #include <sys/param.h>
 #include <sys/hash.h>
@@ -223,7 +223,7 @@
                if (pnc->pnc_cookie == ck) {
                        mutex_exit(&pmp->pmp_lock);
                        puffs_senderr(pmp, PUFFS_ERR_MAKENODE, EEXIST,
-                           "cookie exists", ck);
+                           "newcookie exists", ck);
                        return EPROTO;
                }
        }
@@ -260,7 +260,6 @@
                panic("puffs_putvnode: %p not a puffs vnode", vp);
 #endif
 
-       LIST_REMOVE(pnode, pn_hashent);
        genfs_node_destroy(vp);
        puffs_releasenode(pnode);
        vp->v_data = NULL;
@@ -336,6 +335,9 @@
         */
        mutex_enter(&pmp->pmp_lock);
        if (pmp->pmp_root) {
+               struct puffs_node *pnode = vp->v_data;
+
+               LIST_REMOVE(pnode, pn_hashent);
                mutex_exit(&pmp->pmp_lock);
                puffs_putvnode(vp);
                goto retry;
diff -r eb79b0b1a7eb -r 6487f8467d56 sys/fs/puffs/puffs_vnops.c
--- a/sys/fs/puffs/puffs_vnops.c        Wed Sep 30 18:17:22 2009 +0000
+++ b/sys/fs/puffs/puffs_vnops.c        Wed Sep 30 18:19:17 2009 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: puffs_vnops.c,v 1.133 2009/09/19 11:44:19 pooka Exp $  */
+/*     $NetBSD: puffs_vnops.c,v 1.134 2009/09/30 18:19:17 pooka Exp $  */
 
 /*
  * Copyright (c) 2005, 2006, 2007  Antti Kantee.  All Rights Reserved.
@@ -30,7 +30,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: puffs_vnops.c,v 1.133 2009/09/19 11:44:19 pooka Exp $");
+__KERNEL_RCSID(0, "$NetBSD: puffs_vnops.c,v 1.134 2009/09/30 18:19:17 pooka Exp $");
 
 #include <sys/param.h>
 #include <sys/buf.h>
@@ -645,16 +645,7 @@
        create_msg->pvnr_va = *ap->a_vap;
        puffs_msg_setinfo(park_create, PUFFSOP_VN,
            PUFFS_VN_CREATE, VPTOPNC(dvp));
-
-       /*
-        * Do the dance:
-        * + insert into queue ("interlock")
-        * + unlock vnode
-        * + wait for response
-        */
-       puffs_msg_enqueue(pmp, park_create);
-       REFPN_AND_UNLOCKVP(dvp, dpn);
-       error = puffs_msg_wait2(pmp, park_create, dpn, NULL);
+       PUFFS_MSG_ENQUEUEWAIT2(pmp, park_create, dvp->v_data, NULL, error);
 
        error = checkerr(pmp, error, __func__);
        if (error)
@@ -667,10 +658,10 @@
                    create_msg->pvnr_newnode, cnp);
 
  out:
+       vput(dvp);
        if (error || (cnp->cn_flags & SAVESTART) == 0)
                PNBUF_PUT(cnp->cn_pnbuf);
 
-       RELEPN_AND_VP(dvp, dpn);
        DPRINTF(("puffs_create: return %d\n", error));
        PUFFS_MSG_RELEASE(create);
        return error;
@@ -701,9 +692,7 @@
        puffs_msg_setinfo(park_mknod, PUFFSOP_VN,
            PUFFS_VN_MKNOD, VPTOPNC(dvp));
 
-       puffs_msg_enqueue(pmp, park_mknod);
-       REFPN_AND_UNLOCKVP(dvp, dpn);
-       error = puffs_msg_wait2(pmp, park_mknod, dpn, NULL);
+       PUFFS_MSG_ENQUEUEWAIT2(pmp, park_mknod, dvp->v_data, NULL, error);
 
        error = checkerr(pmp, error, __func__);
        if (error)
@@ -717,10 +706,10 @@
                    mknod_msg->pvnr_newnode, cnp);
 
  out:
+       vput(dvp);
        PUFFS_MSG_RELEASE(mknod);
        if (error || (cnp->cn_flags & SAVESTART) == 0)
                PNBUF_PUT(cnp->cn_pnbuf);
-       RELEPN_AND_VP(dvp, dpn);
        return error;
 }
 
@@ -1074,6 +1063,8 @@
        } */ *ap = v;
        struct vnode *vp = ap->a_vp;
        struct puffs_mount *pmp = MPTOPUFFSMP(vp->v_mount);
+       struct puffs_node *pnode = vp->v_data;
+       bool notifyserver = true;
 
        /*
         * first things first: check if someone is trying to reclaim the
@@ -1086,14 +1077,23 @@
                KASSERT(pmp->pmp_root != NULL);
                pmp->pmp_root = NULL;
                mutex_exit(&pmp->pmp_lock);
-               goto out;
+               notifyserver = false;
        }
 
-       callreclaim(MPTOPUFFSMP(vp->v_mount), VPTOPNC(vp));
-
- out:
+       /*
+        * purge info from kernel before issueing FAF, since we
+        * don't really know when we'll get around to it after
+        * that and someone might race us into node creation
+        */
+       mutex_enter(&pmp->pmp_lock);
+       LIST_REMOVE(pnode, pn_hashent);
+       mutex_exit(&pmp->pmp_lock);
        if (PUFFS_USE_NAMECACHE(pmp))
                cache_purge(vp);
+
+       if (notifyserver)
+               callreclaim(MPTOPUFFSMP(vp->v_mount), VPTOPNC(vp));
+
        puffs_putvnode(vp);
 
        return 0;
@@ -1476,9 +1476,7 @@
        puffs_msg_setinfo(park_mkdir, PUFFSOP_VN,
            PUFFS_VN_MKDIR, VPTOPNC(dvp));
 
-       puffs_msg_enqueue(pmp, park_mkdir);
-       REFPN_AND_UNLOCKVP(dvp, dpn);
-       error = puffs_msg_wait2(pmp, park_mkdir, dpn, NULL);
+       PUFFS_MSG_ENQUEUEWAIT2(pmp, park_mkdir, dvp->v_data, NULL, error);
 
        error = checkerr(pmp, error, __func__);
        if (error)
@@ -1491,10 +1489,10 @@
                    mkdir_msg->pvnr_newnode, cnp);
 
  out:
+       vput(dvp);
        PUFFS_MSG_RELEASE(mkdir);
        if (error || (cnp->cn_flags & SAVESTART) == 0)
                PNBUF_PUT(cnp->cn_pnbuf);
-       RELEPN_AND_VP(dvp, dpn);
        return error;
 }
 
@@ -1637,9 +1635,7 @@
        puffs_msg_setinfo(park_symlink, PUFFSOP_VN,
            PUFFS_VN_SYMLINK, VPTOPNC(dvp));
 
-       puffs_msg_enqueue(pmp, park_symlink);
-       REFPN_AND_UNLOCKVP(dvp, dpn);
-       error = puffs_msg_wait2(pmp, park_symlink, dpn, NULL);
+       PUFFS_MSG_ENQUEUEWAIT2(pmp, park_symlink, dvp->v_data, NULL, error);
 
        error = checkerr(pmp, error, __func__);
        if (error)
@@ -1652,10 +1648,10 @@
                    symlink_msg->pvnr_newnode, cnp);
 
  out:
+       vput(dvp);
        PUFFS_MSG_RELEASE(symlink);
        if (error || (cnp->cn_flags & SAVESTART) == 0)
                PNBUF_PUT(cnp->cn_pnbuf);
-       RELEPN_AND_VP(dvp, dpn);
 
        return error;
 }



Home | Main Index | Thread Index | Old Index