Source-Changes-HG archive

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

[src/trunk]: src/sys/nfs Use a random opaque cookie, not kva pointer, for nfs...



details:   https://anonhg.NetBSD.org/src/rev/79b49f3c3e31
branches:  trunk
changeset: 829315:79b49f3c3e31
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Thu Jan 25 17:14:36 2018 +0000

description:
Use a random opaque cookie, not kva pointer, for nfssvc(2).

(What were they smoking?!)

I suspect most of this is actually dead code that wasn't properly
amputated along with the rest of the gangrene of NFSKERB a decade
ago, but I'm out of time to investigate further.  If someone else
wants to kill NFSSVC_AUTHIN/NFSSVC_AUTHINFAIL and the rest of the
tentacular kerberosity, be my guest.

Noted by Silvio Cesare of InfoSect.

diffstat:

 sys/nfs/nfs.h          |    7 +-
 sys/nfs/nfs_syscalls.c |  151 ++++++++++++++++++++++++++++++++++++++++++++----
 2 files changed, 141 insertions(+), 17 deletions(-)

diffs (truncated from 309 to 300 lines):

diff -r 74848c5dcedb -r 79b49f3c3e31 sys/nfs/nfs.h
--- a/sys/nfs/nfs.h     Thu Jan 25 16:05:11 2018 +0000
+++ b/sys/nfs/nfs.h     Thu Jan 25 17:14:36 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs.h,v 1.76 2018/01/21 20:36:49 christos Exp $        */
+/*     $NetBSD: nfs.h,v 1.77 2018/01/25 17:14:36 riastradh Exp $       */
 /*
  * Copyright (c) 1989, 1993, 1995
  *     The Regents of the University of California.  All rights reserved.
@@ -41,6 +41,7 @@
 #include <sys/fstypes.h>
 #include <sys/mbuf.h>
 #include <sys/mutex.h>
+#include <sys/rbtree.h>
 #endif
 
 /*
@@ -486,7 +487,7 @@
  * One of these structures is allocated for each nfsd.
  */
 struct nfsd {
-       TAILQ_ENTRY(nfsd) nfsd_chain;   /* List of all nfsd's */
+       struct rb_node  nfsd_node;      /* Tree of all nfsd's */
        SLIST_ENTRY(nfsd) nfsd_idle;    /* List of idle nfsd's */
        kcondvar_t      nfsd_cv;
        int             nfsd_flag;      /* NFSD_ flags */
@@ -497,6 +498,7 @@
        u_char          nfsd_verfstr[RPCVERF_MAXSIZ];
        struct proc     *nfsd_procp;    /* Proc ptr */
        struct nfsrv_descript *nfsd_nd; /* Associated nfsrv_descript */
+       uint32_t        nfsd_cookie;    /* Userland cookie, fits 32bit ptr */
 };
 
 /* Bits for "nfsd_flag" */
@@ -557,7 +559,6 @@
 
 extern kmutex_t nfsd_lock;
 extern kcondvar_t nfsd_initcv;
-extern TAILQ_HEAD(nfsdhead, nfsd) nfsd_head;
 extern SLIST_HEAD(nfsdidlehead, nfsd) nfsd_idle_head;
 extern int nfsd_head_flag;
 #define        NFSD_CHECKSLP   0x01
