Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/opencrypto crypto(4): Fix possible use-after-free in rac...



details:   https://anonhg.NetBSD.org/src/rev/684c5c07cd97
branches:  trunk
changeset: 366310:684c5c07cd97
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Sun May 22 11:29:54 2022 +0000

description:
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!

diffstat:

 sys/opencrypto/cryptodev.c |  41 ++++++++++++++++++++---------------------
 1 files changed, 20 insertions(+), 21 deletions(-)

diffs (73 lines):

diff -r 40a494aad4a2 -r 684c5c07cd97 sys/opencrypto/cryptodev.c
--- a/sys/opencrypto/cryptodev.c        Sun May 22 11:29:25 2022 +0000
+++ b/sys/opencrypto/cryptodev.c        Sun May 22 11:29:54 2022 +0000
@@ -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 <sys/cdefs.h>
-__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 <sys/param.h>
 #include <sys/systm.h>
@@ -715,20 +715,18 @@
 cryptodev_cb(struct cryptop *crp)
 {
        struct csession *cse = crp->crp_opaque;
-       int error = 0;
+       int error;
+
+       if ((error = crp->crp_etype) == EAGAIN) {
+               error = crypto_dispatch(crp);
+               if (error == 0)
+                       return 0;
+       }
 
        mutex_enter(&cryptodev_mtx);
-       cse->error = crp->crp_etype;
-       if (crp->crp_etype == EAGAIN) {
-               /* always drop mutex to call dispatch routine */
-               mutex_exit(&cryptodev_mtx);
-               error = crypto_dispatch(crp);
-               mutex_enter(&cryptodev_mtx);
-       }
-       if (error != 0 || (crp->crp_flags & CRYPTO_F_DONE)) {
-               crp->crp_devflags |= CRYPTODEV_F_RET;
-               cv_signal(&crp->crp_cv);
-       }
+       cse->error = error;
+       crp->crp_devflags |= CRYPTODEV_F_RET;
+       cv_signal(&crp->crp_cv);
        mutex_exit(&cryptodev_mtx);
        return 0;
 }
@@ -737,15 +735,16 @@
 cryptodev_mcb(struct cryptop *crp)
 {
        struct csession *cse = crp->crp_opaque;
+       int error;
+
+       if ((error = crp->crp_etype) == EAGAIN) {
+               error = crypto_dispatch(crp);
+               if (error == 0)
+                       return 0;
+       }
 
        mutex_enter(&cryptodev_mtx);
-       cse->error = crp->crp_etype;
-       if (crp->crp_etype == EAGAIN) {
-               mutex_exit(&cryptodev_mtx);
-               (void)crypto_dispatch(crp);
-               mutex_enter(&cryptodev_mtx);
-       }
-
+       cse->error = error;
        TAILQ_INSERT_TAIL(&crp->fcrp->crp_ret_mq, crp, crp_next);
        selnotify(&crp->fcrp->sinfo, 0, 0);
        mutex_exit(&cryptodev_mtx);



Home | Main Index | Thread Index | Old Index