On 2023/11/8 22:20, Quentin Schulz wrote:

From: Quentin Schulz quentin.schulz@theobroma-systems.com

env_get_default suffers from a particular issue int that it can only return a value truncated to gd->env_buf (32) characters. This may be enough for most variables but it isn't for others, so let's allow users to provide a preallocated buffer to copy the value into instead, allowing for more control, though it'll still be truncated if the value size is bigger than the preallocated buffer.

Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

env/common.c | 8 ++++++++ include/env.h | 10 ++++++++++ 2 files changed, 18 insertions(+)

diff --git a/env/common.c b/env/common.c index eb1a9137953..307003099dd 100644 --- a/env/common.c +++ b/env/common.c @@ -254,6 +254,14 @@ char *env_get_default(const char *name) return NULL; }

+/*

• • Look up the variable from the default environment and store its value in buf
• */

+int env_get_default_into(const char *name, char *buf, unsigned int len) +{

• return env_get_from_linear(default_environment, name, buf, len);

+}

• void env_set_default(const char *s, int flags) { if (s) {

diff --git a/include/env.h b/include/env.h index 430c4fa94a4..9a406de3781 100644 --- a/include/env.h +++ b/include/env.h @@ -348,6 +348,16 @@ int env_import_redund(const char *buf1, int buf1_read_fail, */ char *env_get_default(const char *name);

+/**

• • env_get_default_into() - Look up a variable from the default environment and
• • copy its value in buf.
• • @name: Variable to look up
• • Return: actual length of the variable value excluding the terminating
• • NULL-byte, or -1 if the variable is not found
• */

+int env_get_default_into(const char *name, char *buf, unsigned int len);

• /* [re]set to the default environment */ void env_set_default(const char *s, int flags);

On 2023/11/8 22:20, Quentin Schulz wrote:

From: Quentin Schulz quentin.schulz@theobroma-systems.com

Those functions are useless in anything but U-Boot proper for now, so let's compile them out explicitly.

Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

board/theobroma-systems/puma_rk3399/puma-rk3399.c | 2 ++ 1 file changed, 2 insertions(+)

diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index 614a60ee8f9..1b7a39b0474 100644 --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c @@ -26,6 +26,7 @@ #include <power/regulator.h> #include <u-boot/sha256.h>

+#ifndef CONFIG_SPL_BUILD static void setup_iodomain(void) { const u32 GRF_IO_VSEL_GPIO4CD_SHIFT = 3; @@ -192,3 +193,4 @@ int misc_init_r(void)

return 0; } +#endif

On 2023/11/8 22:20, Quentin Schulz wrote:

From: Quentin Schulz quentin.schulz@theobroma-systems.com

U-Boot proper automatically modifies boot_targets to swap the order in which MMC storage media are used for standard boot based on which MMC storage medium was used to load U-Boot proper. This is however only done if the user has not changed it manually, therefore a check between the default and current value is done.

This used to work fine until the migration to standard boot where boot_targets value size in the default environment went above the 32 characters that env_get_default function can return, thus resulting in a truncated variable.

Therefore the check between default and current value would always fail.

By using the newly added env_get_default_into function, a buffer of the appropriate size can be allocated on the stack to get the whole value of boot_targets in the default environment and thus fixing the check.

Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

board/theobroma-systems/puma_rk3399/puma-rk3399.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/board/theobroma-systems/puma_rk3399/puma-rk3399.c b/board/theobroma-systems/puma_rk3399/puma-rk3399.c index 1b7a39b0474..df667fae5fc 100644 --- a/board/theobroma-systems/puma_rk3399/puma-rk3399.c +++ b/board/theobroma-systems/puma_rk3399/puma-rk3399.c @@ -54,7 +54,9 @@ static int setup_boottargets(void) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device");

• char *env_default, *env;

• char env_default[sizeof(BOOT_TARGETS)];

• char *env;

• int ret;

  if (!boot_device) { debug("%s: /chosen/u-boot,spl-boot-device not set\n",

@@ -63,7 +65,10 @@ static int setup_boottargets(void) } debug("%s: booted from %s\n", __func__, boot_device);

• env_default = env_get_default("boot_targets");

• ret = env_get_default_into("boot_targets", env_default,

• 		   sizeof(env_default));
  

• if (ret < 0)

• env_default[0] = '\0';
  

  env = env_get("boot_targets"); if (!env) { debug("%s: boot_targets does not exist\n", __func__);

On 2023/11/8 22:20, Quentin Schulz wrote:

From: Quentin Schulz quentin.schulz@theobroma-systems.com

U-Boot proper automatically modifies boot_targets to swap the order in which MMC storage media are used for standard boot based on which MMC storage medium was used to load U-Boot proper. This is however only done if the user has not changed it manually, therefore a check between the default and current value is done.

This used to work fine until the migration to standard boot where boot_targets value size in the default environment went above the 32 characters that env_get_default function can return, thus resulting in a truncated variable.

Therefore the check between default and current value would always fail.

By using the newly added env_get_default_into function, a buffer of the appropriate size can be allocated on the stack to get the whole value of boot_targets in the default environment and thus fixing the check.

Cc: Quentin Schulz foss+uboot@0leil.net Signed-off-by: Quentin Schulz quentin.schulz@theobroma-systems.com

Reviewed-by: Kever Yang kever.yang@rock-chips.com

Thanks, - Kever

---

board/theobroma-systems/ringneck_px30/ringneck-px30.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/board/theobroma-systems/ringneck_px30/ringneck-px30.c b/board/theobroma-systems/ringneck_px30/ringneck-px30.c index dd711cd05de..2bce8147eeb 100644 --- a/board/theobroma-systems/ringneck_px30/ringneck-px30.c +++ b/board/theobroma-systems/ringneck_px30/ringneck-px30.c @@ -39,7 +39,9 @@ static int setup_boottargets(void) { const char *boot_device = ofnode_read_chosen_string("u-boot,spl-boot-device");

• char *env_default, *env;

• char env_default[sizeof(BOOT_TARGETS)];

• char *env;

• int ret;

  if (!boot_device) { debug("%s: /chosen/u-boot,spl-boot-device not set\n",

@@ -48,7 +50,10 @@ static int setup_boottargets(void) } debug("%s: booted from %s\n", __func__, boot_device);

• env_default = env_get_default("boot_targets");

• ret = env_get_default_into("boot_targets", env_default,

• 		   sizeof(env_default));
  

• if (ret < 0)

• env_default[0] = '\0';
  

  env = env_get("boot_targets"); if (!env) { debug("%s: boot_targets does not exist\n", __func__);