| Message ID | 20200529064130.28332-6-takahiro.akashi@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | efi_loader: rework/improve UEFI secure boot code | expand |
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.akashi at 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.glpk at 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; > } >
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) { - *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; }
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.akashi at linaro.org> --- lib/efi_loader/efi_signature.c | 44 +++++++++++++--------------------- 1 file changed, 16 insertions(+), 28 deletions(-)