| Message ID | 20220218180559.1432559-1-jannh@google.com |
|---|---|
| State | Accepted |
| Commit | 258dd902022cb10c83671176688074879517fd21 |
| Headers | show |
| Series | efivars: Respect "block" flag in efivar_entry_set_safe() | expand |
On Fri, 18 Feb 2022 at 19:06, Jann Horn <jannh@google.com> wrote: > > When the "block" flag is false, the old code would sometimes still call > check_var_size(), which wrongly tells ->query_variable_store() that it can > block. > > As far as I can tell, this can't really materialize as a bug at the moment, > because ->query_variable_store only does something on X86 with generic EFI, > and in that configuration we always take the efivar_entry_set_nonblocking() > path. So I'm not marking this for stable backporting. > > Fixes: ca0e30dcaa53 ("efi: Add nonblocking option to efi_query_variable_store()") > Signed-off-by: Jann Horn <jannh@google.com> Thanks Jann. I'll queue this up. > --- > > drivers/firmware/efi/vars.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c > index abdc8a6a3963..cae590bd08f2 100644 > --- a/drivers/firmware/efi/vars.c > +++ b/drivers/firmware/efi/vars.c > @@ -742,6 +742,7 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes, > { > const struct efivar_operations *ops; > efi_status_t status; > + unsigned long varsize; > > if (!__efivars) > return -EINVAL; > @@ -764,15 +765,17 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes, > return efivar_entry_set_nonblocking(name, vendor, attributes, > size, data); > > + varsize = size + ucs2_strsize(name, 1024); > if (!block) { > if (down_trylock(&efivars_lock)) > return -EBUSY; > + status = check_var_size_nonblocking(attributes, varsize); > } else { > if (down_interruptible(&efivars_lock)) > return -EINTR; > + status = check_var_size(attributes, varsize); > } > > - status = check_var_size(attributes, size + ucs2_strsize(name, 1024)); > if (status != EFI_SUCCESS) { > up(&efivars_lock); > return -ENOSPC; > > base-commit: 83e396641110663d3c7bb25b9bc0c6a750359ecf > -- > 2.35.1.473.g83b2b277ed-goog >
diff --git a/drivers/firmware/efi/vars.c b/drivers/firmware/efi/vars.c index abdc8a6a3963..cae590bd08f2 100644 --- a/drivers/firmware/efi/vars.c +++ b/drivers/firmware/efi/vars.c @@ -742,6 +742,7 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes, { const struct efivar_operations *ops; efi_status_t status; + unsigned long varsize; if (!__efivars) return -EINVAL; @@ -764,15 +765,17 @@ int efivar_entry_set_safe(efi_char16_t *name, efi_guid_t vendor, u32 attributes, return efivar_entry_set_nonblocking(name, vendor, attributes, size, data); + varsize = size + ucs2_strsize(name, 1024); if (!block) { if (down_trylock(&efivars_lock)) return -EBUSY; + status = check_var_size_nonblocking(attributes, varsize); } else { if (down_interruptible(&efivars_lock)) return -EINTR; + status = check_var_size(attributes, varsize); } - status = check_var_size(attributes, size + ucs2_strsize(name, 1024)); if (status != EFI_SUCCESS) { up(&efivars_lock); return -ENOSPC;
When the "block" flag is false, the old code would sometimes still call check_var_size(), which wrongly tells ->query_variable_store() that it can block. As far as I can tell, this can't really materialize as a bug at the moment, because ->query_variable_store only does something on X86 with generic EFI, and in that configuration we always take the efivar_entry_set_nonblocking() path. So I'm not marking this for stable backporting. Fixes: ca0e30dcaa53 ("efi: Add nonblocking option to efi_query_variable_store()") Signed-off-by: Jann Horn <jannh@google.com> --- drivers/firmware/efi/vars.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) base-commit: 83e396641110663d3c7bb25b9bc0c6a750359ecf