CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Tue Jul 11 10:42:16 UTC 2023 Modified Files: src/sys/opencrypto: cryptodev.h Log Message: opencrypto/cryptodev.h: Fix includes. - Move sys/condvar.h under #ifdef _KERNEL. - Add some other necessary includes and forward declarations. - Sort. To generate a diff of this commit: cvs rdiff -u -r1.50 -r1.51 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Tue Jul 11 10:42:16 UTC 2023 Modified Files: src/sys/opencrypto: cryptodev.h Log Message: opencrypto/cryptodev.h: Fix includes. - Move sys/condvar.h under #ifdef _KERNEL. - Add some other necessary includes and forward declarations. - Sort. To generate a diff of this commit: cvs rdiff -u -r1.50 -r1.51 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.h diff -u src/sys/opencrypto/cryptodev.h:1.50 src/sys/opencrypto/cryptodev.h:1.51 --- src/sys/opencrypto/cryptodev.h:1.50 Sun May 22 11:40:29 2022 +++ src/sys/opencrypto/cryptodev.h Tue Jul 11 10:42:16 2023 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.h,v 1.50 2022/05/22 11:40:29 riastradh Exp $ */ +/* $NetBSD: cryptodev.h,v 1.51 2023/07/11 10:42:16 riastradh 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 $ */ @@ -85,8 +85,9 @@ #ifndef _CRYPTO_CRYPTO_H_ #define _CRYPTO_CRYPTO_H_ +#include + #include -#include #include #if defined(_KERNEL_OPT) @@ -409,6 +410,16 @@ struct cryptostats { }; #ifdef _KERNEL + +#include +#include +#include +#include +#include + +struct cpu_info; +struct uio; + /* Standard initialization structure beginning */ struct cryptoini { int cri_alg; /* Algorithm to use */
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun Jun 26 22:52:30 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto(9): Fix missing initialization in error branch. Reported-by: syzbot+8c519140cac567be1...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.130 -r1.131 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.130 src/sys/opencrypto/crypto.c:1.131 --- src/sys/opencrypto/crypto.c:1.130 Sun May 22 11:40:54 2022 +++ src/sys/opencrypto/crypto.c Sun Jun 26 22:52:30 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.130 2022/05/22 11:40:54 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.131 2022/06/26 22:52:30 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.130 2022/05/22 11:40:54 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.131 2022/06/26 22:52:30 riastradh Exp $"); #include #include @@ -1485,6 +1485,7 @@ crypto_kinvoke(struct cryptkop *krp, int return error; } else { krp->krp_status = ENODEV; + krp->reqcpu = curcpu(); crypto_kdone(krp); return 0; }
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun Jun 26 22:52:30 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto(9): Fix missing initialization in error branch. Reported-by: syzbot+8c519140cac567be1...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.130 -r1.131 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:40:54 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert session id is valid in crypto_freesession. This gives us the opportunity to detect usage mistakes like use-after-free. Exception: Continue to silently ignore sid=0. To generate a diff of this commit: cvs rdiff -u -r1.129 -r1.130 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.129 src/sys/opencrypto/crypto.c:1.130 --- src/sys/opencrypto/crypto.c:1.129 Sun May 22 11:40:29 2022 +++ src/sys/opencrypto/crypto.c Sun May 22 11:40:54 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.129 2022/05/22 11:40:29 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.130 2022/05/22 11:40:54 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.129 2022/05/22 11:40:29 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.130 2022/05/22 11:40:54 riastradh Exp $"); #include #include @@ -870,11 +870,10 @@ crypto_freesession(u_int64_t sid) /* Determine two IDs. */ cap = crypto_checkdriver_lock(CRYPTO_SESID2HID(sid)); - if (cap == NULL) /* XXX should assert; need to audit callers */ - return; + KASSERTMSG(cap != NULL, "sid=%"PRIx64, sid); - if (cap->cc_sessions) - (cap->cc_sessions)--; + KASSERT(cap->cc_sessions > 0); + cap->cc_sessions--; /* Call the driver cleanup routine, if available. */ if (cap->cc_freesession)
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:40:54 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert session id is valid in crypto_freesession. This gives us the opportunity to detect usage mistakes like use-after-free. Exception: Continue to silently ignore sid=0. To generate a diff of this commit: cvs rdiff -u -r1.129 -r1.130 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:40:38 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: opencrypto: Prune dead code now that crypto_dispatch never fails. To generate a diff of this commit: cvs rdiff -u -r1.123 -r1.124 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.123 src/sys/opencrypto/cryptodev.c:1.124 --- src/sys/opencrypto/cryptodev.c:1.123 Sun May 22 11:40:29 2022 +++ src/sys/opencrypto/cryptodev.c Sun May 22 11:40:38 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.123 2022/05/22 11:40:29 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.124 2022/05/22 11:40:38 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.123 2022/05/22 11:40:29 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.124 2022/05/22 11:40:38 riastradh Exp $"); #include #include @@ -1292,30 +1292,8 @@ cryptodev_mop(struct fcrypt *fcr, crp->crp_reqid = cnop[req].reqid; crp->crp_usropaque = cnop[req].opaque; cv_init(>crp_cv, "crydev"); -#ifdef notyet -eagain: -#endif crypto_dispatch(crp); cnop[req].status = 0; - mutex_enter(_mtx); /* XXX why mutex? */ - - switch (cnop[req].status) { -#ifdef notyet /* don't loop forever -- but EAGAIN not possible here yet */ - case EAGAIN: - mutex_exit(_mtx); - goto eagain; - break; -#endif - case 0: - break; - default: - DPRINTF("not waiting, error.\n"); - mutex_exit(_mtx); - cv_destroy(>crp_cv); - goto bail; - } - - mutex_exit(_mtx); cv_destroy(>crp_cv); bail: if (cnop[req].status) {
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:40:38 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: opencrypto: Prune dead code now that crypto_dispatch never fails. To generate a diff of this commit: cvs rdiff -u -r1.123 -r1.124 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:40:15 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert driver process routine returns 0 or ERESTART. No other errors are allowed -- other errors must be transmitted by crypto_done. All drivers in tree (sun8i_crypto, glxsb, via_padlock, mvcesa, mvxpsec, hifn, qat, ubsec, cryptosoft) have been audited for this. To generate a diff of this commit: cvs rdiff -u -r1.127 -r1.128 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.127 src/sys/opencrypto/crypto.c:1.128 --- src/sys/opencrypto/crypto.c:1.127 Sun May 22 11:40:03 2022 +++ src/sys/opencrypto/crypto.c Sun May 22 11:40:15 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.127 2022/05/22 11:40:03 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.128 2022/05/22 11:40:15 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.127 2022/05/22 11:40:03 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.128 2022/05/22 11:40:15 riastradh Exp $"); #include #include @@ -1318,7 +1318,6 @@ crypto_dispatch(struct cryptop *crp) softint_schedule(crypto_q_si); kpreempt_enable(); } - return 0; } @@ -1336,7 +1335,6 @@ crypto_dispatch(struct cryptop *crp) * to other drivers in cryptointr() later. */ TAILQ_INSERT_TAIL(crp_q, crp, crp_next); - result = 0; goto out; } @@ -1347,7 +1345,6 @@ crypto_dispatch(struct cryptop *crp) * it unblocks and the swi thread gets kicked. */ TAILQ_INSERT_TAIL(crp_q, crp, crp_next); - result = 0; goto out; } @@ -1358,6 +1355,7 @@ crypto_dispatch(struct cryptop *crp) */ crypto_driver_unlock(cap); result = crypto_invoke(crp, 0); + KASSERTMSG(result == 0 || result == ERESTART, "result=%d", result); if (result == ERESTART) { /* * The driver ran out of resources, mark the @@ -1369,18 +1367,11 @@ crypto_dispatch(struct cryptop *crp) crypto_driver_unlock(cap); TAILQ_INSERT_HEAD(crp_q, crp, crp_next); cryptostats.cs_blocks++; - - /* - * The crp is enqueued to crp_q, that is, - * no error occurs. So, this function should - * not return error. - */ - result = 0; } out: crypto_put_crp_qs(); - return result; + return 0; } /* @@ -1411,7 +1402,6 @@ crypto_kdispatch(struct cryptkop *krp) */ if (cap == NULL) { TAILQ_INSERT_TAIL(crp_kq, krp, krp_next); - result = 0; goto out; } @@ -1422,12 +1412,12 @@ crypto_kdispatch(struct cryptkop *krp) * it unblocks and the swi thread gets kicked. */ TAILQ_INSERT_TAIL(crp_kq, krp, krp_next); - result = 0; goto out; } crypto_driver_unlock(cap); result = crypto_kinvoke(krp, 0); + KASSERTMSG(result == 0 || result == ERESTART, "result=%d", result); if (result == ERESTART) { /* * The driver ran out of resources, mark the @@ -1439,18 +1429,11 @@ crypto_kdispatch(struct cryptkop *krp) crypto_driver_unlock(cap); TAILQ_INSERT_HEAD(crp_kq, krp, krp_next); cryptostats.cs_kblocks++; - - /* - * The krp is enqueued to crp_kq, that is, - * no error occurs. So, this function should - * not return error. - */ - result = 0; } out: crypto_put_crp_qs(); - return result; + return 0; } /* @@ -1500,15 +1483,14 @@ crypto_kinvoke(struct cryptkop *krp, int krp->reqcpu = curcpu(); crypto_driver_unlock(cap); error = (*process)(arg, krp, hint); + KASSERTMSG(error == 0 || error == ERESTART, "error=%d", + error); + return error; } else { - error = ENODEV; - } - - if (error) { - krp->krp_status = error; + krp->krp_status = ENODEV; crypto_kdone(krp); + return 0; } - return 0; } #ifdef CRYPTO_TIMING @@ -1542,6 +1524,7 @@ static int crypto_invoke(struct cryptop *crp, int hint) { struct cryptocap *cap; + int error; KASSERT(crp != NULL); KASSERT(crp->crp_callback != NULL); @@ -1567,7 +1550,10 @@ crypto_invoke(struct cryptop *crp, int h */ DPRINTF("calling process for %p\n", crp); crypto_driver_unlock(cap); - return (*process)(arg, crp, hint); + error = (*process)(arg, crp, hint); + KASSERTMSG(error == 0 || error == ERESTART, "error=%d", + error); + return error; } else { if (cap != NULL) { crypto_driver_unlock(cap); @@ -1880,6 +1866,8 @@ cryptointr(void *arg __unused) if (submit != NULL) { TAILQ_REMOVE(crp_q, submit, crp_next); result = crypto_invoke(submit, hint); + KASSERTMSG(result == 0 || result == ERESTART, + "result=%d", result); /* we must take here as the TAILQ op or kinvoke may need this mutex below. sigh. */ if (result == ERESTART) { @@ -1924,6 +1912,8 @@ cryptointr(void *arg
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:40:15 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert driver process routine returns 0 or ERESTART. No other errors are allowed -- other errors must be transmitted by crypto_done. All drivers in tree (sun8i_crypto, glxsb, via_padlock, mvcesa, mvxpsec, hifn, qat, ubsec, cryptosoft) have been audited for this. To generate a diff of this commit: cvs rdiff -u -r1.127 -r1.128 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:39:54 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert nonnull callback up front in crypto_dispatch. Same with crypto_kdispatch. Convert some dead branches downstream to assertions too. To generate a diff of this commit: cvs rdiff -u -r1.125 -r1.126 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.125 src/sys/opencrypto/crypto.c:1.126 --- src/sys/opencrypto/crypto.c:1.125 Sun May 22 11:39:37 2022 +++ src/sys/opencrypto/crypto.c Sun May 22 11:39:54 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.125 2022/05/22 11:39:37 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.126 2022/05/22 11:39:54 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.125 2022/05/22 11:39:37 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.126 2022/05/22 11:39:54 riastradh Exp $"); #include #include @@ -1283,6 +1283,7 @@ crypto_dispatch(struct cryptop *crp) struct crypto_crp_q *crp_q; KASSERT(crp != NULL); + KASSERT(crp->crp_callback != NULL); KASSERT(crp->crp_desc != NULL); KASSERT(crp->crp_buf != NULL); KASSERT(!cpu_intr_p()); @@ -1395,6 +1396,7 @@ crypto_kdispatch(struct cryptkop *krp) struct crypto_crp_kq *crp_kq; KASSERT(krp != NULL); + KASSERT(krp->krp_callback != NULL); KASSERT(!cpu_intr_p()); cryptostats.cs_kops++; @@ -1462,15 +1464,9 @@ crypto_kinvoke(struct cryptkop *krp, int int error; KASSERT(krp != NULL); + KASSERT(krp->krp_callback != NULL); KASSERT(!cpu_intr_p()); - /* Sanity checks. */ - if (krp->krp_callback == NULL) { - cv_destroy(>krp_cv); - crypto_kfreereq(krp); - return EINVAL; - } - mutex_enter(_drv_mtx); for (hid = 0; hid < crypto_drivers_num; hid++) { cap = crypto_checkdriver(hid); @@ -1548,21 +1544,14 @@ crypto_invoke(struct cryptop *crp, int h struct cryptocap *cap; KASSERT(crp != NULL); + KASSERT(crp->crp_callback != NULL); + KASSERT(crp->crp_desc != NULL); KASSERT(!cpu_intr_p()); #ifdef CRYPTO_TIMING if (crypto_timing) crypto_tstat(_invoke, >crp_tstamp); #endif - /* Sanity checks. */ - if (crp->crp_callback == NULL) { - return EINVAL; - } - if (crp->crp_desc == NULL) { - crp->crp_etype = EINVAL; - crypto_done(crp); - return 0; - } cap = crypto_checkdriver_lock(CRYPTO_SESID2HID(crp->crp_sid)); if (cap != NULL && (cap->cc_flags & CRYPTOCAP_F_CLEANUP) == 0) {
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:39:54 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert nonnull callback up front in crypto_dispatch. Same with crypto_kdispatch. Convert some dead branches downstream to assertions too. To generate a diff of this commit: cvs rdiff -u -r1.125 -r1.126 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:39:46 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Nix dead code now that crypto_freesession never fails. To generate a diff of this commit: cvs rdiff -u -r1.120 -r1.121 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.120 src/sys/opencrypto/cryptodev.c:1.121 --- src/sys/opencrypto/cryptodev.c:1.120 Sun May 22 11:39:37 2022 +++ src/sys/opencrypto/cryptodev.c Sun May 22 11:39:45 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.120 2022/05/22 11:39:37 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.121 2022/05/22 11:39:45 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.120 2022/05/22 11:39:37 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.121 2022/05/22 11:39:45 riastradh Exp $"); #include #include @@ -182,11 +182,11 @@ static struct csession *csecreate(struct u_int64_t, void *, u_int64_t, u_int32_t, u_int32_t, u_int32_t, const struct enc_xform *, const struct auth_hash *, const struct comp_algo *); -static int csefree(struct csession *); +static void csefree(struct csession *); static int cryptodev_key(struct crypt_kop *); static int cryptodev_mkey(struct fcrypt *, struct crypt_n_kop *, int); -static int cryptodev_msessionfin(struct fcrypt *, int, u_int32_t *); +static void cryptodev_msessionfin(struct fcrypt *, int, u_int32_t *); static void cryptodev_cb(struct cryptop *); static void cryptodevkey_cb(struct cryptkop *); @@ -317,7 +317,7 @@ mbail: } csedelete(fcr, cse); mutex_exit(_mtx); - error = csefree(cse); + csefree(cse); break; case CIOCNFSESSION: mutex_enter(_mtx); @@ -334,7 +334,7 @@ mbail: error = copyin(sfop->sesid, sesid, (sfop->count * sizeof(u_int32_t))); if (!error) { - error = cryptodev_msessionfin(fcr, sfop->count, sesid); + cryptodev_msessionfin(fcr, sfop->count, sesid); } kmem_free(sesid, (sfop->count * sizeof(u_int32_t))); break; @@ -922,7 +922,7 @@ cryptof_close(struct file *fp) while ((cse = TAILQ_FIRST(>csessions))) { TAILQ_REMOVE(>csessions, cse, next); mutex_exit(_mtx); - (void)csefree(cse); + csefree(cse); mutex_enter(_mtx); } seldestroy(>sinfo); @@ -950,7 +950,7 @@ csefind(struct fcrypt *fcr, u_int ses) TAILQ_FOREACH_SAFE(cse, >csessions, next, cnext) if (cse->ses == ses) ret = cse; - + return ret; } @@ -1014,19 +1014,16 @@ csecreate(struct fcrypt *fcr, u_int64_t } } -static int +static void csefree(struct csession *cse) { - int error; crypto_freesession(cse->sid); - error = 0; if (cse->key) free(cse->key, M_XDATA); if (cse->mackey) free(cse->mackey, M_XDATA); pool_put(, cse); - return error; } static int @@ -1757,11 +1754,11 @@ cryptodev_msession(struct fcrypt *fcr, s return 0; } -static int +static void cryptodev_msessionfin(struct fcrypt *fcr, int count, u_int32_t *sesid) { struct csession *cse; - int req, error = 0; + int req; mutex_enter(_mtx); for(req = 0; req < count; req++) { @@ -1770,11 +1767,10 @@ cryptodev_msessionfin(struct fcrypt *fcr continue; csedelete(fcr, cse); mutex_exit(_mtx); - error = csefree(cse); + csefree(cse); mutex_enter(_mtx); } mutex_exit(_mtx); - return error; } /*
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:39:46 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Nix dead code now that crypto_freesession never fails. To generate a diff of this commit: cvs rdiff -u -r1.120 -r1.121 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:39:17 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): crypto_freesession should never fail here. It can only fail if we pass it an invalid sid, which the logic to maintain the user sessions should not do. So kassert error=0 here. To generate a diff of this commit: cvs rdiff -u -r1.118 -r1.119 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.118 src/sys/opencrypto/cryptodev.c:1.119 --- src/sys/opencrypto/cryptodev.c:1.118 Sun May 22 11:34:29 2022 +++ src/sys/opencrypto/cryptodev.c Sun May 22 11:39:17 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.118 2022/05/22 11:34:29 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.119 2022/05/22 11:39:17 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.118 2022/05/22 11:34:29 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.119 2022/05/22 11:39:17 riastradh Exp $"); #include #include @@ -1020,6 +1020,7 @@ csefree(struct csession *cse) int error; error = crypto_freesession(cse->sid); + KASSERTMSG(error == 0, "error=%d", error); if (cse->key) free(cse->key, M_XDATA); if (cse->mackey)
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:39:17 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): crypto_freesession should never fail here. It can only fail if we pass it an invalid sid, which the logic to maintain the user sessions should not do. So kassert error=0 here. To generate a diff of this commit: cvs rdiff -u -r1.118 -r1.119 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:38:59 UTC 2022 Modified Files: src/sys/opencrypto: cryptosoft.c Log Message: cryptosoft(4): Prune dead branches. Assert session id validity. To generate a diff of this commit: cvs rdiff -u -r1.62 -r1.63 src/sys/opencrypto/cryptosoft.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptosoft.c diff -u src/sys/opencrypto/cryptosoft.c:1.62 src/sys/opencrypto/cryptosoft.c:1.63 --- src/sys/opencrypto/cryptosoft.c:1.62 Sun May 22 11:29:25 2022 +++ src/sys/opencrypto/cryptosoft.c Sun May 22 11:38:59 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptosoft.c,v 1.62 2022/05/22 11:29:25 riastradh Exp $ */ +/* $NetBSD: cryptosoft.c,v 1.63 2022/05/22 11:38:59 riastradh Exp $ */ /* $FreeBSD: src/sys/opencrypto/cryptosoft.c,v 1.2.2.1 2002/11/21 23:34:23 sam Exp $ */ /* $OpenBSD: cryptosoft.c,v 1.35 2002/04/26 08:43:50 deraadt Exp $ */ @@ -24,7 +24,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.62 2022/05/22 11:29:25 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.63 2022/05/22 11:38:59 riastradh Exp $"); #include #include @@ -766,9 +766,6 @@ swcr_newsession(void *arg, u_int32_t *si u_int32_t i; int k, error; - if (sid == NULL || cri == NULL) - return EINVAL; - if (swcr_sessions) { for (i = 1; i < swcr_sesnum; i++) if (swcr_sessions[i] == NULL) @@ -1128,9 +1125,9 @@ swcr_freesession(void *arg, u_int64_t ti struct swcr_data *swd; u_int32_t sid = ((u_int32_t) tid) & 0x; - if (sid > swcr_sesnum || swcr_sessions == NULL || - swcr_sessions[sid] == NULL) - return EINVAL; + KASSERTMSG(sid < swcr_sesnum, "sid=%"PRIu32" swcr_sesnum=%"PRIu32, + sid, swcr_sesnum); + KASSERT(swcr_sessions[sid]); swd = swcr_sessions[sid]; swcr_sessions[sid] = NULL;
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:38:59 UTC 2022 Modified Files: src/sys/opencrypto: cryptosoft.c Log Message: cryptosoft(4): Prune dead branches. Assert session id validity. To generate a diff of this commit: cvs rdiff -u -r1.62 -r1.63 src/sys/opencrypto/cryptosoft.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:34:40 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert crp_desc and crp_buf are nonnull. - crypto_getreq ensures crp_desc is nonnull. - Caller is responsible for setting crp_buf. To generate a diff of this commit: cvs rdiff -u -r1.122 -r1.123 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.122 src/sys/opencrypto/crypto.c:1.123 --- src/sys/opencrypto/crypto.c:1.122 Sun May 22 11:34:17 2022 +++ src/sys/opencrypto/crypto.c Sun May 22 11:34:40 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.122 2022/05/22 11:34:17 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.123 2022/05/22 11:34:40 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.122 2022/05/22 11:34:17 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.123 2022/05/22 11:34:40 riastradh Exp $"); #include #include @@ -1287,6 +1287,8 @@ crypto_dispatch(struct cryptop *crp) struct crypto_crp_q *crp_q; KASSERT(crp != NULL); + KASSERT(crp->crp_desc != NULL); + KASSERT(crp->crp_buf != NULL); KASSERT(!cpu_intr_p()); DPRINTF("crp %p, alg %d\n", crp, crp->crp_desc->crd_alg);
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:34:40 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert crp_desc and crp_buf are nonnull. - crypto_getreq ensures crp_desc is nonnull. - Caller is responsible for setting crp_buf. To generate a diff of this commit: cvs rdiff -u -r1.122 -r1.123 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:34:29 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Refuse crypto operations with nothing in them earlier. This way we avoid passing 0 to crypto_getreq -- makes it easier to reason about everything downstream. To generate a diff of this commit: cvs rdiff -u -r1.117 -r1.118 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.117 src/sys/opencrypto/cryptodev.c:1.118 --- src/sys/opencrypto/cryptodev.c:1.117 Sun May 22 11:30:41 2022 +++ src/sys/opencrypto/cryptodev.c Sun May 22 11:34:29 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.117 2022/05/22 11:30:41 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.118 2022/05/22 11:34:29 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.117 2022/05/22 11:30:41 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.118 2022/05/22 11:34:29 riastradh Exp $"); #include #include @@ -471,6 +471,9 @@ cryptodev_op(struct csession *cse, struc return EINVAL; } + if (cse->tcomp == NULL && cse->txform == NULL && cse->thash == NULL) + return EINVAL; + DPRINTF("cryptodev_op[%u]: iov_len %d\n", CRYPTO_SESID2LID(cse->sid), iov_len); if ((cse->tcomp) && cop->dst_len) { @@ -1131,6 +1134,13 @@ cryptodev_mop(struct fcrypt *fcr, } } + if (cse->txform == NULL && + cse->thash == NULL && + cse->tcomp == NULL) { + cnop[req].status = EINVAL; + goto bail; + } + /* sanitize */ if (cnop[req].len <= 0) { cnop[req].status = ENOMEM;
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:34:29 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Refuse crypto operations with nothing in them earlier. This way we avoid passing 0 to crypto_getreq -- makes it easier to reason about everything downstream. To generate a diff of this commit: cvs rdiff -u -r1.117 -r1.118 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:30:05 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c cryptodev.h Log Message: opencrypto: Nix CRYPTO_F_DONE. Nothing uses it any more. To generate a diff of this commit: cvs rdiff -u -r1.120 -r1.121 src/sys/opencrypto/crypto.c cvs rdiff -u -r1.44 -r1.45 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:30:05 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c cryptodev.h Log Message: opencrypto: Nix CRYPTO_F_DONE. Nothing uses it any more. To generate a diff of this commit: cvs rdiff -u -r1.120 -r1.121 src/sys/opencrypto/crypto.c cvs rdiff -u -r1.44 -r1.45 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.120 src/sys/opencrypto/crypto.c:1.121 --- src/sys/opencrypto/crypto.c:1.120 Sun May 22 11:25:14 2022 +++ src/sys/opencrypto/crypto.c Sun May 22 11:30:05 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.120 2022/05/22 11:25:14 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.121 2022/05/22 11:30:05 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.120 2022/05/22 11:25:14 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.121 2022/05/22 11:30:05 riastradh Exp $"); #include #include @@ -1748,8 +1748,6 @@ crypto_done(struct cryptop *crp) #endif DPRINTF("lid[%u]: crp %p\n", CRYPTO_SESID2LID(crp->crp_sid), crp); - crp->crp_flags |= CRYPTO_F_DONE; - qs = crypto_get_crp_ret_qs(crp->reqcpu); crp_ret_q = >crp_ret_q; wasempty = TAILQ_EMPTY(crp_ret_q); @@ -1780,8 +1778,6 @@ crypto_kdone(struct cryptkop *krp) if (krp->krp_status != 0) cryptostats.cs_kerrs++; - krp->krp_flags |= CRYPTO_F_DONE; - qs = crypto_get_crp_ret_qs(krp->reqcpu); crp_ret_kq = >crp_ret_kq; Index: src/sys/opencrypto/cryptodev.h diff -u src/sys/opencrypto/cryptodev.h:1.44 src/sys/opencrypto/cryptodev.h:1.45 --- src/sys/opencrypto/cryptodev.h:1.44 Sun May 22 11:25:14 2022 +++ src/sys/opencrypto/cryptodev.h Sun May 22 11:30:05 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.h,v 1.44 2022/05/22 11:25:14 riastradh Exp $ */ +/* $NetBSD: cryptodev.h,v 1.45 2022/05/22 11:30:05 riastradh 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 $ */ @@ -470,10 +470,10 @@ struct cryptop { #define CRYPTO_F_REL 0x0004 /* Must return data in same place */ #define CRYPTO_F_BATCH 0x0008 /* Batch op if possible possible */ #define CRYPTO_F_UNUSED0 0x0010 /* was CRYPTO_F_CBIMM */ -#define CRYPTO_F_DONE 0x0020 /* Operation completed */ -#define CRYPTO_F_UNUSED1 0x0040 /* was CRYPTO_F_CBIFSYNC */ +#define CRYPTO_F_UNUSED1 0x0020 /* was CRYPTO_F_DONE */ +#define CRYPTO_F_UNUSED2 0x0040 /* was CRYPTO_F_CBIFSYNC */ #define CRYPTO_F_ONRETQ 0x0080 /* Request is on return queue */ -#define CRYPTO_F_UNUSED2 0x0100 /* was CRYPTO_F_USER */ +#define CRYPTO_F_UNUSED3 0x0100 /* was CRYPTO_F_USER */ #define CRYPTO_F_MORE 0x0200 /* more data to follow */ int crp_devflags; /* other than cryptodev.c must not use. */
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:29:54 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Fix possible use-after-free in race around detach. This is extremely unlikely because I don't think we have any drivers for removable crypto decelerators^Waccelerators...but if we were to sprout one, and someone ran crypto_dispatch concurrently with crypto_unregister, cryptodev_cb/mcb would enter with crp->crp_etype = EAGAIN and with CRYPTO_F_DONE set in crp->crp_flags. In this case, cryptodev_cb/mcb would issue crypto_dispatch but -- since nothing clears CRYPTO_F_DONE -- it would _also_ consider the request done and notify the ioctl thread of that. With this change, we return early if crypto_dispatch succeeds. No need to consult CRYPTO_F_DONE: if the callback is invoked it's done, and if we try to redispatch it on EAGAIN but crypto_dispatch fails, it's done. (Soon we'll get rid of the possibility of crypto_dispatch failing synchronously, but not just yet.) XXX This path could really use some testing! To generate a diff of this commit: cvs rdiff -u -r1.115 -r1.116 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.115 src/sys/opencrypto/cryptodev.c:1.116 --- src/sys/opencrypto/cryptodev.c:1.115 Sat May 21 23:11:03 2022 +++ src/sys/opencrypto/cryptodev.c Sun May 22 11:29:54 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.115 2022/05/21 23:11:03 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.116 2022/05/22 11:29:54 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.115 2022/05/21 23:11:03 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.116 2022/05/22 11:29:54 riastradh Exp $"); #include #include @@ -715,20 +715,18 @@ static int cryptodev_cb(struct cryptop *crp) { struct csession *cse = crp->crp_opaque; - int error = 0; + int error; - mutex_enter(_mtx); - cse->error = crp->crp_etype; - if (crp->crp_etype == EAGAIN) { - /* always drop mutex to call dispatch routine */ - mutex_exit(_mtx); + if ((error = crp->crp_etype) == EAGAIN) { error = crypto_dispatch(crp); - mutex_enter(_mtx); - } - if (error != 0 || (crp->crp_flags & CRYPTO_F_DONE)) { - crp->crp_devflags |= CRYPTODEV_F_RET; - cv_signal(>crp_cv); + if (error == 0) + return 0; } + + mutex_enter(_mtx); + cse->error = error; + crp->crp_devflags |= CRYPTODEV_F_RET; + cv_signal(>crp_cv); mutex_exit(_mtx); return 0; } @@ -737,15 +735,16 @@ static int cryptodev_mcb(struct cryptop *crp) { struct csession *cse = crp->crp_opaque; + int error; - mutex_enter(_mtx); - cse->error = crp->crp_etype; - if (crp->crp_etype == EAGAIN) { - mutex_exit(_mtx); - (void)crypto_dispatch(crp); - mutex_enter(_mtx); + if ((error = crp->crp_etype) == EAGAIN) { + error = crypto_dispatch(crp); + if (error == 0) + return 0; } + mutex_enter(_mtx); + cse->error = error; TAILQ_INSERT_TAIL(>fcrp->crp_ret_mq, crp, crp_next); selnotify(>fcrp->sinfo, 0, 0); mutex_exit(_mtx);
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:29:54 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Fix possible use-after-free in race around detach. This is extremely unlikely because I don't think we have any drivers for removable crypto decelerators^Waccelerators...but if we were to sprout one, and someone ran crypto_dispatch concurrently with crypto_unregister, cryptodev_cb/mcb would enter with crp->crp_etype = EAGAIN and with CRYPTO_F_DONE set in crp->crp_flags. In this case, cryptodev_cb/mcb would issue crypto_dispatch but -- since nothing clears CRYPTO_F_DONE -- it would _also_ consider the request done and notify the ioctl thread of that. With this change, we return early if crypto_dispatch succeeds. No need to consult CRYPTO_F_DONE: if the callback is invoked it's done, and if we try to redispatch it on EAGAIN but crypto_dispatch fails, it's done. (Soon we'll get rid of the possibility of crypto_dispatch failing synchronously, but not just yet.) XXX This path could really use some testing! To generate a diff of this commit: cvs rdiff -u -r1.115 -r1.116 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:29:25 UTC 2022 Modified Files: src/sys/opencrypto: cryptosoft.c Log Message: cryptosoft(4): Rip out nonsense to quietly ignore sid=0. This is no longer necessary because crypto_freesession no longer calls into the driver for session ids that were never allocated in the first place. To generate a diff of this commit: cvs rdiff -u -r1.61 -r1.62 src/sys/opencrypto/cryptosoft.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptosoft.c diff -u src/sys/opencrypto/cryptosoft.c:1.61 src/sys/opencrypto/cryptosoft.c:1.62 --- src/sys/opencrypto/cryptosoft.c:1.61 Tue Apr 6 03:38:04 2021 +++ src/sys/opencrypto/cryptosoft.c Sun May 22 11:29:25 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptosoft.c,v 1.61 2021/04/06 03:38:04 knakahara Exp $ */ +/* $NetBSD: cryptosoft.c,v 1.62 2022/05/22 11:29:25 riastradh Exp $ */ /* $FreeBSD: src/sys/opencrypto/cryptosoft.c,v 1.2.2.1 2002/11/21 23:34:23 sam Exp $ */ /* $OpenBSD: cryptosoft.c,v 1.35 2002/04/26 08:43:50 deraadt Exp $ */ @@ -24,7 +24,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.61 2021/04/06 03:38:04 knakahara Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.62 2022/05/22 11:29:25 riastradh Exp $"); #include #include @@ -1132,10 +1132,6 @@ swcr_freesession(void *arg, u_int64_t ti swcr_sessions[sid] == NULL) return EINVAL; - /* Silently accept and return */ - if (sid == 0) - return 0; - swd = swcr_sessions[sid]; swcr_sessions[sid] = NULL; swcr_freesession_internal(swd);
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:29:25 UTC 2022 Modified Files: src/sys/opencrypto: cryptosoft.c Log Message: cryptosoft(4): Rip out nonsense to quietly ignore sid=0. This is no longer necessary because crypto_freesession no longer calls into the driver for session ids that were never allocated in the first place. To generate a diff of this commit: cvs rdiff -u -r1.61 -r1.62 src/sys/opencrypto/cryptosoft.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:25:14 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c cryptodev.h Log Message: opencrypto: Make sid=0 always invalid, but OK to free. Previously, crypto_newsession could sometimes return 0 as the driver-specific part of the session id, and 0 as the hid, for sid=0. But netipsec assumes that it is always safe to free sid=0 from zero-initialized memory even if crypto_newsession has never succeeded. So it was up to every driver in tree to gracefully handle sid=0, if it happened to get assigned hid=0. And, as long as the freesession callback was expected to just return an error code when given a bogus session id, that worked out fine...because nothing ever used the error code. That was a terrible fragile system that should never have been invented. Instead, let's just ensure that valid session ids are nonzero, and make crypto_freesession with sid=0 be a no-op. To generate a diff of this commit: cvs rdiff -u -r1.119 -r1.120 src/sys/opencrypto/crypto.c cvs rdiff -u -r1.43 -r1.44 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.119 src/sys/opencrypto/crypto.c:1.120 --- src/sys/opencrypto/crypto.c:1.119 Thu May 19 20:51:59 2022 +++ src/sys/opencrypto/crypto.c Sun May 22 11:25:14 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.119 2022/05/19 20:51:59 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.120 2022/05/22 11:25:14 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.119 2022/05/19 20:51:59 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.120 2022/05/22 11:25:14 riastradh Exp $"); #include #include @@ -800,6 +800,16 @@ crypto_newsession(u_int64_t *sid, struct struct cryptocap *cap; int err = EINVAL; + /* + * On failure, leave *sid initialized to a sentinel value that + * crypto_freesession will ignore. This is the same as what + * you get from zero-initialized memory -- some callers (I'm + * looking at you, netipsec!) have paths that lead from + * zero-initialized memory into crypto_freesession without any + * crypto_newsession. + */ + *sid = 0; + mutex_enter(_drv_mtx); cap = crypto_select_driver_lock(cri, hard); @@ -807,6 +817,7 @@ crypto_newsession(u_int64_t *sid, struct u_int32_t hid, lid; hid = cap - crypto_drivers; + KASSERT(hid < 0xff); /* * Can't do everything in one session. * @@ -820,10 +831,11 @@ crypto_newsession(u_int64_t *sid, struct err = cap->cc_newsession(cap->cc_arg, , cri); crypto_driver_lock(cap); if (err == 0) { - (*sid) = hid; + (*sid) = hid + 1; (*sid) <<= 32; (*sid) |= (lid & 0x); - (cap->cc_sessions)++; + KASSERT(*sid != 0); + cap->cc_sessions++; } else { DPRINTF("crypto_drivers[%d].cc_newsession() failed. error=%d\n", hid, err); @@ -846,6 +858,17 @@ crypto_freesession(u_int64_t sid) struct cryptocap *cap; int err = 0; + /* + * crypto_newsession never returns 0 as a sid (by virtue of + * never returning 0 as a hid, which is part of the sid). + * However, some callers assume that freeing zero is safe. + * Previously this relied on all drivers to agree that freeing + * invalid sids is a no-op, but that's a terrible API contract + * that we're getting rid of. + */ + if (sid == 0) + return; + /* Determine two IDs. */ cap = crypto_checkdriver_lock(CRYPTO_SESID2HID(sid)); if (cap == NULL) Index: src/sys/opencrypto/cryptodev.h diff -u src/sys/opencrypto/cryptodev.h:1.43 src/sys/opencrypto/cryptodev.h:1.44 --- src/sys/opencrypto/cryptodev.h:1.43 Thu May 19 20:51:46 2022 +++ src/sys/opencrypto/cryptodev.h Sun May 22 11:25:14 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.h,v 1.43 2022/05/19 20:51:46 riastradh Exp $ */ +/* $NetBSD: cryptodev.h,v 1.44 2022/05/22 11:25:14 riastradh 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 $ */ @@ -589,7 +589,7 @@ struct cryptocap { * a copy of the driver's capabilities that can be used by client code to * optimize operation. */ -#define CRYPTO_SESID2HID(_sid) (((_sid) >> 32) & 0xff) +#define CRYPTO_SESID2HID(_sid) _sid) >> 32) & 0xff) - 1) #define CRYPTO_SESID2CAPS(_sid) (((_sid) >> 56) & 0xff) #define CRYPTO_SESID2LID(_sid) (((u_int32_t) (_sid)) & 0x)
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sun May 22 11:25:14 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c cryptodev.h Log Message: opencrypto: Make sid=0 always invalid, but OK to free. Previously, crypto_newsession could sometimes return 0 as the driver-specific part of the session id, and 0 as the hid, for sid=0. But netipsec assumes that it is always safe to free sid=0 from zero-initialized memory even if crypto_newsession has never succeeded. So it was up to every driver in tree to gracefully handle sid=0, if it happened to get assigned hid=0. And, as long as the freesession callback was expected to just return an error code when given a bogus session id, that worked out fine...because nothing ever used the error code. That was a terrible fragile system that should never have been invented. Instead, let's just ensure that valid session ids are nonzero, and make crypto_freesession with sid=0 be a no-op. To generate a diff of this commit: cvs rdiff -u -r1.119 -r1.120 src/sys/opencrypto/crypto.c cvs rdiff -u -r1.43 -r1.44 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sat May 21 23:11:03 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Fix set-but-unused variable warning. This deliberately ignores the error code returned by crypto_dispatch, but that error code is fundamentally incoherent and the issue will be mooted by subsequent changes to make it return void and always pass the error through the callback, as well as subsequent changes to rip out the EAGAIN logic anyway. To generate a diff of this commit: cvs rdiff -u -r1.114 -r1.115 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.114 src/sys/opencrypto/cryptodev.c:1.115 --- src/sys/opencrypto/cryptodev.c:1.114 Sat May 21 20:37:18 2022 +++ src/sys/opencrypto/cryptodev.c Sat May 21 23:11:03 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.114 2022/05/21 20:37:18 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.115 2022/05/21 23:11:03 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.114 2022/05/21 20:37:18 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.115 2022/05/21 23:11:03 riastradh Exp $"); #include #include @@ -737,13 +737,12 @@ static int cryptodev_mcb(struct cryptop *crp) { struct csession *cse = crp->crp_opaque; - int error = 0; mutex_enter(_mtx); cse->error = crp->crp_etype; if (crp->crp_etype == EAGAIN) { mutex_exit(_mtx); - error = crypto_dispatch(crp); + (void)crypto_dispatch(crp); mutex_enter(_mtx); }
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sat May 21 23:11:03 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Fix set-but-unused variable warning. This deliberately ignores the error code returned by crypto_dispatch, but that error code is fundamentally incoherent and the issue will be mooted by subsequent changes to make it return void and always pass the error through the callback, as well as subsequent changes to rip out the EAGAIN logic anyway. To generate a diff of this commit: cvs rdiff -u -r1.114 -r1.115 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sat May 21 20:37:18 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Don't signal the condvar for multi-operation completion. The condvar may be destroyed by the time we got here, and nothing waits on it anyway -- instead the caller is expected to select/poll for completion in userland. The bug was already here, but the recent change to eliminate CRYPTO_F_CBIMM made it happen more often by causing the callback to _always_ be run asynchronously instead of sometimes being run synchronously. To generate a diff of this commit: cvs rdiff -u -r1.113 -r1.114 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.113 src/sys/opencrypto/cryptodev.c:1.114 --- src/sys/opencrypto/cryptodev.c:1.113 Thu May 19 20:51:46 2022 +++ src/sys/opencrypto/cryptodev.c Sat May 21 20:37:18 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.113 2022/05/19 20:51:46 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.114 2022/05/21 20:37:18 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.113 2022/05/19 20:51:46 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.114 2022/05/21 20:37:18 riastradh Exp $"); #include #include @@ -746,9 +746,6 @@ cryptodev_mcb(struct cryptop *crp) error = crypto_dispatch(crp); mutex_enter(_mtx); } - if (error != 0 || (crp->crp_flags & CRYPTO_F_DONE)) { - cv_signal(>crp_cv); - } TAILQ_INSERT_TAIL(>fcrp->crp_ret_mq, crp, crp_next); selnotify(>fcrp->sinfo, 0, 0);
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sat May 21 20:37:18 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Don't signal the condvar for multi-operation completion. The condvar may be destroyed by the time we got here, and nothing waits on it anyway -- instead the caller is expected to select/poll for completion in userland. The bug was already here, but the recent change to eliminate CRYPTO_F_CBIMM made it happen more often by causing the callback to _always_ be run asynchronously instead of sometimes being run synchronously. To generate a diff of this commit: cvs rdiff -u -r1.113 -r1.114 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Thu May 19 20:51:59 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert !cpu_intr_p() on dispatch and invoke. These should only ever have been potentially called from hard interrupt context by CRYPTO_F_CBIMM callbacks (CBIMM = call back immediately). CRYPTO_F_CBIMM is no more, so there is no more need to allow this case of call from hard interrupt context. To generate a diff of this commit: cvs rdiff -u -r1.118 -r1.119 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.118 src/sys/opencrypto/crypto.c:1.119 --- src/sys/opencrypto/crypto.c:1.118 Thu May 19 20:51:46 2022 +++ src/sys/opencrypto/crypto.c Thu May 19 20:51:59 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.118 2022/05/19 20:51:46 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.119 2022/05/19 20:51:59 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.118 2022/05/19 20:51:46 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.119 2022/05/19 20:51:59 riastradh Exp $"); #include #include @@ -1264,6 +1264,7 @@ crypto_dispatch(struct cryptop *crp) struct crypto_crp_q *crp_q; KASSERT(crp != NULL); + KASSERT(!cpu_intr_p()); DPRINTF("crp %p, alg %d\n", crp, crp->crp_desc->crd_alg); @@ -1373,6 +1374,7 @@ crypto_kdispatch(struct cryptkop *krp) struct crypto_crp_kq *crp_kq; KASSERT(krp != NULL); + KASSERT(!cpu_intr_p()); cryptostats.cs_kops++; @@ -1439,6 +1441,7 @@ crypto_kinvoke(struct cryptkop *krp, int int error; KASSERT(krp != NULL); + KASSERT(!cpu_intr_p()); /* Sanity checks. */ if (krp->krp_callback == NULL) { @@ -1524,6 +1527,7 @@ crypto_invoke(struct cryptop *crp, int h struct cryptocap *cap; KASSERT(crp != NULL); + KASSERT(!cpu_intr_p()); #ifdef CRYPTO_TIMING if (crypto_timing)
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Thu May 19 20:51:59 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Assert !cpu_intr_p() on dispatch and invoke. These should only ever have been potentially called from hard interrupt context by CRYPTO_F_CBIMM callbacks (CBIMM = call back immediately). CRYPTO_F_CBIMM is no more, so there is no more need to allow this case of call from hard interrupt context. To generate a diff of this commit: cvs rdiff -u -r1.118 -r1.119 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Thu May 19 20:51:46 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c cryptodev.c cryptodev.h Log Message: opencrypto: Nix CRYPTO_F_USER, CRYPTO_F_CBIMM, CRYPTO_F_CBIFSYNC. CRYPTO_F_USER is no longer needed. It was introduced in 2008 by darran@ in crypto.c 1.30, cryptodev.c 1.45 in an attempt to avoid double-free between the issuing thread and asynchronous callback. But the `fix' didn't work. In 2017, knakahara@ fixed it properly in cryptodev.c 1.87 by distinguishing `the crypto operation has completed' (CRYPTO_F_DONE) from `the callback is done touching the crp object' (CRYPTO_F_DQRETQ, now renamed to CRYPTODEV_F_RET). CRYPTO_F_CBIMM formerly served to invoke the callback synchronously from the driver's interrupt completion routine, to reduce contention on what was once a single cryptoret thread. Now, there is a per-CPU queue and softint for much cheaper processing, so there is less motivation for this in the first place. So let's remove the complicated logic. This means the callbacks never run in hard interrupt context, which means we don't need to worry about recursion into crypto_dispatch in hard interrupt context. To generate a diff of this commit: cvs rdiff -u -r1.117 -r1.118 src/sys/opencrypto/crypto.c cvs rdiff -u -r1.112 -r1.113 src/sys/opencrypto/cryptodev.c cvs rdiff -u -r1.42 -r1.43 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.117 src/sys/opencrypto/crypto.c:1.118 --- src/sys/opencrypto/crypto.c:1.117 Tue May 17 10:32:58 2022 +++ src/sys/opencrypto/crypto.c Thu May 19 20:51:46 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.117 2022/05/17 10:32:58 riastradh Exp $ */ +/* $NetBSD: crypto.c,v 1.118 2022/05/19 20:51:46 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.117 2022/05/17 10:32:58 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.118 2022/05/19 20:51:46 riastradh Exp $"); #include #include @@ -1707,6 +1707,9 @@ crypto_kgetreq(int num __unused, int prf void crypto_done(struct cryptop *crp) { + int wasempty; + struct crypto_crp_ret_qs *qs; + struct crypto_crp_ret_q *crp_ret_q; KASSERT(crp != NULL); @@ -1720,70 +1723,19 @@ crypto_done(struct cryptop *crp) crp->crp_flags |= CRYPTO_F_DONE; - /* - * Normal case; queue the callback for the thread. - * - * The return queue is manipulated by the swi thread - * and, potentially, by crypto device drivers calling - * back to mark operations completed. Thus we need - * to mask both while manipulating the return queue. - */ - if (crp->crp_flags & CRYPTO_F_CBIMM) { - /* - * Do the callback directly. This is ok when the - * callback routine does very little (e.g. the - * /dev/crypto callback method just does a wakeup). - */ -#ifdef CRYPTO_TIMING - if (crypto_timing) { - /* - * NB: We must copy the timestamp before - * doing the callback as the cryptop is - * likely to be reclaimed. - */ - struct timespec t = crp->crp_tstamp; - crypto_tstat(_cb, ); - crp->crp_callback(crp); - crypto_tstat(_finis, ); - } else -#endif - crp->crp_callback(crp); - } else { -#if 0 - if (crp->crp_flags & CRYPTO_F_USER) { - /* - * TODO: - * If crp->crp_flags & CRYPTO_F_USER and the used - * encryption driver does all the processing in - * the same context, we can skip enqueueing crp_ret_q - * and softint_schedule(crypto_ret_si). - */ - DPRINTF("lid[%u]: crp %p CRYPTO_F_USER\n", -CRYPTO_SESID2LID(crp->crp_sid), crp); - } else -#endif - { - int wasempty; - struct crypto_crp_ret_qs *qs; - struct crypto_crp_ret_q *crp_ret_q; - - qs = crypto_get_crp_ret_qs(crp->reqcpu); - crp_ret_q = >crp_ret_q; - wasempty = TAILQ_EMPTY(crp_ret_q); - DPRINTF("lid[%u]: queueing %p\n", -CRYPTO_SESID2LID(crp->crp_sid), crp); - crp->crp_flags |= CRYPTO_F_ONRETQ; - TAILQ_INSERT_TAIL(crp_ret_q, crp, crp_next); - qs->crp_ret_q_len++; - if (wasempty && !qs->crp_ret_q_exit_flag) { -DPRINTF("lid[%u]: waking cryptoret," - "crp %p hit empty queue\n.", - CRYPTO_SESID2LID(crp->crp_sid), crp); -softint_schedule_cpu(crypto_ret_si, crp->reqcpu); - } - crypto_put_crp_ret_qs(crp->reqcpu); - } + qs = crypto_get_crp_ret_qs(crp->reqcpu); + crp_ret_q = >crp_ret_q; + wasempty = TAILQ_EMPTY(crp_ret_q); + DPRINTF("lid[%u]: queueing %p\n", CRYPTO_SESID2LID(crp->crp_sid), crp); + crp->crp_flags |= CRYPTO_F_ONRETQ; + TAILQ_INSERT_TAIL(crp_ret_q, crp, crp_next); + qs->crp_ret_q_len++; + if (wasempty && !qs->crp_ret_q_exit_flag) { + DPRINTF("lid[%u]: waking cryptoret, crp %p hit empty queue\n.", +
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Thu May 19 20:51:46 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c cryptodev.c cryptodev.h Log Message: opencrypto: Nix CRYPTO_F_USER, CRYPTO_F_CBIMM, CRYPTO_F_CBIFSYNC. CRYPTO_F_USER is no longer needed. It was introduced in 2008 by darran@ in crypto.c 1.30, cryptodev.c 1.45 in an attempt to avoid double-free between the issuing thread and asynchronous callback. But the `fix' didn't work. In 2017, knakahara@ fixed it properly in cryptodev.c 1.87 by distinguishing `the crypto operation has completed' (CRYPTO_F_DONE) from `the callback is done touching the crp object' (CRYPTO_F_DQRETQ, now renamed to CRYPTODEV_F_RET). CRYPTO_F_CBIMM formerly served to invoke the callback synchronously from the driver's interrupt completion routine, to reduce contention on what was once a single cryptoret thread. Now, there is a per-CPU queue and softint for much cheaper processing, so there is less motivation for this in the first place. So let's remove the complicated logic. This means the callbacks never run in hard interrupt context, which means we don't need to worry about recursion into crypto_dispatch in hard interrupt context. To generate a diff of this commit: cvs rdiff -u -r1.117 -r1.118 src/sys/opencrypto/crypto.c cvs rdiff -u -r1.112 -r1.113 src/sys/opencrypto/cryptodev.c cvs rdiff -u -r1.42 -r1.43 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:03:58 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Simplify error test in cryptodev_op. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.111 -r1.112 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.111 src/sys/opencrypto/cryptodev.c:1.112 --- src/sys/opencrypto/cryptodev.c:1.111 Wed May 18 20:03:45 2022 +++ src/sys/opencrypto/cryptodev.c Wed May 18 20:03:58 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.111 2022/05/18 20:03:45 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.112 2022/05/18 20:03:58 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.111 2022/05/18 20:03:45 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.112 2022/05/18 20:03:58 riastradh Exp $"); #include #include @@ -647,22 +647,12 @@ cryptodev_op(struct csession *cse, struc error = EINVAL; goto bail; } - crp->crp_mac=cse->tmp_mac; + crp->crp_mac = cse->tmp_mac; } cv_init(>crp_cv, "crydev"); - error = crypto_dispatch(crp); - - /* - * Don't touch crp before returned by any error or received - * cv_signal(>crp_cv). It is required to restructure locks. - */ - - switch (error) { - case 0: - break; - default: + if (error) { DPRINTF("not waiting, error.\n"); cv_destroy(>crp_cv); goto bail;
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:03:58 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Simplify error test in cryptodev_op. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.111 -r1.112 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:03:45 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Narrow scope of cryptodev_mtx to cover wait. No functional change intended -- this only removes an unnecessary lock/unlock cycle in the error case. To generate a diff of this commit: cvs rdiff -u -r1.110 -r1.111 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.110 src/sys/opencrypto/cryptodev.c:1.111 --- src/sys/opencrypto/cryptodev.c:1.110 Wed May 18 20:03:32 2022 +++ src/sys/opencrypto/cryptodev.c Wed May 18 20:03:45 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.110 2022/05/18 20:03:32 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.111 2022/05/18 20:03:45 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.110 2022/05/18 20:03:32 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.111 2022/05/18 20:03:45 riastradh Exp $"); #include #include @@ -653,7 +653,6 @@ cryptodev_op(struct csession *cse, struc cv_init(>crp_cv, "crydev"); error = crypto_dispatch(crp); - mutex_enter(_mtx); /* * Don't touch crp before returned by any error or received @@ -665,11 +664,11 @@ cryptodev_op(struct csession *cse, struc break; default: DPRINTF("not waiting, error.\n"); - mutex_exit(_mtx); cv_destroy(>crp_cv); goto bail; } + mutex_enter(_mtx); while (!(crp->crp_devflags & CRYPTODEV_F_RET)) { DPRINTF("cse->sid[%d]: sleeping on cv %p for crp %p\n", (uint32_t)cse->sid, >crp_cv, crp);
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:03:45 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Narrow scope of cryptodev_mtx to cover wait. No functional change intended -- this only removes an unnecessary lock/unlock cycle in the error case. To generate a diff of this commit: cvs rdiff -u -r1.110 -r1.111 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:03:32 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Nix long-dead code and comments. To generate a diff of this commit: cvs rdiff -u -r1.109 -r1.110 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.109 src/sys/opencrypto/cryptodev.c:1.110 --- src/sys/opencrypto/cryptodev.c:1.109 Wed May 18 20:02:49 2022 +++ src/sys/opencrypto/cryptodev.c Wed May 18 20:03:32 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.109 2022/05/18 20:02:49 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.110 2022/05/18 20:03:32 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.109 2022/05/18 20:02:49 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.110 2022/05/18 20:03:32 riastradh Exp $"); #include #include @@ -652,34 +652,15 @@ cryptodev_op(struct csession *cse, struc cv_init(>crp_cv, "crydev"); - /* - * XXX there was a comment here which said that we went to - * XXX splcrypto() but needed to only if CRYPTO_F_CBIMM, - * XXX disabled on NetBSD since 1.6O due to a race condition. - * XXX But crypto_dispatch went to splcrypto() itself! (And - * XXX now takes the cryptodev_mtx mutex itself). We do, however, - * XXX need to hold the mutex across the call to cv_wait(). - * XXX (should we arrange for crypto_dispatch to return to - * XXX us with it held? it seems quite ugly to do so.) - */ -#ifdef notyet -eagain: -#endif error = crypto_dispatch(crp); mutex_enter(_mtx); - /* + /* * Don't touch crp before returned by any error or received * cv_signal(>crp_cv). It is required to restructure locks. */ switch (error) { -#ifdef notyet /* don't loop forever -- but EAGAIN not possible here yet */ - case EAGAIN: - mutex_exit(_mtx); - goto eagain; - break; -#endif case 0: break; default: @@ -1056,7 +1037,6 @@ csecreate(struct fcrypt *fcr, u_int64_t } } -/* csefree: call with cryptodev_mtx held. */ static int csefree(struct csession *cse) {
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:03:32 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Nix long-dead code and comments. To generate a diff of this commit: cvs rdiff -u -r1.109 -r1.110 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:02:49 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Use IPL_NONE, not IPL_NET, for /dev/crypto pools. These are used (pool_get/put) only from thread context, never from interrupt or even soft interrupt context. To generate a diff of this commit: cvs rdiff -u -r1.108 -r1.109 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.108 src/sys/opencrypto/cryptodev.c:1.109 --- src/sys/opencrypto/cryptodev.c:1.108 Tue May 17 09:53:09 2022 +++ src/sys/opencrypto/cryptodev.c Wed May 18 20:02:49 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.108 2022/05/17 09:53:09 riastradh Exp $ */ +/* $NetBSD: cryptodev.c,v 1.109 2022/05/18 20:02:49 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.108 2022/05/17 09:53:09 riastradh Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.109 2022/05/18 20:02:49 riastradh Exp $"); #include #include @@ -2118,9 +2118,9 @@ cryptoattach(int num) mutex_init(_mtx, MUTEX_DEFAULT, IPL_NONE); pool_init(, sizeof(struct fcrypt), 0, 0, 0, "fcrpl", - NULL, IPL_NET); /* XXX IPL_NET ("splcrypto") */ + NULL, IPL_NONE); pool_init(, sizeof(struct csession), 0, 0, 0, "csepl", - NULL, IPL_NET); /* XXX IPL_NET ("splcrypto") */ + NULL, IPL_NONE); /* * Preallocate space for 64 users, with 5 sessions each.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Wed May 18 20:02:49 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): Use IPL_NONE, not IPL_NET, for /dev/crypto pools. These are used (pool_get/put) only from thread context, never from interrupt or even soft interrupt context. To generate a diff of this commit: cvs rdiff -u -r1.108 -r1.109 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Tue May 17 10:32:58 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Factor setting CRYPTO_F_DONE out of branches. This had been done in 1.30 when the locking was different. No need any more. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.116 -r1.117 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Tue May 17 10:32:58 UTC 2022 Modified Files: src/sys/opencrypto: crypto.c Log Message: opencrypto: Factor setting CRYPTO_F_DONE out of branches. This had been done in 1.30 when the locking was different. No need any more. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.116 -r1.117 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.116 src/sys/opencrypto/crypto.c:1.117 --- src/sys/opencrypto/crypto.c:1.116 Sat Aug 14 20:43:05 2021 +++ src/sys/opencrypto/crypto.c Tue May 17 10:32:58 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.116 2021/08/14 20:43:05 andvar Exp $ */ +/* $NetBSD: crypto.c,v 1.117 2022/05/17 10:32:58 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.116 2021/08/14 20:43:05 andvar Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.117 2022/05/17 10:32:58 riastradh Exp $"); #include #include @@ -1718,6 +1718,8 @@ crypto_done(struct cryptop *crp) #endif DPRINTF("lid[%u]: crp %p\n", CRYPTO_SESID2LID(crp->crp_sid), crp); + crp->crp_flags |= CRYPTO_F_DONE; + /* * Normal case; queue the callback for the thread. * @@ -1732,8 +1734,6 @@ crypto_done(struct cryptop *crp) * callback routine does very little (e.g. the * /dev/crypto callback method just does a wakeup). */ - crp->crp_flags |= CRYPTO_F_DONE; - #ifdef CRYPTO_TIMING if (crypto_timing) { /* @@ -1749,7 +1749,6 @@ crypto_done(struct cryptop *crp) #endif crp->crp_callback(crp); } else { - crp->crp_flags |= CRYPTO_F_DONE; #if 0 if (crp->crp_flags & CRYPTO_F_USER) { /*
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Tue May 17 09:53:09 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: opencrypto(9): Omit needless casts around callbacks. Just declare the right types to begin with. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.107 -r1.108 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.107 src/sys/opencrypto/cryptodev.c:1.108 --- src/sys/opencrypto/cryptodev.c:1.107 Thu Mar 31 19:30:17 2022 +++ src/sys/opencrypto/cryptodev.c Tue May 17 09:53:09 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.107 2022/03/31 19:30:17 pgoyette Exp $ */ +/* $NetBSD: cryptodev.c,v 1.108 2022/05/17 09:53:09 riastradh 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.107 2022/03/31 19:30:17 pgoyette Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.108 2022/05/17 09:53:09 riastradh Exp $"); #include #include @@ -188,11 +188,11 @@ static int cryptodev_key(struct crypt_ko static int cryptodev_mkey(struct fcrypt *, struct crypt_n_kop *, int); static int cryptodev_msessionfin(struct fcrypt *, int, u_int32_t *); -static int cryptodev_cb(void *); -static int cryptodevkey_cb(void *); +static int cryptodev_cb(struct cryptop *); +static int cryptodevkey_cb(struct cryptkop *); -static int cryptodev_mcb(void *); -static int cryptodevkey_mcb(void *); +static int cryptodev_mcb(struct cryptop *); +static int cryptodevkey_mcb(struct cryptkop *); static int cryptodev_getmstatus(struct fcrypt *, struct crypt_result *, int); @@ -612,9 +612,9 @@ cryptodev_op(struct csession *cse, struc crp->crp_flags = CRYPTO_F_IOV | (cop->flags & COP_F_BATCH) | CRYPTO_F_USER | flags; crp->crp_buf = (void *)>uio; - crp->crp_callback = (int (*) (struct cryptop *)) cryptodev_cb; + crp->crp_callback = cryptodev_cb; crp->crp_sid = cse->sid; - crp->crp_opaque = (void *)cse; + crp->crp_opaque = cse; if (cop->iv) { if (crde == NULL) { @@ -748,10 +748,9 @@ bail: } static int -cryptodev_cb(void *op) +cryptodev_cb(struct cryptop *crp) { - struct cryptop *crp = (struct cryptop *) op; - struct csession *cse = (struct csession *)crp->crp_opaque; + struct csession *cse = crp->crp_opaque; int error = 0; mutex_enter(_mtx); @@ -771,11 +770,10 @@ cryptodev_cb(void *op) } static int -cryptodev_mcb(void *op) +cryptodev_mcb(struct cryptop *crp) { - struct cryptop *crp = (struct cryptop *) op; - struct csession *cse = (struct csession *)crp->crp_opaque; - int error=0; + struct csession *cse = crp->crp_opaque; + int error = 0; mutex_enter(_mtx); cse->error = crp->crp_etype; @@ -795,10 +793,9 @@ cryptodev_mcb(void *op) } static int -cryptodevkey_cb(void *op) +cryptodevkey_cb(struct cryptkop *krp) { - struct cryptkop *krp = op; - + mutex_enter(_mtx); krp->krp_devflags |= CRYPTODEV_F_RET; cv_signal(>krp_cv); @@ -807,9 +804,8 @@ cryptodevkey_cb(void *op) } static int -cryptodevkey_mcb(void *op) +cryptodevkey_mcb(struct cryptkop *krp) { - struct cryptkop *krp = op; mutex_enter(_mtx); cv_signal(>krp_cv); @@ -892,7 +888,7 @@ cryptodev_key(struct crypt_kop *kop) krp->krp_iparams = kop->crk_iparams; krp->krp_oparams = kop->crk_oparams; krp->krp_status = 0; - krp->krp_callback = (int (*) (struct cryptkop *)) cryptodevkey_cb; + krp->krp_callback = cryptodevkey_cb; for (i = 0; i < CRK_MAXPARAM; i++) krp->krp_param[i].crp_nbits = kop->crk_param[i].crp_nbits; @@ -1306,9 +1302,9 @@ cryptodev_mop(struct fcrypt *fcr, crp->crp_flags = CRYPTO_F_IOV | CRYPTO_F_CBIMM | (cnop[req].flags & COP_F_BATCH) | flags; crp->crp_buf = (void *)>uio; - crp->crp_callback = (int (*) (struct cryptop *)) cryptodev_mcb; + crp->crp_callback = cryptodev_mcb; crp->crp_sid = cse->sid; - crp->crp_opaque = (void *)cse; + crp->crp_opaque = cse; crp->fcrp = fcr; crp->dst = cnop[req].dst; crp->len = cnop[req].len; /* input len, iov may be larger */ @@ -1482,8 +1478,7 @@ cryptodev_mkey(struct fcrypt *fcr, struc krp->krp_iparams = kop[req].crk_iparams; krp->krp_oparams = kop[req].crk_oparams; krp->krp_status = 0; - krp->krp_callback = - (int (*) (struct cryptkop *)) cryptodevkey_mcb; + krp->krp_callback = cryptodevkey_mcb; (void)memcpy(krp->crk_param, kop[req].crk_param, sizeof(kop[req].crk_param));
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Tue May 17 09:53:09 UTC 2022 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: opencrypto(9): Omit needless casts around callbacks. Just declare the right types to begin with. No functional change intended. To generate a diff of this commit: cvs rdiff -u -r1.107 -r1.108 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sat Mar 12 17:15:04 UTC 2022 Modified Files: src/sys/opencrypto: ocryptodev.c Log Message: crypto(4): Refuse count>1 for old CIOCNCRYPTM. This hasn't worked since it was written in 2009; if anyone cared surely they would have fixed it by now! (Fixing this properly -- and putting a more reasonable upper bound than the maximum that size_t arithmetic allows -- left as an exercise or the reader.) Reported-by: syzbot+798d4a16bc15ae885...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.16 -r1.17 src/sys/opencrypto/ocryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: riastradh Date: Sat Mar 12 17:15:04 UTC 2022 Modified Files: src/sys/opencrypto: ocryptodev.c Log Message: crypto(4): Refuse count>1 for old CIOCNCRYPTM. This hasn't worked since it was written in 2009; if anyone cared surely they would have fixed it by now! (Fixing this properly -- and putting a more reasonable upper bound than the maximum that size_t arithmetic allows -- left as an exercise or the reader.) Reported-by: syzbot+798d4a16bc15ae885...@syzkaller.appspotmail.com To generate a diff of this commit: cvs rdiff -u -r1.16 -r1.17 src/sys/opencrypto/ocryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/ocryptodev.c diff -u src/sys/opencrypto/ocryptodev.c:1.16 src/sys/opencrypto/ocryptodev.c:1.17 --- src/sys/opencrypto/ocryptodev.c:1.16 Mon Jan 27 17:09:17 2020 +++ src/sys/opencrypto/ocryptodev.c Sat Mar 12 17:15:04 2022 @@ -1,4 +1,4 @@ -/* $NetBSD: ocryptodev.c,v 1.16 2020/01/27 17:09:17 pgoyette Exp $ */ +/* $NetBSD: ocryptodev.c,v 1.17 2022/03/12 17:15:04 riastradh 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 $ */ @@ -69,7 +69,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: ocryptodev.c,v 1.16 2020/01/27 17:09:17 pgoyette Exp $"); +__KERNEL_RCSID(0, "$NetBSD: ocryptodev.c,v 1.17 2022/03/12 17:15:04 riastradh Exp $"); #include #include @@ -167,8 +167,7 @@ mbail: break; case OCIOCNCRYPTM: omop = (struct ocrypt_mop *)data; - if ((omop->count <= 0) || - (SIZE_MAX/sizeof(struct ocrypt_n_op) <= omop->count)) { + if (omop->count <= 0 || omop->count > 1) { error = EINVAL; break; }
CVS commit: src/sys/opencrypto
Module Name:src Committed By: hikaru Date: Fri Nov 29 08:30:31 UTC 2019 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): accept CRYPTO_SHA2_384_HMAC and CRYPTO_SHA2_512_HMAC. To generate a diff of this commit: cvs rdiff -u -r1.101 -r1.102 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.101 src/sys/opencrypto/cryptodev.c:1.102 --- src/sys/opencrypto/cryptodev.c:1.101 Thu Jun 13 02:02:45 2019 +++ src/sys/opencrypto/cryptodev.c Fri Nov 29 08:30:30 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.101 2019/06/13 02:02:45 christos Exp $ */ +/* $NetBSD: cryptodev.c,v 1.102 2019/11/29 08:30:30 hikaru 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.101 2019/06/13 02:02:45 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.102 2019/11/29 08:30:30 hikaru Exp $"); #include #include @@ -1644,6 +1644,12 @@ cryptodev_session(struct fcrypt *fcr, st return EINVAL; } break; + case CRYPTO_SHA2_384_HMAC: + thash = _hash_hmac_sha2_384; + break; + case CRYPTO_SHA2_512_HMAC: + thash = _hash_hmac_sha2_512; + break; case CRYPTO_RIPEMD160_HMAC: thash = _hash_hmac_ripemd_160; break;
CVS commit: src/sys/opencrypto
Module Name:src Committed By: hikaru Date: Fri Nov 29 08:30:31 UTC 2019 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: crypto(4): accept CRYPTO_SHA2_384_HMAC and CRYPTO_SHA2_512_HMAC. To generate a diff of this commit: cvs rdiff -u -r1.101 -r1.102 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: hikaru Date: Fri Nov 29 07:20:03 UTC 2019 Modified Files: src/sys/opencrypto: cryptodev.h Log Message: HMAC-SHA-512 has 32 bytes MAC. To generate a diff of this commit: cvs rdiff -u -r1.39 -r1.40 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: hikaru Date: Fri Nov 29 07:20:03 UTC 2019 Modified Files: src/sys/opencrypto: cryptodev.h Log Message: HMAC-SHA-512 has 32 bytes MAC. To generate a diff of this commit: cvs rdiff -u -r1.39 -r1.40 src/sys/opencrypto/cryptodev.h Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.h diff -u src/sys/opencrypto/cryptodev.h:1.39 src/sys/opencrypto/cryptodev.h:1.40 --- src/sys/opencrypto/cryptodev.h:1.39 Wed Jul 26 06:44:50 2017 +++ src/sys/opencrypto/cryptodev.h Fri Nov 29 07:20:03 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.h,v 1.39 2017/07/26 06:44:50 knakahara Exp $ */ +/* $NetBSD: cryptodev.h,v 1.40 2019/11/29 07:20:03 hikaru 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 $ */ @@ -262,7 +262,7 @@ struct crypt_sgop { struct session_n_op * sessions; }; -#define CRYPTO_MAX_MAC_LEN 20 +#define CRYPTO_MAX_MAC_LEN 32 /* Keep this updated */ /* bignum parameter, in packed bytes, ... */ struct crparam {
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Sat Oct 12 00:49:30 UTC 2019 Modified Files: src/sys/opencrypto: cryptosoft.c cryptosoft_xform.c Log Message: add (void *) intermediate casts to elide gcc function cast warnings. This is the simplest solution; choices: - add pragmas, complex and ugly (need to be gcc-specific) - add -Wno to COPTS. Needs to be done in many makefiles because of rump - add intermediate functions: slows down things To generate a diff of this commit: cvs rdiff -u -r1.53 -r1.54 src/sys/opencrypto/cryptosoft.c cvs rdiff -u -r1.27 -r1.28 src/sys/opencrypto/cryptosoft_xform.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptosoft.c diff -u src/sys/opencrypto/cryptosoft.c:1.53 src/sys/opencrypto/cryptosoft.c:1.54 --- src/sys/opencrypto/cryptosoft.c:1.53 Thu Jul 11 19:27:24 2019 +++ src/sys/opencrypto/cryptosoft.c Fri Oct 11 20:49:30 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptosoft.c,v 1.53 2019/07/11 23:27:24 christos Exp $ */ +/* $NetBSD: cryptosoft.c,v 1.54 2019/10/12 00:49:30 christos Exp $ */ /* $FreeBSD: src/sys/opencrypto/cryptosoft.c,v 1.2.2.1 2002/11/21 23:34:23 sam Exp $ */ /* $OpenBSD: cryptosoft.c,v 1.35 2002/04/26 08:43:50 deraadt Exp $ */ @@ -24,7 +24,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.53 2019/07/11 23:27:24 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.54 2019/10/12 00:49:30 christos Exp $"); #include #include @@ -500,7 +500,7 @@ swcr_authcompute(struct cryptop *crp, st break; case CRYPTO_BUF_MBUF: err = m_apply((struct mbuf *) buf, crd->crd_skip, crd->crd_len, - (int (*)(void*, void *, unsigned int)) axf->Update, + (int (*)(void*, void *, unsigned int))(void *)axf->Update, (void *) ); if (err) return err; @@ -508,7 +508,7 @@ swcr_authcompute(struct cryptop *crp, st case CRYPTO_BUF_IOV: err = cuio_apply((struct uio *) buf, crd->crd_skip, crd->crd_len, - (int (*)(void *, void *, unsigned int)) axf->Update, + (int (*)(void *, void *, unsigned int))(void *)axf->Update, (void *) ); if (err) { return err; Index: src/sys/opencrypto/cryptosoft_xform.c diff -u src/sys/opencrypto/cryptosoft_xform.c:1.27 src/sys/opencrypto/cryptosoft_xform.c:1.28 --- src/sys/opencrypto/cryptosoft_xform.c:1.27 Thu Nov 27 15:30:21 2014 +++ src/sys/opencrypto/cryptosoft_xform.c Fri Oct 11 20:49:30 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptosoft_xform.c,v 1.27 2014/11/27 20:30:21 christos Exp $ */ +/* $NetBSD: cryptosoft_xform.c,v 1.28 2019/10/12 00:49:30 christos Exp $ */ /* $FreeBSD: src/sys/opencrypto/xform.c,v 1.1.2.1 2002/11/21 23:34:23 sam Exp $ */ /* $OpenBSD: xform.c,v 1.19 2002/08/16 22:47:25 dhartmei Exp $ */ @@ -40,7 +40,7 @@ */ #include -__KERNEL_RCSID(1, "$NetBSD: cryptosoft_xform.c,v 1.27 2014/11/27 20:30:21 christos Exp $"); +__KERNEL_RCSID(1, "$NetBSD: cryptosoft_xform.c,v 1.28 2019/10/12 00:49:30 christos Exp $"); #include #include @@ -313,26 +313,26 @@ static const struct swcr_auth_hash swcr_ static const struct swcr_auth_hash swcr_auth_hash_hmac_sha2_256 = { _hash_hmac_sha2_256, sizeof(SHA256_CTX), - (void (*)(void *)) SHA256_Init, NULL, NULL, SHA256Update_int, - (void (*)(u_int8_t *, void *)) SHA256_Final + (void (*)(void *))(void *)SHA256_Init, NULL, NULL, SHA256Update_int, + (void (*)(u_int8_t *, void *))(void *)SHA256_Final }; static const struct swcr_auth_hash swcr_auth_hash_hmac_sha2_384 = { _hash_hmac_sha2_384, sizeof(SHA384_CTX), - (void (*)(void *)) SHA384_Init, NULL, NULL, SHA384Update_int, - (void (*)(u_int8_t *, void *)) SHA384_Final + (void (*)(void *))(void *)SHA384_Init, NULL, NULL, SHA384Update_int, + (void (*)(u_int8_t *, void *))(void *)SHA384_Final }; static const struct swcr_auth_hash swcr_auth_hash_hmac_sha2_512 = { _hash_hmac_sha2_512, sizeof(SHA512_CTX), - (void (*)(void *)) SHA512_Init, NULL, NULL, SHA512Update_int, - (void (*)(u_int8_t *, void *)) SHA512_Final + (void (*)(void *))(void *)SHA512_Init, NULL, NULL, SHA512Update_int, + (void (*)(u_int8_t *, void *))(void *)SHA512_Final }; static const struct swcr_auth_hash swcr_auth_hash_aes_xcbc_mac = { _hash_aes_xcbc_mac_96, sizeof(aesxcbc_ctx), null_init, - (void (*)(void *, const u_int8_t *, u_int16_t))aes_xcbc_mac_init, + (void (*)(void *, const u_int8_t *, u_int16_t))(void *)aes_xcbc_mac_init, NULL, aes_xcbc_mac_loop, aes_xcbc_mac_result };
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Sat Oct 12 00:49:30 UTC 2019 Modified Files: src/sys/opencrypto: cryptosoft.c cryptosoft_xform.c Log Message: add (void *) intermediate casts to elide gcc function cast warnings. This is the simplest solution; choices: - add pragmas, complex and ugly (need to be gcc-specific) - add -Wno to COPTS. Needs to be done in many makefiles because of rump - add intermediate functions: slows down things To generate a diff of this commit: cvs rdiff -u -r1.53 -r1.54 src/sys/opencrypto/cryptosoft.c cvs rdiff -u -r1.27 -r1.28 src/sys/opencrypto/cryptosoft_xform.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jul 11 23:28:17 UTC 2019 Modified Files: src/sys/opencrypto: crypto.c Log Message: relinguish our lock while we are autoloading. To generate a diff of this commit: cvs rdiff -u -r1.107 -r1.108 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jul 11 23:28:17 UTC 2019 Modified Files: src/sys/opencrypto: crypto.c Log Message: relinguish our lock while we are autoloading. To generate a diff of this commit: cvs rdiff -u -r1.107 -r1.108 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.107 src/sys/opencrypto/crypto.c:1.108 --- src/sys/opencrypto/crypto.c:1.107 Wed Jun 12 22:07:31 2019 +++ src/sys/opencrypto/crypto.c Thu Jul 11 19:28:17 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.107 2019/06/13 02:07:31 christos Exp $ */ +/* $NetBSD: crypto.c,v 1.108 2019/07/11 23:28:17 christos 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.107 2019/06/13 02:07:31 christos Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.108 2019/07/11 23:28:17 christos Exp $"); #include #include @@ -812,7 +812,9 @@ again: } if (best == NULL && hard == 0 && error == 0) { + mutex_exit(_drv_mtx); error = module_autoload("swcrypto", MODULE_CLASS_DRIVER); + mutex_enter(_drv_mtx); if (error == 0) { error = EINVAL; goto again;
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jul 11 23:27:24 UTC 2019 Modified Files: src/sys/opencrypto: cryptosoft.c Log Message: Disable unloading until we keep track of references To generate a diff of this commit: cvs rdiff -u -r1.52 -r1.53 src/sys/opencrypto/cryptosoft.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptosoft.c diff -u src/sys/opencrypto/cryptosoft.c:1.52 src/sys/opencrypto/cryptosoft.c:1.53 --- src/sys/opencrypto/cryptosoft.c:1.52 Fri Jun 23 07:41:58 2017 +++ src/sys/opencrypto/cryptosoft.c Thu Jul 11 19:27:24 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptosoft.c,v 1.52 2017/06/23 11:41:58 knakahara Exp $ */ +/* $NetBSD: cryptosoft.c,v 1.53 2019/07/11 23:27:24 christos Exp $ */ /* $FreeBSD: src/sys/opencrypto/cryptosoft.c,v 1.2.2.1 2002/11/21 23:34:23 sam Exp $ */ /* $OpenBSD: cryptosoft.c,v 1.35 2002/04/26 08:43:50 deraadt Exp $ */ @@ -24,7 +24,7 @@ */ #include -__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.52 2017/06/23 11:41:58 knakahara Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptosoft.c,v 1.53 2019/07/11 23:27:24 christos Exp $"); #include #include @@ -1447,6 +1447,10 @@ swcrypto_modcmd(modcmd_t cmd, void *arg) #endif return error; case MODULE_CMD_FINI: +#if 1 + // XXX: Need to keep track if we are in use. + return ENOTTY; +#else error = config_cfdata_detach(swcrypto_cfdata); if (error) { return error; @@ -1456,6 +1460,7 @@ swcrypto_modcmd(modcmd_t cmd, void *arg) config_cfdriver_detach(_cd); return 0; +#endif default: return ENOTTY; }
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jul 11 23:27:24 UTC 2019 Modified Files: src/sys/opencrypto: cryptosoft.c Log Message: Disable unloading until we keep track of references To generate a diff of this commit: cvs rdiff -u -r1.52 -r1.53 src/sys/opencrypto/cryptosoft.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jun 13 02:07:31 UTC 2019 Modified Files: src/sys/opencrypto: crypto.c Log Message: Try to load swcrypto if we we did not find any software drivers. To generate a diff of this commit: cvs rdiff -u -r1.106 -r1.107 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/crypto.c diff -u src/sys/opencrypto/crypto.c:1.106 src/sys/opencrypto/crypto.c:1.107 --- src/sys/opencrypto/crypto.c:1.106 Tue Jun 5 21:49:09 2018 +++ src/sys/opencrypto/crypto.c Wed Jun 12 22:07:31 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: crypto.c,v 1.106 2018/06/06 01:49:09 maya Exp $ */ +/* $NetBSD: crypto.c,v 1.107 2019/06/13 02:07:31 christos 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 -__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.106 2018/06/06 01:49:09 maya Exp $"); +__KERNEL_RCSID(0, "$NetBSD: crypto.c,v 1.107 2019/06/13 02:07:31 christos Exp $"); #include #include @@ -746,6 +746,7 @@ crypto_select_driver_lock(struct cryptoi u_int32_t hid; int accept; struct cryptocap *cap, *best; + int error = 0; best = NULL; /* @@ -810,6 +811,14 @@ again: goto again; } + if (best == NULL && hard == 0 && error == 0) { + error = module_autoload("swcrypto", MODULE_CLASS_DRIVER); + if (error == 0) { + error = EINVAL; + goto again; + } + } + return best; }
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jun 13 02:07:31 UTC 2019 Modified Files: src/sys/opencrypto: crypto.c Log Message: Try to load swcrypto if we we did not find any software drivers. To generate a diff of this commit: cvs rdiff -u -r1.106 -r1.107 src/sys/opencrypto/crypto.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jun 13 02:02:45 UTC 2019 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: don't always panic when modunload crypto (int the pool destroy code, because the pools are busy). XXX: this is still racy; we need to prevent creating more sessions while destroying. To generate a diff of this commit: cvs rdiff -u -r1.100 -r1.101 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. Modified files: Index: src/sys/opencrypto/cryptodev.c diff -u src/sys/opencrypto/cryptodev.c:1.100 src/sys/opencrypto/cryptodev.c:1.101 --- src/sys/opencrypto/cryptodev.c:1.100 Fri Mar 1 06:06:57 2019 +++ src/sys/opencrypto/cryptodev.c Wed Jun 12 22:02:45 2019 @@ -1,4 +1,4 @@ -/* $NetBSD: cryptodev.c,v 1.100 2019/03/01 11:06:57 pgoyette Exp $ */ +/* $NetBSD: cryptodev.c,v 1.101 2019/06/13 02:02:45 christos 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 -__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.100 2019/03/01 11:06:57 pgoyette Exp $"); +__KERNEL_RCSID(0, "$NetBSD: cryptodev.c,v 1.101 2019/06/13 02:02:45 christos Exp $"); #include #include @@ -2246,6 +2246,8 @@ crypto_modcmd(modcmd_t cmd, void *arg) return error; case MODULE_CMD_FINI: #ifdef _MODULE + if (crypto_refcount != 0) + return EBUSY; error = config_cfdata_detach(crypto_cfdata); if (error) { return error;
CVS commit: src/sys/opencrypto
Module Name:src Committed By: christos Date: Thu Jun 13 02:02:45 UTC 2019 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: don't always panic when modunload crypto (int the pool destroy code, because the pools are busy). XXX: this is still racy; we need to prevent creating more sessions while destroying. To generate a diff of this commit: cvs rdiff -u -r1.100 -r1.101 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files.
Re: CVS commit: src/sys/opencrypto
On Tue, Apr 18, 2017 at 05:05:06PM +, Maya Rashish wrote: > XXX surrounding code seems fishy > > @@ -759,7 +759,6 @@ swcr_compdec(struct cryptodesc *crd, con > if (result < crd->crd_len) { > adj = result - crd->crd_len; > if (outtype == CRYPTO_BUF_MBUF) { > - adj = result - crd->crd_len; > m_adj((struct mbuf *)buf, adj); > } > /* Don't adjust the iov_len, it breaks the kmem_free */ > calling m_adj with negative adj sounds possibly unintended
Re: CVS commit: src/sys/opencrypto
On Fri, 27 Nov 2015, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sat Nov 28 03:40:43 UTC 2015 Modified Files: src/sys/opencrypto: crypto.c Log Message: fix the build Thanks, and sorry for the breakage. +--+--+-+ | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | (Retired)| FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Kernel Developer | 0786 F758 55DE 53BA 7731 | pgoyette at netbsd.org | +--+--+-+
Re: CVS commit: src/sys/opencrypto
Taylor R Campbell wrote: It seems to me that the crux of the problem is that devsw_detach doesn't fail if the device is still in use, because we do keep no books about which devsws are still in use, so there's nothing to stop you from unloading a device's module before you've given a its close routine a chance to clean up. Would it suffice to add an open count to struct bdevsw and struct cdevsw, to be maintained by {bdev,cdev}_{open,close}? I think this would be sufficient. A lesser problem is that the steps to finalize a device driver module are complicated (attach/detach the devsw, the cfdriver, the cfattach, the cfdata) and different drivers do them in a different order and may or may not back out the same way (e.g., dtv(4) ignores devsw_detach failure), so we ought to formalize what the right way to do this is once we have a way that can work. That would definitely be appreciated. It would seem that config_{init,fini}_component() are intended to handle much/most of this, but these routines are not documented and not used universally. (see sys/kern/subr_autoconf.c) - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/opencrypto
On Sun, Jan 26, 2014 at 11:04:31AM +1100, matthew green wrote: phase one: disable auto-unload. Phase 1 has been done. for the whole kernel? No. Only for this specific module. right - my suggestion was that since this problem affects a large class of modules, until that is fixed, we should disable auto unload globally. Does almost everything get loaded and auto-unloaded at boot time? If so that that isn't really a good idea. It also sounds like manual unloads of drivers can happen when the device is open - and that is also bad. A manual unload probably isn't going to race with open or close though. Disallowing unload completely would be a pain when developing drivers. David -- David Laight: da...@l8s.co.uk
Re: CVS commit: src/sys/opencrypto
Date: Sun, 26 Jan 2014 11:36:32 + From: David Laight da...@l8s.co.uk It also sounds like manual unloads of drivers can happen when the device is open - and that is also bad. A manual unload probably isn't going to race with open or close though. Disallowing unload completely would be a pain when developing drivers. It seems to me that the crux of the problem is that devsw_detach doesn't fail if the device is still in use, because we do keep no books about which devsws are still in use, so there's nothing to stop you from unloading a device's module before you've given a its close routine a chance to clean up. Would it suffice to add an open count to struct bdevsw and struct cdevsw, to be maintained by {bdev,cdev}_{open,close}? FreeBSD has a fancier reference-counting API for their devsw, but I'm not sure why they need that. A lesser problem is that the steps to finalize a device driver module are complicated (attach/detach the devsw, the cfdriver, the cfattach, the cfdata) and different drivers do them in a different order and may or may not back out the same way (e.g., dtv(4) ignores devsw_detach failure), so we ought to formalize what the right way to do this is once we have a way that can work.
re: CVS commit: src/sys/opencrypto
Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread preempted. If auto-unload XXX happens before that thread can return from the module's code, bad XXX things (tm) could happen. in this case, please simply disallow unload for this module always. if the race is fixed, it can be enabled again. I think that most module unloads suffer from this race, any ideas how to fix it? phase one: disable auto-unload. phase two: ??? the rest left as an exercise for the reader. :-)
re: CVS commit: src/sys/opencrypto
Phase 1 has been done. On Sat, 25 Jan 2014, matthew green wrote: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread preempted. If auto-unload XXX happens before that thread can return from the module's code, bad XXX things (tm) could happen. in this case, please simply disallow unload for this module always. if the race is fixed, it can be enabled again. I think that most module unloads suffer from this race, any ideas how to fix it? phase one: disable auto-unload. phase two: ??? the rest left as an exercise for the reader. :-) !DSPAM:52e37018210366601841969! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
re: CVS commit: src/sys/opencrypto
phase one: disable auto-unload. Phase 1 has been done. for the whole kernel?
re: CVS commit: src/sys/opencrypto
On Sun, 26 Jan 2014, matthew green wrote: phase one: disable auto-unload. Phase 1 has been done. for the whole kernel? No. Only for this specific module. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
re: CVS commit: src/sys/opencrypto
phase one: disable auto-unload. Phase 1 has been done. for the whole kernel? No. Only for this specific module. right - my suggestion was that since this problem affects a large class of modules, until that is fixed, we should disable auto unload globally. .mrg.
Re: CVS commit: src/sys/opencrypto
In article 7458.1390534...@splode.eterna.com.au, matthew green m...@eterna.com.au wrote: Log Message: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread preempted. If auto-unload XXX happens before that thread can return from the module's code, bad XXX things (tm) could happen. in this case, please simply disallow unload for this module always. if the race is fixed, it can be enabled again. I think that most module unloads suffer from this race, any ideas how to fix it? christos
Re: CVS commit: src/sys/opencrypto
Some modules get auto-loaded by the syscall mechanism. We already have a mechanism to determine if any lwp's are currently executing these syscalls, and if yes we prevent auto-unload. For device-driver modules, there is currently no equivalent mechanism. They get auto-loaded from code in specfs/vfsops as a result of trying to access the block/char device. We would need to specify exactly what constitutes a reference and then keep track. Is it only open that creates a reference? Someone mentioned that it might be possible to read/write without opening the device... Also need to consider what happens when the device is cloned ... There is a refcount in the struct module, and a generic module_hold() / module_rele() mechanism to manipulate. But it is currently used only for tracking inter-module references (ie, dependencies) as far as I can see. It could be used here, but as currently implemented (linear search for module name) it's probably too expensive. I'm not sure how we would handle miscellaneous modules... :) On Fri, 24 Jan 2014, Christos Zoulas wrote: In article 7458.1390534...@splode.eterna.com.au, matthew green m...@eterna.com.au wrote: Log Message: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread preempted. If auto-unload XXX happens before that thread can return from the module's code, bad XXX things (tm) could happen. in this case, please simply disallow unload for this module always. if the race is fixed, it can be enabled again. I think that most module unloads suffer from this race, any ideas how to fix it? christos !DSPAM:52e2a490250671524318036! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/opencrypto
Date: Fri, 24 Jan 2014 17:35:41 + (UTC) From: chris...@astron.com (Christos Zoulas) In article 7458.1390534...@splode.eterna.com.au, matthew green m...@eterna.com.au wrote: Log Message: XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread preempted. If auto-unload XXX happens before that thread can return from the module's code, bad XXX things (tm) could happen. in this case, please simply disallow unload for this module always. if the race is fixed, it can be enabled again. I think that most module unloads suffer from this race, any ideas how to fix it? Shouldn't devsw_detach or config_fini_component handle this? Why does the crypto device need special reference counting that other devices don't?
Re: CVS commit: src/sys/opencrypto
On Fri, 24 Jan 2014, Taylor R Campbell wrote: Shouldn't devsw_detach or config_fini_component handle this? Why does the crypto device need special reference counting that other devices don't? The crypto device isn't special in this regard. Pretty much all device driver modules need this sort of ref-counting. Without it, the race exists and at some future time something will have reused/overwritten the memory previously occupied by the module, with non-deterministic results. Crypto is special only in that it tried to do some clean-up during the detach. The pool_destroy() code correctly noticed that there were still some outstanding allocations that had not been returned. So we sort of got lucky here and found out about the problem immediately, rather than waiting for some non-deterministic future. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
re: CVS commit: src/sys/opencrypto
Log Message: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread preempted. If auto-unload XXX happens before that thread can return from the module's code, bad XXX things (tm) could happen. in this case, please simply disallow unload for this module always. if the race is fixed, it can be enabled again. .mrg.
Re: CVS commit: src/sys/opencrypto
On Sun, 19 Jan 2014, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sun Jan 19 18:16:13 UTC 2014 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: bail out unloading for now How about the following changes? @@ -143,6 +143,8 @@ static int cryptoread(dev_t dev, struct static int cryptowrite(dev_t dev, struct uio *uio, int ioflag); static int cryptoselect(dev_t dev, int rw, struct lwp *l); +static int crypto_refcount = 0;/* Prevent detaching while in use */ + /* Declaration of cloned-device (per-ctxt) entrypoints */ static int cryptof_read(struct file *, off_t *, struct uio *, kauth_cred_t, int); @@ -262,6 +264,7 @@ cryptof_ioctl(struct file *fp, u_long cm */ criofcr-sesn = 1; criofcr-requestid = 1; + crypto_refcount++; mutex_exit(crypto_mtx); (void)fd_clone(criofp, criofd, (FREAD|FWRITE), cryptofops, criofcr); @@ -951,6 +954,7 @@ cryptof_close(struct file *fp) } seldestroy(fcr-sinfo); fp-f_data = NULL; + crypto_refcount--; mutex_exit(crypto_mtx); pool_put(fcrpl, fcr); @@ -1080,6 +1084,7 @@ cryptoopen(dev_t dev, int flag, int mode */ fcr-sesn = 1; fcr-requestid = 1; + crypto_refcount++; mutex_exit(crypto_mtx); return fd_clone(fp, fd, flag, cryptofops, fcr); } @@ -2109,6 +2114,10 @@ int crypto_detach(device_t, int); int crypto_detach(device_t self, int num) { + + if (crypto_refcount != 0 || self-dv_unit != 0) + return EBUSY; + pool_destroy(fcrpl); pool_destroy(csepl); To generate a diff of this commit: cvs rdiff -u -r1.71 -r1.72 src/sys/opencrypto/cryptodev.c Please note that diffs are not public domain; they are subject to the copyright notices on the relevant files. !DSPAM:52dc1675236281199521295! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/opencrypto
On Jan 19, 10:22am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | How about the following changes? You need to handle the regular open too, not justthe get (look for the other fd_clone) | @@ -143,6 +143,8 @@ static intcryptoread(dev_t dev, struct | static int cryptowrite(dev_t dev, struct uio *uio, int ioflag); | static int cryptoselect(dev_t dev, int rw, struct lwp *l); | | +static int crypto_refcount = 0;/* Prevent detaching while in use */ | + | /* Declaration of cloned-device (per-ctxt) entrypoints */ | static int cryptof_read(struct file *, off_t *, struct uio *, | kauth_cred_t, int); | @@ -262,6 +264,7 @@ cryptof_ioctl(struct file *fp, u_long cm |*/ | criofcr-sesn = 1; | criofcr-requestid = 1; | + crypto_refcount++; | mutex_exit(crypto_mtx); | (void)fd_clone(criofp, criofd, (FREAD|FWRITE), | cryptofops, criofcr); | @@ -951,6 +954,7 @@ cryptof_close(struct file *fp) | } | seldestroy(fcr-sinfo); | fp-f_data = NULL; | + crypto_refcount--; | mutex_exit(crypto_mtx); | | pool_put(fcrpl, fcr); | @@ -1080,6 +1084,7 @@ cryptoopen(dev_t dev, int flag, int mode |*/ | fcr-sesn = 1; | fcr-requestid = 1; | + crypto_refcount++; | mutex_exit(crypto_mtx); | return fd_clone(fp, fd, flag, cryptofops, fcr); | } | @@ -2109,6 +2114,10 @@ intcrypto_detach(device_t, int); It is not just the detach we need to handle, it is the module unload too (look for FINI). | int | crypto_detach(device_t self, int num) | { | + | + if (crypto_refcount != 0 || self-dv_unit != 0) | + return EBUSY; | + | pool_destroy(fcrpl); | pool_destroy(csepl); christos
Re: CVS commit: src/sys/opencrypto
On Jan 19, 10:37am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | That's covered in cryptoopen() at line 1060 I missed that patch | Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() | which attempts to detach each device instance. If a detach fails, then | config_cfdata_detach fails, and the unload will fail. Ok then! Did you test it? If it works, I guess commit it. christos
Re: CVS commit: src/sys/opencrypto
On Sun, 19 Jan 2014, Christos Zoulas wrote: On Jan 19, 10:37am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | That's covered in cryptoopen() at line 1060 I missed that patch No worry. | Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() | which attempts to detach each device instance. If a detach fails, then | config_cfdata_detach fails, and the unload will fail. Ok then! Did you test it? If it works, I guess commit it. It does address the simple case I described. It has a minor/cosmetic issue of printing an error message crypto0: unable to detach instance from config_cfdata_detach(). But David Laight's post has me more concerned, that perhaps we really need to solve this sort of issue more generically. So, not sure if this should be committed, or if we should leave your code in to prevent unload in all cases. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/opencrypto
On Jan 19, 11:21am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | | Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() | | which attempts to detach each device instance. If a detach fails, then | | config_cfdata_detach fails, and the unload will fail. | | Ok then! Did you test it? If it works, I guess commit it. | | It does address the simple case I described. It has a minor/cosmetic | issue of printing an error message | | crypto0: unable to detach instance | | from config_cfdata_detach(). yes, it looks like it is not designed to be called if it is not going to work. This is why most other things do the test in the MODULE_FINI part, see bpf for example. | But David Laight's post has me more concerned, that perhaps we really | need to solve this sort of issue more generically. Yes, this is why I took the EOPNOTSUPP shortcut. | So, not sure if this should be committed, or if we should leave your | code in to prevent unload in all cases. Well, you can move your test from detach to MODULE_FINI and it should work just fine. But yes, we should solve it more generically, but I still think the reference counting code is needed. christos
Re: CVS commit: src/sys/opencrypto
On Sun, 19 Jan 2014, John Nemeth wrote: } Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() } which attempts to detach each device instance. If a detach fails, then } config_cfdata_detach fails, and the unload will fail. Does this mean that you'll end up with some device instances detached and not others? Nope. All non-zero units are prevented from unload. And unit zero is permitted only if there are no references, ie no clones. So the master gets deleted only if there are no non-zero units. - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/opencrypto
In article pine.neb.4.64.1401191416060.22...@screamer.whooppee.com, Paul Goyette p...@whooppee.com wrote: On Sun, 19 Jan 2014, John Nemeth wrote: } Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() } which attempts to detach each device instance. If a detach fails, then } config_cfdata_detach fails, and the unload will fail. Does this mean that you'll end up with some device instances detached and not others? Nope. All non-zero units are prevented from unload. And unit zero is permitted only if there are no references, ie no clones. I don't see why non-zero units are special, can you explain? Open zero open 1, close 0, close 1. Should the close 1 unload? christos
Re: CVS commit: src/sys/opencrypto
On Jan 19, 10:37am, Paul Goyette wrote: } On Sun, 19 Jan 2014, Christos Zoulas wrote: } On Jan 19, 10:22am, p...@whooppee.com (Paul Goyette) wrote: } } | How about the following changes? } } You need to handle the regular open too, not justthe get (look for the } other fd_clone) } } That's covered in cryptoopen() at line 1060 } } | @@ -143,6 +143,8 @@ static intcryptoread(dev_t dev, struct } | static int cryptowrite(dev_t dev, struct uio *uio, int ioflag); } | static int cryptoselect(dev_t dev, int rw, struct lwp *l); } | } | +static int crypto_refcount = 0;/* Prevent detaching while in use */ } | + } | /* Declaration of cloned-device (per-ctxt) entrypoints */ } | static int cryptof_read(struct file *, off_t *, struct uio *, } | kauth_cred_t, int); } | @@ -262,6 +264,7 @@ cryptof_ioctl(struct file *fp, u_long cm } |*/ } | criofcr-sesn = 1; } | criofcr-requestid = 1; } | + crypto_refcount++; } | mutex_exit(crypto_mtx); } | (void)fd_clone(criofp, criofd, (FREAD|FWRITE), } | cryptofops, criofcr); } | @@ -951,6 +954,7 @@ cryptof_close(struct file *fp) } | } } | seldestroy(fcr-sinfo); } | fp-f_data = NULL; } | + crypto_refcount--; } | mutex_exit(crypto_mtx); } | } | pool_put(fcrpl, fcr); } | @@ -1080,6 +1084,7 @@ cryptoopen(dev_t dev, int flag, int mode } |*/ } | fcr-sesn = 1; } | fcr-requestid = 1; } | + crypto_refcount++; } | mutex_exit(crypto_mtx); } | return fd_clone(fp, fd, flag, cryptofops, fcr); } | } } | @@ -2109,6 +2114,10 @@ intcrypto_detach(device_t, int); } } It is not just the detach we need to handle, it is the module unload too } (look for FINI). } } Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() } which attempts to detach each device instance. If a detach fails, then } config_cfdata_detach fails, and the unload will fail. Does this mean that you'll end up with some device instances detached and not others? }-- End of excerpt from Paul Goyette
Re: CVS commit: src/sys/opencrypto
The mere existence of a non-zero unit is a reference that needs to prevent unloading. The two checks (unit# and ref-count) are equivalent and redundant, and only one of them needs to be there. On Sun, 19 Jan 2014, Christos Zoulas wrote: In article pine.neb.4.64.1401191416060.22...@screamer.whooppee.com, Paul Goyette p...@whooppee.com wrote: On Sun, 19 Jan 2014, John Nemeth wrote: } Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() } which attempts to detach each device instance. If a detach fails, then } config_cfdata_detach fails, and the unload will fail. Does this mean that you'll end up with some device instances detached and not others? Nope. All non-zero units are prevented from unload. And unit zero is permitted only if there are no references, ie no clones. I don't see why non-zero units are special, can you explain? Open zero open 1, close 0, close 1. Should the close 1 unload? christos !DSPAM:52dc4fde23811308416388! - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -
Re: CVS commit: src/sys/opencrypto
On Jan 19, 3:04pm, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | The mere existence of a non-zero unit is a reference that needs to | prevent unloading. What if it is the last reference? Then the ref count will go to zero after close without unloading, and you'll never unload. | The two checks (unit# and ref-count) are equivalent and redundant, and | only one of them needs to be there. Well, just keep the refcount one then. christos
Re: CVS commit: src/sys/opencrypto
On Sun, 19 Jan 2014, Christos Zoulas wrote: On Jan 19, 3:04pm, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | The mere existence of a non-zero unit is a reference that needs to | prevent unloading. What if it is the last reference? Then the ref count will go to zero after close without unloading, and you'll never unload. | The two checks (unit# and ref-count) are equivalent and redundant, and | only one of them needs to be there. Well, just keep the refcount one then. Yup - that's what I committed. The device_t wasn't available anyway, in crypto_modcmd() :) - | Paul Goyette | PGP Key fingerprint: | E-mail addresses: | | Customer Service | FA29 0E3B 35AF E8AE 6651 | paul at whooppee.com| | Network Engineer | 0786 F758 55DE 53BA 7731 | pgoyette at juniper.net | | Kernel Developer | | pgoyette at netbsd.org | -