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>