commit 80d47d0aba0f7810a1c56caa0cf86a280d8b9d0b
author Jerome Forissier <jerome@forissier.org> Thu Jan 09 10:43:42 2020 +0100
committer Jérôme Forissier <jerome@forissier.org> Thu Jan 09 13:49:59 2020 +0100
tree 6f1c3c823aab96b98122a68936adcd163b31f7f4
parent 17d4286fd991ce36d91d98943bf0059b577c9599

core: ltc: SM2 PKE: fix bignum memory leaks

crypto_acipher_sm2_pke_encrypt() and crypto_acipher_sm2_pke_decrypt()
leak some bignum memory due to missing calls to ecc_free() and also due
to the fact that the x and y fields of an ecc_point are overwritten.
This is all the more problematic that bignum allocations use a special
memory pool [1] that can easily cause deadlocks when misused.

This commit fixes these problems.

[1] Commit a2eb5b55d169 ("libutils: add mempool API")

Fixes: f9a78287dd12 ("core: ltc: add support for SM2 PKE")
Signed-off-by: Jerome Forissier <jerome@forissier.org>
Reviewed-by: Jens Wiklander <jens.wiklander@linaro.org>

core/lib/libtomcrypt/sm2-pke.c

diff --git a/core/lib/libtomcrypt/sm2-pke.c b/core/lib/libtomcrypt/sm2-pke.c
index c2b3c18..315dc8e 100644
--- a/core/lib/libtomcrypt/sm2-pke.c
+++ b/core/lib/libtomcrypt/sm2-pke.c
@@ -21,49 +21,31 @@
 				size_t *consumed)
 {
 	uint8_t *ptr = (uint8_t *)x1y1;
-	TEE_Result res = TEE_SUCCESS;
 	uint8_t one[] = { 1 };
 	int ltc_res = 0;
-	void *x1 = NULL;
-	void *y1 = NULL;
 
 	if (max_size < (size_t)(2 * dp->size))
 		return TEE_ERROR_BAD_PARAMETERS;
 
-	ltc_res = mp_init_multi(&x1, &y1, &p->z, NULL);
+	ltc_res = mp_read_unsigned_bin(p->x, ptr, dp->size);
 	if (ltc_res != CRYPT_OK)
-		return TEE_ERROR_OUT_OF_MEMORY;
-
-	ltc_res = mp_read_unsigned_bin(x1, ptr, dp->size);
-	if (ltc_res != CRYPT_OK) {
-		res = TEE_ERROR_BAD_PARAMETERS;
-		goto err;
-	}
+		return TEE_ERROR_BAD_PARAMETERS;
 
 	ptr += dp->size;
 
-	ltc_res = mp_read_unsigned_bin(y1, ptr, dp->size);
-	if (ltc_res != CRYPT_OK) {
-		res = TEE_ERROR_BAD_PARAMETERS;
-		goto err;
-	}
+	ltc_res = mp_read_unsigned_bin(p->y, ptr, dp->size);
+	if (ltc_res != CRYPT_OK)
+		return TEE_ERROR_BAD_PARAMETERS;
 
-	ltc_res = ltc_ecc_is_point(dp, x1, y1);
-	if (ltc_res != CRYPT_OK) {
-		res = TEE_ERROR_BAD_PARAMETERS;
-		goto err;
-	}
+	ltc_res = ltc_ecc_is_point(dp, p->x, p->y);
+	if (ltc_res != CRYPT_OK)
+		return TEE_ERROR_BAD_PARAMETERS;
 
-	p->x = x1;
-	p->y = y1;
 	mp_read_unsigned_bin(p->z, one, sizeof(one));
 
 	*consumed = 2 * dp->size + 1; /* PC */
 
 	return TEE_SUCCESS;
-err:
-	mp_clear_multi(x1, y1, p->z, NULL);
-	return res;
 }
 
 /*
@@ -359,6 +341,7 @@
 	ltc_ecc_del_point(S);
 	ltc_ecc_del_point(C1);
 	mp_clear_multi(h, NULL);
+	ecc_free(&ltc_key);
 	return res;
 }
 
@@ -567,6 +550,7 @@
 	ltc_ecc_del_point(x2y2p);
 	ltc_ecc_del_point(S);
 	ltc_ecc_del_point(C1);
+	ecc_free(&ltc_key);
 	mp_clear_multi(k, h, NULL);
 	return res;
 }