[RFC PATCH v2 0/1] cmd: nvedit: Forbid environment key to be empty.

[RFC PATCH v2 0/1] cmd: nvedit: Forbid environment key to be empty.

[Francis Laniel]

Hi.


First of all, I hope you are fine and the same for your relatives.

In this patch, I modified the setenv command to decline empty variable
name.
Indeed, it was strangely possible to give the following to this command:
setenv '' foo
Which results in the following belonging to environment:
=foo
And which in turns leads to problem while rebooting:
Loading Environment from Flash... Cannot import environment: errno = 22
*** Warning - import failed, using default environment
This error message is due to varname being empty and was added in [1].

With this patch, executing the above command will lead to the following being
printed:
## Error: variable name cannot be empty
And the environment will not be changed.

Here is the diffstat for this patch:
Francis Laniel (1):
  cmd: nvedit: Forbid key to be empty.

 cmd/nvedit.c | 5 +++++
 1 file changed, 5 insertions(+)

Change since:
  v1:
    * Replace call to strlen by test if variable name is '\0'.
    * Modify error message printed.

If you see any way to improve the patch, feel free to share it.


Best regards.

---
[1] https://u-boot.denx.narkive.com/P4aKxVFu/patch-env-don-t-add-an-empty-key-to-the-env-hashtable

--
2.25.1

[RFC PATCH v2 1/1] cmd: nvedit: Forbid key to be empty.

[Francis Laniel]

Before this patch, it was possible to do the following using setenv:
setenv '' foo
Then, on next reboot, U-Boot will not be able to parse environment due to it
having:
=foo

Now, if the above command is given, an error message is thrown and environment
is not modified.

Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
---
 cmd/nvedit.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/cmd/nvedit.c b/cmd/nvedit.c
index d14ba10cef..6f99a85a9c 100644
--- a/cmd/nvedit.c
+++ b/cmd/nvedit.c
@@ -262,6 +262,11 @@ static int _do_env_set(int flag, int argc, char *const argv[], int env_flag)
 		return 1;
 	}
 
+	if (*name == '\0') {
+		printf("## Error: variable name must no be empty\n");
+		return 1;
+	}
+
 	env_id++;
 
 	/* Delete only ? */
-- 
2.25.1

Re: [RFC PATCH v2 1/1] cmd: nvedit: Forbid key to be empty.

[Wolfgang Denk]

Dear Francis Laniel,

In message <20210629161859.298630-2-francis.laniel@amarulasolutions.com> you wrote:
> Before this patch, it was possible to do the following using setenv:
> setenv '' foo
> Then, on next reboot, U-Boot will not be able to parse environment due to it
> having:
> =foo
>
> Now, if the above command is given, an error message is thrown and environment
> is not modified.
>
> Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> ---
>  cmd/nvedit.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> index d14ba10cef..6f99a85a9c 100644
> --- a/cmd/nvedit.c
> +++ b/cmd/nvedit.c
> @@ -262,6 +262,11 @@ static int _do_env_set(int flag, int argc, char *const argv[], int env_flag)
>  		return 1;
>  	}
>  
> +	if (*name == '\0') {
> +		printf("## Error: variable name must no be empty\n");
> +		return 1;
> +	}
> +
>  	env_id++;
>  
>  	/* Delete only ? */

Reviewed-by: Wolfgang Denk <wd@denx.de>


Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de
"I can call spirits from the vasty deep."
"Why so can I, or so can any man; but will they come when you do call
for them?"          - Shakespeare, 1 King Henry IV, Act III, Scene I.

Re: [RFC PATCH v2 1/1] cmd: nvedit: Forbid key to be empty.

[Francis Laniel]

Hi.


Le mercredi 30 juin 2021, 09:38:16 CEST Wolfgang Denk a écrit :
> Dear Francis Laniel,
> 
> In message <20210629161859.298630-2-francis.laniel@amarulasolutions.com> you 
wrote:
> > Before this patch, it was possible to do the following using setenv:
> > setenv '' foo
> > Then, on next reboot, U-Boot will not be able to parse environment due to
> > it having:
> > =foo
> > 
> > Now, if the above command is given, an error message is thrown and
> > environment is not modified.
> > 
> > Signed-off-by: Francis Laniel <francis.laniel@amarulasolutions.com>
> > ---
> > 
> >  cmd/nvedit.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/cmd/nvedit.c b/cmd/nvedit.c
> > index d14ba10cef..6f99a85a9c 100644
> > --- a/cmd/nvedit.c
> > +++ b/cmd/nvedit.c
> > @@ -262,6 +262,11 @@ static int _do_env_set(int flag, int argc, char
> > *const argv[], int env_flag)> 
> >  		return 1;
> >  	
> >  	}
> > 
> > +	if (*name == '\0') {
> > +		printf("## Error: variable name must no be empty\n");
> > +		return 1;
> > +	}
> > +
> > 
> >  	env_id++;
> >  	
> >  	/* Delete only ? */
> 
> Reviewed-by: Wolfgang Denk <wd@denx.de>

Thank you for the review!

> 
> Best regards,
> 
> Wolfgang Denk