diff mbox

[U-Boot] fix redundant environment for serial flash

Message ID 3E5A05AB39640047B326BEF8E2FEBBDB0AAB40F20A@mucse402.eu.infineon.com
State Accepted
Commit 2dc55d9ede62cd2af2a6d813115373a462c2b3dc
Headers show

Commit Message

thomas.langer@lantiq.com March 29, 2011, 12:35 p.m. UTC 
This patch fixes problems in the handling of redundant environment in env_sf.c

The major problem are double calls of free() on the allocated buffers,
which damages the internal data of malloc and crashes on next call.

In addition, the selection of the active environment had errors and compiler 
warnings, which are corrected by this patch.

Signed-off-by: Thomas Langer <thomas.langer@lantiq.com>
---
This patch is done and tested against the version 2010.12 and 
applies also cleanly against the u-boot/master.

Comments

Wolfgang Denk April 27, 2011, 10:46 p.m. UTC | #1 
Dear thomas.langer@lantiq.com,

In message <3E5A05AB39640047B326BEF8E2FEBBDB0AAB40F20A@mucse402.eu.infineon.com> you wrote:
> This patch fixes problems in the handling of redundant environment in env_sf.c
> 
> The major problem are double calls of free() on the allocated buffers,
> which damages the internal data of malloc and crashes on next call.
> 
> In addition, the selection of the active environment had errors and compiler 
> warnings, which are corrected by this patch.
> 
> Signed-off-by: Thomas Langer <thomas.langer@lantiq.com>
> ---
> This patch is done and tested against the version 2010.12 and 
> applies also cleanly against the u-boot/master.

Applied, thanks.

Best regards,

Wolfgang Denk
diff mbox

Patch

--- a/common/env_sf.c
+++ b/common/env_sf.c
@@ -59,7 +59,6 @@  DECLARE_GLOBAL_DATA_PTR;
 extern uchar default_environment[];
 
 char * env_name_spec = "SPI Flash";
-env_t *env_ptr;
 
 static struct spi_flash *env_flash;
 
@@ -79,7 +78,7 @@  int saveenv(void)
 	char	*saved_buffer = NULL;
 	u32	sector = 1;
 	int	ret;
-	char	flag = OBSOLETE_FLAG, new_flag = ACTIVE_FLAG;
+	char	flag = OBSOLETE_FLAG;
 
 	if (!env_flash) {
 		env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS,
@@ -159,7 +158,7 @@  int saveenv(void)
 
 	gd->env_valid = (gd->env_valid == 2 ? 1 : 2);
 
-	printf("Valid environment: %d\n", gd->env_valid);
+	printf("Valid environment: %d\n", (int)gd->env_valid);
 
  done:
 	if (saved_buffer)
@@ -174,25 +173,20 @@  void env_relocate_spec(void)
 	env_t *tmp_env1 = NULL;
 	env_t *tmp_env2 = NULL;
 	env_t *ep = NULL;
-	uchar flag1, flag2;
-	/* current_env is set only in case both areas are valid! */
-	int current_env = 0;
 
 	tmp_env1 = (env_t *)malloc(CONFIG_ENV_SIZE);
 	tmp_env2 = (env_t *)malloc(CONFIG_ENV_SIZE);
 
 	if (!tmp_env1 || !tmp_env2) {
-		free(tmp_env1);
-		free(tmp_env2);
 		set_default_env("!malloc() failed");
-		return;
+		goto out;
 	}
 
 	env_flash = spi_flash_probe(CONFIG_ENV_SPI_BUS, CONFIG_ENV_SPI_CS,
 			CONFIG_ENV_SPI_MAX_HZ, CONFIG_ENV_SPI_MODE);
 	if (!env_flash) {
 		set_default_env("!spi_flash_probe() failed");
-		return;
+		goto out;
 	}
 
 	ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET,
@@ -204,33 +198,30 @@  void env_relocate_spec(void)
 
 	if (crc32(0, tmp_env1->data, ENV_SIZE) == tmp_env1->crc)
 		crc1_ok = 1;
-	flag1 = tmp_env1->flags;
 
 	ret = spi_flash_read(env_flash, CONFIG_ENV_OFFSET_REDUND,
 				CONFIG_ENV_SIZE, tmp_env2);
 	if (!ret) {
 		if (crc32(0, tmp_env2->data, ENV_SIZE) == tmp_env2->crc)
 			crc2_ok = 1;
-		flag2 = tmp_env2->flags;
 	}
 
 	if (!crc1_ok && !crc2_ok) {
-		free(tmp_env1);
-		free(tmp_env2);
 		set_default_env("!bad CRC");
-		return;
+		goto err_read;
 	} else if (crc1_ok && !crc2_ok) {
 		gd->env_valid = 1;
-		ep = tmp_env1;
 	} else if (!crc1_ok && crc2_ok) {
+		gd->env_valid = 2;
+	} else if (tmp_env1->flags == ACTIVE_FLAG &&
+		   tmp_env2->flags == OBSOLETE_FLAG) {
 		gd->env_valid = 1;
-	} else if (flag1 == ACTIVE_FLAG && flag2 == OBSOLETE_FLAG) {
-		gd->env_valid = 1;
-	} else if (flag1 == OBSOLETE_FLAG && flag2 == ACTIVE_FLAG) {
+	} else if (tmp_env1->flags == OBSOLETE_FLAG &&
+		   tmp_env2->flags == ACTIVE_FLAG) {
 		gd->env_valid = 2;
-	} else if (flag1 == flag2) {
+	} else if (tmp_env1->flags == tmp_env2->flags) {
 		gd->env_valid = 2;
-	} else if (flag1 == 0xFF) {
+	} else if (tmp_env1->flags == 0xFF) {
 		gd->env_valid = 2;
 	} else {
 		/*
@@ -240,8 +231,6 @@  void env_relocate_spec(void)
 		gd->env_valid = 2;
 	}
 
-	free(env_ptr);
-
 	if (gd->env_valid == 1)
 		ep = tmp_env1;
 	else
@@ -257,10 +246,6 @@  err_read:
 	spi_flash_free(env_flash);
 	env_flash = NULL;
 out:
-	if (tmp_env1)
-		free(tmp_env1);
-	if (tmp_env2)
-		free(tmp_env2);
 	free(tmp_env1);
 	free(tmp_env2);