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>
8 years ago · 33f0086cb7
parent 00c234f38f
commit 33f0086cb7
2 changed files with 70 additions and 19 deletions
--- 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)
--- 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
 *