From owner-svn-src-releng@freebsd.org Tue May 12 16:57:48 2020 Return-Path: Delivered-To: svn-src-releng@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 557E82F4A66; Tue, 12 May 2020 16:57:48 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 49M3rr1Z1wz4VCj; Tue, 12 May 2020 16:57:48 +0000 (UTC) (envelope-from gordon@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 mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 2C648B2DC; Tue, 12 May 2020 16:57:48 +0000 (UTC) (envelope-from gordon@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 04CGvmJK063999; Tue, 12 May 2020 16:57:48 GMT (envelope-from gordon@FreeBSD.org) Received: (from gordon@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 04CGvlFH063998; Tue, 12 May 2020 16:57:47 GMT (envelope-from gordon@FreeBSD.org) Message-Id: <202005121657.04CGvlFH063998@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: gordon set sender to gordon@FreeBSD.org using -f From: Gordon Tetlow Date: Tue, 12 May 2020 16:57:47 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-releng@freebsd.org Subject: svn commit: r360976 - in releng: 11.3/sys/opencrypto 12.1/sys/opencrypto X-SVN-Group: releng X-SVN-Commit-Author: gordon X-SVN-Commit-Paths: in releng: 11.3/sys/opencrypto 12.1/sys/opencrypto X-SVN-Commit-Revision: 360976 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-releng@freebsd.org X-Mailman-Version: 2.1.33 Precedence: list List-Id: SVN commit messages for the release engineering / security commits to the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 12 May 2020 16:57:48 -0000 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); }