Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 21 Sep 2017 19:30:32 +0000 (UTC)
From:      Marius Strobl <marius@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-10@freebsd.org
Subject:   svn commit: r323871 - stable/10/sys/crypto/aesni
Message-ID:  <201709211930.v8LJUW1B003724@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: marius
Date: Thu Sep 21 19:30:32 2017
New Revision: 323871
URL: https://svnweb.freebsd.org/changeset/base/323871

Log:
  MFC: r285215
  
  remove _NORMAL flag which isn't suppose to be used w/ _alloc_ctx...
  
  MFC: r285289
  
  address an issue where consumers, like IPsec, can reuse the same
  session in multiple threads w/o locking..  There was a single fpu
  context shared per session, if multiple threads were using the session,
  and both migrated away, they could corrupt each other's fpu context...
  
  MFC: r285297
  
  upon further examination, it turns out that _unregister_all already
  provides the guarantee that no threads will be in the _newsession code..
  
  MFC: r298332
  
  aesni(4): Initialize error before use [1]
  
  Reported by:	Coverity [1]
  CID:		1331554 [1]

Modified:
  stable/10/sys/crypto/aesni/aesni.c
  stable/10/sys/crypto/aesni/aesni.h
Directory Properties:
  stable/10/   (props changed)

Modified: stable/10/sys/crypto/aesni/aesni.c
==============================================================================
--- stable/10/sys/crypto/aesni/aesni.c	Thu Sep 21 19:24:11 2017	(r323870)
+++ stable/10/sys/crypto/aesni/aesni.c	Thu Sep 21 19:30:32 2017	(r323871)
@@ -39,16 +39,34 @@ __FBSDID("$FreeBSD$");
 #include <sys/rwlock.h>
 #include <sys/bus.h>
 #include <sys/uio.h>
+#include <sys/smp.h>
 #include <crypto/aesni/aesni.h>
 #include <cryptodev_if.h>
 
+static struct mtx_padalign *ctx_mtx;
+static struct fpu_kern_ctx **ctx_fpu;
+
 struct aesni_softc {
+	int	dieing;
 	int32_t cid;
 	uint32_t sid;
 	TAILQ_HEAD(aesni_sessions_head, aesni_session) sessions;
 	struct rwlock lock;
 };
 
+#define AQUIRE_CTX(i, ctx)					\
+	do {							\
+		(i) = PCPU_GET(cpuid);				\
+		mtx_lock(&ctx_mtx[(i)]);			\
+		(ctx) = ctx_fpu[(i)];				\
+	} while (0)
+#define RELEASE_CTX(i, ctx)					\
+	do {							\
+		mtx_unlock(&ctx_mtx[(i)]);			\
+		(i) = -1;					\
+		(ctx) = NULL;					\
+	} while (0)
+
 static int aesni_newsession(device_t, uint32_t *sidp, struct cryptoini *cri);
 static int aesni_freesession(device_t, uint64_t tid);
 static void aesni_freesession_locked(struct aesni_softc *sc,
@@ -88,14 +106,36 @@ aesni_probe(device_t dev)
 	return (0);
 }
 
