Hi Heinrich,

On Mon, 31 Jul 2023 at 15:11, Heinrich Schuchardt heinrich.schuchardt@canonical.com wrote:

If the hard coded array hash_algo_list[] contains an entry for an unsupported algorithm, we should not leak resources new_efi and regs.

We should still extend the log with the digests for the supported algorithms and not write any message.

The same holds true of tcg2_create_digest(): just continue in case hash_algo_list[] contains an unsupported entry.

Fixes: 163a0d7e2cbd ("efi_loader: add PE/COFF image measurement") Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com

---

v2: drop the error message fix tcg2_create_digest() too

---

lib/efi_loader/efi_tcg2.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/lib/efi_loader/efi_tcg2.c b/lib/efi_loader/efi_tcg2.c index 49f8a5e77c..7b7926a0d4 100644 --- a/lib/efi_loader/efi_tcg2.c +++ b/lib/efi_loader/efi_tcg2.c @@ -706,8 +706,7 @@ static efi_status_t tcg2_create_digest(const u8 *input, u32 length, sha512_finish(&ctx_512, final); break; default:

•                   EFI_PRINT("Unsupported algorithm %x\n", hash_alg);
  

•                   return EFI_INVALID_PARAMETER;
  

•                   continue;
            }
            digest_list->digests[digest_list->count].hash_alg = hash_alg;
            memcpy(&digest_list->digests[digest_list->count].digest, final,
  

@@ -930,8 +929,7 @@ static efi_status_t tcg2_hash_pe_image(void *efi, u64 efi_size, hash_calculate("sha512", regs->reg, regs->num, hash); break; default:

•                   EFI_PRINT("Unsupported algorithm %x\n", hash_alg);
  

•                   return EFI_INVALID_PARAMETER;
  

•                   continue;
            }
            digest_list->digests[digest_list->count].hash_alg = hash_alg;
            memcpy(&digest_list->digests[digest_list->count].digest, hash,
  

-- 2.40.1

I am fine with the change. It's worth noting that this will only happen if we ever add an invalid algorithm to hash_algo_list[]. But the algorithms are described in the spec, so we should never end up in that case regardless. Can you pick this up via the EFI tree?

Reviewed-by: Ilias Apalodimas ilias.apalodimas@linaro.org