[src/trunk]: src/sys/opencrypto fix race among crypto_done(), cryptoret(), an...

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

details:   https://anonhg.NetBSD.org/src/rev/48e0dc341cdc
branches:  trunk
changeset: 822793:48e0dc341cdc
user:      knakahara <knakahara%NetBSD.org@localhost>
date:      Fri Apr 07 12:15:51 2017 +0000

description:
fix race among crypto_done(), cryptoret(), and {cryptodev_op(), cryptodev_key()}.

crypto_op() waited to be set CRYPTO_F_DONE with crp->crp_cv.
However, there is context switch chances between being set CRYPTO_F_DONE in
crypto_done() and done cv_signal(crp->crp_cv) in cryptodev_cb(), that is,
cryptodev_op() thread can run to cv_destroy(crp->crp_cv) before cryptoret()
thread is waken up. As a result, cryptodev_cb() can call invalid(destroyed)
cv_signal(crp->crp_cv).

Furthermore, below two implementations cause other races.
    - waiting CRYPTO_F_DONE with crp->crp_cv
    - context witch chances between set CRYPTO_F_DONE and cv_signal(crp->crp_cv)

So, use other flag(CRYPTO_F_DQRETQ) for cryptodev_op() and cryptodev_key(),
and then call cv_signal(crp->crp_cv) immediately after set CRYPTO_F_DQRETQ.

Tested concurrent over 20 processes with software and hardware drivers.

diffstat:

 sys/opencrypto/cryptodev.c |  10 ++++++----
 sys/opencrypto/cryptodev.h |   3 ++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diffs (69 lines):

diff -r 866dbdc07f42 -r 48e0dc341cdc sys/opencrypto/cryptodev.c
--- a/sys/opencrypto/cryptodev.c        Fri Apr 07 07:21:58 2017 +0000
+++ b/sys/opencrypto/cryptodev.c        Fri Apr 07 12:15:51 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cryptodev.c,v 1.86 2017/04/05 08:51:04 knakahara Exp $ */
+/*     $NetBSD: cryptodev.c,v 1.87 2017/04/07 12:15:51 knakahara Exp $ */
 /*     $FreeBSD: src/sys/opencrypto/cryptodev.c,v 1.4.2.4 2003/06/03 00:09:02 sam Exp $        */
 /*     $OpenBSD: cryptodev.c,v 1.53 2002/07/10 22:21:30 mickey Exp $   */
 
@@ -64,7 +64,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.86 2017/04/05 08:51:04 knakahara Exp $");
+__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.87 2017/04/07 12:15:51 knakahara Exp $");
 
 #include <sys/param.h>
 #include <sys/systm.h>
@@ -663,7 +663,7 @@
                goto bail;
        }
 
-       while (!(crp->crp_flags & CRYPTO_F_DONE)) {
+       while (!(crp->crp_flags & CRYPTO_F_DQRETQ)) {
                DPRINTF(("cryptodev_op[%d]: sleeping on cv %p for crp %p\n",
                        (uint32_t)cse->sid, &crp->crp_cv, crp));
                cv_wait(&crp->crp_cv, &crypto_mtx);     /* XXX cv_wait_sig? */
@@ -744,6 +744,7 @@
                mutex_enter(&crypto_mtx);
        }
        if (error != 0 || (crp->crp_flags & CRYPTO_F_DONE)) {
+               crp->crp_flags |= CRYPTO_F_DQRETQ;
                cv_signal(&crp->crp_cv);
        }
        mutex_exit(&crypto_mtx);
@@ -780,6 +781,7 @@
        struct cryptkop *krp = op;
        
        mutex_enter(&crypto_mtx);
+       krp->krp_flags |= CRYPTO_F_DQRETQ;
        cv_signal(&krp->krp_cv);
        mutex_exit(&crypto_mtx);
        return 0;
@@ -890,7 +892,7 @@
        }
 
        mutex_enter(&crypto_mtx);
-       while (!(krp->krp_flags & CRYPTO_F_DONE)) {
+       while (!(krp->krp_flags & CRYPTO_F_DQRETQ)) {
                cv_wait(&krp->krp_cv, &crypto_mtx);     /* XXX cv_wait_sig? */
        }
        if (krp->krp_flags & CRYPTO_F_ONRETQ) {
diff -r 866dbdc07f42 -r 48e0dc341cdc sys/opencrypto/cryptodev.h
--- a/sys/opencrypto/cryptodev.h        Fri Apr 07 07:21:58 2017 +0000
+++ b/sys/opencrypto/cryptodev.h        Fri Apr 07 12:15:51 2017 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: cryptodev.h,v 1.29 2017/03/06 09:59:05 knakahara Exp $ */
+/*     $NetBSD: cryptodev.h,v 1.30 2017/04/07 12:15:51 knakahara Exp $ */
 /*     $FreeBSD: src/sys/opencrypto/cryptodev.h,v 1.2.2.6 2003/07/02 17:04:50 sam Exp $        */
 /*     $OpenBSD: cryptodev.h,v 1.33 2002/07/17 23:52:39 art Exp $      */
 
@@ -473,6 +473,7 @@
 #define        CRYPTO_F_ONRETQ         0x0080  /* Request is on return queue */
 #define        CRYPTO_F_USER           0x0100  /* Request is in user context */
 #define        CRYPTO_F_MORE           0x0200  /* more data to follow */
+#define        CRYPTO_F_DQRETQ         0x0400  /* Dequeued from crp_ret_{,k}q */
 
        void *          crp_buf;        /* Data to be processed */
        void *          crp_opaque;     /* Opaque pointer, passed along */