From owner-svn-src-stable-11@freebsd.org Mon Jan 20 11:19:56 2020 Return-Path: Delivered-To: svn-src-stable-11@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 C73E01F6531; Mon, 20 Jan 2020 11:19:56 +0000 (UTC) (envelope-from jhb@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 481Tj84zSwz4NNh; Mon, 20 Jan 2020 11:19:56 +0000 (UTC) (envelope-from jhb@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 A615122BD6; Mon, 20 Jan 2020 11:19:56 +0000 (UTC) (envelope-from jhb@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id 00KBJuq2058215; Mon, 20 Jan 2020 11:19:56 GMT (envelope-from jhb@FreeBSD.org) Received: (from jhb@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id 00KBJulX058214; Mon, 20 Jan 2020 11:19:56 GMT (envelope-from jhb@FreeBSD.org) Message-Id: <202001201119.00KBJulX058214@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: jhb set sender to jhb@FreeBSD.org using -f From: John Baldwin Date: Mon, 20 Jan 2020 11:19:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-11@freebsd.org Subject: svn commit: r356908 - in stable: 11/sys/opencrypto 12/sys/opencrypto X-SVN-Group: stable-11 X-SVN-Commit-Author: jhb X-SVN-Commit-Paths: in stable: 11/sys/opencrypto 12/sys/opencrypto X-SVN-Commit-Revision: 356908 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-stable-11@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for only the 11-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 20 Jan 2020 11:19:56 -0000 Author: jhb Date: Mon Jan 20 11:19:55 2020 New Revision: 356908 URL: https://svnweb.freebsd.org/changeset/base/356908 Log: MFC 356507,356520: Add a reference count to cryptodev sessions. 356507: Add a reference count to cryptodev sessions. This prevents use-after-free races with crypto requests (which may sleep) and CIOCFSESSION as well as races from current CIOCFSESSION requests. 356520: Remove no-longer-used function prototype. admbugs: 949 Sponsored by: Chelsio Communications Modified: stable/11/sys/opencrypto/cryptodev.c Directory Properties: stable/11/ (props changed) Changes in other areas also in this revision: Modified: stable/12/sys/opencrypto/cryptodev.c Directory Properties: stable/12/ (props changed) Modified: stable/11/sys/opencrypto/cryptodev.c ============================================================================== --- stable/11/sys/opencrypto/cryptodev.c Mon Jan 20 09:16:06 2020 (r356907) +++ stable/11/sys/opencrypto/cryptodev.c Mon Jan 20 11:19:55 2020 (r356908) @@ -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 struct timeval warninterval = { .tv_sec = 60, .tv_usec = 0 }; @@ -325,8 +327,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 *); @@ -617,13 +618,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 @@ -640,6 +637,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); @@ -706,6 +704,7 @@ bail: return (EINVAL); } error = cryptodev_aead(cse, caead, active_cred, td); + (void)csefree(cse); break; default: error = EINVAL; @@ -1323,6 +1322,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); @@ -1343,34 +1345,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, @@ -1382,6 +1385,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; @@ -1391,7 +1395,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); } @@ -1400,6 +1407,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) @@ -1437,13 +1446,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); }