Source-Changes-HG archive

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

[src/trunk]: src/sys/nfs fix access-after-free bugs in dircache code by refco...



details:   https://anonhg.NetBSD.org/src/rev/d284be9c0325
branches:  trunk
changeset: 569983:d284be9c0325
user:      yamt <yamt%NetBSD.org@localhost>
date:      Wed Sep 15 09:50:56 2004 +0000

description:
fix access-after-free bugs in dircache code by refcounting nfsdircache.
PR/26864.

diffstat:

 sys/nfs/nfs_bio.c  |   12 ++-
 sys/nfs/nfs_subs.c |  189 ++++++++++++++++++++++++++++++++++++++++------------
 sys/nfs/nfs_var.h  |    3 +-
 sys/nfs/nfsnode.h  |    8 +-
 4 files changed, 163 insertions(+), 49 deletions(-)

diffs (truncated from 449 to 300 lines):

diff -r f133252cf722 -r d284be9c0325 sys/nfs/nfs_bio.c
--- a/sys/nfs/nfs_bio.c Wed Sep 15 09:21:22 2004 +0000
+++ b/sys/nfs/nfs_bio.c Wed Sep 15 09:50:56 2004 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_bio.c,v 1.119 2004/07/18 07:43:00 yamt Exp $       */
+/*     $NetBSD: nfs_bio.c,v 1.120 2004/09/15 09:50:56 yamt Exp $       */
 
 /*
  * Copyright (c) 1989, 1993
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_bio.c,v 1.119 2004/07/18 07:43:00 yamt Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_bio.c,v 1.120 2004/09/15 09:50:56 yamt Exp $");
 
 #include "opt_nfs.h"
 #include "opt_ddb.h"
@@ -282,6 +282,7 @@
 
                if (uio->uio_offset != 0 &&
                    ndp->dc_cookie == np->n_direofoffset) {
+                       nfs_putdircache(np, ndp);
                        nfsstats.direofcache_hits++;
                        return (0);
                }
@@ -299,6 +300,7 @@
                         * server. Punt and let the userland code
                         * deal with it.
                         */
+                       nfs_putdircache(np, ndp);
                        brelse(bp);
                        if (error == NFSERR_BAD_COOKIE) {
                            nfs_invaldircache(vp, 0);
@@ -317,6 +319,7 @@
                 */
                if (np->n_direofoffset != 0 && 
                        ndp->dc_blkcookie == np->n_direofoffset) {
+                       nfs_putdircache(np, ndp);
                        brelse(bp);
                        return (0);
                }
@@ -351,6 +354,7 @@
                                (unsigned long)uio->uio_offset,
                                (unsigned long)NFS_GETCOOKIE(pdp));
 #endif
+                       nfs_putdircache(np, ndp);
                        brelse(bp);
                        nfs_invaldircache(vp, 0);
                        nfs_vinvalbuf(vp, 0, cred, p, 0);
@@ -393,11 +397,13 @@
                                        NFS_STASHCOOKIE32(pdp,
                                            nndp->dc_cookie32);
                                }
+                               nfs_putdircache(np, nndp);
                        }
                        pdp = dp;
                        dp = (struct dirent *)((caddr_t)dp + dp->d_reclen);
                        enn++;
                }
