Author: jhb
Date: Mon Jan 20 11:19:55 2020
New Revision: 356908
URL: https://svnweb.freebsd.org/changeset/base/356908

Log:
  MFC 356507,356520: Add a reference count to cryptodev sessions.
  
  356507:
  Add a reference count to cryptodev sessions.
  
  This prevents use-after-free races with crypto requests (which may
  sleep) and CIOCFSESSION as well as races from current CIOCFSESSION
  requests.
  
  356520:
  Remove no-longer-used function prototype.
  
  admbugs:      949
  Sponsored by: Chelsio Communications

Modified:
  stable/11/sys/opencrypto/cryptodev.c
Directory Properties:
  stable/11/   (props changed)

Changes in other areas also in this revision:
Modified:
  stable/12/sys/opencrypto/cryptodev.c
Directory Properties:
  stable/12/   (props changed)

Modified: stable/11/sys/opencrypto/cryptodev.c
==============================================================================
--- stable/11/sys/opencrypto/cryptodev.c        Mon Jan 20 09:16:06 2020        
(r356907)
+++ stable/11/sys/opencrypto/cryptodev.c        Mon Jan 20 11:19:55 2020        
(r356908)
@@ -268,6 +268,7 @@ crypt_kop_to_32(const struct crypt_kop *from, struct c
 struct csession {
        TAILQ_ENTRY(csession) next;
        u_int64_t       sid;
+       volatile u_int  refs;
        u_int32_t       ses;
        struct mtx      lock;           /* for op submission */
 
@@ -294,6 +295,7 @@ struct cryptop_data {
 struct fcrypt {
        TAILQ_HEAD(csessionlist, csession) csessions;
        int             sesn;
+       struct mtx      lock;
 };
 
 static struct timeval warninterval = { .tv_sec = 60, .tv_usec = 0 };
@@ -325,8 +327,7 @@ static struct fileops cryptofops = {
 };
 
 static struct csession *csefind(struct fcrypt *, u_int);
-static int csedelete(struct fcrypt *, struct csession *);
-static struct csession *cseadd(struct fcrypt *, struct csession *);
+static int csedelete(struct fcrypt *, u_int);
 static struct csession *csecreate(struct fcrypt *, u_int64_t, caddr_t,
     u_int64_t, caddr_t, u_int64_t, u_int32_t, u_int32_t, struct enc_xform *,
     struct auth_hash *);
@@ -617,13 +618,9 @@ bail:
                break;
        case CIOCFSESSION:
                ses = *(u_int32_t *)data;
-               cse = csefind(fcr, ses);
-               if (cse == NULL) {
+               error = csedelete(fcr, ses);
+               if (error != 0)
                        SDT_PROBE1(opencrypto, dev, ioctl, error, __LINE__);
-                       return (EINVAL);
-               }
-               csedelete(fcr, cse);
-               error = csefree(cse);
                break;
        case CIOCCRYPT:
 #ifdef COMPAT_FREEBSD32
@@ -640,6 +637,7 @@ bail:
                        return (EINVAL);
                }
                error = cryptodev_op(cse, cop, active_cred, td);
+               (void)csefree(cse);
 #ifdef COMPAT_FREEBSD32
                if (error == 0 && cmd == CIOCCRYPT32)
                        crypt_op_to_32(cop, data);
@@ -706,6 +704,7 @@ bail:
                        return (EINVAL);
                }
                error = cryptodev_aead(cse, caead, active_cred, td);
+               (void)csefree(cse);
                break;
        default:
                error = EINVAL;
@@ -1323,6 +1322,9 @@ cryptof_close(struct file *fp, struct thread *td)
 
        while ((cse = TAILQ_FIRST(&fcr->csessions))) {
                TAILQ_REMOVE(&fcr->csessions, cse, next);
+               KASSERT(cse->refs == 1,
+                   ("%s: crypto session %p with %d refs", __func__, cse,
+                   cse->refs));
                (void)csefree(cse);
        }
        free(fcr, M_XDATA);
@@ -1343,34 +1345,35 @@ csefind(struct fcrypt *fcr, u_int ses)
 {
        struct csession *cse;
 
-       TAILQ_FOREACH(cse, &fcr->csessions, next)
-               if (cse->ses == ses)
+       mtx_lock(&fcr->lock);
+       TAILQ_FOREACH(cse, &fcr->csessions, next) {
+               if (cse->ses == ses) {
+                       refcount_acquire(&cse->refs);
+                       mtx_unlock(&fcr->lock);
                        return (cse);
+               }
+       }
+       mtx_unlock(&fcr->lock);
        return (NULL);
 }
 
 static int
-csedelete(struct fcrypt *fcr, struct csession *cse_del)
+csedelete(struct fcrypt *fcr, u_int ses)
 {
        struct csession *cse;
 
+       mtx_lock(&fcr->lock);
        TAILQ_FOREACH(cse, &fcr->csessions, next) {
-               if (cse == cse_del) {
+               if (cse->ses == ses) {
                        TAILQ_REMOVE(&fcr->csessions, cse, next);
-                       return (1);
+                       mtx_unlock(&fcr->lock);
+                       return (csefree(cse));
                }
        }
-       return (0);
+       mtx_unlock(&fcr->lock);
+       return (EINVAL);
 }
        
-static struct csession *
-cseadd(struct fcrypt *fcr, struct csession *cse)
-{
-       TAILQ_INSERT_TAIL(&fcr->csessions, cse, next);
-       cse->ses = fcr->sesn++;
-       return (cse);
-}
-
 struct csession *
 csecreate(struct fcrypt *fcr, u_int64_t sid, caddr_t key, u_int64_t keylen,
     caddr_t mackey, u_int64_t mackeylen, u_int32_t cipher, u_int32_t mac,
@@ -1382,6 +1385,7 @@ csecreate(struct fcrypt *fcr, u_int64_t sid, caddr_t k
        if (cse == NULL)
                return NULL;
        mtx_init(&cse->lock, "cryptodev", "crypto session lock", MTX_DEF);
+       refcount_init(&cse->refs, 1);
        cse->key = key;
        cse->keylen = keylen/8;
        cse->mackey = mackey;
@@ -1391,7 +1395,10 @@ csecreate(struct fcrypt *fcr, u_int64_t sid, caddr_t k
        cse->mac = mac;
        cse->txform = txform;
        cse->thash = thash;
-       cseadd(fcr, cse);
+       mtx_lock(&fcr->lock);
+       TAILQ_INSERT_TAIL(&fcr->csessions, cse, next);
+       cse->ses = fcr->sesn++;
+       mtx_unlock(&fcr->lock);
        return (cse);
 }
 
@@ -1400,6 +1407,8 @@ csefree(struct csession *cse)
 {
        int error;
 
+       if (!refcount_release(&cse->refs))
+               return (0);
        error = crypto_freesession(cse->sid);
        mtx_destroy(&cse->lock);
        if (cse->key)
@@ -1437,13 +1446,14 @@ cryptoioctl(struct cdev *dev, u_long cmd, caddr_t data
 
        switch (cmd) {
        case CRIOGET:
-               fcr = malloc(sizeof(struct fcrypt), M_XDATA, M_WAITOK);
+               fcr = malloc(sizeof(struct fcrypt), M_XDATA, M_WAITOK | M_ZERO);
                TAILQ_INIT(&fcr->csessions);
-               fcr->sesn = 0;
+               mtx_init(&fcr->lock, "fcrypt", NULL, MTX_DEF);
 
                error = falloc(td, &f, &fd, 0);
 
                if (error) {
+                       mtx_destroy(&fcr->lock);
                        free(fcr, M_XDATA);
                        return (error);
                }
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to