svn commit: r360976 - in releng: 11.3/sys/opencrypto 12.1/sys/opencrypto

Gordon Tetlow gordon at FreeBSD.org
Tue May 12 16:57:48 UTC 2020


Author: gordon
Date: Tue May 12 16:57:47 2020
New Revision: 360976
URL: https://svnweb.freebsd.org/changeset/base/360976

Log:
  Fix use after free in cryptodev module.
  
  Approved by:	so
  Security:	FreeBSD-SA-20:15.cryptodev
  Security:	CVE-2019-15879

Modified:
  releng/11.3/sys/opencrypto/cryptodev.c
  releng/12.1/sys/opencrypto/cryptodev.c

Modified: releng/11.3/sys/opencrypto/cryptodev.c
==============================================================================
--- releng/11.3/sys/opencrypto/cryptodev.c	Tue May 12 16:55:32 2020	(r360975)
+++ releng/11.3/sys/opencrypto/cryptodev.c	Tue May 12 16:57:47 2020	(r360976)
@@ -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	int cryptof_ioctl(struct file *, u_long, void *,
@@ -320,8 +322,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 *);
@@ -612,13 +613,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
@@ -635,6 +632,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);
@@ -701,6 +699,7 @@ bail:
 			return (EINVAL);
 		}
 		error = cryptodev_aead(cse, caead, active_cred, td);
+		(void)csefree(cse);
 		break;
 	default:
 		error = EINVAL;
@@ -1275,6 +1274,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);
@@ -1295,34 +1297,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,
@@ -1334,6 +1337,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;
@@ -1343,7 +1347,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);
 }
 
@@ -1352,6 +1359,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)
@@ -1389,13 +1398,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);
 		}

Modified: releng/12.1/sys/opencrypto/cryptodev.c
==============================================================================
--- releng/12.1/sys/opencrypto/cryptodev.c	Tue May 12 16:55:32 2020	(r360975)
+++ releng/12.1/sys/opencrypto/cryptodev.c	Tue May 12 16:57:47 2020	(r360976)
@@ -266,6 +266,7 @@ crypt_kop_to_32(const struct crypt_kop *from, struct c
 struct csession {
 	TAILQ_ENTRY(csession) next;
 	crypto_session_t cses;
+	volatile u_int	refs;
 	u_int32_t	ses;
 	struct mtx	lock;		/* for op submission */
 
@@ -292,6 +293,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 };
@@ -323,8 +325,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 bool csedelete(struct fcrypt *, u_int);
 static struct csession *csecreate(struct fcrypt *, crypto_session_t, caddr_t,
     u_int64_t, caddr_t, u_int64_t, u_int32_t, u_int32_t, struct enc_xform *,
     struct auth_hash *);
@@ -685,13 +686,10 @@ bail:
 		break;
 	case CIOCFSESSION:
 		ses = *(u_int32_t *)data;
-		cse = csefind(fcr, ses);
-		if (cse == NULL) {
+		if (!csedelete(fcr, ses)) {
 			SDT_PROBE1(opencrypto, dev, ioctl, error, __LINE__);
 			return (EINVAL);
 		}
-		csedelete(fcr, cse);
-		csefree(cse);
 		break;
 	case CIOCCRYPT:
 #ifdef COMPAT_FREEBSD32
@@ -708,6 +706,7 @@ bail:
 			return (EINVAL);
 		}
 		error = cryptodev_op(cse, cop, active_cred, td);
+		csefree(cse);
 #ifdef COMPAT_FREEBSD32
 		if (error == 0 && cmd == CIOCCRYPT32)
 			crypt_op_to_32(cop, data);
@@ -774,6 +773,7 @@ bail:
 			return (EINVAL);
 		}
 		error = cryptodev_aead(cse, caead, active_cred, td);
+		csefree(cse);
 		break;
 	default:
 		error = EINVAL;
@@ -1349,6 +1349,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));
 		csefree(cse);
 	}
 	free(fcr, M_XDATA);
@@ -1369,34 +1372,36 @@ 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)
+static bool
+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);
+			csefree(cse);
+			return (true);
 		}
 	}
-	return (0);
+	mtx_unlock(&fcr->lock);
+	return (false);
 }
 	
-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, crypto_session_t cses, caddr_t key, u_int64_t keylen,
     caddr_t mackey, u_int64_t mackeylen, u_int32_t cipher, u_int32_t mac,
@@ -1408,6 +1413,7 @@ csecreate(struct fcrypt *fcr, crypto_session_t cses, c
 	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;
@@ -1417,7 +1423,10 @@ csecreate(struct fcrypt *fcr, crypto_session_t cses, c
 	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);
 }
 
@@ -1425,6 +1434,8 @@ static void
 csefree(struct csession *cse)
 {
 
+	if (!refcount_release(&cse->refs))
+		return;
 	crypto_freesession(cse->cses);
 	mtx_destroy(&cse->lock);
 	if (cse->key)
@@ -1461,13 +1472,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);
 		}


More information about the svn-src-all mailing list