env: fix memory leak in fw_env routines

fw_env_open allocates buffers to store the environment, but these
buffers are never freed. This becomes quite nasty using the fw_ tools as
library, because each access to the environment (even just reading a
variable) generates a memory leak equal to the size of the environment.

Fix this renaming fw_env_close() as fw_env_flush(), because the function
really flushes the environment from RAM to storage, and add a
fw_env_close function to free the allocated resources.

Signed-off-by: Stefano Babic <sbabic@denx.de>

diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
index fc3f4ce47fab5551970daa8d410941a6f8aee1f5..299e0c9608bb2bc3b9e0298929726c14a337f96c 100644 (file)
--- a/tools/env/fw_env.c
+++ b/tools/env/fw_env.c
@@ -278,6 +278,7 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
 
                        printf ("%s\n", env);
                }
+               fw_env_close(opts);
                return 0;
        }
 
@@ -300,10 +301,12 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts)
                printf("%s=%s\n", name, val);
        }
 
+       fw_env_close(opts);
+
        return rc;
 }
 
-int fw_env_close(struct env_opts *opts)
+int fw_env_flush(struct env_opts *opts)
 {
        int ret;
 
@@ -472,6 +475,7 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts)
        char *name, **valv;
        char *value = NULL;
        int valc;
+       int ret;
 
        if (!opts)
                opts = &default_opts;
@@ -491,8 +495,10 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts)
        valv = argv + 1;
        valc = argc - 1;
 
-       if (env_flags_validate_env_set_params(name, valv, valc) < 0)
+       if (env_flags_validate_env_set_params(name, valv, valc) < 0) {
+               fw_env_close(opts);
                return -1;
+       }
 
        len = 0;
        for (i = 0; i < valc; ++i) {
@@ -518,7 +524,10 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts)
 
        free(value);
 
-       return fw_env_close(opts);
+       ret = fw_env_flush(opts);
+       fw_env_close(opts);
+
+       return ret;
 }
 
 /*
@@ -639,7 +648,9 @@ int fw_parse_script(char *fname, struct env_opts *opts)
        if (strcmp(fname, "-") != 0)
                fclose(fp);
 
-       ret |= fw_env_close(opts);
+       ret |= fw_env_flush(opts);
+
+       fw_env_close(opts);
 
        return ret;
 }
@@ -1105,11 +1116,11 @@ int fw_env_open(struct env_opts *opts)
 {
        int crc0, crc0_ok;
        unsigned char flag0;
-       void *addr0;
+       void *addr0 = NULL;
 
        int crc1, crc1_ok;
        unsigned char flag1;
-       void *addr1;
+       void *addr1 = NULL;
 
        int ret;
 
@@ -1120,14 +1131,15 @@ int fw_env_open(struct env_opts *opts)
                opts = &default_opts;
 
        if (parse_config(opts))         /* should fill envdevices */
-               return -1;
+               return -EINVAL;
 
        addr0 = calloc(1, CUR_ENVSIZE);
        if (addr0 == NULL) {
                fprintf(stderr,
                        "Not enough memory for environment (%ld bytes)\n",
                        CUR_ENVSIZE);
-               return -1;
+               ret = -ENOMEM;
+               goto open_cleanup;
        }
 
        /* read environment from FLASH to local buffer */
@@ -1146,8 +1158,10 @@ int fw_env_open(struct env_opts *opts)
        }
 
        dev_current = 0;
-       if (flash_io (O_RDONLY))
-               return -1;
+       if (flash_io(O_RDONLY)) {
+               ret = -EIO;
+               goto open_cleanup;
+       }
 
        crc0 = crc32 (0, (uint8_t *) environment.data, ENV_SIZE);
 
@@ -1155,7 +1169,7 @@ int fw_env_open(struct env_opts *opts)
                ret = env_aes_cbc_crypt(environment.data, 0,
                                        opts->aes_key);
                if (ret)
-                       return ret;
+                       goto open_cleanup;
        }
 
        crc0_ok = (crc0 == *environment.crc);
