| Message ID | 20240417101928.119115-4-ilias.apalodimas@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable SetVariable at runtime | expand |
On 17.04.24 12:19, 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 > and will be updated when GetVariable is called. > > Some adjustments are needed to do that. > Currently we discard BS-only variables in EBS(). We need to preserve > those on the RAM backend that exposes the variables. Since BS-only > variables can't appear at runtime 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 an efi_var_collect() variant available at runtime, in order > to construct the "VarToFile" buffer on the fly. > > 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 > > Co-developed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com> > Signed-off-by: Ilias Apalodimas <ilias.apalodimas@linaro.org> > --- > include/efi_variable.h | 14 ++- > lib/charset.c | 2 +- > lib/efi_loader/efi_runtime.c | 19 ++++ > lib/efi_loader/efi_var_common.c | 6 +- > lib/efi_loader/efi_var_mem.c | 146 ++++++++++++++++++------------ > lib/efi_loader/efi_variable.c | 6 +- > lib/efi_loader/efi_variable_tee.c | 5 - > 7 files changed, 130 insertions(+), 68 deletions(-) > > diff --git a/include/efi_variable.h b/include/efi_variable.h > index 42a2b7c52bef..b545a36aac50 100644 > --- a/include/efi_variable.h > +++ b/include/efi_variable.h > @@ -271,13 +271,16 @@ const efi_guid_t *efi_auth_var_get_guid(const u16 *name); > * > * @variable_name_size: size of variable_name buffer in bytes > * @variable_name: name of uefi variable's name in u16 > + * @mask: bitmask with required attributes of variables to be collected. > + * variables are only collected if all of the required > + * attributes match. Use 0 to skip matching > * @vendor: vendor's guid > * > * Return: status code > */ > efi_status_t __efi_runtime > efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_name, > - efi_guid_t *vendor); > + efi_guid_t *vendor, u32 mask); > /** > * efi_get_variable_mem() - Runtime common code across efi variable > * implementations for GetVariable() from > @@ -289,12 +292,15 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_na > * @data_size: size of the buffer to which the variable value is copied > * @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 > + * attributes match. Use 0 to skip matching > * 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); > > /** > * efi_get_variable_runtime() - runtime implementation of GetVariable() > @@ -334,4 +340,8 @@ efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, > */ > void efi_var_buf_update(struct efi_var_file *var_buf); > > +efi_status_t __efi_runtime efi_var_collect_mem(struct efi_var_file *buf, > + efi_uintn_t *lenp, > + u32 check_attr_mask); > + > #endif > diff --git a/lib/charset.c b/lib/charset.c > index df4f04074852..182c92a50c48 100644 > --- a/lib/charset.c > +++ b/lib/charset.c > @@ -387,7 +387,7 @@ int u16_strcasecmp(const u16 *s1, const u16 *s2) > * > 0 if the first different u16 in s1 is greater than the > * corresponding u16 in s2 > */ > -int u16_strncmp(const u16 *s1, const u16 *s2, size_t n) > +int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n) > { > int ret = 0; > > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c > index c8f7a88ba8db..99ad1f35d8f1 100644 > --- a/lib/efi_loader/efi_runtime.c > +++ b/lib/efi_loader/efi_runtime.c > @@ -130,6 +130,8 @@ efi_status_t efi_init_runtime_supported(void) > EFI_RT_SUPPORTED_CONVERT_POINTER; > > if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { > + int s = 0; u8 s = 0; should be enough. > + > ret = efi_set_variable_int(u"RTStorageVolatile", > &efi_guid_efi_rt_var_file, > EFI_VARIABLE_BOOTSERVICE_ACCESS | > @@ -141,6 +143,23 @@ efi_status_t efi_init_runtime_supported(void) > log_err("Failed to set RTStorageVolatile\n"); > return ret; > } > + ret = efi_set_variable_int(u"VarToFile", > + &efi_guid_efi_rt_var_file, > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > + EFI_VARIABLE_RUNTIME_ACCESS, Why is the variable set here? A comment would be helpful. If you set it for protection, shouldn't it be EFI_VARIABLE_READ_ONLY? An alternative would be to return EFI_INVALID_PARAMETER in efi_set_variable_int for any attempt to set a variable with GUID efi_guid_efi_rt_var_file. > + sizeof(s), > + &s, false); > + if (ret != EFI_SUCCESS) { > + log_err("Failed to set VarToFile\n"); > + 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, NULL, false); > + > + return ret; > + } > rt_table->runtime_services_supported |= EFI_RT_SUPPORTED_SET_VARIABLE; > } > > diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c > index aa8feffd3ec1..7862f2d6ce8a 100644 > --- a/lib/efi_loader/efi_var_common.c > +++ b/lib/efi_loader/efi_var_common.c > @@ -182,7 +182,8 @@ efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid, > { > efi_status_t ret; > > - ret = efi_get_variable_mem(variable_name, guid, attributes, data_size, data, NULL); > + ret = efi_get_variable_mem(variable_name, guid, attributes, data_size, > + data, NULL, EFI_VARIABLE_RUNTIME_ACCESS); > > /* Remove EFI_VARIABLE_READ_ONLY flag */ > if (attributes) > @@ -195,7 +196,8 @@ efi_status_t __efi_runtime EFIAPI > efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, > u16 *variable_name, efi_guid_t *guid) > { > - return efi_get_next_variable_name_mem(variable_name_size, variable_name, guid); > + return efi_get_next_variable_name_mem(variable_name_size, variable_name, > + guid, EFI_VARIABLE_RUNTIME_ACCESS); > } > > /** > diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c > index 6c21cec5d457..65ab858c926e 100644 > --- a/lib/efi_loader/efi_var_mem.c > +++ b/lib/efi_loader/efi_var_mem.c > @@ -36,9 +36,11 @@ efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid, > const u16 *data, *var_name; > bool match = true; > > - for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0; > - i < sizeof(efi_guid_t) && match; ++i) > - match = (guid1[i] == guid2[i]); > + if (guid) { > + for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0; > + i < sizeof(efi_guid_t) && match; ++i) > + match = (guid1[i] == guid2[i]); > + } > > for (data = var->name, var_name = name;; ++data) { > if (match) > @@ -184,53 +186,6 @@ u64 __efi_runtime efi_var_mem_free(void) > sizeof(struct efi_var_entry); > } > > -/** > - * efi_var_mem_bs_del() - delete boot service only variables > - */ > -static void efi_var_mem_bs_del(void) > -{ > - struct efi_var_entry *var = efi_var_buf->var; > - > - for (;;) { > - struct efi_var_entry *last; > - > - last = (struct efi_var_entry *) > - ((uintptr_t)efi_var_buf + efi_var_buf->length); > - if (var >= last) > - break; > - if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) { > - u16 *data; > - > - /* skip variable */ > - for (data = var->name; *data; ++data) > - ; > - ++data; > - var = (struct efi_var_entry *) > - ALIGN((uintptr_t)data + var->length, 8); > - } else { > - /* delete variable */ > - efi_var_mem_del(var); > - } > - } > -} > - > -/** > - * efi_var_mem_notify_exit_boot_services() - ExitBootService callback > - * > - * @event: callback event > - * @context: callback context > - */ > -static void EFIAPI > -efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context) > -{ > - EFI_ENTRY("%p, %p", event, context); > - > - /* Delete boot service only variables */ > - efi_var_mem_bs_del(); > - > - EFI_EXIT(EFI_SUCCESS); > -} > - > /** > * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback > * > @@ -261,11 +216,7 @@ efi_status_t efi_var_mem_init(void) > efi_var_buf->magic = EFI_VAR_FILE_MAGIC; > efi_var_buf->length = (uintptr_t)efi_var_buf->var - > (uintptr_t)efi_var_buf; > - /* crc32 for 0 bytes = 0 */ > > - ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, > - efi_var_mem_notify_exit_boot_services, NULL, > - NULL, &event); > if (ret != EFI_SUCCESS) > return ret; > ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK, > @@ -276,10 +227,75 @@ efi_status_t efi_var_mem_init(void) > return ret; > } > > +/** > + * efi_var_collect_mem() - Copy EFI variables matching attributes mask from > + * efi_var_buf > + * > + * @buf: buffer containing variable collection > + * @lenp: buffer length > + * @mask: mask of matched attributes > + * > + * Return: Status code > + */ > +efi_status_t __efi_runtime > +efi_var_collect_mem(struct efi_var_file *buf, efi_uintn_t *lenp, u32 mask) > +{ > + static struct efi_var_file __efi_runtime_data hdr = { > + .magic = EFI_VAR_FILE_MAGIC, > + }; > + struct efi_var_entry *last, *var, *var_to; > + > + hdr.length = sizeof(struct efi_var_file); > + > + var = efi_var_buf->var; > + last = (struct efi_var_entry *) > + ((uintptr_t)efi_var_buf + efi_var_buf->length); > + if (buf) > + var_to = buf->var; > + > + while (var < last) { > + u32 len; > + struct efi_var_entry *var_next; > + > + efi_var_mem_compare(var, NULL, u"", &var_next); On IRC you suggested to make u16_strlen __efi_runtime_code to replace this efi_var_mem_compare() call and avoid the modifications of said function. That might be more readable. Best regards Heinrich > + len = (uintptr_t)var_next - (uintptr_t)var; > + > + if ((var->attr & mask) != mask) { > + var = (void *)var + len; > + continue; > + } > + > + hdr.length += len; > + > + if (buf && hdr.length <= *lenp) { > + efi_memcpy_runtime(var_to, var, len); > + var_to = (void *)var_to + len; > + } > + var = (void *)var + len; > + } > + > + if (!buf && hdr.length <= *lenp) { > + *lenp = hdr.length; > + return EFI_INVALID_PARAMETER; > + } > + > + if (!buf || hdr.length > *lenp) { > + *lenp = hdr.length; > + return EFI_BUFFER_TOO_SMALL; > + } > + hdr.crc32 = crc32(0, (u8 *)buf->var, > + hdr.length - sizeof(struct efi_var_file)); > + > + efi_memcpy_runtime(buf, &hdr, sizeof(hdr)); > + *lenp = hdr.length; > + > + return EFI_SUCCESS; > +} > + > 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) > { > efi_uintn_t old_size; > struct efi_var_entry *var; > @@ -291,11 +307,22 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, > if (!var) > return EFI_NOT_FOUND; > > + /* > + * This function is used at runtime to dump EFI variables. > + * The memory backend we keep around has BS-only variables as > + * well. At runtime we filter them here > + */ > + if (mask && !((var->attr & mask) == mask)) > + return EFI_NOT_FOUND; > + > if (attributes) > *attributes = var->attr; > if (timep) > *timep = var->time; > > + if (!u16_strcmp(variable_name, u"VarToFile")) > + return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE); > + > old_size = *data_size; > *data_size = var->length; > if (old_size < var->length) > @@ -315,7 +342,8 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, > > efi_status_t __efi_runtime > efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, > - u16 *variable_name, efi_guid_t *vendor) > + u16 *variable_name, efi_guid_t *vendor, > + u32 mask) > { > struct efi_var_entry *var; > efi_uintn_t len, old_size; > @@ -324,6 +352,7 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, > if (!variable_name_size || !variable_name || !vendor) > return EFI_INVALID_PARAMETER; > > +skip: > len = *variable_name_size >> 1; > if (u16_strnlen(variable_name, len) == len) > return EFI_INVALID_PARAMETER; > @@ -347,6 +376,11 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, > efi_memcpy_runtime(variable_name, var->name, *variable_name_size); > efi_memcpy_runtime(vendor, &var->guid, sizeof(efi_guid_t)); > > + if (mask && !((var->attr & mask) == mask)) { > + *variable_name_size = old_size; > + goto skip; > + } > + > return EFI_SUCCESS; > } > > diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c > index abc2a3402f42..4aaa05a617d7 100644 > --- a/lib/efi_loader/efi_variable.c > +++ b/lib/efi_loader/efi_variable.c > @@ -209,14 +209,16 @@ efi_get_variable_int(const u16 *variable_name, const efi_guid_t *vendor, > u32 *attributes, efi_uintn_t *data_size, void *data, > u64 *timep) > { > - return efi_get_variable_mem(variable_name, vendor, attributes, data_size, data, timep); > + return efi_get_variable_mem(variable_name, vendor, attributes, data_size, > + data, timep, 0); > } > > efi_status_t __efi_runtime > efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, > u16 *variable_name, efi_guid_t *vendor) > { > - return efi_get_next_variable_name_mem(variable_name_size, variable_name, vendor); > + return efi_get_next_variable_name_mem(variable_name_size, variable_name, > + vendor, 0); > } > > /** > diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c > index dde135fd9f81..4f1aa298da13 100644 > --- a/lib/efi_loader/efi_variable_tee.c > +++ b/lib/efi_loader/efi_variable_tee.c > @@ -959,11 +959,6 @@ void efi_variables_boot_exit_notify(void) > log_err("Unable to notify the MM partition for ExitBootServices\n"); > free(comm_buf); > > - /* > - * Populate the list for runtime variables. > - * asking EFI_VARIABLE_RUNTIME_ACCESS is redundant, since > - * efi_var_mem_notify_exit_boot_services will clean those, but that's fine > - */ > ret = efi_var_collect(&var_buf, &len, EFI_VARIABLE_RUNTIME_ACCESS); > if (ret != EFI_SUCCESS) > log_err("Can't populate EFI variables. No runtime variables will be available\n");
Hi Heinrich, [...] > > { > > int ret = 0; > > > > diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c > > index c8f7a88ba8db..99ad1f35d8f1 100644 > > --- a/lib/efi_loader/efi_runtime.c > > +++ b/lib/efi_loader/efi_runtime.c > > @@ -130,6 +130,8 @@ efi_status_t efi_init_runtime_supported(void) > > EFI_RT_SUPPORTED_CONVERT_POINTER; > > > > if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { > > + int s = 0; > > u8 s = 0; > should be enough. Sure, > > } > > + ret = efi_set_variable_int(u"VarToFile", > > + &efi_guid_efi_rt_var_file, > > + EFI_VARIABLE_BOOTSERVICE_ACCESS | > > + EFI_VARIABLE_RUNTIME_ACCESS, > > Why is the variable set here? A comment would be helpful. We need to set it so it shows up in the efivarfs, fo linux, and the variable list in general for other OSes. Otherwise, people won't be able to read it, unless they specifically search for it. I'll add a comment though > If you set it for protection, shouldn't it be EFI_VARIABLE_READ_ONLY? > > An alternative would be to return EFI_INVALID_PARAMETER in > efi_set_variable_int for any attempt to set a variable with GUID > efi_guid_efi_rt_var_file. It's not volatile, so it will be treated as RO at runtime. But I'll add EFI_VARIABLE_READ_ONLY as well. I prefer using the existing EFI APIs instead of treating GUIDs specially [...] > > +/** > > + * efi_var_collect_mem() - Copy EFI variables matching attributes mask from > > + * efi_var_buf > > + * > > + * @buf: buffer containing variable collection > > + * @lenp: buffer length > > + * @mask: mask of matched attributes > > + * > > + * Return: Status code > > + */ > > +efi_status_t __efi_runtime > > +efi_var_collect_mem(struct efi_var_file *buf, efi_uintn_t *lenp, u32 mask) > > +{ > > + static struct efi_var_file __efi_runtime_data hdr = { > > + .magic = EFI_VAR_FILE_MAGIC, > > + }; > > + struct efi_var_entry *last, *var, *var_to; > > + > > + hdr.length = sizeof(struct efi_var_file); > > + > > + var = efi_var_buf->var; > > + last = (struct efi_var_entry *) > > + ((uintptr_t)efi_var_buf + efi_var_buf->length); > > + if (buf) > > + var_to = buf->var; > > + > > + while (var < last) { > > + u32 len; > > + struct efi_var_entry *var_next; > > + > > + efi_var_mem_compare(var, NULL, u"", &var_next); > > On IRC you suggested to make u16_strlen __efi_runtime_code to replace > this efi_var_mem_compare() call and avoid the modifications of said > function. That might be more readable. Yes it will, I'll change that on v3. That function will also help us clean various open-coded sequences of length calculations. FWIW I just managed to run FWTS over this -- and I also fixed QueryVariableInfo at runtime We had 22 tests passing and 1 failing. I am still trying to figure out what failed, but I'll fix that as well for v3 > > Best regards > > Heinrich > > > + len = (uintptr_t)var_next - (uintptr_t)var; > > + > > + if ((var->attr & mask) != mask) { [...] Thanks /Ilias
diff --git a/include/efi_variable.h b/include/efi_variable.h index 42a2b7c52bef..b545a36aac50 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -271,13 +271,16 @@ const efi_guid_t *efi_auth_var_get_guid(const u16 *name); * * @variable_name_size: size of variable_name buffer in bytes * @variable_name: name of uefi variable's name in u16 + * @mask: bitmask with required attributes of variables to be collected. + * variables are only collected if all of the required + * attributes match. Use 0 to skip matching * @vendor: vendor's guid * * Return: status code */ efi_status_t __efi_runtime efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_name, - efi_guid_t *vendor); + efi_guid_t *vendor, u32 mask); /** * efi_get_variable_mem() - Runtime common code across efi variable * implementations for GetVariable() from @@ -289,12 +292,15 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, u16 *variable_na * @data_size: size of the buffer to which the variable value is copied * @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 + * attributes match. Use 0 to skip matching * 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); /** * efi_get_variable_runtime() - runtime implementation of GetVariable() @@ -334,4 +340,8 @@ efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, */ void efi_var_buf_update(struct efi_var_file *var_buf); +efi_status_t __efi_runtime efi_var_collect_mem(struct efi_var_file *buf, + efi_uintn_t *lenp, + u32 check_attr_mask); + #endif diff --git a/lib/charset.c b/lib/charset.c index df4f04074852..182c92a50c48 100644 --- a/lib/charset.c +++ b/lib/charset.c @@ -387,7 +387,7 @@ int u16_strcasecmp(const u16 *s1, const u16 *s2) * > 0 if the first different u16 in s1 is greater than the * corresponding u16 in s2 */ -int u16_strncmp(const u16 *s1, const u16 *s2, size_t n) +int __efi_runtime u16_strncmp(const u16 *s1, const u16 *s2, size_t n) { int ret = 0; diff --git a/lib/efi_loader/efi_runtime.c b/lib/efi_loader/efi_runtime.c index c8f7a88ba8db..99ad1f35d8f1 100644 --- a/lib/efi_loader/efi_runtime.c +++ b/lib/efi_loader/efi_runtime.c @@ -130,6 +130,8 @@ efi_status_t efi_init_runtime_supported(void) EFI_RT_SUPPORTED_CONVERT_POINTER; if (IS_ENABLED(CONFIG_EFI_RT_VOLATILE_STORE)) { + int s = 0; + ret = efi_set_variable_int(u"RTStorageVolatile", &efi_guid_efi_rt_var_file, EFI_VARIABLE_BOOTSERVICE_ACCESS | @@ -141,6 +143,23 @@ efi_status_t efi_init_runtime_supported(void) log_err("Failed to set RTStorageVolatile\n"); return ret; } + ret = efi_set_variable_int(u"VarToFile", + &efi_guid_efi_rt_var_file, + EFI_VARIABLE_BOOTSERVICE_ACCESS | + EFI_VARIABLE_RUNTIME_ACCESS, + sizeof(s), + &s, false); + if (ret != EFI_SUCCESS) { + log_err("Failed to set VarToFile\n"); + 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, NULL, false); + + return ret; + } rt_table->runtime_services_supported |= EFI_RT_SUPPORTED_SET_VARIABLE; } diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index aa8feffd3ec1..7862f2d6ce8a 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -182,7 +182,8 @@ efi_get_variable_runtime(u16 *variable_name, const efi_guid_t *guid, { efi_status_t ret; - ret = efi_get_variable_mem(variable_name, guid, attributes, data_size, data, NULL); + ret = efi_get_variable_mem(variable_name, guid, attributes, data_size, + data, NULL, EFI_VARIABLE_RUNTIME_ACCESS); /* Remove EFI_VARIABLE_READ_ONLY flag */ if (attributes) @@ -195,7 +196,8 @@ efi_status_t __efi_runtime EFIAPI efi_get_next_variable_name_runtime(efi_uintn_t *variable_name_size, u16 *variable_name, efi_guid_t *guid) { - return efi_get_next_variable_name_mem(variable_name_size, variable_name, guid); + return efi_get_next_variable_name_mem(variable_name_size, variable_name, + guid, EFI_VARIABLE_RUNTIME_ACCESS); } /** diff --git a/lib/efi_loader/efi_var_mem.c b/lib/efi_loader/efi_var_mem.c index 6c21cec5d457..65ab858c926e 100644 --- a/lib/efi_loader/efi_var_mem.c +++ b/lib/efi_loader/efi_var_mem.c @@ -36,9 +36,11 @@ efi_var_mem_compare(struct efi_var_entry *var, const efi_guid_t *guid, const u16 *data, *var_name; bool match = true; - for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0; - i < sizeof(efi_guid_t) && match; ++i) - match = (guid1[i] == guid2[i]); + if (guid) { + for (guid1 = (u8 *)&var->guid, guid2 = (u8 *)guid, i = 0; + i < sizeof(efi_guid_t) && match; ++i) + match = (guid1[i] == guid2[i]); + } for (data = var->name, var_name = name;; ++data) { if (match) @@ -184,53 +186,6 @@ u64 __efi_runtime efi_var_mem_free(void) sizeof(struct efi_var_entry); } -/** - * efi_var_mem_bs_del() - delete boot service only variables - */ -static void efi_var_mem_bs_del(void) -{ - struct efi_var_entry *var = efi_var_buf->var; - - for (;;) { - struct efi_var_entry *last; - - last = (struct efi_var_entry *) - ((uintptr_t)efi_var_buf + efi_var_buf->length); - if (var >= last) - break; - if (var->attr & EFI_VARIABLE_RUNTIME_ACCESS) { - u16 *data; - - /* skip variable */ - for (data = var->name; *data; ++data) - ; - ++data; - var = (struct efi_var_entry *) - ALIGN((uintptr_t)data + var->length, 8); - } else { - /* delete variable */ - efi_var_mem_del(var); - } - } -} - -/** - * efi_var_mem_notify_exit_boot_services() - ExitBootService callback - * - * @event: callback event - * @context: callback context - */ -static void EFIAPI -efi_var_mem_notify_exit_boot_services(struct efi_event *event, void *context) -{ - EFI_ENTRY("%p, %p", event, context); - - /* Delete boot service only variables */ - efi_var_mem_bs_del(); - - EFI_EXIT(EFI_SUCCESS); -} - /** * efi_var_mem_notify_exit_boot_services() - SetVirtualMemoryMap callback * @@ -261,11 +216,7 @@ efi_status_t efi_var_mem_init(void) efi_var_buf->magic = EFI_VAR_FILE_MAGIC; efi_var_buf->length = (uintptr_t)efi_var_buf->var - (uintptr_t)efi_var_buf; - /* crc32 for 0 bytes = 0 */ - ret = efi_create_event(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_CALLBACK, - efi_var_mem_notify_exit_boot_services, NULL, - NULL, &event); if (ret != EFI_SUCCESS) return ret; ret = efi_create_event(EVT_SIGNAL_VIRTUAL_ADDRESS_CHANGE, TPL_CALLBACK, @@ -276,10 +227,75 @@ efi_status_t efi_var_mem_init(void) return ret; } +/** + * efi_var_collect_mem() - Copy EFI variables matching attributes mask from + * efi_var_buf + * + * @buf: buffer containing variable collection + * @lenp: buffer length + * @mask: mask of matched attributes + * + * Return: Status code + */ +efi_status_t __efi_runtime +efi_var_collect_mem(struct efi_var_file *buf, efi_uintn_t *lenp, u32 mask) +{ + static struct efi_var_file __efi_runtime_data hdr = { + .magic = EFI_VAR_FILE_MAGIC, + }; + struct efi_var_entry *last, *var, *var_to; + + hdr.length = sizeof(struct efi_var_file); + + var = efi_var_buf->var; + last = (struct efi_var_entry *) + ((uintptr_t)efi_var_buf + efi_var_buf->length); + if (buf) + var_to = buf->var; + + while (var < last) { + u32 len; + struct efi_var_entry *var_next; + + efi_var_mem_compare(var, NULL, u"", &var_next); + len = (uintptr_t)var_next - (uintptr_t)var; + + if ((var->attr & mask) != mask) { + var = (void *)var + len; + continue; + } + + hdr.length += len; + + if (buf && hdr.length <= *lenp) { + efi_memcpy_runtime(var_to, var, len); + var_to = (void *)var_to + len; + } + var = (void *)var + len; + } + + if (!buf && hdr.length <= *lenp) { + *lenp = hdr.length; + return EFI_INVALID_PARAMETER; + } + + if (!buf || hdr.length > *lenp) { + *lenp = hdr.length; + return EFI_BUFFER_TOO_SMALL; + } + hdr.crc32 = crc32(0, (u8 *)buf->var, + hdr.length - sizeof(struct efi_var_file)); + + efi_memcpy_runtime(buf, &hdr, sizeof(hdr)); + *lenp = hdr.length; + + return EFI_SUCCESS; +} + 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) { efi_uintn_t old_size; struct efi_var_entry *var; @@ -291,11 +307,22 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, if (!var) return EFI_NOT_FOUND; + /* + * This function is used at runtime to dump EFI variables. + * The memory backend we keep around has BS-only variables as + * well. At runtime we filter them here + */ + if (mask && !((var->attr & mask) == mask)) + return EFI_NOT_FOUND; + if (attributes) *attributes = var->attr; if (timep) *timep = var->time; + if (!u16_strcmp(variable_name, u"VarToFile")) + return efi_var_collect_mem(data, data_size, EFI_VARIABLE_NON_VOLATILE); + old_size = *data_size; *data_size = var->length; if (old_size < var->length) @@ -315,7 +342,8 @@ efi_get_variable_mem(const u16 *variable_name, const efi_guid_t *vendor, efi_status_t __efi_runtime efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, - u16 *variable_name, efi_guid_t *vendor) + u16 *variable_name, efi_guid_t *vendor, + u32 mask) { struct efi_var_entry *var; efi_uintn_t len, old_size; @@ -324,6 +352,7 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, if (!variable_name_size || !variable_name || !vendor) return EFI_INVALID_PARAMETER; +skip: len = *variable_name_size >> 1; if (u16_strnlen(variable_name, len) == len) return EFI_INVALID_PARAMETER; @@ -347,6 +376,11 @@ efi_get_next_variable_name_mem(efi_uintn_t *variable_name_size, efi_memcpy_runtime(variable_name, var->name, *variable_name_size); efi_memcpy_runtime(vendor, &var->guid, sizeof(efi_guid_t)); + if (mask && !((var->attr & mask) == mask)) { + *variable_name_size = old_size; + goto skip; + } + return EFI_SUCCESS; } diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index abc2a3402f42..4aaa05a617d7 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -209,14 +209,16 @@ efi_get_variable_int(const u16 *variable_name, const efi_guid_t *vendor, u32 *attributes, efi_uintn_t *data_size, void *data, u64 *timep) { - return efi_get_variable_mem(variable_name, vendor, attributes, data_size, data, timep); + return efi_get_variable_mem(variable_name, vendor, attributes, data_size, + data, timep, 0); } efi_status_t __efi_runtime efi_get_next_variable_name_int(efi_uintn_t *variable_name_size, u16 *variable_name, efi_guid_t *vendor) { - return efi_get_next_variable_name_mem(variable_name_size, variable_name, vendor); + return efi_get_next_variable_name_mem(variable_name_size, variable_name, + vendor, 0); } /** diff --git a/lib/efi_loader/efi_variable_tee.c b/lib/efi_loader/efi_variable_tee.c index dde135fd9f81..4f1aa298da13 100644 --- a/lib/efi_loader/efi_variable_tee.c +++ b/lib/efi_loader/efi_variable_tee.c @@ -959,11 +959,6 @@ void efi_variables_boot_exit_notify(void) log_err("Unable to notify the MM partition for ExitBootServices\n"); free(comm_buf); - /* - * Populate the list for runtime variables. - * asking EFI_VARIABLE_RUNTIME_ACCESS is redundant, since - * efi_var_mem_notify_exit_boot_services will clean those, but that's fine - */ ret = efi_var_collect(&var_buf, &len, EFI_VARIABLE_RUNTIME_ACCESS); if (ret != EFI_SUCCESS) log_err("Can't populate EFI variables. No runtime variables will be available\n");