[PATCH v1 3/4] efi_loader: add an EFI variable with the variable file contents
Heinrich Schuchardt
xypron.glpk at gmx.de
Mon Apr 8 10:29:37 CEST 2024
On 4/8/24 10:12, Ilias Apalodimas wrote:
> Hi Heinrich
>
> On Mon, 8 Apr 2024 at 09:41, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>> On 4/6/24 16:01, Ilias Apalodimas wrote:
>>> Previous patches enabled SetVariableRT using a RAM backend.
>>> Although EBBR [0] defines a variable format we can teach userspace tools
>>> and write the altered variables, it's better if we skip the ABI
>>> requirements completely.
>>>
>>> So let's add a new variable, in its own namespace called "VarToFile"
>>> which contains a binary dump of the updated RT, BS and, NV variables.
>>>
>>> Some adjustments are needed to do that. Currently we discard BS-only
>>> variables in EBS(). We need to preserve those on the OS RAM backend
>>> that exposes the variables. Since BS-only variables can't appear at RT
>>> we need to move the memory masking checks from efi_var_collect() to
>>> efi_get_next_variable_name_mem()/efi_get_variable_mem() and do the
>>> filtering at runtime. We also need to make efi_var_collect() available
>>> at runtime, in order to construct the "VarToFile" buffer with BS, RT &
>>> NV variables.
>>>
>>> All users and applications (for linux) have to do when updating a variable
>>> is dd that variable in the file described by "RTStorageVolatile".
>>>
>>> Linux efivarfs uses a first 4 bytes of the output to represent attributes
>>> in little-endian format. So, storing variables works like this:
>>>
>>> $~ efibootmgr -n 0001
>>> $~ dd if=/sys/firmware/efi/efivars/VarToFile-b2ac5fc9-92b7-4acd-aeac-11e818c3130c of=/boot/efi/ubootefi.var skip=4 bs=1
>>>
>>> [0] https://arm-software.github.io/ebbr/index.html#document-chapter5-variable-storage
>>
>> This change actually doubles the amount of memory needed to store a
>> variable at runtime. How do you reflect this in QueryVariableInfo()?
>>
>
> It doesn't double it. Only BS, RT, NV variables are added to VarToFile.
> QueryVariableInfo isn't supported at runtime, but with the current
> code we have at boot time the VarToFile would be included in the
> calculations.
VarToFile does not exist at boot time. Nothing stops the user from
writing NV variables to fill up the memory buffer. VarToFile will be as
large as the sum of NV variables and will not fit into the buffer when
efi_variables_boot_exit_notify() is invoked.
>
>>>
>>> Suggested-by:Ard Biesheuvel <ardb at kernel.org> # dumping all variables to a variable
>>> Signed-off-by: Ilias Apalodimas <ilias.apalodimas at linaro.org>
>>> ---
>>> include/efi_variable.h | 15 +++-
>>> lib/efi_loader/efi_boottime.c | 2 +
>>> lib/efi_loader/efi_var_common.c | 43 +++++------
>>> lib/efi_loader/efi_var_file.c | 1 -
>>> lib/efi_loader/efi_var_mem.c | 90 ++++++++++-------------
>>> lib/efi_loader/efi_variable.c | 118 ++++++++++++++++++++++++------
>
> [...]
>
>>> * @data: buffer to which the variable value is copied
>>> * @timep: authentication time (seconds since start of epoch)
>>> + * @mask: bitmask with required attributes of variables to be collected.
>>> + * variables are only collected if all of the required
>>
>> This line looks incomplete.
>
> Yes, fixed
>
>>
>>> * Return: status code
>>> */
>>> efi_status_t __efi_runtime
>>> efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor,
>>> u32 *attributes, efi_uintn_t *data_size, void *data,
>>> - u64 *timep);
>>> + u64 *timep, u32 mask);
>>>
>>> /**
>
> [...]
>
>>> return EFI_UNSUPPORTED;
>>> @@ -557,11 +593,11 @@ void efi_variables_boot_exit_notify(void)
>>> const efi_guid_t efi_guid_efi_rt_var_file = U_BOOT_EFI_RT_VAR_FILE_GUID;
>>> const efi_guid_t rt_prop_guid = EFI_RT_PROPERTIES_TABLE_GUID;
>>> efi_status_t ret;
>>> + struct efi_var_file *buf;
>>> + loff_t len;
>>> + bool fail = false;
>>>
>>> if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) {
>>> - struct efi_rt_properties_table *rt_prop =
>>> - efi_get_configuration_table(&rt_prop_guid);
>>> -
>>> ret = efi_set_variable_int(u"RTStorageVolatile",
>>> &efi_guid_efi_rt_var_file,
>>> EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>> @@ -569,11 +605,47 @@ void efi_variables_boot_exit_notify(void)
>>> EFI_VARIABLE_READ_ONLY,
>>> sizeof(EFI_VAR_FILE_NAME),
>>> EFI_VAR_FILE_NAME, false);
>>> + if (ret != EFI_SUCCESS) {
>>> + fail = true;
>>> + goto out;
>>> + }
>>> +
>>> + ret = efi_var_collect(&buf, &len, EFI_VARIABLE_NON_VOLATILE);
>>> + if (ret != EFI_SUCCESS) {
>>> + fail = true;
>>> + goto out;
>>> + }
>>> +
>>> + ret = efi_set_variable_int(u"VarToFile",
>>> + &efi_guid_efi_rt_var_file,
>>> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>> + EFI_VARIABLE_RUNTIME_ACCESS,
>>> + len,
>>> + buf, false);
>>> if (ret != EFI_SUCCESS)
>>> - rt_prop->runtime_services_supported |= ~EFI_RT_SUPPORTED_SET_VARIABLE;
>>> - else
>>> - log_err("Can't RTStorage. SetVariableRT won't be available\n");
>>> + fail = true;
>>> +out:
>>> + if (fail) {
>>> + efi_set_variable_int(u"RTStorageVolatile",
>>> + &efi_guid_efi_rt_var_file,
>>> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>> + EFI_VARIABLE_RUNTIME_ACCESS |
>>> + EFI_VARIABLE_READ_ONLY, 0, 0,
>>> + false);
>>
>> Not providing VarToFile because the memory buffer is more than half
>> filled before ExitBootServices() is unexpected behavior. This needs rework.
>
> Why? We should add the SetVariable at runtime to the documentation.
Depending on the number of variables existing at boot time support you
will be creating VarToFile or not. I can't see that users will fancy this.
Best regards
Heinrich
>
>>
>> It is not necessary to create VarToFile in the internal memory buffer.
>> You could generate it when the user calls GetVariable() in the user
>> provided buffer.
>
> Yes, but that would make some functions look a bit messier, since we
> have to inject code specifically looking for VarToFile.
> OTOH it would make SetVariable at runtime easier to read since all the
> size calculations and variable creation would go away.
> But in any case, we have to add the variable at runtime (perhaps with
> a value of 0?) -- users need to know it's there to be able to read it.
> But if you are fine with the special conditions in GetVariable at
> runtime, I can easily move the logic to GetVariable.
>
> Thanks
> /Ilias
>
>>
>> Best regards
>>
>> Heinrich
>>
>>> + efi_set_variable_int(u"VarToFile",
>>> + &efi_guid_efi_rt_var_file,
>>> + EFI_VARIABLE_BOOTSERVICE_ACCESS |
>>> + EFI_VARIABLE_RUNTIME_ACCESS, 0, 0,
>>> + false);
>>> + } else {
>>> + struct efi_rt_properties_table *rt_prop =
>>> + efi_get_configuration_table(&rt_prop_guid);
>>> +
>>> + rt_prop->runtime_services_supported |=
>>> + EFI_RT_SUPPORTED_SET_VARIABLE;
>>> + }
>>> }
>>> +
>>> /* Switch variable services functions to runtime version */
>>> efi_runtime_services.get_variable = efi_get_variable_runtime;
>>> efi_runtime_services.get_next_variable_name =
>>> diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c
>>> index dde135fd9f81..9d0e270591ea 100644
>>> --- a/lib/efi_loader/efi_variable_tee.c
>>> +++ b/lib/efi_loader/efi_variable_tee.c
>>> @@ -969,7 +969,6 @@ void efi_variables_boot_exit_notify(void)
>>> log_err("Can't populate EFI variables. No runtime variables will be available\n");
>>> else
>>> efi_var_buf_update(var_buf);
>>> - free(var_buf);
>>>
>>> /* Update runtime service table */
>>> efi_runtime_services.query_variable_info =
>>> --
>>> 2.37.2
>>>
>>
More information about the U-Boot
mailing list