From 5f18deaf240278bdac9065c1721a367b1a0abfef Mon Sep 17 00:00:00 2001 From: k-nakahara Date: Fri, 2 Jun 2017 21:41:49 +0900 Subject: [PATCH 3/3] XXXX fix panic, but network down after detach swcrypto0 --- sys/opencrypto/crypto.c | 60 +++++++++++++++++++++++++++++++++++++--------- sys/opencrypto/cryptodev.h | 6 +++++ 2 files changed, 55 insertions(+), 11 deletions(-) diff --git a/sys/opencrypto/crypto.c b/sys/opencrypto/crypto.c index 0a6842b1e56..6ff2b63d22a 100644 --- a/sys/opencrypto/crypto.c +++ b/sys/opencrypto/crypto.c @@ -675,8 +675,12 @@ crypto_get_driverid(u_int32_t flags) cap->cc_sessions = 1; /* Mark */ cap->cc_flags = flags; localcount_init(&cap->cc_ref); + cap->cc_psz = pserialize_create(); + cap->cc_state = CRYPTOCAP_S_RESERVED; cv_init(&cap->cc_cv, "cryptocap"); - mutex_init(&cap->cc_lock, MUTEX_DEFAULT, IPL_NET); +// mutex_init(&cap->cc_lock, MUTEX_DEFAULT, IPL_NET); + /* to use pserialize, must be adaptive */ + mutex_init(&cap->cc_lock, MUTEX_DEFAULT, IPL_NONE); if (bootverbose) printf("crypto: assign driver %u, flags %u\n", i, flags); @@ -707,6 +711,7 @@ crypto_checkdriver(u_int32_t hid) static struct cryptocap * crypto_checkdriver_ref(u_int32_t hid) { + int s; struct cryptocap *cap; if (crypto_drivers == NULL) @@ -722,10 +727,18 @@ crypto_checkdriver_ref(u_int32_t hid) * crypto_register(), therefore, the "cap" is not done * crypto_register() yet. */ + /* XXX not needed because enough to check cap->cc_state */ if (cap->cc_process == NULL && cap->cc_sessions == 0) return NULL; + s = pserialize_read_enter(); + if (cap->cc_state != CRYPTOCAP_S_VALID) { + pserialize_read_exit(s); + return NULL; + } + crypto_driver_ref(cap); + pserialize_read_exit(s); return cap; } @@ -792,6 +805,8 @@ crypto_kregister(u_int32_t driverid, int kalg, u_int32_t flags, if (cap->cc_kprocess == NULL) { cap->cc_karg = karg; cap->cc_kprocess = kprocess; + KASSERT(cap->cc_state == CRYPTOCAP_S_RESERVED); + cap->cc_state = CRYPTOCAP_S_VALID; } err = 0; } else @@ -852,6 +867,8 @@ crypto_register(u_int32_t driverid, int alg, u_int16_t maxoplen, cap->cc_process = process; cap->cc_freesession = freeses; cap->cc_sessions = 0; /* Unmark */ + KASSERT(cap->cc_state == CRYPTOCAP_S_RESERVED); + cap->cc_state = CRYPTOCAP_S_VALID; } err = 0; } else @@ -902,9 +919,13 @@ crypto_unregister_locked(u_int32_t driverid, int alg, bool all) if (lastalg) { mutex_enter(&cap->cc_lock); mutex_exit(&crypto_drv_mtx); /* XXX */ + cap->cc_state = CRYPTOCAP_S_RESERVED; + pserialize_perform(cap->cc_psz); + /* ok, none begin to call localcount_acquire() any more. */ localcount_drain(&cap->cc_ref, &cap->cc_cv, &cap->cc_lock); mutex_exit(&cap->cc_lock); localcount_fini(&cap->cc_ref); + pserialize_destroy(cap->cc_psz); mutex_destroy(&cap->cc_lock); KASSERT(cap->cc_sessions == 0); memset(cap, 0, sizeof(struct cryptocap)); @@ -966,7 +987,7 @@ crypto_unblock(u_int32_t driverid, int what) struct cryptocap *cap; int needwakeup = 0; - mutex_spin_enter(&crypto_q_mtx); + mutex_spin_enter(&crypto_q_mtx); /* XXX should drv_mtx */ cap = crypto_checkdriver(driverid); KASSERTMSG(cap != NULL, "You have not registered yet or already unregistered."); @@ -982,7 +1003,7 @@ crypto_unblock(u_int32_t driverid, int what) } crypto_driver_unref(cap); - mutex_spin_exit(&crypto_q_mtx); + mutex_spin_exit(&crypto_q_mtx); /* XXX should drv_mtx */ if (needwakeup) setsoftcrypto(softintr_cookie); @@ -1010,6 +1031,13 @@ crypto_dispatch(struct cryptop *crp) nanouptime(&crp->crp_tstamp); #endif + cap = crypto_checkdriver_ref(CRYPTO_SESID2HID(crp->crp_sid)); + if (cap == NULL) { + DPRINTF("crypto_driverid[%lu] is freed concurrently", + CRYPTO_SESID2HID(crp->crp_sid)); + return ENODEV; + } + if ((crp->crp_flags & CRYPTO_F_BATCH) != 0) { int wasempty = TAILQ_EMPTY(&crp_q); /* @@ -1027,10 +1055,6 @@ crypto_dispatch(struct cryptop *crp) return 0; } - cap = crypto_checkdriver_ref(CRYPTO_SESID2HID(crp->crp_sid)); - KASSERTMSG(cap != NULL, "crypto_driverid[%lu] is free unexpectedly.", - CRYPTO_SESID2HID(crp->crp_sid)); - /* * TODO: * cap->cc_qblocked should be protected by a spin lock other than @@ -1566,8 +1590,14 @@ cryptointr(void) hint = 0; TAILQ_FOREACH_SAFE(crp, &crp_q, crp_next, cnext) { u_int32_t hid = CRYPTO_SESID2HID(crp->crp_sid); - cap = crypto_checkdriver(hid); - KASSERTMSG(cap != NULL, + cap = crypto_checkdriver_ref(hid); + /* + * At this point, cap->cc_state can be CRYPTOCAP_S_RESERVED + * as crypto_unresiger() already called. + * However cap->cc_state does not rearched CRYPTOCAP_S_UNUSED + * as referenced by this crp. + */ + KASSERTMSG(cap != NULL && cap->cc_state != CRYPTOCAP_S_UNUSED, "crypto_driverid[%u] is free unexpectedly.", hid); @@ -1589,6 +1619,7 @@ cryptointr(void) hint = CRYPTO_HINT_MORE; } + crypto_driver_unref(cap); continue; } @@ -1622,9 +1653,16 @@ cryptointr(void) /* validate sid again */ /* divert ref got in crypto_dispatch() */ cap = crypto_checkdriver(CRYPTO_SESID2HID(submit->crp_sid)); - KASSERTMSG(cap != NULL, + /* + * At this point, cap->cc_state can be CRYPTOCAP_S_RESERVED + * as crypto_unresiger() already called. + * However cap->cc_state does not rearched CRYPTOCAP_S_UNUSED + * as referenced by this crp. + */ + KASSERTMSG(cap != NULL && cap->cc_state != CRYPTOCAP_S_UNUSED, "crypto_driverid[%lu] is free unexpectedly.", - CRYPTO_SESID2HID(crp->crp_sid)); + CRYPTO_SESID2HID(submit->crp_sid)); + cap->cc_qblocked = 1; TAILQ_INSERT_HEAD(&crp_q, submit, crp_next); cryptostats.cs_blocks++; diff --git a/sys/opencrypto/cryptodev.h b/sys/opencrypto/cryptodev.h index 95a3af10ccf..3d1b4b8da11 100644 --- a/sys/opencrypto/cryptodev.h +++ b/sys/opencrypto/cryptodev.h @@ -89,6 +89,7 @@ #include #include #include +#include #if defined(_KERNEL_OPT) #include "opt_ocf.h" @@ -565,6 +566,11 @@ struct cryptocap { kmutex_t cc_lock; struct localcount cc_ref; kcondvar_t cc_cv; + pserialize_t cc_psz; + int cc_state; +#define CRYPTOCAP_S_UNUSED 0 /* just allocated, not get_driverid()'d yet */ +#define CRYPTOCAP_S_RESERVED 1 /* already get_driverid()'d, but not register() yet */ +#define CRYPTOCAP_S_VALID 2 /* already register(), can be used from ipsec/cryptodev */ }; /* -- 2.11.0