+static void
+aensi_cleanctx(void)
+{
+	int i;
+
+	/* XXX - no way to return driverid */
+	CPU_FOREACH(i) {
+		if (ctx_fpu[i] != NULL) {
+			mtx_destroy(&ctx_mtx[i]);
+			fpu_kern_free_ctx(ctx_fpu[i]);
+		}
+		ctx_fpu[i] = NULL;
+	}
+	free(ctx_mtx, M_AESNI);
+	ctx_mtx = NULL;
+	free(ctx_fpu, M_AESNI);
+	ctx_fpu = NULL;
+}
+
 static int
 aesni_attach(device_t dev)
 {
 	struct aesni_softc *sc;
+	int i;
 
 	sc = device_get_softc(dev);
+	sc->dieing = 0;
 	TAILQ_INIT(&sc->sessions);
 	sc->sid = 1;
+
 	sc->cid = crypto_get_driverid(dev, CRYPTOCAP_F_HARDWARE |
 	    CRYPTOCAP_F_SYNC);
 	if (sc->cid < 0) {
@@ -103,6 +143,16 @@ aesni_attach(device_t dev)
 		return (ENOMEM);
 	}
 
+	ctx_mtx = malloc(sizeof *ctx_mtx * (mp_maxid + 1), M_AESNI,
+	    M_WAITOK|M_ZERO);
+	ctx_fpu = malloc(sizeof *ctx_fpu * (mp_maxid + 1), M_AESNI,
+	    M_WAITOK|M_ZERO);
+
+	CPU_FOREACH(i) {
+		ctx_fpu[i] = fpu_kern_alloc_ctx(0);
+		mtx_init(&ctx_mtx[i], "anifpumtx", NULL, MTX_DEF|MTX_NEW);
+	}
+
 	rw_init(&sc->lock, "aesni_lock");
 	crypto_register(sc->cid, CRYPTO_AES_CBC, 0, 0);
 	crypto_register(sc->cid, CRYPTO_AES_XTS, 0, 0);
@@ -116,6 +166,7 @@ aesni_detach(device_t dev)
 	struct aesni_session *ses;
 
 	sc = device_get_softc(dev);
+
 	rw_wlock(&sc->lock);
 	TAILQ_FOREACH(ses, &sc->sessions, next) {
 		if (ses->used) {
@@ -125,14 +176,18 @@ aesni_detach(device_t dev)
 			return (EBUSY);
 		}
 	}
+	sc->dieing = 1;
 	while ((ses = TAILQ_FIRST(&sc->sessions)) != NULL) {
 		TAILQ_REMOVE(&sc->sessions, ses, next);
-		fpu_kern_free_ctx(ses->fpu_ctx);
 		free(ses, M_AESNI);
 	}
 	rw_wunlock(&sc->lock);
-	rw_destroy(&sc->lock);
 	crypto_unregister_all(sc->cid);
+
+	rw_destroy(&sc->lock);
+
+	aensi_cleanctx();
+
 	return (0);
 }
 
