Hi!

On 12/4/22 11:39, Pali Rohár wrote:
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).

Yes, please update the commit subject here.

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

Yes.

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.

Understood.

Signed-off-by: Derek LaHousse <de...@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?

I would prefer to get Pali's corrected version included. Could you
please prepare a v2 patch with this update and also with the added
or changed patch subject.

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

Yes, please do.

Thanks,
Stefan

Reply via email to