[PATCH v2 2/7] efi: add a helper to generate dynamic UUIDs

[Caleb Connolly]

On 30/05/2024 16:56, Ilias Apalodimas wrote:
> Hi Caleb,

Hi Ilias,
> 
> [...]
> 
>>
>>   #include <crypto/pkcs7.h>
>>   #include <crypto/pkcs7_parser.h>
>>   #include <linux/err.h>
>> diff --git a/lib/efi_loader/efi_firmware.c b/lib/efi_loader/efi_firmware.c
>> index ba5aba098c0f..a8dafe4f01a5 100644
>> --- a/lib/efi_loader/efi_firmware.c
>> +++ b/lib/efi_loader/efi_firmware.c
>> @@ -244,8 +244,71 @@ void efi_firmware_fill_version_info(struct efi_firmware_image_descriptor *image_
>>
>>          free(var_state);
>>   }
>>
>> +#if CONFIG_IS_ENABLED(EFI_CAPSULE_DYNAMIC_UUIDS)
>> +/**
>> + * efi_capsule_update_info_gen_ids - generate GUIDs for the images
>> + *
>> + * Generate the image_type_id for each image in the update_info.images array
>> + * using the first compatible from the device tree and a salt
>> + * UUID defined at build time.
>> + *
>> + * Returns:            status code
>> + */
>> +static efi_status_t efi_capsule_update_info_gen_ids(void)
>> +{
>> +       int ret, i;
>> +       struct uuid namespace;
>> +       const char *compatible; /* Full array including null bytes */
>> +       struct efi_fw_image *fw_array;
>> +
>> +       fw_array = update_info.images;
>> +       /* Check if we need to run (there are images and we didn't already generate their IDs) */
>> +       if (!update_info.num_images ||
>> +           memchr_inv(&fw_array[0].image_type_id, 0, sizeof(fw_array[0].image_type_id)))
> 
> Why not just go a guidcmp()? memchr_inv() will return the first
> invalid match, but we don't need that

guidcmp() would require allocating a zero guid to compare to, this is 
just a check for any non-zero byte in the GUID so memchr_inv() seemed 
more fitting.

I can switch to guidcmp() for readability.
> 
>> +               return EFI_SUCCESS;
>> +
>> +       ret = uuid_str_to_bin(CONFIG_EFI_CAPSULE_NAMESPACE_UUID,
>> +                       (unsigned char *)&namespace, UUID_STR_FORMAT_GUID);
>> +       if (ret) {
>> +               log_debug("%s: CONFIG_EFI_CAPSULE_NAMESPACE_UUID is invalid: %d\n", __func__, ret);
>> +               return EFI_UNSUPPORTED;
>> +       }
>> +
>> +       compatible = ofnode_read_string(ofnode_root(), "compatible");
>> +
>> +       if (!compatible) {
>> +               log_debug("%s: model or compatible not defined\n", __func__);
>> +               return EFI_UNSUPPORTED;
>> +       }
>> +
>> +       if (!update_info.num_images) {
>> +               log_debug("%s: no fw_images, make sure update_info.num_images is set\n", __func__);
>> +               return -ENODATA;
>> +       }
> 
> [...]
> 
> Cheers
> /Ilias

-- 
// Caleb (they/them)