[PATCH 1/1] mvebu: fix end-of-array check

[Pali Rohár]

Hello!

I would suggest to change description of the patch from

  "mvebu: fix end-of-array check"

to something like:

  "arm: mvebu: Espressobin: fix end-of-array check in env"

as it affects only Espressobin boards (not other mvebu).

Stefan, please look below as this issue/fix is important.

On Wednesday 30 November 2022 13:33:40 Derek LaHousse wrote:
> Properly seek the end of default_environment variables.
> 
> The current algorithm overwrites from the second variable.  This
> replacement finds the end of the array of strings.
> 
> Stomped variables include "board", "soc", "loadaddr".  These can be
> seen on a "env default -a" after patch, but they are not seen with a
> version before the patch.

This is a real issue which I introduced in the past. So it some fix for
this issue should be pulled into the v2023.01 release.

> Signed-off-by: Derek LaHousse <derek at seaofdirac.org>
> ---
>  board/Marvell/mvebu_armada-37xx/board.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/board/Marvell/mvebu_armada-37xx/board.c
> b/board/Marvell/mvebu_armada-37xx/board.c
> index c6ecc323bb..ac29ac5b95 100644
> --- a/board/Marvell/mvebu_armada-37xx/board.c
> +++ b/board/Marvell/mvebu_armada-37xx/board.c
> @@ -100,8 +100,11 @@ int board_late_init(void)
>  		return 0;
>  
>  	/* Find free buffer in default_environment[] for new variables
> */
> -	while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
> -	ptr += 2;
> +	if (*ptr != '\0') { // Defending against empty default env
> +		while ((i = strlen(ptr)) != 0) {
> +			ptr += i + 1;
> +		}
> +	}

If I'm looking at the outer-if condition correctly then it is not
needed. strlen("") returns zero and so inner-while loop stops
immediately.

My proposed fix for this issue was just changing correct while loop
check to ensure that ptr is set after the _last_ variable.

-	while (*ptr != '\0' && *(ptr+1) != '\0') ptr++;
-	ptr += 2;
+	while (*ptr != '\0' || *(ptr+1) != '\0') ptr++;
+	ptr++;

Both changes should be equivalent, but I'm not sure which one is more
readable. The original issue was introduced really by non-readable
code...

Stefan, do you have a preference which one fix is better / more
readable?

It would be really a good idea if you try to check if any of those
proposed fixes/patches are now really correct.

>  
>  	/*
>  	 * Ensure that 'env default -a' does not erase permanent MAC
> addresses
> -- 
> 2.30.2
> 
> 
>