Locking down U-Boot env with ENV_WRITEABLE_LIST

[Marek Vasut]

On 4/6/21 9:52 PM, Tim Harvey wrote:
> 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.

Can you maybe write some better env documentation and submit a patch, 
now that you got it all figured out ?

> 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?

set bootdelay=-2 disables the prompt access.