kernel - Fix NFS panic when competing clients collide on hardlink
authorMatthew Dillon <dillon@apollo.backplane.com>
Wed, 21 Nov 2012 17:59:55 +0000 (09:59 -0800)
committerMatthew Dillon <dillon@apollo.backplane.com>
Wed, 21 Nov 2012 17:59:55 +0000 (09:59 -0800)
* Fix recursive execlusive lock on vnode.

* For example, if several boxes are nfs booting and have the same writable
  /var/log, simultanious attempts to rotate logs can cause a panic due to
  higher layers of the OS's hardlink code not being able to detect
  duplicate vnodes which are created by some other client racing the same
  operation.

* Adds CNP_NOTVP and cn_notvp flag which vop_compat_nlink() uses to notify
  NFS's VOP_LOOKUP function to check for a vnode lookup collision.

-Matt

sys/kern/vfs_default.c
sys/sys/namei.h
sys/vfs/nfs/nfs_node.c
sys/vfs/nfs/nfs_vfsops.c
sys/vfs/nfs/nfs_vnops.c
sys/vfs/nfs/nfsm_subs.c
sys/vfs/nfs/nfsnode.h

index 6b12932..8ff58b9 100644 (file)
@@ -580,14 +580,21 @@ vop_compat_nlink(struct vop_nlink_args *ap)
         * caches all information required to create the entry in the
         * directory inode.  We expect a return code of EJUSTRETURN for
         * the CREATE case.  The cnp must simulated a saved-name situation.
+        *
+        * It should not be possible for there to be a vnode collision
+        * between the source vp and target (name lookup).  However NFS
+        * clients racing each other can cause NFS to alias the same vnode
+        * across several names without the rest of the system knowing it.
+        * Use CNP_NOTVP to avoid a panic in this situation.
         */
        bzero(&cnp, sizeof(cnp));
        cnp.cn_nameiop = NAMEI_CREATE;
-       cnp.cn_flags = CNP_LOCKPARENT;
+       cnp.cn_flags = CNP_LOCKPARENT | CNP_NOTVP;
        cnp.cn_nameptr = ncp->nc_name;
        cnp.cn_namelen = ncp->nc_nlen;
        cnp.cn_cred = ap->a_cred;
        cnp.cn_td = td;
+       cnp.cn_notvp = ap->a_vp;
 
        tvp = NULL;
        error = vop_old_lookup(ap->a_head.a_ops, dvp, &tvp, &cnp);
index 8a87586..1b2dbfa 100644 (file)
@@ -71,6 +71,7 @@ struct componentname {
        long    cn_namelen;     /* length of looked up component */
        long    cn_consume;     /* chars to consume in lookup() */
        int     cn_timeout;     /* if CNP_CACHETIMEOUT is set, in ticks */
+       struct vnode *cn_notvp; /* used by NFS to check for collision */
 };
 
 #ifdef _KERNEL
@@ -97,7 +98,7 @@ struct componentname {
  */
        /* (NOCROSSMOUNT)   0x00000100 */
 #define        CNP_RDONLY          0x00000200 /* lookup with read-only semantics */
-       /* (HASBUF)         0x00000400 */
+#define CNP_NOTVP          0x00000400 /* test cn_notvp, fail if matches */
        /* (SAVENAME)       0x00000800 */
        /* (CNP_SAVESTART)  0x00001000 */
 #define CNP_ISDOTDOT       0x00002000 /* current component name is .. */
index 4520a2c..69b3170 100644 (file)
@@ -88,7 +88,8 @@ nfs_nhinit(void)
  */
 
 int
-nfs_nget(struct mount *mntp, nfsfh_t *fhp, int fhsize, struct nfsnode **npp)
+nfs_nget(struct mount *mntp, nfsfh_t *fhp, int fhsize, struct nfsnode **npp,
+        struct vnode *notvp)
 {
        struct nfsnode *np, *np2;
        struct nfsnodehashhead *nhpp;
@@ -118,6 +119,13 @@ loop:
                        continue;
                }
                vp = NFSTOV(np);
+               if (vp == notvp) {
+                       kprintf("nfs warning: client-client collision "
+                               "during rename/link/softlink\n");
+                       *npp = NULL;
+                       lwkt_reltoken(&nfsnhash_token);
+                       return (ESTALE);
+               }
                if (vget(vp, LK_EXCLUSIVE))
                        goto loop;
                LIST_FOREACH(np, nhpp, n_hash) {
@@ -213,7 +221,7 @@ loop:
  */
 int
 nfs_nget_nonblock(struct mount *mntp, nfsfh_t *fhp, int fhsize,
-                 struct nfsnode **npp)
+                 struct nfsnode **npp, struct vnode *notvp)
 {
        struct nfsnode *np, *np2;
        struct nfsnodehashhead *nhpp;
@@ -246,6 +254,12 @@ loop:
                }
                if (vp == NULL) {
                        vp = NFSTOV(np);
+                       if (vp == notvp) {
+                               kprintf("nfs warning: client-client collision "
+                                       "during rename/link/softlink\n");
+                               error = ESTALE;
+                               goto fail;
+                       }
                        if (vget(vp, LK_EXCLUSIVE | LK_NOWAIT)) {
                                error = EWOULDBLOCK;
                                goto fail;
index f96d77d..7438dea 100644 (file)
@@ -331,7 +331,7 @@ nfs_statfs(struct mount *mp, struct statfs *sbp, struct ucred *cred)
 #ifndef nolint
        sfp = NULL;
 #endif
-       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np);
+       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np, NULL);
        if (error) {
                lwkt_reltoken(&nmp->nm_token);
                return (error);
@@ -416,7 +416,7 @@ nfs_statvfs(struct mount *mp, struct statvfs *sbp, struct ucred *cred)
 #ifndef nolint
        sfp = NULL;
 #endif
-       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np);
+       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np, NULL);
        if (error) {
                lwkt_reltoken(&nmp->nm_token);
                return (error);
@@ -1119,7 +1119,7 @@ mountnfs(struct nfs_args *argp, struct mount *mp, struct sockaddr *nam,
         * this problem, because one can identify root inodes by their
         * number == ROOTINO (2).
         */
-       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np);
+       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np, NULL);
        if (error)
                goto bad;
        *vpp = NFSTOV(np);
