Locking down U-Boot env with ENV_WRITEABLE_LIST

From: Marek Vasut <marex@denx.de>
To: u-boot@lists.denx.de
Subject: Locking down U-Boot env with ENV_WRITEABLE_LIST
Date: Mon, 5 Apr 2021 19:36:00 +0200	[thread overview]
Message-ID: <541706f0-9baa-0c28-7b0a-122286120849@denx.de> (raw)
In-Reply-To: <CAJ+vNU0iXmQgQrzOUrxfh95Oabp2DT7ZdKZ9DtG-astCg81p5w@mail.gmail.com>

On 4/5/21 6:27 PM, Tim Harvey wrote:
> On Sat, Apr 3, 2021 at 12:09 PM Marek Vasut <marex@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.