diff options
author | sam <sam@FreeBSD.org> | 2003-06-02 23:28:06 +0000 |
---|---|---|
committer | sam <sam@FreeBSD.org> | 2003-06-02 23:28:06 +0000 |
commit | fd016d74481eb1d01de3f6e9f3c43ebec302988e (patch) | |
tree | a080c149a3ddedf4ec627ed9e75477eb1532bdec | |
parent | cc4569f1717f17c3c99d389ca96bd1bf83d4f534 (diff) | |
download | FreeBSD-src-fd016d74481eb1d01de3f6e9f3c43ebec302988e.zip FreeBSD-src-fd016d74481eb1d01de3f6e9f3c43ebec302988e.tar.gz |
Flush my local cache of cryto subsystem fixes:
o add a ``done'' flag for crypto operations; this is set when the operation
completes and is intended for callers to check operations that may complete
``prematurely'' because of direct callbacks
o close a race for operations where the crypto driver returns ERESTART: we
need to hold the q lock to insure the blocked state for the driver and any
driver-private state is consistent; otherwise drivers may take an interrupt
and notify the crypto subsystem that it can unblock the driver but operations
will be left queued and never be processed
o close a race in /dev/crypto where operations can complete before the caller
can sleep waiting for the callback: use a per-session mutex and the new done
flag to handle this
o correct crypto_dispatch's handling of operations where the driver returns
ERESTART: the return value must be zero and not ERESTART, otherwise the
caller may free the crypto request despite it being queued for later handling
(this typically results in a later panic)
o change crypto mutex ``names'' so witness printouts and the like are more
meaningful
-rw-r--r-- | sys/opencrypto/crypto.c | 28 | ||||
-rw-r--r-- | sys/opencrypto/cryptodev.c | 32 | ||||
-rw-r--r-- | sys/opencrypto/cryptodev.h | 1 |
3 files changed, 43 insertions, 18 deletions
diff --git a/sys/opencrypto/crypto.c b/sys/opencrypto/crypto.c index 7f6c71c..e559912 100644 --- a/sys/opencrypto/crypto.c +++ b/sys/opencrypto/crypto.c @@ -114,16 +114,16 @@ crypto_init(void) { int error; - mtx_init(&crypto_drivers_mtx, "crypto driver table", - NULL, MTX_DEF|MTX_QUIET); + mtx_init(&crypto_drivers_mtx, "crypto", "crypto driver table", + MTX_DEF|MTX_QUIET); TAILQ_INIT(&crp_q); TAILQ_INIT(&crp_kq); - mtx_init(&crypto_q_mtx, "crypto op queues", NULL, MTX_DEF); + mtx_init(&crypto_q_mtx, "crypto", "crypto op queues", MTX_DEF); TAILQ_INIT(&crp_ret_q); TAILQ_INIT(&crp_ret_kq); - mtx_init(&crypto_ret_q_mtx, "crypto return queues", NULL, MTX_DEF); + mtx_init(&crypto_ret_q_mtx, "crypto", "crypto return queues", MTX_DEF); cryptop_zone = uma_zcreate("cryptop", sizeof (struct cryptop), 0, 0, 0, 0, @@ -664,6 +664,7 @@ crypto_dispatch(struct cryptop *crp) binuptime(&crp->crp_tstamp); #endif + CRYPTO_Q_LOCK(); if ((crp->crp_flags & CRYPTO_F_BATCH) == 0) { struct cryptocap *cap; /* @@ -679,21 +680,22 @@ crypto_dispatch(struct cryptop *crp) * The driver ran out of resources, mark the * driver ``blocked'' for cryptop's and put * the request on the queue. + * + * XXX ops are placed at the tail so their + * order is preserved but this can place them + * behind batch'd ops. */ - CRYPTO_Q_LOCK(); crypto_drivers[hid].cc_qblocked = 1; - TAILQ_INSERT_HEAD(&crp_q, crp, crp_next); - CRYPTO_Q_UNLOCK(); + TAILQ_INSERT_TAIL(&crp_q, crp, crp_next); cryptostats.cs_blocks++; + result = 0; } } else { /* * The driver is blocked, just queue the op until * it unblocks and the kernel thread gets kicked. */ - CRYPTO_Q_LOCK(); TAILQ_INSERT_TAIL(&crp_q, crp, crp_next); - CRYPTO_Q_UNLOCK(); result = 0; } } else { @@ -704,14 +706,13 @@ crypto_dispatch(struct cryptop *crp) * when the operation is low priority and/or suitable * for batching. */ - CRYPTO_Q_LOCK(); wasempty = TAILQ_EMPTY(&crp_q); TAILQ_INSERT_TAIL(&crp_q, crp, crp_next); if (wasempty) wakeup_one(&crp_q); - CRYPTO_Q_UNLOCK(); result = 0; } + CRYPTO_Q_UNLOCK(); return result; } @@ -743,7 +744,7 @@ crypto_kdispatch(struct cryptkop *krp) * it at the end does not work. */ crypto_drivers[krp->krp_hid].cc_kqblocked = 1; - TAILQ_INSERT_HEAD(&crp_kq, krp, krp_next); + TAILQ_INSERT_TAIL(&crp_kq, krp, krp_next); cryptostats.cs_kblocks++; } } else { @@ -939,6 +940,9 @@ crypto_getreq(int num) void crypto_done(struct cryptop *crp) { + KASSERT((crp->crp_flags & CRYPTO_F_DONE) == 0, + ("crypto_done: op already done, flags 0x%x", crp->crp_flags)); + crp->crp_flags |= CRYPTO_F_DONE; if (crp->crp_etype != 0) cryptostats.cs_errs++; #ifdef CRYPTO_TIMING diff --git a/sys/opencrypto/cryptodev.c b/sys/opencrypto/cryptodev.c index a40c859..e9a1129 100644 --- a/sys/opencrypto/cryptodev.c +++ b/sys/opencrypto/cryptodev.c @@ -56,6 +56,7 @@ struct csession { TAILQ_ENTRY(csession) next; u_int64_t sid; u_int32_t ses; + struct mtx lock; /* for op submission */ u_int32_t cipher; struct enc_xform *txform; @@ -419,12 +420,21 @@ cryptodev_op( crp->crp_mac=cse->tmp_mac; } - crypto_dispatch(crp); - error = tsleep(cse, PSOCK, "crydev", 0); - if (error) { - /* XXX can this happen? if so, how do we recover? */ + /* + * Let the dispatch run unlocked, then, interlock against the + * callback before checking if the operation completed and going + * to sleep. This insures drivers don't inherit our lock which + * results in a lock order reversal between crypto_dispatch forced + * entry and the crypto_done callback into us. + */ + error = crypto_dispatch(crp); + mtx_lock(&cse->lock); + if (error == 0 && (crp->crp_flags & CRYPTO_F_DONE) == 0) + error = msleep(crp, &cse->lock, PWAIT, "crydev", 0); + mtx_unlock(&cse->lock); + + if (error != 0) goto bail; - } if (crp->crp_etype != 0) { error = crp->crp_etype; @@ -462,7 +472,9 @@ cryptodev_cb(void *op) cse->error = crp->crp_etype; if (crp->crp_etype == EAGAIN) return crypto_dispatch(crp); - wakeup(cse); + mtx_lock(&cse->lock); + wakeup_one(crp); + mtx_unlock(&cse->lock); return (0); } @@ -661,10 +673,17 @@ csecreate(struct fcrypt *fcr, u_int64_t sid, caddr_t key, u_int64_t keylen, { struct csession *cse; +#ifdef INVARIANTS + /* NB: required when mtx_init is built with INVARIANTS */ + MALLOC(cse, struct csession *, sizeof(struct csession), + M_XDATA, M_NOWAIT | M_ZERO); +#else MALLOC(cse, struct csession *, sizeof(struct csession), M_XDATA, M_NOWAIT); +#endif if (cse == NULL) return NULL; + mtx_init(&cse->lock, "cryptodev", "crypto session lock", MTX_DEF); cse->key = key; cse->keylen = keylen/8; cse->mackey = mackey; @@ -684,6 +703,7 @@ csefree(struct csession *cse) int error; error = crypto_freesession(cse->sid); + mtx_destroy(&cse->lock); if (cse->key) FREE(cse->key, M_XDATA); if (cse->mackey) diff --git a/sys/opencrypto/cryptodev.h b/sys/opencrypto/cryptodev.h index b9d22d5..d7894a6 100644 --- a/sys/opencrypto/cryptodev.h +++ b/sys/opencrypto/cryptodev.h @@ -264,6 +264,7 @@ struct cryptop { #define CRYPTO_F_REL 0x0004 /* Must return data in same place */ #define CRYPTO_F_BATCH 0x0008 /* Batch op if possible */ #define CRYPTO_F_CBIMM 0x0010 /* Do callback immediately */ +#define CRYPTO_F_DONE 0x0020 /* Operation completed */ caddr_t crp_buf; /* Data to be processed */ caddr_t crp_opaque; /* Opaque pointer, passed along */ |