From 40c1001792de63e0f90e977eb05393fd71f78692 Mon Sep 17 00:00:00 2001 From: Mimi Zohar Date: Mon, 20 Dec 2010 12:37:18 -0500 Subject: [PATCH 1/4] trusted-keys: free memory bugfix Add missing kfree(td) in tpm_seal() before the return, freeing td on error paths as well. Reported-by: Dan Carpenter Signed-off-by: Mimi Zohar Acked-by: David Safford Acked-by: David Howells Signed-off-by: Serge Hallyn Signed-off-by: James Morris --- security/keys/trusted_defined.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/security/keys/trusted_defined.c b/security/keys/trusted_defined.c index 975e9f29a52c..932f8687df16 100644 --- a/security/keys/trusted_defined.c +++ b/security/keys/trusted_defined.c @@ -511,7 +511,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, /* get session for sealing key */ ret = osap(tb, &sess, keyauth, keytype, keyhandle); if (ret < 0) - return ret; + goto out; dump_sess(&sess); /* calculate encrypted authorization value */ @@ -519,11 +519,11 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, memcpy(td->xorwork + SHA1_DIGEST_SIZE, sess.enonce, SHA1_DIGEST_SIZE); ret = TSS_sha1(td->xorwork, SHA1_DIGEST_SIZE * 2, td->xorhash); if (ret < 0) - return ret; + goto out; ret = tpm_get_random(tb, td->nonceodd, TPM_NONCE_SIZE); if (ret < 0) - return ret; + goto out; ordinal = htonl(TPM_ORD_SEAL); datsize = htonl(datalen); pcrsize = htonl(pcrinfosize); @@ -552,7 +552,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, &datsize, datalen, data, 0, 0); } if (ret < 0) - return ret; + goto out; /* build and send the TPM request packet */ INIT_BUF(tb); @@ -572,7 +572,7 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, ret = trusted_tpm_send(TPM_ANY_NUM, tb->data, MAX_BUF_SIZE); if (ret < 0) - return ret; + goto out; /* calculate the size of the returned Blob */ sealinfosize = LOAD32(tb->data, TPM_DATA_OFFSET + sizeof(uint32_t)); @@ -591,6 +591,8 @@ static int tpm_seal(struct tpm_buf *tb, uint16_t keytype, memcpy(blob, tb->data + TPM_DATA_OFFSET, storedsize); *bloblen = storedsize; } +out: + kfree(td); return ret; } From 35576eab390df313095306e2a8216134910e7014 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 17 Jan 2011 09:22:47 +0900 Subject: [PATCH 2/4] trusted-keys: another free memory bugfix TSS_rawhmac() forgot to call va_end()/kfree() when data == NULL and forgot to call va_end() when crypto_shash_update() < 0. Fix these bugs by escaping from the loop using "break" (rather than "return"/"goto") in order to make sure that va_end()/kfree() are always called. Signed-off-by: Tetsuo Handa Reviewed-by: Jesper Juhl Acked-by: Mimi Zohar Acked-by: David Howells Signed-off-by: James Morris --- security/keys/trusted_defined.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/security/keys/trusted_defined.c b/security/keys/trusted_defined.c index 932f8687df16..7b2179589063 100644 --- a/security/keys/trusted_defined.c +++ b/security/keys/trusted_defined.c @@ -101,11 +101,13 @@ static int TSS_rawhmac(unsigned char *digest, const unsigned char *key, if (dlen == 0) break; data = va_arg(argp, unsigned char *); - if (data == NULL) - return -EINVAL; + if (data == NULL) { + ret = -EINVAL; + break; + } ret = crypto_shash_update(&sdesc->shash, data, dlen); if (ret < 0) - goto out; + break; } va_end(argp); if (!ret) From 0e7491f685cbc962f2ef977f7b5f8ed0b3100e88 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 17 Jan 2011 09:25:34 +0900 Subject: [PATCH 3/4] trusted-keys: check for NULL before using it TSS_rawhmac() checks for data != NULL before using it. We should do the same thing for TSS_authhmac(). Signed-off-by: Tetsuo Handa Reviewed-by: Jesper Juhl Acked-by: Mimi Zohar Acked-by: David Howells Signed-off-by: James Morris --- security/keys/trusted_defined.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/security/keys/trusted_defined.c b/security/keys/trusted_defined.c index 7b2179589063..f7d06776faf0 100644 --- a/security/keys/trusted_defined.c +++ b/security/keys/trusted_defined.c @@ -148,6 +148,11 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key, if (dlen == 0) break; data = va_arg(argp, unsigned char *); + if (!data) { + ret = -EINVAL; + va_end(argp); + goto out; + } ret = crypto_shash_update(&sdesc->shash, data, dlen); if (ret < 0) { va_end(argp); From 154a96bfcd53b8e5020718c64769e542c44788b9 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 17 Jan 2011 09:27:27 +0900 Subject: [PATCH 4/4] trusted-keys: avoid scattring va_end() We can avoid scattering va_end() within the va_start(); for (;;) { } va_end(); loop, assuming that crypto_shash_init()/crypto_shash_update() return 0 on success and negative value otherwise. Make TSS_authhmac()/TSS_checkhmac1()/TSS_checkhmac2() similar to TSS_rawhmac() by removing "va_end()/goto" from the loop. Signed-off-by: Tetsuo Handa Reviewed-by: Jesper Juhl Acked-by: Mimi Zohar Acked-by: David Howells Signed-off-by: James Morris --- security/keys/trusted_defined.c | 30 +++++++++++++----------------- 1 file changed, 13 insertions(+), 17 deletions(-) diff --git a/security/keys/trusted_defined.c b/security/keys/trusted_defined.c index f7d06776faf0..2836c6dc18a3 100644 --- a/security/keys/trusted_defined.c +++ b/security/keys/trusted_defined.c @@ -150,17 +150,15 @@ static int TSS_authhmac(unsigned char *digest, const unsigned char *key, data = va_arg(argp, unsigned char *); if (!data) { ret = -EINVAL; - va_end(argp); - goto out; + break; } ret = crypto_shash_update(&sdesc->shash, data, dlen); - if (ret < 0) { - va_end(argp); - goto out; - } + if (ret < 0) + break; } va_end(argp); - ret = crypto_shash_final(&sdesc->shash, paramdigest); + if (!ret) + ret = crypto_shash_final(&sdesc->shash, paramdigest); if (!ret) ret = TSS_rawhmac(digest, key, keylen, SHA1_DIGEST_SIZE, paramdigest, TPM_NONCE_SIZE, h1, @@ -229,13 +227,12 @@ static int TSS_checkhmac1(unsigned char *buffer, break; dpos = va_arg(argp, unsigned int); ret = crypto_shash_update(&sdesc->shash, buffer + dpos, dlen); - if (ret < 0) { - va_end(argp); - goto out; - } + if (ret < 0) + break; } va_end(argp); - ret = crypto_shash_final(&sdesc->shash, paramdigest); + if (!ret) + ret = crypto_shash_final(&sdesc->shash, paramdigest); if (ret < 0) goto out; @@ -323,13 +320,12 @@ static int TSS_checkhmac2(unsigned char *buffer, break; dpos = va_arg(argp, unsigned int); ret = crypto_shash_update(&sdesc->shash, buffer + dpos, dlen); - if (ret < 0) { - va_end(argp); - goto out; - } + if (ret < 0) + break; } va_end(argp); - ret = crypto_shash_final(&sdesc->shash, paramdigest); + if (!ret) + ret = crypto_shash_final(&sdesc->shash, paramdigest); if (ret < 0) goto out;