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 *