[PATCH v2 1/1] arm: mvebu: Espressobin: Fix default env variables

[Derek LaHousse]

Default env variables on Espressobin boards are broken since
commit c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
$fdtfile env variable") as well as the 'env default -a' command.

The algorithm to find free space in the default_environment[] array
returns after the first env variable instead of the correct position of
the last variable, where there is allocated free space.

This causes that U-Boot board_late_init() function to overwrite a
portion of the default environment with $ethXaddr and $fdtfile
variables immediately after the first env variable and so it is
overwriting other variables.

This patch also adds an additional null byte to terminate the
environment array.

But U-Boot board_late_init() function do not fill this nul byte
explicitly. And because of that, U-Boot is later trying to interpret
remaining buffer as a continuation of variable list. Normally buffer
should be empty but due to the above issue, it contains garbage from
remaining env variables.

For example 'env default -a' command results in damaging variable
names. It was observed that scritaddr variable name was changed to
criptaddr (without leading 's').

This bug was reported and discussed on the Armbian forum:
https://forum.armbian.com/topic/19564-making-espressobin-v7-work-in-2022/?do=findComment&comment=138136

Fix these issues in two steps:

1) Change code which finds free space for dynamic env variables in
   default_environment[] array by jumping to the end of the variable
list instead of jumping after the first defined variable. [By Derek]

2) Add code which appends terminating nul byte as indication of the end
of the env list, after the last nul term env string. [By Pali]

Fixes: c4df0f6f315c ("arm: mvebu: Espressobin: Set default value for
$fdtfile env variable")
Signed-off-by: Derek LaHousse <derek at seaofdirac.org>
Signed-off-by: Pali Rohár <pali at kernel.org>
---

Changes in v2:
- Include Pali's end-of-array null
- Better tags
- More documentation of what is fixed

diff --git a/board/Marvell/mvebu_armada-37xx/board.c
b/board/Marvell/mvebu_armada-37xx/board.c
index c6ecc323bb99..44c72344e8be 100644
--- a/board/Marvell/mvebu_armada-37xx/board.c
+++ b/board/Marvell/mvebu_armada-37xx/board.c
@@ -99,9 +99,16 @@ int board_late_init(void)
 	if (!of_machine_is_compatible("globalscale,espressobin"))
 		return 0;
 
-	/* Find free buffer in default_environment[] for new variables
*/
-	while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
-	ptr += 2;
+	/*
+	 * Find free space for new variables in default_environment[]
array.
+	 * Free space is after the last variable, each variable is
termined
+	 * by nul byte and after the last variable is additional nul
byte.
+	 * Move ptr to the position where new variable can be filled.
+	 */
+	while (*ptr != '\0') {
+		do { ptr++; } while (*ptr != '\0');
+		ptr++;
+	}
 
 	/*
 	 * Ensure that 'env default -a' does not erase permanent MAC
addresses
@@ -145,6 +152,13 @@ int board_late_init(void)
 		strcpy(ptr, "fdtfile=marvell/armada-3720-espressobin-
emmc.dtb");
 	else
 		strcpy(ptr, "fdtfile=marvell/armada-3720-
espressobin.dtb");
+	ptr += strlen(ptr) + 1;
+
+	/*
+	 * After the last variable (which is nul term string) append
another nul
+	 * byte which terminates the list. So everything after ptr is
ignored.
+	 */
+	*ptr = '\0';
 
 	return 0;
 }