[U-Boot] [PATCH] env: common: load read-only variables after reset
U-Boot fails to load read-only variables from storage after a reset. It happens because the environment hash table prevents creating read-only variables unless the H_FORCE flag is passed.
In the following example, the variable "test" is set to read-only in the board header file (#define CONFIG_ENV_FLAGS_LIST_DEFAULT "test:sr"):
U-Boot> printenv .flags .flags=test:sr U-Boot> setenv -f test 1 U-Boot> printenv test test=1 U-Boot> savee Saving Environment to SPI Flash ... OK U-Boot> reset ... Loading Environment from SPI Flash... ## Error: Can't create "test" himport_r: can't insert "test=1" into hash table ... U-Boot> printenv test ## Error: "test" not defined
Pass the H_FORCE flag when importing the environment from storage.
Signed-off-by: Yaniv Levinsky yaniv.levinsky@compulab.co.il --- env/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/common.c b/env/common.c index 3317cef355..de8dd47e9b 100644 --- a/env/common.c +++ b/env/common.c @@ -119,7 +119,7 @@ int env_import(const char *buf, int check) } }
- if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0, + if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', H_FORCE, 0, 0, NULL)) { gd->flags |= GD_FLG_ENV_READY; return 0;
Dear Yaniv,
In message 20180727153444.5602-1-yaniv.levinsky@compulab.co.il you wrote:
U-Boot fails to load read-only variables from storage after a reset. It happens because the environment hash table prevents creating read-only variables unless the H_FORCE flag is passed.
This is NOT a good idea. "env import" should respect read-only settings in exactly the same way as "env set" does. Please keep in minf that the user my set a variable to read-only exactly just to prevent if from bein (accidentially) overwritten when importing an (not exactly know) set of environment settings. Your patch would kill any such protection.
IMO the correct approach would be to add a "-f" flag to "env import" and the, and ONLY then, also set the H_FORCE flag.
Naked-by: Wolfgang Denk wd@denx.de
Best regards,
Wolfgang Denk
Hello Wolfgang,
On 07/28/2018 03:56 PM, Wolfgang Denk wrote:
Dear Yaniv,
In message 20180727153444.5602-1-yaniv.levinsky@compulab.co.il you wrote:
U-Boot fails to load read-only variables from storage after a reset. It happens because the environment hash table prevents creating read-only variables unless the H_FORCE flag is passed.
This is NOT a good idea. "env import" should respect read-only settings in exactly the same way as "env set" does. Please keep in minf that the user my set a variable to read-only exactly just to prevent if from bein (accidentially) overwritten when importing an (not exactly know) set of environment settings. Your patch would kill any such protection.
IMO the correct approach would be to add a "-f" flag to "env import" and the, and ONLY then, also set the H_FORCE flag.
Naked-by: Wolfgang Denk wd@denx.de
Best regards,
Wolfgang Denk
Thank you for reviewing the patch.
The command "env import" should of course respect access restriction flags.
Confusingly, the function env_import() in env/common.c has nothing to do with the implementation of the "env import" command. This is done entirely by do_env_import() in cmd/nvedit.c.
To the best of my knowledge, the function env_import() is used only by storage devices to load the environment on boot. In this scenario, I think we do need to ignore access restriction flags. Otherwise, read-only variables won't load from storage with the rest of the environment.
Please correct me if I'm wrong.
Thanks, Yaniv
On 07/27/2018 06:34 PM, Yaniv Levinsky wrote:
U-Boot fails to load read-only variables from storage after a reset. It happens because the environment hash table prevents creating read-only variables unless the H_FORCE flag is passed.
In the following example, the variable "test" is set to read-only in the board header file (#define CONFIG_ENV_FLAGS_LIST_DEFAULT "test:sr"):
U-Boot> printenv .flags .flags=test:sr U-Boot> setenv -f test 1 U-Boot> printenv test test=1 U-Boot> savee Saving Environment to SPI Flash ... OK U-Boot> reset ... Loading Environment from SPI Flash... ## Error: Can't create "test" himport_r: can't insert "test=1" into hash table ... U-Boot> printenv test ## Error: "test" not defined
Pass the H_FORCE flag when importing the environment from storage.
Signed-off-by: Yaniv Levinsky yaniv.levinsky@compulab.co.il
env/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/env/common.c b/env/common.c index 3317cef355..de8dd47e9b 100644 --- a/env/common.c +++ b/env/common.c @@ -119,7 +119,7 @@ int env_import(const char *buf, int check) } }
- if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', 0, 0,
- if (himport_r(&env_htab, (char *)ep->data, ENV_SIZE, '\0', H_FORCE, 0, 0, NULL)) { gd->flags |= GD_FLG_ENV_READY; return 0;
Gentle ping.
participants (2)
-
Wolfgang Denk -
Yaniv Levinsky