From owner-svn-src-head@freebsd.org Fri Aug 17 04:40:02 2018 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4F53A108206E; Fri, 17 Aug 2018 04:40:02 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 0121E8C1B3; Fri, 17 Aug 2018 04:40:02 +0000 (UTC) (envelope-from cem@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 D65EB23891; Fri, 17 Aug 2018 04:40:01 +0000 (UTC) (envelope-from cem@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id w7H4e1Wu056994; Fri, 17 Aug 2018 04:40:01 GMT (envelope-from cem@FreeBSD.org) Received: (from cem@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id w7H4e1Cj056992; Fri, 17 Aug 2018 04:40:01 GMT (envelope-from cem@FreeBSD.org) Message-Id: <201808170440.w7H4e1Cj056992@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: cem set sender to cem@FreeBSD.org using -f From: Conrad Meyer Date: Fri, 17 Aug 2018 04:40:01 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r337958 - head/sys/opencrypto X-SVN-Group: head X-SVN-Commit-Author: cem X-SVN-Commit-Paths: head/sys/opencrypto X-SVN-Commit-Revision: 337958 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.27 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 17 Aug 2018 04:40:02 -0000 Author: cem Date: Fri Aug 17 04:40:01 2018 New Revision: 337958 URL: https://svnweb.freebsd.org/changeset/base/337958 Log: cryptosoft: Reduce generality of supported algorithm composition Fix a regression introduced in r336439. Rather than allowing any linked list of algorithms, allow at most two (typically, some combination of encrypt and/or MAC). Removes a WAITOK malloc in an unsleepable context (classic LOR) by placing both software algorithm contexts within the OCF-managed session object. Tested with 'cryptocheck -a all -d cryptosoft0', which includes some encrypt-and-MAC modes. PR: 230304 Reported by: sef@ Modified: head/sys/opencrypto/cryptosoft.c head/sys/opencrypto/cryptosoft.h Modified: head/sys/opencrypto/cryptosoft.c ============================================================================== --- head/sys/opencrypto/cryptosoft.c Fri Aug 17 04:17:51 2018 (r337957) +++ head/sys/opencrypto/cryptosoft.c Fri Aug 17 04:40:01 2018 (r337958) @@ -497,6 +497,7 @@ swcr_authenc(struct cryptop *crp) u_char uaalg[AALG_MAX_RESULT_LEN]; u_char iv[EALG_MAX_BLOCK_LEN]; union authctx ctx; + struct swcr_session *ses; struct cryptodesc *crd, *crda = NULL, *crde = NULL; struct swcr_data *sw, *swa, *swe = NULL; struct auth_hash *axf = NULL; @@ -507,14 +508,16 @@ swcr_authenc(struct cryptop *crp) ivlen = blksz = iskip = oskip = 0; + ses = crypto_get_driver_session(crp->crp_session); + for (crd = crp->crp_desc; crd; crd = crd->crd_next) { - for (sw = crypto_get_driver_session(crp->crp_session); - sw && sw->sw_alg != crd->crd_alg; - sw = sw->sw_next) + for (i = 0; i < nitems(ses->swcr_algorithms) && + ses->swcr_algorithms[i].sw_alg != crd->crd_alg; i++) ; - if (sw == NULL) + if (i == nitems(ses->swcr_algorithms)) return (EINVAL); + sw = &ses->swcr_algorithms[i]; switch (sw->sw_alg) { case CRYPTO_AES_NIST_GCM_16: case CRYPTO_AES_NIST_GMAC: @@ -749,10 +752,12 @@ swcr_compdec(struct cryptodesc *crd, struct swcr_data static int swcr_newsession(device_t dev, crypto_session_t cses, struct cryptoini *cri) { - struct swcr_data **swd, *ses; + struct swcr_session *ses; + struct swcr_data *swd; struct auth_hash *axf; struct enc_xform *txf; struct comp_algo *cxf; + size_t i; int len; int error; @@ -760,16 +765,9 @@ swcr_newsession(device_t dev, crypto_session_t cses, s return EINVAL; ses = crypto_get_driver_session(cses); - swd = &ses; - while (cri) { - if (*swd == NULL) - *swd = malloc(sizeof(struct swcr_data), - M_CRYPTO_DATA, M_WAITOK | M_ZERO); - if (*swd == NULL) { - swcr_freesession(dev, cses); - return ENOBUFS; - } + for (i = 0; cri != NULL && i < nitems(ses->swcr_algorithms); i++) { + swd = &ses->swcr_algorithms[i]; switch (cri->cri_alg) { case CRYPTO_DES_CBC: @@ -801,7 +799,7 @@ swcr_newsession(device_t dev, crypto_session_t cses, s goto enccommon; case CRYPTO_AES_NIST_GMAC: txf = &enc_xform_aes_nist_gmac; - (*swd)->sw_exf = txf; + swd->sw_exf = txf; break; case CRYPTO_CAMELLIA_CBC: txf = &enc_xform_camellia; @@ -814,14 +812,14 @@ swcr_newsession(device_t dev, crypto_session_t cses, s goto enccommon; enccommon: if (cri->cri_key != NULL) { - error = txf->setkey(&((*swd)->sw_kschedule), + error = txf->setkey(&swd->sw_kschedule, cri->cri_key, cri->cri_klen / 8); if (error) { swcr_freesession(dev, cses); return error; } } - (*swd)->sw_exf = txf; + swd->sw_exf = txf; break; case CRYPTO_MD5_HMAC: @@ -848,22 +846,22 @@ swcr_newsession(device_t dev, crypto_session_t cses, s case CRYPTO_RIPEMD160_HMAC: axf = &auth_hash_hmac_ripemd_160; authcommon: - (*swd)->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, + swd->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_ictx == NULL) { + if (swd->sw_ictx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } - (*swd)->sw_octx = malloc(axf->ctxsize, M_CRYPTO_DATA, + swd->sw_octx = malloc(axf->ctxsize, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_octx == NULL) { + if (swd->sw_octx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } if (cri->cri_key != NULL) { - error = swcr_authprepare(axf, *swd, + error = swcr_authprepare(axf, swd, cri->cri_key, cri->cri_klen); if (error != 0) { swcr_freesession(dev, cses); @@ -871,8 +869,8 @@ swcr_newsession(device_t dev, crypto_session_t cses, s } } - (*swd)->sw_mlen = cri->cri_mlen; - (*swd)->sw_axf = axf; + swd->sw_mlen = cri->cri_mlen; + swd->sw_axf = axf; break; case CRYPTO_MD5_KPDK: @@ -882,23 +880,23 @@ swcr_newsession(device_t dev, crypto_session_t cses, s case CRYPTO_SHA1_KPDK: axf = &auth_hash_key_sha1; auth2common: - (*swd)->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, + swd->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_ictx == NULL) { + if (swd->sw_ictx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } - (*swd)->sw_octx = malloc(cri->cri_klen / 8, + swd->sw_octx = malloc(cri->cri_klen / 8, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_octx == NULL) { + if (swd->sw_octx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } /* Store the key so we can "append" it to the payload */ if (cri->cri_key != NULL) { - error = swcr_authprepare(axf, *swd, + error = swcr_authprepare(axf, swd, cri->cri_key, cri->cri_klen); if (error != 0) { swcr_freesession(dev, cses); @@ -906,8 +904,8 @@ swcr_newsession(device_t dev, crypto_session_t cses, s } } - (*swd)->sw_mlen = cri->cri_mlen; - (*swd)->sw_axf = axf; + swd->sw_mlen = cri->cri_mlen; + swd->sw_axf = axf; break; #ifdef notdef case CRYPTO_MD5: @@ -931,16 +929,16 @@ swcr_newsession(device_t dev, crypto_session_t cses, s axf = &auth_hash_sha2_512; auth3common: - (*swd)->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, + swd->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_ictx == NULL) { + if (swd->sw_ictx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } - axf->Init((*swd)->sw_ictx); - (*swd)->sw_mlen = cri->cri_mlen; - (*swd)->sw_axf = axf; + axf->Init(swd->sw_ictx); + swd->sw_mlen = cri->cri_mlen; + swd->sw_axf = axf; break; case CRYPTO_AES_128_NIST_GMAC: @@ -960,15 +958,15 @@ swcr_newsession(device_t dev, crypto_session_t cses, s return EINVAL; } - (*swd)->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, + swd->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_ictx == NULL) { + if (swd->sw_ictx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } - axf->Init((*swd)->sw_ictx); - axf->Setkey((*swd)->sw_ictx, cri->cri_key, len); - (*swd)->sw_axf = axf; + axf->Init(swd->sw_ictx); + axf->Setkey(swd->sw_ictx, cri->cri_key, len); + swd->sw_axf = axf; break; case CRYPTO_BLAKE2B: @@ -980,45 +978,52 @@ swcr_newsession(device_t dev, crypto_session_t cses, s case CRYPTO_POLY1305: axf = &auth_hash_poly1305; auth5common: - (*swd)->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, + swd->sw_ictx = malloc(axf->ctxsize, M_CRYPTO_DATA, M_NOWAIT); - if ((*swd)->sw_ictx == NULL) { + if (swd->sw_ictx == NULL) { swcr_freesession(dev, cses); return ENOBUFS; } - axf->Setkey((*swd)->sw_ictx, cri->cri_key, + axf->Setkey(swd->sw_ictx, cri->cri_key, cri->cri_klen / 8); - axf->Init((*swd)->sw_ictx); - (*swd)->sw_axf = axf; + axf->Init(swd->sw_ictx); + swd->sw_axf = axf; break; case CRYPTO_DEFLATE_COMP: cxf = &comp_algo_deflate; - (*swd)->sw_cxf = cxf; + swd->sw_cxf = cxf; break; default: swcr_freesession(dev, cses); return EINVAL; } - (*swd)->sw_alg = cri->cri_alg; + swd->sw_alg = cri->cri_alg; cri = cri->cri_next; - swd = &((*swd)->sw_next); + ses->swcr_nalgs++; } + + if (cri != NULL) { + CRYPTDEB("Bogus session request for three or more algorithms"); + return EINVAL; + } return 0; } static void swcr_freesession(device_t dev, crypto_session_t cses) { - struct swcr_data *ses, *swd, *next; + struct swcr_session *ses; + struct swcr_data *swd; struct enc_xform *txf; struct auth_hash *axf; + size_t i; ses = crypto_get_driver_session(cses); - for (swd = ses; swd != NULL; swd = next) { - next = swd->sw_next; + for (i = 0; i < nitems(ses->swcr_algorithms); i++) { + swd = &ses->swcr_algorithms[i]; switch (swd->sw_alg) { case CRYPTO_DES_CBC: @@ -1095,10 +1100,6 @@ swcr_freesession(device_t dev, crypto_session_t cses) /* Nothing to do */ break; } - - /* OCF owns and frees the primary session object */ - if (swd != ses) - free(swd, M_CRYPTO_DATA); } } @@ -1108,8 +1109,10 @@ swcr_freesession(device_t dev, crypto_session_t cses) static int swcr_process(device_t dev, struct cryptop *crp, int hint) { + struct swcr_session *ses; struct cryptodesc *crd; - struct swcr_data *sw, *ses; + struct swcr_data *sw; + size_t i; /* Sanity check */ if (crp == NULL) @@ -1134,15 +1137,16 @@ swcr_process(device_t dev, struct cryptop *crp, int hi * XXX between the various instances of an algorithm (so we can * XXX locate the correct crypto context). */ - for (sw = ses; sw && sw->sw_alg != crd->crd_alg; - sw = sw->sw_next) + for (i = 0; i < nitems(ses->swcr_algorithms) && + ses->swcr_algorithms[i].sw_alg != crd->crd_alg; i++) ; /* No such context ? */ - if (sw == NULL) { + if (i == nitems(ses->swcr_algorithms)) { crp->crp_etype = EINVAL; goto done; } + sw = &ses->swcr_algorithms[i]; switch (sw->sw_alg) { case CRYPTO_DES_CBC: case CRYPTO_3DES_CBC: @@ -1235,7 +1239,7 @@ swcr_attach(device_t dev) memset(hmac_ipad_buffer, HMAC_IPAD_VAL, HMAC_MAX_BLOCK_LEN); memset(hmac_opad_buffer, HMAC_OPAD_VAL, HMAC_MAX_BLOCK_LEN); - swcr_id = crypto_get_driverid(dev, sizeof(struct swcr_data), + swcr_id = crypto_get_driverid(dev, sizeof(struct swcr_session), CRYPTOCAP_F_SOFTWARE | CRYPTOCAP_F_SYNC); if (swcr_id < 0) { device_printf(dev, "cannot initialize!"); Modified: head/sys/opencrypto/cryptosoft.h ============================================================================== --- head/sys/opencrypto/cryptosoft.h Fri Aug 17 04:17:51 2018 (r337957) +++ head/sys/opencrypto/cryptosoft.h Fri Aug 17 04:40:01 2018 (r337958) @@ -55,8 +55,11 @@ struct swcr_data { #define sw_exf SWCR_UN.SWCR_ENC.SW_exf #define sw_size SWCR_UN.SWCR_COMP.SW_size #define sw_cxf SWCR_UN.SWCR_COMP.SW_cxf +}; - struct swcr_data *sw_next; +struct swcr_session { + struct swcr_data swcr_algorithms[2]; + unsigned swcr_nalgs; }; #ifdef _KERNEL