From owner-svn-src-all@freebsd.org Thu Sep 21 20:29:15 2017 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 88891E20404; Thu, 21 Sep 2017 20:29:15 +0000 (UTC) (envelope-from marius@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 63B5C69747; Thu, 21 Sep 2017 20:29:15 +0000 (UTC) (envelope-from marius@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v8LKTEAn028983; Thu, 21 Sep 2017 20:29:14 GMT (envelope-from marius@FreeBSD.org) Received: (from marius@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v8LKTE8F028981; Thu, 21 Sep 2017 20:29:14 GMT (envelope-from marius@FreeBSD.org) Message-Id: <201709212029.v8LKTE8F028981@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: marius set sender to marius@FreeBSD.org using -f From: Marius Strobl Date: Thu, 21 Sep 2017 20:29:14 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org Subject: svn commit: r323875 - releng/10.4/sys/crypto/aesni X-SVN-Group: releng X-SVN-Commit-Author: marius X-SVN-Commit-Paths: releng/10.4/sys/crypto/aesni X-SVN-Commit-Revision: 323875 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 21 Sep 2017 20:29:15 -0000 Author: marius Date: Thu Sep 21 20:29:14 2017 New Revision: 323875 URL: https://svnweb.freebsd.org/changeset/base/323875 Log: MF10: r323871 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] Approved by: re (gjb, kib) Modified: releng/10.4/sys/crypto/aesni/aesni.c releng/10.4/sys/crypto/aesni/aesni.h Directory Properties: releng/10.4/ (props changed) Modified: releng/10.4/sys/crypto/aesni/aesni.c ============================================================================== --- releng/10.4/sys/crypto/aesni/aesni.c Thu Sep 21 20:27:43 2017 (r323874) +++ releng/10.4/sys/crypto/aesni/aesni.c Thu Sep 21 20:29:14 2017 (r323875) @@ -39,16 +39,34 @@ __FBSDID("$FreeBSD$"); #include #include #include +#include #include #include +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: releng/10.4/sys/crypto/aesni/aesni.h ============================================================================== --- releng/10.4/sys/crypto/aesni/aesni.h Thu Sep 21 20:27:43 2017 (r323874) +++ releng/10.4/sys/crypto/aesni/aesni.h Thu Sep 21 20:29:14 2017 (r323875) @@ -65,7 +65,6 @@ struct aesni_session { int used; uint32_t id; TAILQ_ENTRY(aesni_session) next; - struct fpu_kern_ctx *fpu_ctx; }; /*