Add reference counting that prevents freeing of busy aesni sessions.

The early freeing has become possible because aesni_process() and
crypto_freesession() are no longer serialized by the kernel lock.
The flaw has caused kernel panics with IPsec traffic.

Issue seen by some, fix tested by mabi on bugs@
OK mikeb@, mpi@

1 files changed, 53 insertions, 26 deletions

diff --git a/sys/arch/amd64/amd64/aesni.c b/sys/arch/amd64/amd64/aesni.c
index ce2808cbfc6..9d5cb606e0a 100644
--- a/sys/arch/amd64/amd64/aesni.c
+++ b/sys/arch/amd64/amd64/aesni.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: aesni.c,v 1.44 2018/04/09 04:34:56 visa Exp $	*/
+/*	$OpenBSD: aesni.c,v 1.45 2018/04/22 02:59:03 visa Exp $	*/
 /*-
  * Copyright (c) 2003 Jason Wright
  * Copyright (c) 2003, 2004 Theo de Raadt
@@ -21,6 +21,7 @@
 
 #include <sys/param.h>
 #include <sys/systm.h>
+#include <sys/atomic.h>
 #include <sys/queue.h>
 #include <sys/malloc.h>
 #include <sys/pool.h>
@@ -50,6 +51,7 @@ struct aesni_session {
 	uint32_t		 ses_dkey[4 * (AES_MAXROUNDS + 1)];
 	uint32_t		 ses_klen;
 	uint8_t			 ses_nonce[AESCTR_NONCESIZE];
+	unsigned int		 ses_refs;
 	int			 ses_sid;
 	GHASH_CTX		*ses_ghash;
 	struct aesni_xts_ctx	*ses_xts;
@@ -105,6 +107,10 @@ int	aesni_newsession(u_int32_t *, struct cryptoini *);
 int	aesni_freesession(u_int64_t);
 int	aesni_process(struct cryptop *);
 
+struct aesni_session *
+	aesni_get(uint32_t);
+void	aesni_put(struct aesni_session *);
+
 int	aesni_swauth(struct cryptop *, struct cryptodesc *, struct swcr_data *,
 	    caddr_t);
 
@@ -181,15 +187,11 @@ aesni_newsession(u_int32_t *sidp, struct cryptoini *cri)
 	if (!ses)
 		return (ENOMEM);
 
+	ses->ses_refs = 1;
 	ses->ses_buf = malloc(PAGE_SIZE, M_DEVBUF, M_NOWAIT|M_ZERO);
 	if (ses->ses_buf != NULL)
 		ses->ses_buflen = PAGE_SIZE;
 
-	mtx_enter(&aesni_sc->sc_mtx);
-	LIST_INSERT_HEAD(&aesni_sc->sc_sessions, ses, ses_entries);
-	mtx_leave(&aesni_sc->sc_mtx);
-	ses->ses_sid = ++aesni_sc->sc_sid;
-
 	for (c = cri; c != NULL; c = c->cri_next) {
 		switch (c->cri_alg) {
 		case CRYPTO_AES_CBC:
@@ -214,7 +216,7 @@ aesni_newsession(u_int32_t *sidp, struct cryptoini *cri)
 			ses->ses_xts = malloc(sizeof(struct aesni_xts_ctx),
 			    M_CRYPTO_DATA, M_NOWAIT | M_ZERO);
 			if (ses->ses_xts == NULL) {
-				aesni_freesession(ses->ses_sid);
+				aesni_put(ses);
 				return (ENOMEM);
 			}
 
@@ -238,7 +240,7 @@ aesni_newsession(u_int32_t *sidp, struct cryptoini *cri)
 			ses->ses_ghash = malloc(sizeof(GHASH_CTX),
 			    M_CRYPTO_DATA, M_NOWAIT | M_ZERO);
 			if (ses->ses_ghash == NULL) {
-				aesni_freesession(ses->ses_sid);
+				aesni_put(ses);
 				return (ENOMEM);
 			}
 
@@ -267,7 +269,7 @@ aesni_newsession(u_int32_t *sidp, struct cryptoini *cri)
 			swd = malloc(sizeof(struct swcr_data), M_CRYPTO_DATA,
 			    M_NOWAIT|M_ZERO);
 			if (swd == NULL) {
-				aesni_freesession(ses->ses_sid);
+				aesni_put(ses);
 				return (ENOMEM);
 			}
 			ses->ses_swd = swd;
@@ -275,14 +277,14 @@ aesni_newsession(u_int32_t *sidp, struct cryptoini *cri)
 			swd->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA,
 			    M_NOWAIT);
 			if (swd->sw_ictx == NULL) {
-				aesni_freesession(ses->ses_sid);
+				aesni_put(ses);
 				return (ENOMEM);
 			}
 
 			swd->sw_octx = malloc(axf->ctxsize, M_CRYPTO_DATA,
 			    M_NOWAIT);
 			if (swd->sw_octx == NULL) {
-				aesni_freesession(ses->ses_sid);
+				aesni_put(ses);
 				return (ENOMEM);
 			}
 
@@ -316,11 +318,16 @@ aesni_newsession(u_int32_t *sidp, struct cryptoini *cri)
 			break;
 
 		default:
-			aesni_freesession(ses->ses_sid);
+			aesni_put(ses);
 			return (EINVAL);
 		}
 	}
 
+	mtx_enter(&aesni_sc->sc_mtx);
+	ses->ses_sid = ++aesni_sc->sc_sid;
+	LIST_INSERT_HEAD(&aesni_sc->sc_sessions, ses, ses_entries);
+	mtx_leave(&aesni_sc->sc_mtx);
+
 	*sidp = ses->ses_sid;
 	return (0);
 }
@@ -329,23 +336,33 @@ int
 aesni_freesession(u_int64_t tid)
 {
 	struct aesni_session *ses;
-	struct swcr_data *swd;
-	struct auth_hash *axf;
 	u_int32_t sid = (u_int32_t)tid;
 
 	mtx_enter(&aesni_sc->sc_mtx);
 	LIST_FOREACH(ses, &aesni_sc->sc_sessions, ses_entries) {
-		if (ses->ses_sid == sid)
+		if (ses->ses_sid == sid) {
+			LIST_REMOVE(ses, ses_entries);
 			break;
+		}
 	}
 	mtx_leave(&aesni_sc->sc_mtx);
 
 	if (ses == NULL)
 		return (EINVAL);
 
-	mtx_enter(&aesni_sc->sc_mtx);
-	LIST_REMOVE(ses, ses_entries);
-	mtx_leave(&aesni_sc->sc_mtx);
+	aesni_put(ses);
+
+	return (0);
+}
+
+void
+aesni_put(struct aesni_session *ses)
+{
+	struct swcr_data *swd;
+	struct auth_hash *axf;
+
+	if (atomic_dec_int_nv(&ses->ses_refs) > 0)
+		return;
 
 	if (ses->ses_ghash) {
 		explicit_bzero(ses->ses_ghash, sizeof(GHASH_CTX));
@@ -379,8 +396,22 @@ aesni_freesession(u_int64_t tid)
 
 	explicit_bzero(ses, sizeof (*ses));
 	pool_put(&aesnipl, ses);
+}
 
-	return (0);
+struct aesni_session *
+aesni_get(uint32_t sid)
+{
+	struct aesni_session *ses = NULL;
+
+	mtx_enter(&aesni_sc->sc_mtx);
+	LIST_FOREACH(ses, &aesni_sc->sc_sessions, ses_entries) {
+		if (ses->ses_sid == sid) {
+			atomic_inc_int(&ses->ses_refs);
+			break;
+		}
+	}
+	mtx_leave(&aesni_sc->sc_mtx);
+	return (ses);
 }
 
 int
@@ -609,13 +640,7 @@ aesni_process(struct cryptop *crp)
 	if (crp->crp_ndesc < 1)
 		return (EINVAL);
 
-	mtx_enter(&aesni_sc->sc_mtx);
-	LIST_FOREACH(ses, &aesni_sc->sc_sessions, ses_entries) {
-		if (ses->ses_sid == (crp->crp_sid & 0xffffffff))
-			break;
-	}
-	mtx_leave(&aesni_sc->sc_mtx);
-
+	ses = aesni_get(crp->crp_sid & 0xffffffff);
 	if (!ses) {
 		err = EINVAL;
 		goto out;
@@ -670,6 +695,8 @@ aesni_process(struct cryptop *crp)
 	}
 
 out:
+	if (ses != NULL)
+		aesni_put(ses);
 	crp->crp_etype = err;
 	crypto_done(crp);
 	return (err);