diff -r 74848c5dcedb -r 79b49f3c3e31 sys/nfs/nfs_syscalls.c
--- a/sys/nfs/nfs_syscalls.c    Thu Jan 25 16:05:11 2018 +0000
+++ b/sys/nfs/nfs_syscalls.c    Thu Jan 25 17:14:36 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nfs_syscalls.c,v 1.158 2017/02/12 18:24:31 maxv Exp $  */
+/*     $NetBSD: nfs_syscalls.c,v 1.159 2018/01/25 17:14:36 riastradh Exp $     */
 
 /*
  * Copyright (c) 1989, 1993
@@ -35,7 +35,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nfs_syscalls.c,v 1.158 2017/02/12 18:24:31 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nfs_syscalls.c,v 1.159 2018/01/25 17:14:36 riastradh Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -61,6 +61,8 @@
 #include <sys/kthread.h>
 #include <sys/kauth.h>
 #include <sys/syscallargs.h>
+#include <sys/cprng.h>
+#include <sys/rbtree.h>
 
 #include <netinet/in.h>
 #include <netinet/tcp.h>
@@ -87,9 +89,11 @@
 struct nfssvc_sockhead nfssvc_sockhead;
 kcondvar_t nfsd_initcv;
 struct nfssvc_sockhead nfssvc_sockpending;
-struct nfsdhead nfsd_head;
 struct nfsdidlehead nfsd_idle_head;
 
+static rb_tree_t nfsd_tree;
+static const rb_tree_ops_t nfsd_tree_ops;
+
 int nfssvc_sockhead_flag;
 int nfsd_head_flag;
 
@@ -102,12 +106,118 @@
 static int nfssvc_nfsd(struct nfssvc_copy_ops *, struct nfsd_srvargs *, void *,
                struct lwp *);
 
+static int nfsd_compare_nodes(void *, const void *, const void *);
+static int nfsd_compare_key(void *, const void *, const void *);
+
+static struct nfsd *nfsd_bake_cookie(struct nfsd *);
+static void nfsd_toss_cookie(struct nfsd *);
+static struct nfsd *nfsd_get(struct nfsd *);
+
 static int nfssvc_addsock_in(struct nfsd_args *, const void *);
 static int nfssvc_setexports_in(struct mountd_exports_list *, const void *);
 static int nfssvc_nsd_in(struct nfsd_srvargs *, const void *);
 static int nfssvc_nsd_out(void *, const struct nfsd_srvargs *);
 static int nfssvc_exp_in(struct export_args *, const void *, size_t);
 
+static const rb_tree_ops_t nfsd_tree_ops = {
+       .rbto_compare_nodes = nfsd_compare_nodes,
+       .rbto_compare_key = nfsd_compare_key,
+       .rbto_node_offset = offsetof(struct nfsd, nfsd_node),
+};
+
+static int
+nfsd_compare_nodes(void *cookie, const void *va, const void *vb)
+{
+       const struct nfsd *na = va;
+       const struct nfsd *nb = vb;
+
+       if (na->nfsd_cookie < nb->nfsd_cookie)
+               return -1;
+       if (na->nfsd_cookie > nb->nfsd_cookie)
+               return +1;
+       return 0;
+}
+
+static int
+nfsd_compare_key(void *cookie, const void *vn, const void *vk)
+{
+       const struct nfsd *n = vn;
+       const uint32_t *k = vk;
+
+       if (n->nfsd_cookie < *k)
+               return -1;
+       if (n->nfsd_cookie > *k)
+               return +1;
+       return 0;
+}
+
+/*
+ * nfsd_bake_cookie(nfsd)
+ *
+ *     Bake a cookie for nfsd, hang it on the tree of nfsds, and
+ *     return a userland-safe pointer nfsdu neatly packed for
+ *     transport in struct nfsd_srvargs::nsd_nfsd.
+ */
+static struct nfsd *
+nfsd_bake_cookie(struct nfsd *nfsd)
+{
+
+       KASSERT(mutex_owned(&nfsd_lock));
+
+       do {
+               nfsd->nfsd_cookie = cprng_fast32();
+       } while (nfsd->nfsd_cookie == 0 ||
+           rb_tree_insert_node(&nfsd_tree, nfsd) != nfsd);
+
+       return (struct nfsd *)(uintptr_t)nfsd->nfsd_cookie;
+}
+
+/*
+ * nfsd_toss_cookie(nfsd)
+ *
+ *     Toss nfsd's cookie.
+ */
+static void
+nfsd_toss_cookie(struct nfsd *nfsd)
+{
+
+       KASSERT(mutex_owned(&nfsd_lock));
+       KASSERT(nfsd->nfsd_cookie != 0);
+
+       rb_tree_remove_node(&nfsd_tree, nfsd);
+       nfsd->nfsd_cookie = 0;  /* paranoia */
+}
+
+/*
+ * nfsd_get(nfsdu)
+ *
+ *     Return the struct nfsd pointer for the userland nfsdu cookie,
+ *     as stored in struct nfsd_srvargs::nsd_nfsd, or NULL if nfsdu is
+ *     not a current valid nfsd cookie.
+ *
+ *     Caller MUST NOT hold nfsd_lock.  Caller MUST NOT pass (struct
+ *     nfsd *)(uintptr_t)0, which is the sentinel value for no nfsd
+ *     cookie, for which the caller should check first.
+ */
+static struct nfsd *
+nfsd_get(struct nfsd *nfsdu)
+{
+       uintptr_t cookie = (uintptr_t)nfsdu;
+       uint32_t key;
+       struct nfsd *nfsd;
+
+       KASSERT(cookie != 0);
+       if (cookie > UINT32_MAX)
+               return NULL;
+       key = cookie;
+
+       mutex_enter(&nfsd_lock);
+       nfsd = rb_tree_find_node(&nfsd_tree, &key);
+       mutex_exit(&nfsd_lock);
+
+       return nfsd;
+}
+
 static int
 nfssvc_addsock_in(struct nfsd_args *nfsdarg, const void *argp)
 {
@@ -184,7 +294,7 @@
        struct mbuf *nam;
        struct nfsd_args nfsdarg;
        struct nfsd_srvargs nfsd_srvargs, *nsd = &nfsd_srvargs;
-       struct nfsd *nfsd;
+       struct nfsd *nfsd = NULL;
        struct nfssvc_sock *slp;
        struct nfsuid *nuidp;
 
@@ -254,8 +364,11 @@
                error = ops->nsd_in(nsd, argp);
                if (error)
                        return (error);
+               if ((uintptr_t)nsd->nsd_nfsd != 0 &&
+                   (nfsd = nfsd_get(nsd->nsd_nfsd)) == NULL)
+                       return (EINVAL);
                if ((flag & NFSSVC_AUTHIN) &&
-                   ((nfsd = nsd->nsd_nfsd)) != NULL &&
+                   nfsd != NULL &&
                    (nfsd->nfsd_slp->ns_flags & SLP_VALID)) {
                        slp = nfsd->nfsd_slp;
 
@@ -340,7 +453,7 @@
                        }
                }
                if ((flag & NFSSVC_AUTHINFAIL) &&
-                   (nfsd = nsd->nsd_nfsd))
+                   nfsd != NULL)
                        nfsd->nfsd_flag |= NFSD_AUTHFAIL;
                error = nfssvc_nfsd(ops, nsd, argp, l);
        }
