From 9ebbebe4f763addd4e6269a2b4b76688f5f2fb04 Mon Sep 17 00:00:00 2001 From: Conrad Meyer Date: Fri, 17 Aug 2018 04:40:01 +0000 Subject: [PATCH] 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@ --- sys/opencrypto/cryptosoft.c | 126 +++++++++++++++++++----------------- sys/opencrypto/cryptosoft.h | 5 +- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/sys/opencrypto/cryptosoft.c b/sys/opencrypto/cryptosoft.c index 41f8a1772508..a4a719b5fbed 100644 --- a/sys/opencrypto/cryptosoft.c +++ b/sys/opencrypto/cryptosoft.c @@ -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 *sw, 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, struct cryptoini *cri) 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, struct cryptoini *cri) 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, struct cryptoini *cri) 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, struct cryptoini *cri) 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, struct cryptoini *cri) } } - (*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, struct cryptoini *cri) 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, struct cryptoini *cri) } } - (*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, struct cryptoini *cri) 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, struct cryptoini *cri) 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,30 +978,35 @@ swcr_newsession(device_t dev, crypto_session_t cses, struct cryptoini *cri) 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; } @@ -1011,14 +1014,16 @@ swcr_newsession(device_t dev, crypto_session_t cses, struct cryptoini *cri) 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 hint) * 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!"); diff --git a/sys/opencrypto/cryptosoft.h b/sys/opencrypto/cryptosoft.h index af78dc18529e..d88b09d4e1c0 100644 --- a/sys/opencrypto/cryptosoft.h +++ b/sys/opencrypto/cryptosoft.h @@ -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