| 123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267 |
- From 5ba7e08c4188064286533e896189f75c3d60af57 Mon Sep 17 00:00:00 2001
- From: Stefano Babic <sbabic@denx.de>
- Date: Wed, 5 Apr 2017 17:23:44 +0200
- Subject: [PATCH v1 4/4] 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>
- ---
- tools/env/fw_env.c | 72 ++++++++++++++++++++++++++++++++++++++++++------------
- tools/env/fw_env.h | 17 ++++++++++---
- 2 files changed, 70 insertions(+), 19 deletions(-)
- diff --git a/tools/env/fw_env.c b/tools/env/fw_env.c
- index fc3f4ce..299e0c9 100644
- --- 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 cf346b3..04bb646 100644
- --- 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
- *
- --
- 2.7.4
|