@@ -1174,7 +1188,8 @@ int fw_env_open(struct env_opts *opts)
                        fprintf(stderr,
                                "Not enough memory for environment (%ld bytes)\n",
                                CUR_ENVSIZE);
-                       return -1;
+                       ret = -ENOMEM;
+                       goto open_cleanup;
                }
                redundant = addr1;
 
@@ -1183,8 +1198,10 @@ int fw_env_open(struct env_opts *opts)
                 * other pointers in environment still point inside addr0
                 */
                environment.image = addr1;
-               if (flash_io (O_RDONLY))
-                       return -1;
+               if (flash_io(O_RDONLY)) {
+                       ret = -EIO;
+                       goto open_cleanup;
+               }
 
                /* Check flag scheme compatibility */
                if (DEVTYPE(dev_current) == MTD_NORFLASH &&
@@ -1204,7 +1221,8 @@ int fw_env_open(struct env_opts *opts)
                        environment.flag_scheme = FLAG_INCREMENTAL;
                } else {
                        fprintf (stderr, "Incompatible flash types!\n");
-                       return -1;
+                       ret = -EINVAL;
+                       goto open_cleanup;
                }
 
                crc1 = crc32 (0, (uint8_t *) redundant->data, ENV_SIZE);
@@ -1213,7 +1231,7 @@ int fw_env_open(struct env_opts *opts)
                        ret = env_aes_cbc_crypt(redundant->data, 0,
                                                opts->aes_key);
                        if (ret)
-                               return ret;
+                               goto open_cleanup;
                }
 
                crc1_ok = (crc1 == redundant->crc);
@@ -1285,6 +1303,28 @@ int fw_env_open(struct env_opts *opts)
 #endif
        }
        return 0;
+
+open_cleanup:
+       if (addr0)
+               free(addr0);
+
+       if (addr1)
+               free(addr0);
+
+       return ret;
+}
+
+/*
+ * Simply free allocated buffer with environment
+ */
+int fw_env_close(struct env_opts *opts)
+{
+       if (environment.image)
+               free(environment.image);
+
+       environment.image = NULL;
+
+       return 0;
 }
 
 static int check_device_config(int dev)

diff --git a/tools/env/fw_env.h b/tools/env/fw_env.h
index cf346b3bab559abd81d16edf270b4af9dec5e892..04bb64602b2561dfbb9982faa128eb2327296407 100644 (file)
--- a/tools/env/fw_env.h
+++ b/tools/env/fw_env.h
@@ -53,7 +53,7 @@ int fw_printenv(int argc, char *argv[], int value_only, struct env_opts *opts);
  * @opts: how to retrieve environment from flash, defaults are used if NULL
  *
  * Description:
- *  Uses fw_env_open, fw_env_write, fw_env_close
+ *  Uses fw_env_open, fw_env_write, fw_env_flush
  *
  * Return:
  *  0 on success, -1 on failure (modifies errno)
@@ -70,7 +70,7 @@ int fw_setenv(int argc, char *argv[], struct env_opts *opts);
  * @opts: encryption key, configuration file, defaults are used if NULL
  *
  * Description:
- *  Uses fw_env_open, fw_env_write, fw_env_close
+ *  Uses fw_env_open, fw_env_write, fw_env_flush
  *
  * Return:
  *  0 success, -1 on failure (modifies errno)
@@ -138,7 +138,17 @@ char *fw_getenv(char *name);
 int fw_env_write(char *name, char *value);
 
 /**
- * fw_env_close - write the environment from RAM cache back to flash
+ * fw_env_flush - write the environment from RAM cache back to flash
+ *
+ * @opts: encryption key, configuration file, defaults are used if NULL
+ *
+ * Return:
+ *  0 on success, -1 on failure (modifies errno)
+ */
+int fw_env_flush(struct env_opts *opts);
+
+/**
+ * fw_env_close - free allocated structure and close env
  *
  * @opts: encryption key, configuration file, defaults are used if NULL
  *
@@ -147,6 +157,7 @@ int fw_env_write(char *name, char *value);
  */
 int fw_env_close(struct env_opts *opts);
 
+
 /**
  * fw_env_version - return the current version of the library
  *