[PATCH] env: fat: Avoid writing to read-only location

[Andre Przywara]

The env_fat_get_dev_part() function mostly returns a fixed string, set
via some Kconfig variable. However when the first character is a colon,
that means that the boot device number is determined at runtime, and
patched in. This requires altering the string.

So far this was done via some ugly and actually illegal direct write to
the .rodata string storage. We got away with this because U-Boot maps
everything as read/write/execute so far.

A proposed patch set actually enforces read-only (and no-execute)
permissions in the page tables, so this routine now causes an exception:
=======================
Loading Environment from FAT... "Synchronous Abort" handler, esr 0x9600004f, far 0xfffb7d4c
elr: 000000004a054228 lr : 000000004a05421c (reloc)
elr: 00000000fff7c228 lr : 00000000fff7c21c
.....
=======================

Rewrite the routine to do away with the dodgy string manipulation,
instead allocate the string in the r/w .data section, where we can
safely manipulate it.

Signed-off-by: Andre Przywara <andre.przywara at arm.com>
---
Hi,

found when naively testing Ilias' series on some Allwinner board.
I am sure there are better ways to implement this (probably as many as
there are developers), but this works for me.
This does away with the optimisation of doing the manipulation only
once, but I am not sure that's really worthwhile?

Cheers,
Andre

 env/fat.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/env/fat.c b/env/fat.c
index b04b1d9c315..1ad301eaaff 100644
--- a/env/fat.c
+++ b/env/fat.c
@@ -41,14 +41,12 @@ __weak const char *env_fat_get_intf(void)
 __weak char *env_fat_get_dev_part(void)
 {
 #ifdef CONFIG_MMC
-	static char *part_str;
-
-	if (!part_str) {
-		part_str = CONFIG_ENV_FAT_DEVICE_AND_PART;
-		if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc") && part_str[0] == ':') {
-			part_str = "0" CONFIG_ENV_FAT_DEVICE_AND_PART;
-			part_str[0] += mmc_get_env_dev();
-		}
+	/* reserve one more char for the manipulation below */
+	static char part_str[] = CONFIG_ENV_FAT_DEVICE_AND_PART "\0";
+
+	if (!strcmp(CONFIG_ENV_FAT_INTERFACE, "mmc") && part_str[0] == ':') {
+		part_str[0] = '0' + mmc_get_env_dev();
+		strcpy(&part_str[1], CONFIG_ENV_FAT_DEVICE_AND_PART);
 	}
 
 	return part_str;
-- 
2.25.1