+               nfs_putdircache(np, ndp);
 
                /*
                 * If the last requested entry was not the last in the
@@ -414,6 +420,7 @@
                                NFS_STASHCOOKIE32(pdp, nndp->dc_cookie32);
                                curoff = nndp->dc_cookie32;
                        }
+                       nfs_putdircache(np, nndp);
                } else
                        curoff = bp->b_dcookie;
 
@@ -452,6 +459,7 @@
                                brelse(rabp);
                        }
                }
+               nfs_putdircache(np, nndp);
                got_buf = 1;
                break;
            default:
diff -r f133252cf722 -r d284be9c0325 sys/nfs/nfs_subs.c
--- a/sys/nfs/nfs_subs.c        Wed Sep 15 09:21:22 2004 +0000
+++ b/sys/nfs/nfs_subs.c        Wed Sep 15 09:50:56 2004 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_subs.c,v 1.134 2004/06/14 12:28:35 yamt Exp $      */
+/*     $NetBSD: nfs_subs.c,v 1.135 2004/09/15 09:50:56 yamt Exp $      */
 
 /*
  * Copyright (c) 1989, 1993
@@ -70,7 +70,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_subs.c,v 1.134 2004/06/14 12:28:35 yamt Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_subs.c,v 1.135 2004/09/15 09:50:56 yamt Exp $");
 
 #include "fs_nfs.h"
 #include "opt_nfs.h"
@@ -1214,19 +1214,32 @@
        return sum;
 }
 
+#define        _NFSDC_MTX(np)          (&NFSTOV(np)->v_interlock)
+#define        NFSDC_LOCK(np)          simple_lock(_NFSDC_MTX(np))
+#define        NFSDC_UNLOCK(np)        simple_unlock(_NFSDC_MTX(np))
+#define        NFSDC_ASSERT_LOCKED(np) LOCK_ASSERT(simple_lock_held(_NFSDC_MTX(np)))
+
 void
 nfs_initdircache(vp)
        struct vnode *vp;
 {
        struct nfsnode *np = VTONFS(vp);
+       struct nfsdirhashhead *dircache;
 
-       KASSERT(np->n_dircache == NULL);
+       dircache = hashinit(NFS_DIRHASHSIZ, HASH_LIST, M_NFSDIROFF,
+           M_WAITOK, &nfsdirhashmask);
 
-       np->n_dircachesize = 0;
-       np->n_dblkno = 1;
-       np->n_dircache = hashinit(NFS_DIRHASHSIZ, HASH_LIST, M_NFSDIROFF,
-           M_WAITOK, &nfsdirhashmask);
-       TAILQ_INIT(&np->n_dirchain);
+       NFSDC_LOCK(np);
+       if (np->n_dircache == NULL) {
+               np->n_dircachesize = 0;
+               np->n_dblkno = 1;
+               np->n_dircache = dircache;
+               dircache = NULL;
+               TAILQ_INIT(&np->n_dirchain);
+       }
+       NFSDC_UNLOCK(np);
+       if (dircache)
+               hashdone(dircache, M_NFSDIROFF);
 }
 
 void
@@ -1234,16 +1247,83 @@
        struct vnode *vp;
 {
        struct nfsnode *np = VTONFS(vp);
+       unsigned *dirgens;
 
        KASSERT(VFSTONFS(vp->v_mount)->nm_flag & NFSMNT_XLATECOOKIE);
-       KASSERT(np->n_dirgens == NULL);
 
-       MALLOC(np->n_dirgens, unsigned *,
-           NFS_DIRHASHSIZ * sizeof (unsigned), M_NFSDIROFF, M_WAITOK);
-       memset((caddr_t)np->n_dirgens, 0, NFS_DIRHASHSIZ * sizeof (unsigned));
+       dirgens = malloc(NFS_DIRHASHSIZ * sizeof (unsigned), M_NFSDIROFF,
+           M_WAITOK|M_ZERO);
+       NFSDC_LOCK(np);
+       if (np->n_dirgens == NULL) {
+               np->n_dirgens = dirgens;
+               dirgens = NULL;
+       }
+       NFSDC_UNLOCK(np);
+       if (dirgens)
+               free(dirgens, M_NFSDIROFF);
 }
 
-static struct nfsdircache dzero = {0, 0, {0, 0}, {0, 0}, 0, 0, 0};
+static const struct nfsdircache dzero;
+
+static void nfs_unlinkdircache __P((struct nfsnode *np, struct nfsdircache *));
+static void nfs_putdircache_unlocked __P((struct nfsnode *,
+    struct nfsdircache *));
+
+static void
+nfs_unlinkdircache(np, ndp)
+       struct nfsnode *np;
+       struct nfsdircache *ndp;
+{
+
+       NFSDC_ASSERT_LOCKED(np);
+       KASSERT(ndp != &dzero);
+
+       if (LIST_NEXT(ndp, dc_hash) == (void *)-1)
+               return;
+
+       TAILQ_REMOVE(&np->n_dirchain, ndp, dc_chain);
+       LIST_REMOVE(ndp, dc_hash);
+       LIST_NEXT(ndp, dc_hash) = (void *)-1; /* mark as unlinked */
+
+       nfs_putdircache_unlocked(np, ndp);
+}
+
+void
+nfs_putdircache(np, ndp)
+       struct nfsnode *np;
+       struct nfsdircache *ndp;
+{
+       int ref;
+
+       if (ndp == &dzero)
+               return;
+
+       KASSERT(ndp->dc_refcnt > 0);
+       NFSDC_LOCK(np);
+       ref = --ndp->dc_refcnt;
+       NFSDC_UNLOCK(np);
+
+       if (ref == 0)
+               free(ndp, M_NFSDIROFF);
+}
+
+static void
+nfs_putdircache_unlocked(np, ndp)
+       struct nfsnode *np;
+       struct nfsdircache *ndp;
+{
+       int ref;
+
+       NFSDC_ASSERT_LOCKED(np);
+
+       if (ndp == &dzero)
+               return;
+
+       KASSERT(ndp->dc_refcnt > 0);
+       ref = --ndp->dc_refcnt;
+       if (ref == 0)
+               free(ndp, M_NFSDIROFF);
+}
 
 struct nfsdircache *
 nfs_searchdircache(vp, off, do32, hashent)
