Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 17 Aug 2018 04:40:01 +0000 (UTC)
From:      Conrad Meyer <cem@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r337958 - head/sys/opencrypto
Message-ID:  <201808170440.w7H4e1Cj056992@repo.freebsd.org>

next in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201808170440.w7H4e1Cj056992>