cert_create: fix memory leak bug caused by key container overwrite
authorMasahiro Yamada <yamada.masahiro@socionext.com>
Mon, 6 Feb 2017 12:15:01 +0000 (21:15 +0900)
committerMasahiro Yamada <yamada.masahiro@socionext.com>
Sat, 11 Feb 2017 03:59:16 +0000 (12:59 +0900)
In the current code, both key_load() and key_create() call key_new()
to allocate a key container (and they do not free it even if they
fail).  If a specific key is not given by the command option,
key_load() fails, then key_create() is called.  At this point, the
key container that has been allocated in key_load() is still alive,
and it is overwritten by a new key container created by key_create().

Move the key_new() call to the main() function to make sure it is
called just once for each descriptor.

While we are here, let's fix one more bug; the error handling code
  ERROR("Malloc error while loading '%s'\n", keys[i].fn);
is wrong because keys[i].fn is NULL pointer unless a specific key is
given by the command option.  This code could be run in either case.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
tools/cert_create/include/key.h
tools/cert_create/src/key.c
tools/cert_create/src/main.c

index f60997f0fb9b13738eabb65f15e637dacd9d404e..433f72cef1b2c055bd3c7a5f76e722b65452d5a2 100644 (file)
@@ -73,6 +73,7 @@ typedef struct key_s {
 /* Exported API */
 int key_init(void);
 key_t *key_get_by_opt(const char *opt);
+int key_new(key_t *key);
 int key_create(key_t *key, int type);
 int key_load(key_t *key, unsigned int *err_code);
 int key_store(key_t *key);
index a7ee75969cbba76a1d677ada2fa196c4b63baaf4..47c152c72ca87cd019f8d3bb4701ef5f5c6b6836 100644 (file)
@@ -49,7 +49,7 @@
 /*
  * Create a new key container
  */
-static int key_new(key_t *key)
+int key_new(key_t *key)
 {
        /* Create key pair container */
        key->key = EVP_PKEY_new();
@@ -123,11 +123,6 @@ int key_create(key_t *key, int type)
                return 0;
        }
 
-       /* Create OpenSSL key container */
-       if (!key_new(key)) {
-               return 0;
-       }
-
        if (key_create_fn[type]) {
                return key_create_fn[type](key);
        }
@@ -140,12 +135,6 @@ int key_load(key_t *key, unsigned int *err_code)
        FILE *fp = NULL;
        EVP_PKEY *k = NULL;
 
-       /* Create OpenSSL key container */
-       if (!key_new(key)) {
-               *err_code = KEY_ERR_MALLOC;
-               return 0;
-       }
-
        if (key->fn) {
                /* Load key from file */
                fp = fopen(key->fn, "r");
index c58f41deab48db1a3599fa691287d5a6ec4f4a7a..dac9e57c0d1955d90f04fc264f24cad2d5c99546 100644 (file)
@@ -367,6 +367,11 @@ int main(int argc, char *argv[])
 
        /* Load private keys from files (or generate new ones) */
        for (i = 0 ; i < num_keys ; i++) {
+               if (!key_new(&keys[i])) {
+                       ERROR("Failed to allocate key container\n");
+                       exit(1);
+               }
+
                /* First try to load the key from disk */
                if (key_load(&keys[i], &err_code)) {
                        /* Key loaded successfully */
@@ -374,11 +379,7 @@ int main(int argc, char *argv[])
                }
 
                /* Key not loaded. Check the error code */
-               if (err_code == KEY_ERR_MALLOC) {
-                       /* Cannot allocate memory. Abort. */
-                       ERROR("Malloc error while loading '%s'\n", keys[i].fn);
-                       exit(1);
-               } else if (err_code == KEY_ERR_LOAD) {
+               if (err_code == KEY_ERR_LOAD) {
                        /* File exists, but it does not contain a valid private
                         * key. Abort. */
                        ERROR("Error loading '%s'\n", keys[i].fn);