arm: mvebu: Espressobin: Fix default env variables
authorDerek LaHousse <derek@seaofdirac.org>
Mon, 12 Dec 2022 06:34:17 +0000 (07:34 +0100)
committerStefan Roese <sr@denx.de>
Mon, 12 Dec 2022 06:36:04 +0000 (07:36 +0100)
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@seaofdirac.org>
Signed-off-by: Pali Rohár <pali@kernel.org>
Signed-off-by: Stefan Roese <sr@denx.de>
board/Marvell/mvebu_armada-37xx/board.c

index c6ecc32..44c7234 100644 (file)
@@ -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;
 }