[PATCH 0/3] efi_loader: fixes for EFI variables
GetVariable() and SetVariable() use an uint32_t value for attributes. The UEFI specification defines the related constants as 32bit.
Add the missing EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS constant.
We don't yet support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS for file based variables, but we should pass it to TEE based variable stores.
Heinrich Schuchardt (3): efi_loader: all variable attributes are 32bit efi_loader: EFI_VARIABLE_READ_ONLY should be 32bit efi_loader: handle EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS
include/efi.h | 18 ++++++++++-------- include/efi_variable.h | 2 +- lib/efi_loader/efi_var_common.c | 2 +- lib/efi_loader/efi_variable.c | 16 ++++++++++------ 4 files changed, 22 insertions(+), 16 deletions(-)
GetVariable() and SetVariable() use an uint32_t value for attributes. The UEFI specification defines the related constants as 32bit.
Add the missing EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS constant.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/efi.h | 15 ++++++++------- lib/efi_loader/efi_var_common.c | 2 +- lib/efi_loader/efi_variable.c | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/include/efi.h b/include/efi.h index f0e5faa7549..62cfb993d2e 100644 --- a/include/efi.h +++ b/include/efi.h @@ -492,13 +492,14 @@ extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[]; /* * Variable Attributes */ -#define EFI_VARIABLE_NON_VOLATILE 0x0000000000000001 -#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002 -#define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004 -#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008 -#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010 -#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0000000000000020 -#define EFI_VARIABLE_APPEND_WRITE 0x0000000000000040 +#define EFI_VARIABLE_NON_VOLATILE 0x00000001 +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 +#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008 +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010 +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020 +#define EFI_VARIABLE_APPEND_WRITE 0x00000040 +#define EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS 0x00000080
#define EFI_VARIABLE_MASK (EFI_VARIABLE_NON_VOLATILE | \ EFI_VARIABLE_BOOTSERVICE_ACCESS | \ diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index d528747f3fb..16b2c3d4882 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -99,7 +99,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, data_size, data);
/* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */ - if (attributes & ~(u32)EFI_VARIABLE_MASK) + if (attributes & ~EFI_VARIABLE_MASK) ret = EFI_INVALID_PARAMETER; else ret = efi_set_variable_int(variable_name, vendor, attributes, diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 40f7a0fb10d..efe17d98f5a 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -259,7 +259,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* check if a variable exists */ var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE); - attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE; + attributes &= ~EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes);
/* check attributes */ @@ -384,7 +384,7 @@ efi_status_t efi_query_variable_info_int(u32 attributes, EFI_VARIABLE_RUNTIME_ACCESS) return EFI_INVALID_PARAMETER;
- if (attributes & ~(u32)EFI_VARIABLE_MASK) + if (attributes & ~EFI_VARIABLE_MASK) return EFI_INVALID_PARAMETER;
*maximum_variable_storage_size = EFI_VAR_BUF_SIZE -
On Wed, 3 Apr 2024 at 18:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
GetVariable() and SetVariable() use an uint32_t value for attributes. The UEFI specification defines the related constants as 32bit.
Add the missing EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS constant.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/efi.h | 15 ++++++++------- lib/efi_loader/efi_var_common.c | 2 +- lib/efi_loader/efi_variable.c | 4 ++-- 3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/include/efi.h b/include/efi.h index f0e5faa7549..62cfb993d2e 100644 --- a/include/efi.h +++ b/include/efi.h @@ -492,13 +492,14 @@ extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[]; /*
- Variable Attributes
*/ -#define EFI_VARIABLE_NON_VOLATILE 0x0000000000000001 -#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x0000000000000002 -#define EFI_VARIABLE_RUNTIME_ACCESS 0x0000000000000004 -#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x0000000000000008 -#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x0000000000000010 -#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x0000000000000020 -#define EFI_VARIABLE_APPEND_WRITE 0x0000000000000040 +#define EFI_VARIABLE_NON_VOLATILE 0x00000001 +#define EFI_VARIABLE_BOOTSERVICE_ACCESS 0x00000002 +#define EFI_VARIABLE_RUNTIME_ACCESS 0x00000004 +#define EFI_VARIABLE_HARDWARE_ERROR_RECORD 0x00000008 +#define EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS 0x00000010 +#define EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS 0x00000020 +#define EFI_VARIABLE_APPEND_WRITE 0x00000040 +#define EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS 0x00000080
#define EFI_VARIABLE_MASK (EFI_VARIABLE_NON_VOLATILE | \ EFI_VARIABLE_BOOTSERVICE_ACCESS | \ diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index d528747f3fb..16b2c3d4882 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -99,7 +99,7 @@ efi_status_t EFIAPI efi_set_variable(u16 *variable_name, data_size, data);
/* Make sure that the EFI_VARIABLE_READ_ONLY flag is not set */
if (attributes & ~(u32)EFI_VARIABLE_MASK)
if (attributes & ~EFI_VARIABLE_MASK) ret = EFI_INVALID_PARAMETER; else ret = efi_set_variable_int(variable_name, vendor, attributes,diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 40f7a0fb10d..efe17d98f5a 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -259,7 +259,7 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* check if a variable exists */ var = efi_var_mem_find(vendor, variable_name, NULL); append = !!(attributes & EFI_VARIABLE_APPEND_WRITE);
attributes &= ~(u32)EFI_VARIABLE_APPEND_WRITE;
attributes &= ~EFI_VARIABLE_APPEND_WRITE; delete = !append && (!data_size || !attributes); /* check attributes */@@ -384,7 +384,7 @@ efi_status_t efi_query_variable_info_int(u32 attributes, EFI_VARIABLE_RUNTIME_ACCESS) return EFI_INVALID_PARAMETER;
if (attributes & ~(u32)EFI_VARIABLE_MASK)
if (attributes & ~EFI_VARIABLE_MASK) return EFI_INVALID_PARAMETER; *maximum_variable_storage_size = EFI_VAR_BUF_SIZE --- 2.43.0
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
GetVariable() and SetVariable() only accept a 32bit value for attributes. It makes not sense to define EFI_VARIABLE_READ_ONLY as unsigned long.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/efi_variable.h | 2 +- lib/efi_loader/efi_variable.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h index 805e6c5f1e0..42a2b7c52be 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -8,7 +8,7 @@
#include <linux/bitops.h>
-#define EFI_VARIABLE_READ_ONLY BIT(31) +#define EFI_VARIABLE_READ_ONLY 0x80000000
enum efi_auth_var_type { EFI_AUTH_VAR_NONE = 0, diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index efe17d98f5a..48ad813d79b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -276,8 +276,8 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* attributes won't be changed */ if (!delete && ((ro_check && var->attr != attributes) || - (!ro_check && ((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY) - != (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) { + (!ro_check && ((var->attr & ~EFI_VARIABLE_READ_ONLY) + != (attributes & ~EFI_VARIABLE_READ_ONLY))))) { return EFI_INVALID_PARAMETER; } time = var->time;
On Wed, 3 Apr 2024 at 18:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
GetVariable() and SetVariable() only accept a 32bit value for attributes. It makes not sense to define EFI_VARIABLE_READ_ONLY as unsigned long.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/efi_variable.h | 2 +- lib/efi_loader/efi_variable.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/efi_variable.h b/include/efi_variable.h index 805e6c5f1e0..42a2b7c52be 100644 --- a/include/efi_variable.h +++ b/include/efi_variable.h @@ -8,7 +8,7 @@
#include <linux/bitops.h>
-#define EFI_VARIABLE_READ_ONLY BIT(31) +#define EFI_VARIABLE_READ_ONLY 0x80000000
enum efi_auth_var_type { EFI_AUTH_VAR_NONE = 0, diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index efe17d98f5a..48ad813d79b 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -276,8 +276,8 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, /* attributes won't be changed */ if (!delete && ((ro_check && var->attr != attributes) ||
(!ro_check && ((var->attr & ~(u32)EFI_VARIABLE_READ_ONLY)!= (attributes & ~(u32)EFI_VARIABLE_READ_ONLY))))) {
(!ro_check && ((var->attr & ~EFI_VARIABLE_READ_ONLY)!= (attributes & ~EFI_VARIABLE_READ_ONLY))))) { return EFI_INVALID_PARAMETER; } time = var->time;-- 2.43.0
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
We don't yet support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS for file based variables, but we should pass it to TEE based variable stores.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/efi.h | 3 ++- lib/efi_loader/efi_variable.c | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/efi.h b/include/efi.h index 62cfb993d2e..c3c4b93f860 100644 --- a/include/efi.h +++ b/include/efi.h @@ -507,7 +507,8 @@ extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[]; EFI_VARIABLE_HARDWARE_ERROR_RECORD | \ EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \ EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \ - EFI_VARIABLE_APPEND_WRITE) + EFI_VARIABLE_APPEND_WRITE | \ + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)
/** * efi_get_priv() - Get access to the EFI-private information diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 48ad813d79b..e09a5e7ccb2 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -235,8 +235,12 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, if (data_size && !data) return EFI_INVALID_PARAMETER;
- /* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */ - if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS) + /* + * EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated. + * We don't support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS. + */ + if (attributes & (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \ + EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)) return EFI_UNSUPPORTED;
/* Make sure if runtime bit is set, boot service bit is set also */
On Wed, 3 Apr 2024 at 18:34, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:
We don't yet support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS for file based variables, but we should pass it to TEE based variable stores.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
include/efi.h | 3 ++- lib/efi_loader/efi_variable.c | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/include/efi.h b/include/efi.h index 62cfb993d2e..c3c4b93f860 100644 --- a/include/efi.h +++ b/include/efi.h @@ -507,7 +507,8 @@ extern char _binary_u_boot_bin_start[], _binary_u_boot_bin_end[]; EFI_VARIABLE_HARDWARE_ERROR_RECORD | \ EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \ EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS | \
EFI_VARIABLE_APPEND_WRITE)
EFI_VARIABLE_APPEND_WRITE | \EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)/**
- efi_get_priv() - Get access to the EFI-private information
diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index 48ad813d79b..e09a5e7ccb2 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -235,8 +235,12 @@ efi_status_t efi_set_variable_int(const u16 *variable_name, if (data_size && !data) return EFI_INVALID_PARAMETER;
/* EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated */if (attributes & EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS)
/** EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS is deprecated.* We don't support EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS.*/if (attributes & (EFI_VARIABLE_AUTHENTICATED_WRITE_ACCESS | \EFI_VARIABLE_ENHANCED_AUTHENTICATED_ACCESS)) return EFI_UNSUPPORTED; /* Make sure if runtime bit is set, boot service bit is set also */-- 2.43.0
Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas