From 1ea133acd64eb0099865b0649b1d039ef63787ee Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 29 Aug 2021 11:52:44 +0200 Subject: efi_loader: sections with zero VirtualSize In a section header VirtualSize may be zero. This is for instance seen in the .sbat section of shim. In this case use SizeOfRawData as section size. Fixes: 9d30a941cce5 ("efi_loader: don't load beyond VirtualSize") Signed-off-by: Heinrich Schuchardt Reviewed-by: Asherah Connor --- lib/efi_loader/efi_image_loader.c | 31 +++++++++++++++++++++++++++---- 1 file changed, 27 insertions(+), 4 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index a0eb63fcebe..838e3a7f021 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -800,6 +800,23 @@ efi_status_t efi_check_pe(void *buffer, size_t size, void **nt_header) return EFI_SUCCESS; } +/** + * section_size() - determine size of section + * + * The size of a section in memory if normally given by VirtualSize. + * If VirtualSize is not provided, use SizeOfRawData. + * + * @sec: section header + * Return: size of section in memory + */ +static u32 section_size(IMAGE_SECTION_HEADER *sec) +{ + if (sec->Misc.VirtualSize) + return sec->Misc.VirtualSize; + else + return sec->SizeOfRawData; +} + /** * efi_load_pe() - relocate EFI binary * @@ -869,8 +886,9 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, /* Calculate upper virtual address boundary */ for (i = num_sections - 1; i >= 0; i--) { IMAGE_SECTION_HEADER *sec = §ions[i]; + virt_size = max_t(unsigned long, virt_size, - sec->VirtualAddress + sec->Misc.VirtualSize); + sec->VirtualAddress + section_size(sec)); } /* Read 32/64bit specific header bits */ @@ -931,11 +949,16 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, /* Load sections into RAM */ for (i = num_sections - 1; i >= 0; i--) { IMAGE_SECTION_HEADER *sec = §ions[i]; - memset(efi_reloc + sec->VirtualAddress, 0, - sec->Misc.VirtualSize); + u32 copy_size = section_size(sec); + + if (copy_size > sec->SizeOfRawData) { + copy_size = sec->SizeOfRawData; + memset(efi_reloc + sec->VirtualAddress, 0, + sec->Misc.VirtualSize); + } memcpy(efi_reloc + sec->VirtualAddress, efi + sec->PointerToRawData, - min(sec->Misc.VirtualSize, sec->SizeOfRawData)); + copy_size); } /* Run through relocations */ -- cgit v1.2.3 From f3a343d7339acf1d531e438e15fef3c7975cfdcf Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Sun, 29 Aug 2021 11:52:44 +0200 Subject: efi_loader: rounding of image size We should not first allocate memory and then report a rounded up value as image size. Instead first round up according to section allocation and then allocate the memory. Fixes: 82786754b9d2 ("efi_loader: ImageSize must be multiple of SectionAlignment") Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_image_loader.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_image_loader.c b/lib/efi_loader/efi_image_loader.c index 838e3a7f021..e9572d4d5db 100644 --- a/lib/efi_loader/efi_image_loader.c +++ b/lib/efi_loader/efi_image_loader.c @@ -898,6 +898,7 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, image_base = opt->ImageBase; efi_set_code_and_data_type(loaded_image_info, opt->Subsystem); handle->image_type = opt->Subsystem; + virt_size = ALIGN(virt_size, opt->SectionAlignment); efi_reloc = efi_alloc(virt_size, loaded_image_info->image_code_type); if (!efi_reloc) { @@ -908,12 +909,12 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, handle->entry = efi_reloc + opt->AddressOfEntryPoint; rel_size = opt->DataDirectory[rel_idx].Size; rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress; - virt_size = ALIGN(virt_size, opt->SectionAlignment); } else if (nt->OptionalHeader.Magic == IMAGE_NT_OPTIONAL_HDR32_MAGIC) { IMAGE_OPTIONAL_HEADER32 *opt = &nt->OptionalHeader; image_base = opt->ImageBase; efi_set_code_and_data_type(loaded_image_info, opt->Subsystem); handle->image_type = opt->Subsystem; + virt_size = ALIGN(virt_size, opt->SectionAlignment); efi_reloc = efi_alloc(virt_size, loaded_image_info->image_code_type); if (!efi_reloc) { @@ -924,7 +925,6 @@ efi_status_t efi_load_pe(struct efi_loaded_image_obj *handle, handle->entry = efi_reloc + opt->AddressOfEntryPoint; rel_size = opt->DataDirectory[rel_idx].Size; rel = efi_reloc + opt->DataDirectory[rel_idx].VirtualAddress; - virt_size = ALIGN(virt_size, opt->SectionAlignment); } else { log_err("Invalid optional header magic %x\n", nt->OptionalHeader.Magic); -- cgit v1.2.3 From 9ef82e29478c76f17b536f8f289fd0406067ab01 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Wed, 25 Aug 2021 19:13:24 +0200 Subject: efi_loader: don't load signature database from file The UEFI specification requires that the signature database may only be stored in tamper-resistant storage. So these variable may not be read from an unsigned file. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_var_common.c | 2 -- lib/efi_loader/efi_var_file.c | 41 +++++++++++++++++++++++++---------------- lib/efi_loader/efi_variable.c | 2 +- 3 files changed, 26 insertions(+), 19 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index 3d92afe2ebd..005c03ea5f8 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -32,10 +32,8 @@ static const struct efi_auth_var_name_type name_type[] = { {u"KEK", &efi_global_variable_guid, EFI_AUTH_VAR_KEK}, {u"db", &efi_guid_image_security_database, EFI_AUTH_VAR_DB}, {u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX}, - /* not used yet {u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT}, {u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR}, - */ }; static bool efi_secure_boot; diff --git a/lib/efi_loader/efi_var_file.c b/lib/efi_loader/efi_var_file.c index de076b8cbc4..c7c6805ed05 100644 --- a/lib/efi_loader/efi_var_file.c +++ b/lib/efi_loader/efi_var_file.c @@ -148,9 +148,10 @@ error: #endif } -efi_status_t efi_var_restore(struct efi_var_file *buf) +efi_status_t efi_var_restore(struct efi_var_file *buf, bool safe) { struct efi_var_entry *var, *last_var; + u16 *data; efi_status_t ret; if (buf->reserved || buf->magic != EFI_VAR_FILE_MAGIC || @@ -160,21 +161,29 @@ efi_status_t efi_var_restore(struct efi_var_file *buf) return EFI_INVALID_PARAMETER; } - var = buf->var; last_var = (struct efi_var_entry *)((u8 *)buf + buf->length); - while (var < last_var) { - u16 *data = var->name + u16_strlen(var->name) + 1; - - if (var->attr & EFI_VARIABLE_NON_VOLATILE && var->length) { - ret = efi_var_mem_ins(var->name, &var->guid, var->attr, - var->length, data, 0, NULL, - var->time); - if (ret != EFI_SUCCESS) - log_err("Failed to set EFI variable %ls\n", - var->name); - } - var = (struct efi_var_entry *) - ALIGN((uintptr_t)data + var->length, 8); + for (var = buf->var; var < last_var; + var = (struct efi_var_entry *) + ALIGN((uintptr_t)data + var->length, 8)) { + + data = var->name + u16_strlen(var->name) + 1; + + /* + * Secure boot related and non-volatile variables shall only be + * restored from U-Boot's preseed. + */ + if (!safe && + (efi_auth_var_get_type(var->name, &var->guid) != + EFI_AUTH_VAR_NONE || + !(var->attr & EFI_VARIABLE_NON_VOLATILE))) + continue; + if (!var->length) + continue; + ret = efi_var_mem_ins(var->name, &var->guid, var->attr, + var->length, data, 0, NULL, + var->time); + if (ret != EFI_SUCCESS) + log_err("Failed to set EFI variable %ls\n", var->name); } return EFI_SUCCESS; } @@ -213,7 +222,7 @@ efi_status_t efi_var_from_file(void) log_err("Failed to load EFI variables\n"); goto error; } - if (buf->length != len || efi_var_restore(buf) != EFI_SUCCESS) + if (buf->length != len || efi_var_restore(buf, false) != EFI_SUCCESS) log_err("Invalid EFI variables file\n"); error: free(buf); diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index ba0874e9e78..a7d305ffbc7 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -426,7 +426,7 @@ efi_status_t efi_init_variables(void) if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) { ret = efi_var_restore((struct efi_var_file *) - __efi_var_file_begin); + __efi_var_file_begin, true); if (ret != EFI_SUCCESS) log_err("Invalid EFI variable seed\n"); } -- cgit v1.2.3 From b191aa429e509ba6bf9eb446ae27b1a4fcd83276 Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 26 Aug 2021 04:30:24 +0200 Subject: efi_loader: efi_auth_var_type for AuditMode, DeployedMode Writing variables AuditMode and DeployedMode serves to switch between Secure Boot modes. Provide a separate value for these in efi_auth_var_type. With this patch the variables will not be read from from file even if they are marked as non-volatile by mistake. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_var_common.c | 2 ++ lib/efi_loader/efi_variable.c | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index 005c03ea5f8..c744e2fd910 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -34,6 +34,8 @@ static const struct efi_auth_var_name_type name_type[] = { {u"dbx", &efi_guid_image_security_database, EFI_AUTH_VAR_DBX}, {u"dbt", &efi_guid_image_security_database, EFI_AUTH_VAR_DBT}, {u"dbr", &efi_guid_image_security_database, EFI_AUTH_VAR_DBR}, + {u"AuditMode", &efi_global_variable_guid, EFI_AUTH_MODE}, + {u"DeployedMode", &efi_global_variable_guid, EFI_AUTH_MODE}, }; static bool efi_secure_boot; diff --git a/lib/efi_loader/efi_variable.c b/lib/efi_loader/efi_variable.c index a7d305ffbc7..fa2b6bc7a86 100644 --- a/lib/efi_loader/efi_variable.c +++ b/lib/efi_loader/efi_variable.c @@ -247,7 +247,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, return EFI_WRITE_PROTECTED; if (IS_ENABLED(CONFIG_EFI_VARIABLES_PRESEED)) { - if (var_type != EFI_AUTH_VAR_NONE) + if (var_type >= EFI_AUTH_VAR_PK) return EFI_WRITE_PROTECTED; } @@ -268,7 +268,7 @@ efi_status_t efi_set_variable_int(u16 *variable_name, const efi_guid_t *vendor, return EFI_NOT_FOUND; } - if (var_type != EFI_AUTH_VAR_NONE) { + if (var_type >= EFI_AUTH_VAR_PK) { /* authentication is mandatory */ if (!(attributes & EFI_VARIABLE_TIME_BASED_AUTHENTICATED_WRITE_ACCESS)) { -- cgit v1.2.3 From 7219856daee8cd28872d2f7ef7405704af07bd7d Mon Sep 17 00:00:00 2001 From: Heinrich Schuchardt Date: Thu, 2 Sep 2021 07:11:45 +0200 Subject: efi_loader: correct determination of secure boot state When U-Boot is started we have to use the existing variables to determine in which secure boot state we are. * If a platform key PK is present and DeployedMode=1, we are in deployed mode. * If no platform key PK is present and AuditMode=1, we are in audit mode. * Otherwise if a platform key is present, we are in user mode. * Otherwise if no platform key is present, we are in setup mode. Signed-off-by: Heinrich Schuchardt --- lib/efi_loader/efi_var_common.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_var_common.c b/lib/efi_loader/efi_var_common.c index c744e2fd910..a00bbf16206 100644 --- a/lib/efi_loader/efi_var_common.c +++ b/lib/efi_loader/efi_var_common.c @@ -314,17 +314,40 @@ err: efi_status_t efi_init_secure_state(void) { - enum efi_secure_mode mode = EFI_MODE_SETUP; + enum efi_secure_mode mode; u8 efi_vendor_keys = 0; - efi_uintn_t size = 0; + efi_uintn_t size; efi_status_t ret; - - ret = efi_get_variable_int(L"PK", &efi_global_variable_guid, - NULL, &size, NULL, NULL); - if (ret == EFI_BUFFER_TOO_SMALL) { - if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) - mode = EFI_MODE_USER; + u8 deployed_mode = 0; + u8 audit_mode = 0; + u8 setup_mode = 1; + + if (IS_ENABLED(CONFIG_EFI_SECURE_BOOT)) { + size = sizeof(deployed_mode); + ret = efi_get_variable_int(u"DeployedMode", &efi_global_variable_guid, + NULL, &size, &deployed_mode, NULL); + size = sizeof(audit_mode); + ret = efi_get_variable_int(u"AuditMode", &efi_global_variable_guid, + NULL, &size, &audit_mode, NULL); + size = 0; + ret = efi_get_variable_int(u"PK", &efi_global_variable_guid, + NULL, &size, NULL, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + setup_mode = 0; + audit_mode = 0; + } else { + setup_mode = 1; + deployed_mode = 0; + } } + if (deployed_mode) + mode = EFI_MODE_DEPLOYED; + else if (audit_mode) + mode = EFI_MODE_AUDIT; + else if (setup_mode) + mode = EFI_MODE_SETUP; + else + mode = EFI_MODE_USER; ret = efi_transfer_secure_state(mode); if (ret != EFI_SUCCESS) -- cgit v1.2.3 From 580d7242b14064f57a9fc392a2a2ce23e73b19e8 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 3 Sep 2021 10:55:50 +0900 Subject: efi_loader: add missing parameter check for EFI_TCG2_PROTOCOL api TCG EFI Protocol Specification defines the required parameter checking and return value for each API. This commit adds the missing parameter check and fixes the wrong return value to comply the specification. Signed-off-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_tcg2.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) (limited to 'lib') diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 35e69b91129..c4e9f61fd6d 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -708,6 +708,18 @@ efi_tcg2_get_eventlog(struct efi_tcg2_protocol *this, EFI_ENTRY("%p, %u, %p, %p, %p", this, log_format, event_log_location, event_log_last_entry, event_log_truncated); + if (!this || !event_log_location || !event_log_last_entry || + !event_log_truncated) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + + /* Only support TPMV2 */ + if (log_format != TCG2_EVENT_LOG_FORMAT_TCG_2) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + ret = platform_get_tpm2_device(&dev); if (ret != EFI_SUCCESS) { event_log_location = NULL; @@ -965,6 +977,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, data_to_hash_len, (void **)&nt); if (ret != EFI_SUCCESS) { log_err("Not a valid PE-COFF file\n"); + ret = EFI_UNSUPPORTED; goto out; } ret = tcg2_hash_pe_image((void *)(uintptr_t)data_to_hash, @@ -1038,9 +1051,15 @@ efi_tcg2_get_active_pcr_banks(struct efi_tcg2_protocol *this, { efi_status_t ret; + if (!this || !active_pcr_banks) { + ret = EFI_INVALID_PARAMETER; + goto out; + } + EFI_ENTRY("%p, %p", this, active_pcr_banks); ret = __get_active_pcr_banks(active_pcr_banks); +out: return EFI_EXIT(ret); } -- cgit v1.2.3 From 538c0f2d3798261161a28a05e445d0c85af56276 Mon Sep 17 00:00:00 2001 From: Masahisa Kojima Date: Fri, 3 Sep 2021 10:55:52 +0900 Subject: efi_loader: fix efi_tcg2_hash_log_extend_event() parameter check TCG EFI Protocol Specification defines that PCRIndex parameter passed from caller must be 0 to 23. TPM2_MAX_PCRS is currently used to check the range of PCRIndex, but TPM2_MAX_PCRS is tpm2 device dependent and may have larger value. This commit newly adds EFI_TCG2_MAX_PCR_INDEX macro, it is used to check the range of PCRIndex parameter. Signed-off-by: Masahisa Kojima Acked-by: Heinrich Schuchardt Reviewed-by: Ilias Apalodimas --- lib/efi_loader/efi_tcg2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'lib') diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index c4e9f61fd6d..b268a02976c 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -958,7 +958,7 @@ efi_tcg2_hash_log_extend_event(struct efi_tcg2_protocol *this, u64 flags, goto out; } - if (efi_tcg_event->header.pcr_index > TPM2_MAX_PCRS) { + if (efi_tcg_event->header.pcr_index > EFI_TCG2_MAX_PCR_INDEX) { ret = EFI_INVALID_PARAMETER; goto out; } -- cgit v1.2.3