Locking down U-Boot env with ENV_WRITEABLE_LIST

[Tim Harvey]

On Mon, Apr 5, 2021 at 10:36 AM Marek Vasut <marex at denx.de> wrote:
>
> On 4/5/21 6:27 PM, Tim Harvey wrote:
> > On Sat, Apr 3, 2021 at 12:09 PM Marek Vasut <marex at denx.de> wrote:
> >>
> >> On 4/3/21 6:43 PM, Tim Harvey wrote:
> >>
> >> Hi,
> >>
> >> [...]
> >>
> >>>>>> And those config options I had enabled in u-boot defconfig:
> >>>>>>
> >>>>>> CONFIG_CMD_ENV_CALLBACK=y
> >>>>>> CONFIG_CMD_ENV_FLAGS=y
> >>>>>> CONFIG_ENV_IS_NOWHERE=y
> >>>>>> CONFIG_ENV_IS_IN_MMC=y
> >>>>>> CONFIG_ENV_APPEND=y
> >>>>>> CONFIG_ENV_WRITEABLE_LIST=y
> >>>>>> CONFIG_ENV_ACCESS_IGNORE_FORCE=y
> >>>>>
> >>>>> Do you really define both ENV_IS_NOWHERE and ENV_IS_IN_MMC? From what
> >>>>> I see if you define ENV_IS_NOWHERE none of the others will be used.
> >>>>
> >>>> Yes, having two ENV drivers is mandatory. One provides the base env (the
> >>>> nowhere) and the other is used to import the filtered extras from
> >>>> external env.
> >>>
> >>> Enabling ENV_IS_NOWHERE does not work as you describe in my testing.
> >>> I'm testing this with imx8mm_venice_defconfig on U-Boot 2021.01 and as
> >>> soon as I define ENV_IS_NOWHERE then env_load is never called because
> >>> when env_relocate is called env is not valid yet so env_set_default is
> >>> called and env_load is 'never' called, thus mmc env is never loaded.
> >>> This is all from
> >>> https://elixir.bootlin.com/u-boot/v2021.01/source/env/common.c#L259
> >>
> >> Maybe this [PATCH V3] env: Fix invalid env handling in env_init() patch
> >> is missing ?
> >>
> >> [...]
> >>
> >>>    /* Link Definitions */
> >>>    #define CONFIG_LOADADDR                        0x40480000
> >>>    #define CONFIG_SYS_LOAD_ADDR           CONFIG_LOADADDR''
> >>>
> >>> With this configuration a blank env in flash results in the entire
> >>> default env showing in an 'env print' (ie all the stuff from
> >>> include/env_default.h 'default_environment') but as soon as I put an
> >>> env in flash (ie saveenv) and reset now the only env is what was set
> >>> via running code (ethaddr's fdtcontroladdr stdin/err/out). The only
> >>> different I expected is that I expected the default env from
> >>> include/env_default.h to be there on an initial boot with no valid mmc
> >>> env.
> >>
> >> Maybe the aforementioned patch is missing ?
> >
> > Marek,
> >
> > Yes, that patch fixes the issue I'm seeing of mmc not initializing and
> > now the default env shows up as expected when MMC env is empty or
> > populated.
> >
> > Thanks - hopefully that patch gets merged soon... I did respond with a
> > Tested-By.
>
> Oh, good. You might want to notify Tom about that, so it would get picked.

Marek,

One additional thing I did get stuck on is that if your writable
var(s) appears in the default environment the default will take
precedence over the FLASH env. This does make sense, but requires that
you create a default FLASH env (ie mkenvimage) and put it in the
appropriate place. I figured I would mention this for anyone else that
ends up reading this thread for help.

One last thing that I have not yet figured out how to work-around is
how to best disable the shell completely so that if there is any
failure in your bootcmd you don't simply drop into a completely
insecure U-Boot shell. What is your recommendation there?

Best regards,

Tim