[src/netbsd-8]: src/sys/opencrypto Pull up following revision(s) (requested b...

[snj <snj%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/1bd41688f8c5
branches:  netbsd-8
changeset: 434029:1bd41688f8c5
user:      snj <snj%NetBSD.org@localhost>
date:      Thu Jun 22 05:36:41 2017 +0000

description:
Pull up following revision(s) (requested by christos/knakahara in ticket #37):
        sys/opencrypto/crypto.c: 1.79-1.86
        sys/opencrypto/cryptodev.h: 1.35, 1.36
use crypto_checkdriver_uninit() when it may touch uninitialized crypto_drivers.
--
fix reading crp_q without holding crypto_q_mtx
--
restructure locks(1/2): make relation between lock and data explicit.
    + crypto_drv_mtx protects
      -  whole crypto_drivers
    + crypto_drivers[i].cc_lock (new) protects
      - crypto_drivers[i] itself
      - member of crypto_drivers[i]
    + crypto_q_mtx protects
      - crp_q
      - crp_kq
    + crypto_ret_q_mtx protects
      - crp_ret_q
      - crp_ret_kq
      - crypto_exit_flag
I will add locking note later.
--
restructure locks(2/2): crypto_q_mtx can be adaptive now.
--
add locking notes.
--
avoid crp_q reordering as hardware interrupts.
crypto_{,k}invoke() can be called with holding crp_q_mtx now.
--
apply the same fix as crypto.c:r1.83 for crypto_dispatch to crypto_kdispatch.
--
- acquire lock
- use c99 loop indexes
- initialize featp
--
Put back crypto_checkdriver(); use it when we need to make sure that we
get back a cryptocap that has been initialized.

diffstat:

 sys/opencrypto/crypto.c    |  357 ++++++++++++++++++++++++++++++--------------
 sys/opencrypto/cryptodev.h |   18 ++-
 2 files changed, 262 insertions(+), 113 deletions(-)

diffs (truncated from 836 to 300 lines):

diff -r 74977de3c2a9 -r 1bd41688f8c5 sys/opencrypto/crypto.c
--- a/sys/opencrypto/crypto.c   Wed Jun 21 18:31:24 2017 +0000
+++ b/sys/opencrypto/crypto.c   Thu Jun 22 05:36:41 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: crypto.c,v 1.78 2017/05/31 02:17:49 knakahara Exp $ */
+/*     $NetBSD: crypto.c,v 1.78.2.1 2017/06/22 05:36:41 snj Exp $ */
 /*     $FreeBSD: src/sys/opencrypto/crypto.c,v 1.4.2.5 2003/02/26 00:14:05 sam Exp $   */
 /*     $OpenBSD: crypto.c,v 1.41 2002/07/17 23:52:38 art Exp $ */
 
@@ -53,7 +53,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.78 2017/05/31 02:17:49 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.78.2.1 2017/06/22 05:36:41 snj Exp $");
 
 #include <sys/param.h>
 #include <sys/reboot.h>
@@ -374,7 +374,12 @@
 static int crypto_invoke(struct cryptop *crp, int hint);
 static int crypto_kinvoke(struct cryptkop *krp, int hint);
 
+static struct cryptocap *crypto_checkdriver_lock(u_int32_t);
+static struct cryptocap *crypto_checkdriver_uninit(u_int32_t);
 static struct cryptocap *crypto_checkdriver(u_int32_t);
+static void crypto_driver_lock(struct cryptocap *);
+static void crypto_driver_unlock(struct cryptocap *);
+static void crypto_driver_clear(struct cryptocap *);
 
 static struct cryptostats cryptostats;
 #ifdef CRYPTO_TIMING
@@ -389,7 +394,7 @@
        int error;
 
        mutex_init(&crypto_drv_mtx, MUTEX_DEFAULT, IPL_NONE);
-       mutex_init(&crypto_q_mtx, MUTEX_DEFAULT, IPL_NET);
+       mutex_init(&crypto_q_mtx, MUTEX_DEFAULT, IPL_NONE);
        mutex_init(&crypto_ret_q_mtx, MUTEX_DEFAULT, IPL_NET);
        cv_init(&cryptoret_cv, "crypto_w");
        pool_init(&cryptop_pool, sizeof(struct cryptop), 0, 0,  
@@ -437,26 +442,33 @@
        if (exit_kthread) {
                struct cryptocap *cap = NULL;
 
-               mutex_spin_enter(&crypto_ret_q_mtx);
-
                /* if we have any in-progress requests, don't unload */
+               mutex_enter(&crypto_q_mtx);
                if (!TAILQ_EMPTY(&crp_q) || !TAILQ_EMPTY(&crp_kq)) {
-                       mutex_spin_exit(&crypto_ret_q_mtx);
+                       mutex_exit(&crypto_q_mtx);
                        return EBUSY;
                }
+               mutex_exit(&crypto_q_mtx);
+               /* FIXME:
+                * prohibit enqueue to crp_q and crp_kq after here.
+                */
 
+               mutex_enter(&crypto_drv_mtx);
                for (i = 0; i < crypto_drivers_num; i++) {
                        cap = crypto_checkdriver(i);
                        if (cap == NULL)
                                continue;
-                       if (cap->cc_sessions != 0)
-                               break;
+                       if (cap->cc_sessions != 0) {
+                               mutex_exit(&crypto_drv_mtx);
+                               return EBUSY;
+                       }
                }
-               if (cap != NULL) {
-                       mutex_spin_exit(&crypto_ret_q_mtx);
-                       return EBUSY;
-               }
+               mutex_exit(&crypto_drv_mtx);
+               /* FIXME:
+                * prohibit touch crypto_drivers[] and each element after here.
+                */
 
+               mutex_spin_enter(&crypto_ret_q_mtx);
                /* kick the cryptoret thread and wait for it to exit */
                crypto_exit_flag = 1;
                cv_signal(&cryptoret_cv);
@@ -515,20 +527,28 @@
                if (cap == NULL)
                        continue;
 
+               crypto_driver_lock(cap);
+
                /*
                 * If it's not initialized or has remaining sessions
                 * referencing it, skip.
                 */
                if (cap->cc_newsession == NULL ||
-                   (cap->cc_flags & CRYPTOCAP_F_CLEANUP))
+                   (cap->cc_flags & CRYPTOCAP_F_CLEANUP)) {
+                       crypto_driver_unlock(cap);
                        continue;
+               }
 
                /* Hardware required -- ignore software drivers. */
-               if (hard > 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE))
+               if (hard > 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE)) {
+                       crypto_driver_unlock(cap);
                        continue;
+               }
                /* Software required -- ignore hardware drivers. */
-               if (hard < 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE) == 0)
+               if (hard < 0 && (cap->cc_flags & CRYPTOCAP_F_SOFTWARE) == 0) {
+                       crypto_driver_unlock(cap);
                        continue;
+               }
 
                /* See if all the algorithms are supported. */
                for (cr = cri; cr; cr = cr->cri_next)
@@ -559,9 +579,12 @@
                                DPRINTF("crypto_drivers[%d].cc_newsession() failed. error=%d\n",
                                        hid, err);
                        }