@@ -1278,7 +1278,7 @@ nfs_root(struct mount *mp, struct vnode **vpp)
 
        nmp = VFSTONFS(mp);
        lwkt_gettoken(&nmp->nm_token);
-       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np);
+       error = nfs_nget(mp, (nfsfh_t *)nmp->nm_fh, nmp->nm_fhsize, &np, NULL);
        if (error) {
                lwkt_reltoken(&nmp->nm_token);
                return (error);
index 49762ee..74855bb 100644 (file)
@@ -1011,7 +1011,7 @@ nfs_nresolve(struct vop_nresolve_args *ap)
                vref(dvp);
                nvp = dvp;
        } else {
-               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np);
+               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np, NULL);
                if (error) {
                        m_freem(info.mrep);
                        info.mrep = NULL;
@@ -1060,6 +1060,7 @@ nfs_lookup(struct vop_old_lookup_args *ap)
        struct vnode **vpp = ap->a_vpp;
        int flags = cnp->cn_flags;
        struct vnode *newvp;
+       struct vnode *notvp;
        struct nfsmount *nmp;
        long len;
        nfsfh_t *fhp;
@@ -1073,6 +1074,8 @@ nfs_lookup(struct vop_old_lookup_args *ap)
        info.v3 = NFS_ISV3(dvp);
        error = 0;
 
+       notvp = (cnp->cn_flags & CNP_NOTVP) ? cnp->cn_notvp : NULL;
+
        /*
         * Read-only mount check and directory check.
         */
@@ -1135,7 +1138,7 @@ nfs_lookup(struct vop_old_lookup_args *ap)
                        lwkt_reltoken(&nmp->nm_token);
                        return (EISDIR);
                }
-               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np);
+               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np, notvp);
                if (error) {
                        m_freem(info.mrep);
                        info.mrep = NULL;
@@ -1165,7 +1168,7 @@ nfs_lookup(struct vop_old_lookup_args *ap)
        if (flags & CNP_ISDOTDOT) {
                vn_unlock(dvp);
                cnp->cn_flags |= CNP_PDIRUNLOCK;
-               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np);
+               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np, notvp);
                if (error) {
                        vn_lock(dvp, LK_EXCLUSIVE | LK_RETRY);
                        cnp->cn_flags &= ~CNP_PDIRUNLOCK;
@@ -1186,7 +1189,7 @@ nfs_lookup(struct vop_old_lookup_args *ap)
                vref(dvp);
                newvp = dvp;
        } else {
-               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np);
+               error = nfs_nget(dvp->v_mount, fhp, fhsize, &np, notvp);
                if (error) {
                        m_freem(info.mrep);
                        info.mrep = NULL;
@@ -2790,7 +2793,8 @@ nfs_readdirplusrpc_uio(struct vnode *vp, struct uio *uiop)
                                        goto rdfail;
                                    cache_setunresolved(&nch);
                                    error = nfs_nget_nonblock(vp->v_mount, fhp,
-                                                             fhsize, &np);
+                                                             fhsize, &np,
+                                                             NULL);
                                    if (error) {
                                        cache_put(&nch);
                                        goto rdfail;
@@ -2979,7 +2983,7 @@ nfs_lookitup(struct vnode *dvp, const char *name, int len, struct ucred *cred,
                    vref(dvp);
                    newvp = dvp;
                } else {
-                   error = nfs_nget(dvp->v_mount, nfhp, fhlen, &np);
+                   error = nfs_nget(dvp->v_mount, nfhp, fhlen, &np, NULL);
                    if (error) {
                        m_freem(info.mrep);
                        info.mrep = NULL;
index bf3f47b..d651982 100644 (file)
@@ -400,7 +400,7 @@ nfsm_mtofh(nfsm_info_t info, struct vnode *dvp, struct vnode **vpp, int *gotvpp)
        }
        if (*gotvpp) {
                NEGATIVEOUT(ttfhsize = nfsm_getfh(info, &ttfhp));
-               error = nfs_nget(dvp->v_mount, ttfhp, ttfhsize, &ttnp);
+               error = nfs_nget(dvp->v_mount, ttfhp, ttfhsize, &ttnp, NULL);
                if (error) {
                        m_freem(info->mrep);
                        info->mrep = NULL;
index 11157e2..db4dc8e 100644 (file)
@@ -226,8 +226,10 @@ int        nfs_flush (struct vnode *, int, struct thread *, int);
 
 /* other stuff */
 int    nfs_removeit (struct sillyrename *);
-int    nfs_nget (struct mount *,nfsfh_t *,int,struct nfsnode **);
-int    nfs_nget_nonblock (struct mount *,nfsfh_t *,int,struct nfsnode **);
+int    nfs_nget (struct mount *, nfsfh_t *, int,
+               struct nfsnode **, struct vnode *);
+int    nfs_nget_nonblock (struct mount *, nfsfh_t *, int,
+               struct nfsnode **, struct vnode *);
 nfsuint64 *nfs_getcookie (struct nfsnode *, off_t, int);
 void   nfs_invaldir (struct vnode *);