VBoot Reference: Fix many memory leaks.
Fix memory leaks found using Valgrind, adds sanity checking to RSAPublicKey parsing code to prevent crazy behavior.
Review URL: http://codereview.chromium.org/858008
diff --git a/utils/firmware_image.c b/utils/firmware_image.c
index c0a660b..cd8e942 100644
--- a/utils/firmware_image.c
+++ b/utils/firmware_image.c
@@ -394,12 +394,12 @@
const int dev_mode) {
int error_code;
int algorithm; /* Signing key algorithm. */
- RSAPublicKey* firmware_sign_key;
+ RSAPublicKey* firmware_sign_key = NULL;
int firmware_sign_key_len, signature_len, header_len, firmware_len;
- const uint8_t* header_ptr; /* Pointer to header. */
- const uint8_t* firmware_sign_key_ptr; /* Pointer to signing key. */
- const uint8_t* preamble_ptr; /* Pointer to preamble block. */
- const uint8_t* firmware_ptr; /* Pointer to firmware signature/data. */
+ const uint8_t* header_ptr = NULL; /* Pointer to header. */
+ const uint8_t* firmware_sign_key_ptr = NULL; /* Pointer to signing key. */
+ const uint8_t* preamble_ptr = NULL; /* Pointer to preamble block. */
+ const uint8_t* firmware_ptr = NULL; /* Pointer to firmware signature/data. */
/* Note: All the offset calculations are based on struct FirmwareImage which
* is defined in include/firmware_image.h. */
@@ -428,9 +428,10 @@
FIELD_LEN(firmware_key_signature));
if ((error_code = VerifyFirmwarePreamble(firmware_sign_key, preamble_ptr,
algorithm,
- &firmware_len)))
+ &firmware_len))) {
+ RSAPublicKeyFree(firmware_sign_key);
return error_code; /* AKA jump to recovery. */
-
+ }
/* Only continue if firmware data verification succeeds. */
firmware_ptr = (preamble_ptr +
FIELD_LEN(firmware_version) +
@@ -440,16 +441,19 @@
if ((error_code = VerifyFirmwareData(firmware_sign_key, firmware_ptr,
firmware_len,
- algorithm)))
+ algorithm))) {
+ RSAPublicKeyFree(firmware_sign_key);
return error_code; /* AKA jump to recovery. */
+ }
+ RSAPublicKeyFree(firmware_sign_key);
return 0; /* Success! */
}
int VerifyFirmwareImage(const RSAPublicKey* root_key,
const FirmwareImage* image,
const int dev_mode) {
- RSAPublicKey* firmware_sign_key;
+ RSAPublicKey* firmware_sign_key = NULL;
uint8_t* header_digest = NULL;
uint8_t* preamble_digest = NULL;
uint8_t* firmware_digest = NULL;
@@ -527,6 +531,7 @@
}
verify_failure:
+ RSAPublicKeyFree(firmware_sign_key);
Free(firmware_digest);
Free(preamble_digest);
Free(header_digest);
diff --git a/utils/kernel_image.c b/utils/kernel_image.c
index 893f49a..8201137 100644
--- a/utils/kernel_image.c
+++ b/utils/kernel_image.c
@@ -500,8 +500,10 @@
config_ptr = (header_ptr + header_len + kernel_key_signature_len);
if ((error_code = VerifyKernelConfig(kernel_sign_key, config_ptr,
kernel_sign_algorithm,
- &kernel_len)))
+ &kernel_len))) {
+ RSAPublicKeyFree(kernel_sign_key);
return error_code; /* AKA jump to recovery. */
+ }
/* Only continue if kernel data verification succeeds. */
kernel_ptr = (config_ptr +
FIELD_LEN(kernel_version) +
@@ -512,15 +514,18 @@
kernel_signature_len);
if ((error_code = VerifyKernelData(kernel_sign_key, kernel_ptr, kernel_len,
- kernel_sign_algorithm)))
+ kernel_sign_algorithm))) {
+ RSAPublicKeyFree(kernel_sign_key);
return error_code; /* AKA jump to recovery. */
+ }
+ RSAPublicKeyFree(kernel_sign_key);
return 0; /* Success! */
}
int VerifyKernelImage(const RSAPublicKey* firmware_key,
const KernelImage* image,
const int dev_mode) {
- RSAPublicKey* kernel_sign_key;
+ RSAPublicKey* kernel_sign_key = NULL;
uint8_t* header_digest = NULL;
uint8_t* config_digest = NULL;
uint8_t* kernel_digest = NULL;
@@ -610,6 +615,7 @@
}
verify_failure:
+ RSAPublicKeyFree(kernel_sign_key);
Free(kernel_digest);
Free(config_digest);
Free(header_digest);
@@ -622,7 +628,7 @@
int AddKernelKeySignature(KernelImage* image, const char* firmware_key_file) {
uint8_t* header_blob = NULL;
- uint8_t* signature;
+ uint8_t* signature = NULL;
int signature_len = siglen_map[image->firmware_sign_algorithm];
if (!image || !firmware_key_file)
return 0;
@@ -645,9 +651,9 @@
int AddKernelSignature(KernelImage* image,
const char* kernel_signing_key_file) {
- uint8_t* config_blob;
- uint8_t* config_signature;
- uint8_t* kernel_signature;
+ uint8_t* config_blob = NULL;
+ uint8_t* config_signature = NULL;
+ uint8_t* kernel_signature = NULL;
int signature_len = siglen_map[image->kernel_sign_algorithm];
config_blob = GetKernelConfigBlob(image);
@@ -659,6 +665,7 @@
Free(config_blob);
return 0;
}
+ Free(config_blob);
image->config_signature = (uint8_t*) Malloc(signature_len);
Memcpy(image->config_signature, config_signature, signature_len);
diff --git a/utils/signature_digest.c b/utils/signature_digest.c
index 23c47df..8f4c238 100644
--- a/utils/signature_digest.c
+++ b/utils/signature_digest.c
@@ -52,6 +52,7 @@
key_fp = fopen(key_file, "r");
if (!key_fp) {
fprintf(stderr, "SignatureBuf(): Couldn't open key file: %s\n", key_file);
+ Free(signature_digest);
return NULL;
}
if ((key = PEM_read_RSAPrivateKey(key_fp, NULL, NULL, NULL)))
@@ -67,6 +68,7 @@
RSA_PKCS1_PADDING)) /* Padding to use. */
fprintf(stderr, "SignatureBuf(): RSA_private_encrypt() failed.\n");
}
+ fclose(key_fp);
if (key)
RSA_free(key);
Free(signature_digest);