+                       crypto_driver_unlock(cap);
                        goto done;
                        /*break;*/
                }
+
+               crypto_driver_unlock(cap);
        }
 done:
        mutex_exit(&crypto_drv_mtx);
@@ -578,14 +601,10 @@
        struct cryptocap *cap;
        int err = 0;
 
-       mutex_enter(&crypto_drv_mtx);
-
        /* Determine two IDs. */
-       cap = crypto_checkdriver(CRYPTO_SESID2HID(sid));
-       if (cap == NULL) {
-               err = ENOENT;
-               goto done;
-       }
+       cap = crypto_checkdriver_lock(CRYPTO_SESID2HID(sid));
+       if (cap == NULL)
+               return ENOENT;
 
        if (cap->cc_sessions)
                (cap->cc_sessions)--;
@@ -601,11 +620,19 @@
         * make the entry available for reuse.
         */
        if ((cap->cc_flags & CRYPTOCAP_F_CLEANUP) && cap->cc_sessions == 0)
-               memset(cap, 0, sizeof(struct cryptocap));
+               crypto_driver_clear(cap);
+
+       crypto_driver_unlock(cap);
+       return err;
+}
 
-done:
-       mutex_exit(&crypto_drv_mtx);
-       return err;
+static bool
+crypto_checkdriver_initialized(const struct cryptocap *cap)
+{
+
+       return cap->cc_process != NULL ||
+           (cap->cc_flags & CRYPTOCAP_F_CLEANUP) != 0 ||
+           cap->cc_sessions != 0;
 }
 
 /*
@@ -623,13 +650,10 @@
 
        mutex_enter(&crypto_drv_mtx);
        for (i = 0; i < crypto_drivers_num; i++) {
-               cap = crypto_checkdriver(i);
-               if (cap == NULL)
+               cap = crypto_checkdriver_uninit(i);
+               if (cap == NULL || crypto_checkdriver_initialized(cap))
                        continue;
-               if (cap->cc_process == NULL &&
-                   (cap->cc_flags & CRYPTOCAP_F_CLEANUP) == 0 &&
-                   cap->cc_sessions == 0)
-                       break;
+               break;
        }
 
        /* Out of entries, allocate some more. */