@@ -1261,7 +1341,8 @@
         * Zero is always a valid cookie.
         */
        if (off == 0)
-               return &dzero;
+               /* LINTED const cast away */
+               return (struct nfsdircache *)&dzero;
 
        if (!np->n_dircache)
                return NULL;
@@ -1281,6 +1362,8 @@
 
        if (hashent)
                *hashent = (int)(ndhp - np->n_dircache);
+
+       NFSDC_LOCK(np);
        if (do32) {
                LIST_FOREACH(ndp, ndhp, dc_hash) {
                        if (ndp->dc_cookie32 == (u_int32_t)off) {
@@ -1289,10 +1372,11 @@
                                 * start of a new block fetched from
                                 * the server.
                                 */
-                               if (ndp->dc_blkno == -1) {
+                               if (ndp->dc_flags & NFSDC_INVALID) {
                                        ndp->dc_blkcookie = ndp->dc_cookie;
                                        ndp->dc_blkno = np->n_dblkno++;
                                        ndp->dc_entry = 0;
+                                       ndp->dc_flags &= ~NFSDC_INVALID;
                                }
                                break;
                        }
@@ -1303,6 +1387,9 @@
                                break;
                }
        }
+       if (ndp != NULL)
+               ndp->dc_refcnt++;
+       NFSDC_UNLOCK(np);
        return ndp;
 }
 
@@ -1316,10 +1403,20 @@
 {
        struct nfsnode *np = VTONFS(vp);
        struct nfsdirhashhead *ndhp;
-       struct nfsdircache *ndp = NULL, *first;
+       struct nfsdircache *ndp = NULL;
+       struct nfsdircache *newndp = NULL;
        struct nfsmount *nmp = VFSTONFS(vp->v_mount);
        int hashent, gen, overwrite;
 
+       /*
+        * XXX refuse entries for offset 0. amd(8) erroneously sets
+        * cookie 0 for the '.' entry, making this necessary. This
+        * isn't so bad, as 0 is a special case anyway.
+        */
+       if (off == 0)
+               /* LINTED const cast away */
+               return (struct nfsdircache *)&dzero;
+
        if (!np->n_dircache)
                /*
                 * XXX would like to do this in nfs_nget but vtype
@@ -1330,34 +1427,34 @@
        if ((nmp->nm_flag & NFSMNT_XLATECOOKIE) && !np->n_dirgens)
                nfs_initdirxlatecookie(vp);
 
-       /*
-        * XXX refuse entries for offset 0. amd(8) erroneously sets
-        * cookie 0 for the '.' entry, making this necessary. This
-        * isn't so bad, as 0 is a special case anyway.
-        */
-       if (off == 0)



Home | Main Index | Thread Index | Old Index