cmd: nvedit: add whitelist option for env import
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
While the env export can take as parameters variables to be exported, env import does not have such a mechanism of variable selection.
Let's add a -w option that asks env import to look for the whitelisted_vars env variable for a space-separated list of variables that are whitelisted.
Every env variable present in env at addr and in whitelisted_vars env variable will override the value of the variable in the current env. All the remaining variables are left untouched.
One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
--- cmd/nvedit.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 4 deletions(-)
cmd/nvedit.c +62 -4
@@ -971,7 +971,7 @@ sep_err:#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the
@@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used when +* importing environment. The space-separated list of variables +* that should override the ones in current environment is stored +* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory
@@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, {ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */
@@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,case 'd' : del =1 ;break ; +case 'w' : + wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" ); +if (!str) { +puts ("## Error: whitelisted_vars is not set.\n" ); +return CMD_RET_USAGE ; + } + + tmp =malloc (sizeof (char ) * (strlen (str) +1 )); +strcpy (tmp, str); + + token =strchr (tmp,' ' ); +while (!token) { + wl_count++; + token =strchr (token +1 ,' ' ); + } + +strcpy (tmp, str); + + array =malloc (sizeof (char *) * wl_count); + wl_count =0 ; + + token =strtok (tmp," " ); +while (token) { +unsigned int size =strlen (token); + + array[wl_count] =malloc (sizeof (char ) * + (size +1 )); +strcpy (array[wl_count], token); + wl_count++; + token =strtok (NULL ," " ); + } + +free (tmp); +break ;default :return CMD_RET_USAGE ; }
@@ -1083,12 +1128,25 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, }if (himport_r (&env_htab, ptr, size, sep, del ?0 :H_NOCLEAR , - crlf_is_lf,0 ,NULL ) ==0 ) { + crlf_is_lf, wl ? wl_count :0 , wl ? array :NULL ) ==0 ) {pr_err ("Environment import failed: errno = %d\n" , errno); + +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +return 1 ; } gd->flags |=GD_FLG_ENV_READY ; +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +return 0 ; sep_err:
@@ -1212,7 +1270,7 @@ static char env_help_text[] =#endif #endif #if defined(CONFIG_CMD_IMPORTENV ) -"env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n" +"env import [-d] [-t [-r] | -b | -c] [-w] addr [size] - import environment\n" #endif "env print [-a | name ...] - print environment\n"#if defined(CONFIG_CMD_RUN )
Signature · 1 line
2.14.1
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
Quoted 69 lines · Quentin Schulz
While the
env exportcan take as parameters variables to be exported,env importdoes not have such a mechanism of variable selection.Let's add a
-woption that asksenv importto look for thewhitelisted_varsenv variable for a space-separated list of variables that are whitelisted.Every env variable present in env at
addrand inwhitelisted_varsenv variable will override the value of the variable in the current env. All the remaining variables are left untouched.One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
Signed-off-by Quentin Schulz <[email protected]>--- cmd/nvedit.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 4 deletions(-)cmd/nvedit.c +10
-2@@ -971,7 +971,7 @@ sep_err:#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used when +* importing environment. The space-separated list of variables +* that should override the ones in current environment is stored +* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, {ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1041
Quoted 8 lines
cmd/nvedit.c +5
-0@@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,case 'd' : del =1 ;break ; +case 'w' : + wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" );
I don't like how this is grabbing the list from the env. I think a comma-separated list should be a parameter following the '-w'.
Quoted 15 lines
cmd/nvedit.c +15
-0+if (!str) { +puts ("## Error: whitelisted_vars is not set.\n" ); +return CMD_RET_USAGE ; + } + + tmp =malloc (sizeof (char ) * (strlen (str) +1 )); +strcpy (tmp, str); + + token =strchr (tmp,' ' ); +while (!token) { + wl_count++; + token =strchr (token +1 ,' ' ); + } + +strcpy (tmp, str);
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1066
Quoted 10 lines
cmd/nvedit.c +10
-0+ + array =malloc (sizeof (char *) * wl_count); + wl_count =0 ; + + token =strtok (tmp," " ); +while (token) { +unsigned int size =strlen (token); + + array[wl_count] =malloc (sizeof (char ) * + (size +1 ));
Why malloc again for every string? Just use the buffer we already have.
Quoted 13 lines
cmd/nvedit.c +7
-0+strcpy (array[wl_count], token); + wl_count++; + token =strtok (NULL ," " ); + } + +free (tmp); +break ;default :return CMD_RET_USAGE ; }@@ -1083,12 +1128,25 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, }
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1138
Quoted 10 lines
cmd/nvedit.c +7
-1if (himport_r (&env_htab, ptr, size, sep, del ?0 :H_NOCLEAR , - crlf_is_lf,0 ,NULL ) ==0 ) { + crlf_is_lf, wl ? wl_count :0 , wl ? array :NULL ) ==0 ) {pr_err ("Environment import failed: errno = %d\n" , errno); + +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + }
It would be better to have a consolidated cleanup at the end of the function.
Quoted 30 lines
cmd/nvedit.c +8
-2+return 1 ; } gd->flags |=GD_FLG_ENV_READY ; +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +return 0 ; sep_err:@@ -1212,7 +1270,7 @@ static char env_help_text[] =#endif #endif #if defined(CONFIG_CMD_IMPORTENV ) -"env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n" +"env import [-d] [-t [-r] | -b | -c] [-w] addr [size] - import environment\n" #endif "env print [-a | name ...] - print environment\n"#if defined(CONFIG_CMD_RUN ) --2.14.1
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
Quoted 13 lines · Joe Hershberger
Quoted 11 lines · Quentin Schulz
While the
env exportcan take as parameters variables to be exported,env importdoes not have such a mechanism of variable selection.Let's add a
-woption that asksenv importto look for thewhitelisted_varsenv variable for a space-separated list of variables that are whitelisted.Every env variable present in env at
addrand inwhitelisted_varsenv variable will override the value of the variable in the current env. All the remaining variables are left untouched.
Quoted 8 lines
Quoted 8 lines
One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
Signed-off-by Quentin Schulz <[email protected]>--- cmd/nvedit.c | 66
++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
Quoted 8 lines
Quoted 8 lines
1 file changed, 62 insertions(+), 4 deletions(-)cmd/nvedit.c +0
-0@@ -971,7 +971,7 @@ sep_err:
Quoted 12 lines
cmd/nvedit.c sep_err: · line 985
Quoted 12 lines
cmd/nvedit.c +2
-1#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used
when
Quoted 1 line
cmd/nvedit.c sep_err: · line 986
Quoted 1 line
cmd/nvedit.c +1
-0+* importing environment. The space-separated list of
variables
Quoted 1 line
cmd/nvedit.c sep_err: · line 987
Quoted 1 line
cmd/nvedit.c +1
-0+* that should override the ones in current environment is
stored
Quoted 5 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int
Quoted 5 lines
cmd/nvedit.c +1
-0+* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, int
flag,
Quoted 16 lines
Quoted 16 lines
cmd/nvedit.c +4
-0{ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;
Quoted 7 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int
Quoted 7 lines
cmd/nvedit.c +1
-1while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */ @@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, int
flag,
Quoted 11 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1041
Quoted 8 lines
cmd/nvedit.c +5
-0case 'd' : del =1 ;break ; +case 'w' : + wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" );I don't like how this is grabbing the list from the env. I think a comma-separated list should be a parameter following the '-w'.
I posted a patch, independently in that vein as I hadn't spotted Quentin's patch:
http://patchwork.ozlabs.org/patch/891422/
I could clean it up to take parameters in an option.
Signature · 1 line
Alex Kiernan
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
Hi Alex,
Quoted 106 lines · Alex Kiernan
Quoted 13 lines · Joe Hershberger
Quoted 11 lines · Quentin Schulz
While the
env exportcan take as parameters variables to be exported,env importdoes not have such a mechanism of variable selection.Let's add a
-woption that asksenv importto look for thewhitelisted_varsenv variable for a space-separated list of variables that are whitelisted.Every env variable present in env at
addrand inwhitelisted_varsenv variable will override the value of the variable in the current env. All the remaining variables are left untouched.Quoted 8 lines
Quoted 8 lines
One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
Signed-off-by Quentin Schulz <[email protected]>--- cmd/nvedit.c | 66++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
Quoted 8 lines
Quoted 8 lines
1 file changed, 62 insertions(+), 4 deletions(-)cmd/nvedit.c +0
-0@@ -971,7 +971,7 @@ sep_err:Quoted 12 lines
cmd/nvedit.c sep_err: · line 985
Quoted 12 lines
cmd/nvedit.c +2
-1#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used when
Quoted 1 line
cmd/nvedit.c sep_err: · line 986
Quoted 1 line
cmd/nvedit.c +1
-0+* importing environment. The space-separated list of variables
Quoted 1 line
cmd/nvedit.c sep_err: · line 987
Quoted 1 line
cmd/nvedit.c +1
-0+* that should override the ones in current environment is stored
Quoted 5 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int
Quoted 5 lines
cmd/nvedit.c +1
-0+* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, intflag,
Quoted 16 lines
Quoted 16 lines
cmd/nvedit.c +4
-0{ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;Quoted 7 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int
Quoted 7 lines
cmd/nvedit.c +1
-1while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */ @@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, intflag,
Quoted 11 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1041
Quoted 8 lines
cmd/nvedit.c +5
-0case 'd' : del =1 ;break ; +case 'w' : + wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" );I don't like how this is grabbing the list from the env. I think a comma-separated list should be a parameter following the '-w'.
I posted a patch, independently in that vein as I hadn't spotted Quentin's patch:
http://patchwork.ozlabs.org/patch/891422/
I could clean it up to take parameters in an option.
--
Alex Kiernan
_______________________________________________
U-Boot mailing list
U-Boot at lists.denx.de
https://lists.denx.de/listinfo/u-boot
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
In reply to Joe HershbergerHi Joe,
Quoted 74 lines · Joe Hershberger
Quoted 72 lines · Quentin Schulz
While the
env exportcan take as parameters variables to be exported,env importdoes not have such a mechanism of variable selection.Let's add a
-woption that asksenv importto look for thewhitelisted_varsenv variable for a space-separated list of variables that are whitelisted.Every env variable present in env at
addrand inwhitelisted_varsenv variable will override the value of the variable in the current env. All the remaining variables are left untouched.One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
Signed-off-by Quentin Schulz <[email protected]>--- cmd/nvedit.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 4 deletions(-)cmd/nvedit.c +10
-2@@ -971,7 +971,7 @@ sep_err:#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used when +* importing environment. The space-separated list of variables +* that should override the ones in current environment is stored +* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, {ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */ @@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,case 'd' : del =1 ;break ;
Quoted 9 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1041
Quoted 5 lines
cmd/nvedit.c +5
-0+case 'w' : + wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" );I don't like how this is grabbing the list from the env. I think a comma-separated list should be a parameter following the '-w'.
Comma are valids in the name of environment variables if I'm not mistaken. (Did a quick setenv test,123 test and it saved test in test,123). So that's not an option.
Using a space separated list after "-w" is also not easy as I don't see from a quick thought how to differentiate the list of vars passed to -w from the addr and size parameters at the end of the function (how to know if size was passed or not?).
Is there any other character aside from a space that can be used to separate the list of variables?
Quoted 19 lines
Quoted 19 lines
cmd/nvedit.c +19
-0+if (!str) { +puts ("## Error: whitelisted_vars is not set.\n" ); +return CMD_RET_USAGE ; + } + + tmp =malloc (sizeof (char ) * (strlen (str) +1 )); +strcpy (tmp, str); + + token =strchr (tmp,' ' ); +while (!token) { + wl_count++; + token =strchr (token +1 ,' ' ); + } + +strcpy (tmp, str); + + array =malloc (sizeof (char *) * wl_count); + wl_count =0 ; +
Quoted 9 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1066
Quoted 6 lines
cmd/nvedit.c +6
-0+ token =strtok (tmp," " ); +while (token) { +unsigned int size =strlen (token); + + array[wl_count] =malloc (sizeof (char ) * + (size +1 ));Why malloc again for every string? Just use the buffer we already have.
Do an array[wl_count] = token instead?
I'm not sure this will work with this patch's code.
From what I understand from strtok[1], token is a pointer to somewhere in the string tmp. So we're screwed once tmp is freed aren't we?
I could move the free(tmp) to the final goto I'm talking about below though and get away with it.
Quoted 13 lines
Quoted 13 lines
cmd/nvedit.c +7
-0+strcpy (array[wl_count], token); + wl_count++; + token =strtok (NULL ," " ); + } + +free (tmp); +break ;default :return CMD_RET_USAGE ; }@@ -1083,12 +1128,25 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, }
Quoted 13 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1138
Quoted 10 lines
cmd/nvedit.c +7
-1if (himport_r (&env_htab, ptr, size, sep, del ?0 :H_NOCLEAR , - crlf_is_lf,0 ,NULL ) ==0 ) { + crlf_is_lf, wl ? wl_count :0 , wl ? array :NULL ) ==0 ) {pr_err ("Environment import failed: errno = %d\n" , errno); + +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + }It would be better to have a consolidated cleanup at the end of the function.
Sure. I'll add a goto with appropriate return value.
Thanks,
Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/7c9283a0/attachment.sig>
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
In reply to Quentin SchulzHi Quentin,
Quoted 16 lines · Quentin Schulz
While the
env exportcan take as parameters variables to be exported,env importdoes not have such a mechanism of variable selection.Let's add a
-woption that asksenv importto look for thewhitelisted_varsenv variable for a space-separated list of variables that are whitelisted.Every env variable present in env at
addrand inwhitelisted_varsenv variable will override the value of the variable in the current env. All the remaining variables are left untouched.One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
Quoted 4 lines
Signed-off-by Quentin Schulz <[email protected]>--- cmd/nvedit.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 4 deletions(-)
Is this something you could write a sandbox test for? I think it would be useful.
Quoted 53 lines
cmd/nvedit.c +11
-2@@ -971,7 +971,7 @@ sep_err:#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used when +* importing environment. The space-separated list of variables +* that should override the ones in current environment is stored +* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, {ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */ @@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,case 'd' : del =1 ;break ; +case 'w' :
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1047
Quoted 10 lines
cmd/nvedit.c +10
-0+ wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" ); +if (!str) { +puts ("## Error: whitelisted_vars is not set.\n" ); +return CMD_RET_USAGE ; + } + + tmp =malloc (sizeof (char ) * (strlen (str) +1 ));
strdup() ?
Also should check for NULL
Quoted 9 lines
cmd/nvedit.c +9
-0+strcpy (tmp, str); + + token =strchr (tmp,' ' ); +while (!token) { + wl_count++; + token =strchr (token +1 ,' ' ); + } + +strcpy (tmp, str);
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1066
Quoted 10 lines
cmd/nvedit.c +10
-0+ + array =malloc (sizeof (char *) * wl_count); + wl_count =0 ; + + token =strtok (tmp," " ); +while (token) { +unsigned int size =strlen (token); + + array[wl_count] =malloc (sizeof (char ) * + (size +1 ));
Check for NULL
Quoted 24 lines
cmd/nvedit.c +15
-1+strcpy (array[wl_count], token); + wl_count++; + token =strtok (NULL ," " ); + } + +free (tmp); +break ;default :return CMD_RET_USAGE ; }@@ -1083,12 +1128,25 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, }if (himport_r (&env_htab, ptr, size, sep, del ?0 :H_NOCLEAR , - crlf_is_lf,0 ,NULL ) ==0 ) { + crlf_is_lf, wl ? wl_count :0 , wl ? array :NULL ) ==0 ) {pr_err ("Environment import failed: errno = %d\n" , errno); + +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1149
Quoted 10 lines
cmd/nvedit.c +6
-0return 1 ; } gd->flags |=GD_FLG_ENV_READY ; +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +
Is there a way to avoid repeating that code twice, perhaps goto or function call.
Quoted 13 lines
cmd/nvedit.c +1
-2return 0 ; sep_err:@@ -1212,7 +1270,7 @@ static char env_help_text[] =#endif #endif #if defined(CONFIG_CMD_IMPORTENV ) -"env import [-d] [-t [-r] | -b | -c] addr [size] - import environment\n" +"env import [-d] [-t [-r] | -b | -c] [-w] addr [size] - import environment\n" #endif "env print [-a | name ...] - print environment\n"#if defined(CONFIG_CMD_RUN ) --
Quoted 2 lines
2.14.1
Regards,
Simon
PATCH RESEND 1/1 cmd: nvedit: add whitelist option for env import
Hi Simon,
Quoted 19 lines · Simon Glass
Hi Quentin,
Quoted 16 lines · Quentin Schulz
While the
env exportcan take as parameters variables to be exported,env importdoes not have such a mechanism of variable selection.Let's add a
-woption that asksenv importto look for thewhitelisted_varsenv variable for a space-separated list of variables that are whitelisted.Every env variable present in env at
addrand inwhitelisted_varsenv variable will override the value of the variable in the current env. All the remaining variables are left untouched.One of its use case could be to load a secure environment from the signed U-Boot binary and load only a handful of variables from an other, unsecure, environment without completely losing control of U-Boot.
Quoted 8 lines
Quoted 4 lines
Signed-off-by Quentin Schulz <[email protected]>--- cmd/nvedit.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 4 deletions(-)Is this something you could write a sandbox test for? I think it would be useful.
I don't know about sandbox tests but I don't see why not.
Quoted 57 lines
Quoted 57 lines
cmd/nvedit.c +15
-2@@ -971,7 +971,7 @@ sep_err:#ifdef CONFIG_CMD_IMPORTENV /* -* env import [-d] [-t [-r] | -b | -c] addr [size] +* env import [-d] [-t [-r] | -b | -c] [-w] addr [size] * -d: delete existing environment before importing; * otherwise overwrite / append to existing definitions * -t: assume text format; either "size" must be given or the @@ -982,6 +982,10 @@ sep_err:* for line endings. Only effective in addition to -t. * -b: assume binary format ('\0' separated, "\0\0" terminated) * -c: assume checksum protected environment format +* -w: specify that whitelisting of variables should be used when +* importing environment. The space-separated list of variables +* that should override the ones in current environment is stored +* in `whitelisted_vars`. * addr: memory address to read from * size: length of input data; if missing, proper '\0' * termination is mandatory @@ -991,17 +995,21 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, {ulong addr;char *cmd, *ptr; +char **array =NULL ;char sep ='\n' ;int chk =0 ;int fmt =0 ;int del =0 ;int crlf_is_lf =0 ; +int wl =0 ; +int wl_count =0 ; +unsigned int i;size_t size; cmd = *argv;while (--argc >0 && **++argv =='-' ) { -char *arg = *argv; +char *arg = *argv, *str, *token, *tmp;while (*++arg) {switch (*arg) {case 'b' :/* raw binary format */ @@ -1026,6 +1034,43 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag,case 'd' : del =1 ;break ; +case 'w' : + wl =1 ; + wl_count =1 ; + + str =env_get ("whitelisted_vars" );
Quoted 9 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1047
Quoted 6 lines
cmd/nvedit.c +6
-0+if (!str) { +puts ("## Error: whitelisted_vars is not set.\n" ); +return CMD_RET_USAGE ; + } + + tmp =malloc (sizeof (char ) * (strlen (str) +1 ));
strdup()?
Thanks for the discovery :)
Quoted 2 lines
Also should check for NULL
Indeed.
Quoted 50 lines
Quoted 9 lines
cmd/nvedit.c +9
-0+strcpy (tmp, str); + + token =strchr (tmp,' ' ); +while (!token) { + wl_count++; + token =strchr (token +1 ,' ' ); + } + +strcpy (tmp, str);cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1066
Quoted 10 lines
cmd/nvedit.c +10
-0+ + array =malloc (sizeof (char *) * wl_count); + wl_count =0 ; + + token =strtok (tmp," " ); +while (token) { +unsigned int size =strlen (token); + + array[wl_count] =malloc (sizeof (char ) * + (size +1 ));Check for NULL
Quoted 28 lines
cmd/nvedit.c +15
-1+strcpy (array[wl_count], token); + wl_count++; + token =strtok (NULL ," " ); + } + +free (tmp); +break ;default :return CMD_RET_USAGE ; }@@ -1083,12 +1128,25 @@ static int do_env_import(cmd_tbl_t *cmdtp, int flag, }if (himport_r (&env_htab, ptr, size, sep, del ?0 :H_NOCLEAR , - crlf_is_lf,0 ,NULL ) ==0 ) { + crlf_is_lf, wl ? wl_count :0 , wl ? array :NULL ) ==0 ) {pr_err ("Environment import failed: errno = %d\n" , errno); + +if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +return 1 ; } gd->flags |=GD_FLG_ENV_READY ;
Quoted 10 lines
cmd/nvedit.c static int do_env_import(cmd_tbl_t *cmdtp, int flag, · line 1139
Quoted 6 lines
cmd/nvedit.c +6
-0+if (wl) { +for (i =0 ; i < wl_count; i++) +free (array[i]); +free (array); + } +Is there a way to avoid repeating that code twice, perhaps goto or function call.
As told to Joe, I think I could use a goto and keep the appropriate return value and that'd be cleaner indeed.
Thanks,
Quentin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: not available
URL: <http://lists.denx.de/pipermail/u-boot/attachments/20180516/cfd2fe0e/attachment.sig>