@@ -148,6 +203,9 @@ aesni_newsession(device_t dev, uint32_t *sidp, struct 
 		return (EINVAL);
 
 	sc = device_get_softc(dev);
+	if (sc->dieing)
+		return (EINVAL);
+
 	ses = NULL;
 	encini = NULL;
 	for (; cri != NULL; cri = cri->cri_next) {
@@ -166,6 +224,10 @@ aesni_newsession(device_t dev, uint32_t *sidp, struct 
 		return (EINVAL);
 
 	rw_wlock(&sc->lock);
+	if (sc->dieing) {
+		rw_wunlock(&sc->lock);
+		return (EINVAL);
+	}
 	/*
 	 * Free sessions goes first, so if first session is used, we need to
 	 * allocate one.
@@ -177,13 +239,6 @@ aesni_newsession(device_t dev, uint32_t *sidp, struct 
 			rw_wunlock(&sc->lock);
 			return (ENOMEM);
 		}
-		ses->fpu_ctx = fpu_kern_alloc_ctx(FPU_KERN_NORMAL |
-		    FPU_KERN_NOWAIT);
-		if (ses->fpu_ctx == NULL) {
-			free(ses, M_AESNI);
-			rw_wunlock(&sc->lock);
-			return (ENOMEM);
-		}
 		ses->id = sc->sid++;
 	} else {
 		TAILQ_REMOVE(&sc->sessions, ses, next);
@@ -208,15 +263,14 @@ aesni_newsession(device_t dev, uint32_t *sidp, struct 
 static void
 aesni_freesession_locked(struct aesni_softc *sc, struct aesni_session *ses)
 {
-	struct fpu_kern_ctx *ctx;
 	uint32_t sid;
 
+	rw_assert(&sc->lock, RA_WLOCKED);
+
 	sid = ses->id;
 	TAILQ_REMOVE(&sc->sessions, ses, next);
-	ctx = ses->fpu_ctx;
 	bzero(ses, sizeof(*ses));
 	ses->id = sid;
-	ses->fpu_ctx = ctx;
 	TAILQ_INSERT_HEAD(&sc->sessions, ses, next);
 }
 
@@ -362,17 +416,27 @@ MODULE_DEPEND(aesni, crypto, 1, 1, 1);
 static int
 aesni_cipher_setup(struct aesni_session *ses, struct cryptoini *encini)
 {
-	struct thread *td;
+	struct fpu_kern_ctx *ctx;
 	int error;
+	int kt, ctxidx;
 
-	td = curthread;
-	error = fpu_kern_enter(td, ses->fpu_ctx, FPU_KERN_NORMAL |
-	    FPU_KERN_KTHR);
-	if (error != 0)
-		return (error);
+	kt = is_fpu_kern_thread(0);
+	if (!kt) {
+		AQUIRE_CTX(ctxidx, ctx);
+		error = fpu_kern_enter(curthread, ctx,
+		    FPU_KERN_NORMAL | FPU_KERN_KTHR);
+		if (error != 0)
+			goto out;
+	}
+
 	error = aesni_cipher_setup_common(ses, encini->cri_key,
 	    encini->cri_klen);
-	fpu_kern_leave(td, ses->fpu_ctx);
+
+	if (!kt) {
+		fpu_kern_leave(curthread, ctx);
+out:
+		RELEASE_CTX(ctxidx, ctx);
+	}
 	return (error);
 }
 
@@ -380,19 +444,24 @@ static int
 aesni_cipher_process(struct aesni_session *ses, struct cryptodesc *enccrd,
     struct cryptop *crp)
 {
-	struct thread *td;
+	struct fpu_kern_ctx *ctx;
 	uint8_t *buf;
 	int error, allocated;
+	int kt, ctxidx;
 
 	buf = aesni_cipher_alloc(enccrd, crp, &allocated);
 	if (buf == NULL)
 		return (ENOMEM);
 
-	td = curthread;
-	error = fpu_kern_enter(td, ses->fpu_ctx, FPU_KERN_NORMAL |
-	    FPU_KERN_KTHR);
-	if (error != 0)
-		goto out1;
+	error = 0;
+	kt = is_fpu_kern_thread(0);
+	if (!kt) {
+		AQUIRE_CTX(ctxidx, ctx);
+		error = fpu_kern_enter(curthread, ctx,
+		    FPU_KERN_NORMAL|FPU_KERN_KTHR);
+		if (error != 0)
+			goto out2;
+	}
 
 	if ((enccrd->crd_flags & CRD_F_KEY_EXPLICIT) != 0) {
 		error = aesni_cipher_setup_common(ses, enccrd->crd_key,
@@ -438,8 +507,12 @@ aesni_cipher_process(struct aesni_session *ses, struct
 		    enccrd->crd_skip + enccrd->crd_len - AES_BLOCK_LEN,
 		    AES_BLOCK_LEN, ses->iv);
 out:
-	fpu_kern_leave(td, ses->fpu_ctx);
-out1:
+	if (!kt) {
+		fpu_kern_leave(curthread, ctx);
+out2:
+		RELEASE_CTX(ctxidx, ctx);
+	}
+
 	if (allocated) {
 		bzero(buf, enccrd->crd_len);
 		free(buf, M_AESNI);

Modified: stable/10/sys/crypto/aesni/aesni.h
==============================================================================
--- stable/10/sys/crypto/aesni/aesni.h	Thu Sep 21 19:24:11 2017	(r323870)
+++ stable/10/sys/crypto/aesni/aesni.h	Thu Sep 21 19:30:32 2017	(r323871)
@@ -65,7 +65,6 @@ struct aesni_session {
 	int used;
 	uint32_t id;
 	TAILQ_ENTRY(aesni_session) next;
-	struct fpu_kern_ctx *fpu_ctx;
 };
 
 /*



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201709211930.v8LJUW1B003724>