@@ -481,7 +594,7 @@
        struct timeval tv;
        struct mbuf *m;
        struct nfssvc_sock *slp;
-       struct nfsd *nfsd = nsd->nsd_nfsd;
+       struct nfsd *nfsd;
        struct nfsrv_descript *nd = NULL;
        struct mbuf *mreq;
        u_quad_t cur_usec;
@@ -493,8 +606,12 @@
        cacherep = RC_DOIT;
        writes_todo = 0;
 #endif
-       if (nfsd == NULL) {
-               nsd->nsd_nfsd = nfsd = kmem_alloc(sizeof(*nfsd), KM_SLEEP);
+       /*
+        * If userland didn't provide an nfsd cookie, bake a fresh one;
+        * if they did provide one, look it up.
+        */
+       if ((uintptr_t)nsd->nsd_nfsd == 0) {
+               nfsd = kmem_alloc(sizeof(*nfsd), KM_SLEEP);
                memset(nfsd, 0, sizeof (struct nfsd));
                cv_init(&nfsd->nfsd_cv, "nfsd");
                nfsd->nfsd_procp = p;
@@ -503,10 +620,15 @@
                        KASSERT(nfs_numnfsd == 0);
                        cv_wait(&nfsd_initcv, &nfsd_lock);
                }
-               TAILQ_INSERT_TAIL(&nfsd_head, nfsd, nfsd_chain);
+               nsd->nsd_nfsd = nfsd_bake_cookie(nfsd);
                nfs_numnfsd++;
                mutex_exit(&nfsd_lock);
+       } else if ((nfsd = nfsd_get(nsd->nsd_nfsd)) == NULL) {
+               return (EINVAL);
        }
+       KASSERT(nfsd != NULL);
+       KASSERT(nsd->nsd_nfsd != (struct nfsd *)(uintptr_t)0);
+
        /*
         * Loop getting rpc requests until SIGKILL.
         */
@@ -759,14 +881,15 @@
        }
 done:
        mutex_enter(&nfsd_lock);
-       TAILQ_REMOVE(&nfsd_head, nfsd, nfsd_chain);
+       nfsd_toss_cookie(nfsd);
        doreinit = --nfs_numnfsd == 0;
        if (doreinit)
                nfssvc_sockhead_flag |= SLP_INIT;
        mutex_exit(&nfsd_lock);
        cv_destroy(&nfsd->nfsd_cv);
        kmem_free(nfsd, sizeof(*nfsd));
-       nsd->nsd_nfsd = NULL;
+       KASSERT(nsd->nsd_nfsd != (struct nfsd *)(uintptr_t)0);
+       nsd->nsd_nfsd = (struct nfsd *)(uintptr_t)0;
        if (doreinit)
                nfsrv_init(true);       /* Reinitialize everything */
        return (error);
@@ -895,7 +1018,7 @@
 
        if (terminating) {
                KASSERT(SLIST_EMPTY(&nfsd_idle_head));
-               KASSERT(TAILQ_EMPTY(&nfsd_head));
+               KASSERT(RB_TREE_MIN(&nfsd_tree) == NULL);
                while ((slp = TAILQ_FIRST(&nfssvc_sockhead)) != NULL) {
                        mutex_exit(&nfsd_lock);
                        KASSERT(slp->ns_sref == 0);



Home | Main Index | Thread Index | Old Index