On 5/29/20 8:41 AM, AKASHI Takahiro wrote:
> There are a couple of occurrences of hash calculations in which a new
> efi_hash_regions will be commonly used.
>
> Signed-off-by: AKASHI Takahiro <takahiro.aka...@linaro.org>
> ---
> lib/efi_loader/efi_signature.c | 44 +++++++++++++---------------------
> 1 file changed, 16 insertions(+), 28 deletions(-)
>
> diff --git a/lib/efi_loader/efi_signature.c b/lib/efi_loader/efi_signature.c
> index 35f678de057e..00e442783059 100644
> --- a/lib/efi_loader/efi_signature.c
> +++ b/lib/efi_loader/efi_signature.c
> @@ -30,6 +30,7 @@ const efi_guid_t efi_guid_cert_type_pkcs7 =
> EFI_CERT_TYPE_PKCS7_GUID;
> /**
> * efi_hash_regions - calculate a hash value
> * @regs: List of regions
> + * @count: Number of regions
> * @hash: Pointer to a pointer to buffer holding a hash value
> * @size: Size of buffer to be returned
> *
> @@ -37,18 +38,20 @@ const efi_guid_t efi_guid_cert_type_pkcs7 =
> EFI_CERT_TYPE_PKCS7_GUID;
> *
> * Return: true on success, false on error
> */
> -static bool efi_hash_regions(struct efi_image_regions *regs, void **hash,
> - size_t *size)
> +static bool efi_hash_regions(struct image_region *regs, int count,
> + void **hash, size_t *size)
What is this size parameter good for if we know all signatures are
SHA256? Should it be eliminiated?
Otherwise
Reviewed-by: Heinrich Schuchardt <xypron.g...@gmx.de>
> {
> - *size = 0;
> - *hash = calloc(1, SHA256_SUM_LEN);
> if (!*hash) {
> - debug("Out of memory\n");
> - return false;
> + *hash = calloc(1, SHA256_SUM_LEN);
> + if (!*hash) {
> + debug("Out of memory\n");
> + return false;
> + }
> }
> - *size = SHA256_SUM_LEN;
> + if (size)
> + *size = SHA256_SUM_LEN;
>
> - hash_calculate("sha256", regs->reg, regs->num, *hash);
> + hash_calculate("sha256", regs, count, *hash);
> #ifdef DEBUG
> debug("hash calculated:\n");
> print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> @@ -73,26 +76,10 @@ static bool efi_hash_msg_content(struct pkcs7_message
> *msg, void **hash,
> {
> struct image_region regtmp;
>
> - *size = 0;
> - *hash = calloc(1, SHA256_SUM_LEN);
> - if (!*hash) {
> - debug("Out of memory\n");
> - free(msg);
> - return false;
> - }
> - *size = SHA256_SUM_LEN;
> -
> regtmp.data = msg->data;
> regtmp.size = msg->data_len;
>
> - hash_calculate("sha256", ®tmp, 1, *hash);
> -#ifdef DEBUG
> - debug("hash calculated based on contentInfo:\n");
> - print_hex_dump(" ", DUMP_PREFIX_OFFSET, 16, 1,
> - *hash, SHA256_SUM_LEN, false);
> -#endif
> -
> - return true;
> + return efi_hash_regions(®tmp, 1, hash, size);
> }
>
> /**
> @@ -168,9 +155,10 @@ static bool efi_signature_verify(struct
> efi_image_regions *regs,
> false);
> #endif
> /* against contentInfo first */
> + hash = NULL;
> if ((msg->data && efi_hash_msg_content(msg, &hash, &size)) ||
> /* for signed image */
> - efi_hash_regions(regs, &hash, &size)) {
> + efi_hash_regions(regs->reg, regs->num, &hash, &size)) {
> /* for authenticated variable */
> if (ps_info->msgdigest_len != size ||
> memcmp(hash, ps_info->msgdigest, size)) {
> @@ -238,7 +226,7 @@ bool efi_signature_verify_with_list(struct
> efi_image_regions *regs,
> regs, signed_info, siglist, valid_cert);
>
> if (!signed_info) {
> - void *hash;
> + void *hash = NULL;
> size_t size;
>
> debug("%s: unsigned image\n", __func__);
> @@ -252,7 +240,7 @@ bool efi_signature_verify_with_list(struct
> efi_image_regions *regs,
> goto out;
> }
>
> - if (!efi_hash_regions(regs, &hash, &size)) {
> + if (!efi_hash_regions(regs->reg, regs->num, &hash, &size)) {
> debug("Digesting unsigned image failed\n");
> goto out;
> }
>
