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

[Ilias Apalodimas]

On Fri, 31 May 2024 at 14:26, Caleb Connolly <caleb.connolly at linaro.org> wrote:
>
>
>
> 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.

Right, I misread what memchr_inv() does, keep it as-is it's fine

Thanks
/Ilias
> >
> >> +               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)