[U-Boot] [RESEND][PATCH] cmd: nvedit: add whitelist option for env import

[Simon Glass]

Hi Alex,

On 15 May 2018 at 12:43, Alex Kiernan <alex.kiernan at gmail.com> wrote:
> On Tue, May 15, 2018 at 7:25 PM Joe Hershberger <joe.hershberger at gmail.com>
> wrote:
>
>> On Tue, May 15, 2018 at 5:03 AM, Quentin Schulz
>> <quentin.schulz at bootlin.com> wrote:
>> > 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.
>> >
>> > Signed-off-by: Quentin Schulz <quentin.schulz at bootlin.com>
>> > ---
>> >  cmd/nvedit.c | 66
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>> >  1 file changed, 62 insertions(+), 4 deletions(-)
>> >
>> > diff --git a/cmd/nvedit.c b/cmd/nvedit.c
>> > index 4e79d03856..4637ec656c 100644
>> > --- a/cmd/nvedit.c
>> > +++ b/cmd/nvedit.c
>> > @@ -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");
>
>> 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