@@ -657,13 +681,14 @@
                free(crypto_drivers, M_CRYPTO_DATA);
                crypto_drivers = newdrv;
 
-               cap = crypto_checkdriver(i);
+               cap = crypto_checkdriver_uninit(i);
                KASSERT(cap != NULL);
        }
 
        /* NB: state is zero'd on free */
        cap->cc_sessions = 1;   /* Mark */
        cap->cc_flags = flags;
+       mutex_init(&cap->cc_lock, MUTEX_DEFAULT, IPL_NET);
 
        if (bootverbose)
                printf("crypto: assign driver %u, flags %u\n", i, flags);
@@ -674,11 +699,97 @@
 }
 
 static struct cryptocap *
+crypto_checkdriver_lock(u_int32_t hid)
+{
+       struct cryptocap *cap;
+
+       KASSERT(crypto_drivers != NULL);
+
+       if (hid >= crypto_drivers_num)
+               return NULL;
+
+       cap = &crypto_drivers[hid];
+       mutex_enter(&cap->cc_lock);
+       return cap;
+}
+
+/*
+ * Use crypto_checkdriver_uninit() instead of crypto_checkdriver() below two
+ * situations
+ *     - crypto_drivers[] may not be allocated
+ *     - crypto_drivers[hid] may not be initialized
+ */
+static struct cryptocap *
+crypto_checkdriver_uninit(u_int32_t hid)
+{
+
+       KASSERT(mutex_owned(&crypto_drv_mtx));
+
+       if (crypto_drivers == NULL)
+               return NULL;
+
+       return (hid >= crypto_drivers_num ? NULL : &crypto_drivers[hid]);
+}
+
+/*
+ * Use crypto_checkdriver_uninit() instead of crypto_checkdriver() below two
+ * situations
+ *     - crypto_drivers[] may not be allocated
+ *     - crypto_drivers[hid] may not be initialized
+ */
+static struct cryptocap *
 crypto_checkdriver(u_int32_t hid)
 {
-       if (crypto_drivers == NULL)
+
+       KASSERT(mutex_owned(&crypto_drv_mtx));
+
+       if (crypto_drivers == NULL || hid >= crypto_drivers_num)
                return NULL;
-       return (hid >= crypto_drivers_num ? NULL : &crypto_drivers[hid]);
+
+       struct cryptocap *cap = &crypto_drivers[hid];
+       return crypto_checkdriver_initialized(cap) ? cap : NULL;
+}
+
+static inline void
+crypto_driver_lock(struct cryptocap *cap)
+{
+
+       KASSERT(cap != NULL);
+
+       mutex_enter(&cap->cc_lock);
+}
+
+static inline void
+crypto_driver_unlock(struct cryptocap *cap)
+{
+
+       KASSERT(cap != NULL);
+
+       mutex_exit(&cap->cc_lock);
+}
+
+static void
+crypto_driver_clear(struct cryptocap *cap)
+{
+
+       if (cap == NULL)
+               return;
+
+       KASSERT(mutex_owned(&cap->cc_lock));
+
+       cap->cc_sessions = 0;
+       memset(&cap->cc_max_op_len, 0, sizeof(cap->cc_max_op_len));
+       memset(&cap->cc_alg, 0, sizeof(cap->cc_alg));
+       memset(&cap->cc_kalg, 0, sizeof(cap->cc_kalg));
+       cap->cc_flags = 0;
+       cap->cc_qblocked = 0;
+       cap->cc_kqblocked = 0;
+
+       cap->cc_arg = NULL;
+       cap->cc_newsession = NULL;
+       cap->cc_process = NULL;
+       cap->cc_freesession = NULL;
+       cap->cc_kprocess = NULL;