diff mbox series

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

Message ID 20240529-b4-dynamic-uuid-v2-2-c26f31057bbe@linaro.org
State Superseded
Headers show
Series efi: CapsuleUpdate: support for dynamic UUIDs | expand

Commit Message

Caleb Connolly May 29, 2024, 2:48 p.m. UTC
Introduce a new helper efi_capsule_update_info_gen_ids() which populates
the capsule update fw images image_type_id field. This allows for
determinstic UUIDs to be used that can scale to a large number of
different boards and board variants without the need to maintain a big
list.

We call this from efi_fill_image_desc_array() to populate the UUIDs
lazily on-demand.

This is behind an additional config option as it depends on V5 UUIDs and
the SHA1 implementation.

Signed-off-by: Caleb Connolly <caleb.connolly@linaro.org>
---
 lib/efi_loader/Kconfig        | 23 +++++++++++++++
 lib/efi_loader/efi_capsule.c  |  1 +
 lib/efi_loader/efi_firmware.c | 66 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 90 insertions(+)

Comments

Ilias Apalodimas May 30, 2024, 2:56 p.m. UTC | #1
Hi Caleb,

[...]

>
>  #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

> +               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 Connolly May 31, 2024, 11:26 a.m. UTC | #2
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
Ilias Apalodimas May 31, 2024, 11:48 a.m. UTC | #3
On Fri, 31 May 2024 at 14:26, Caleb Connolly <caleb.connolly@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)
diff mbox series

Patch

diff --git a/lib/efi_loader/Kconfig b/lib/efi_loader/Kconfig
index 430bb7f0f7dc..e90caf4f8e14 100644
--- a/lib/efi_loader/Kconfig
+++ b/lib/efi_loader/Kconfig
@@ -235,8 +235,31 @@  config EFI_CAPSULE_ON_DISK_EARLY
 	  If this option is enabled, capsules will be enforced to be
 	  executed as part of U-Boot initialisation so that they will
 	  surely take place whatever is set to distro_bootcmd.
 
+config EFI_CAPSULE_DYNAMIC_UUIDS
+	bool "Dynamic UUIDs for capsules"
+	depends on EFI_HAVE_CAPSULE_SUPPORT
+	select UUID_GEN_V5
+	help
+	  Select this option if you want to use dynamically generated v5
+	  UUIDs for your board. To make use of this feature, your board
+	  code should call efi_capsule_update_info_gen_ids() with a seed
+	  UUID to generate the image_type_id field for each fw_image.
+
+	  The CapsuleUpdate payloads are expected to generate matching UUIDs
+	  using the same scheme.
+
+config EFI_CAPSULE_NAMESPACE_UUID
+	string "Namespace UUID for dynamic UUIDs"
+	depends on EFI_CAPSULE_DYNAMIC_UUIDS
+	help
+	  Define the namespace or "salt" UUID used to generate the per-image
+	  UUIDs. This should be a UUID in the standard 8-4-4-4-12 format.
+
+	  Device vendors are expected to generate their own namespace UUID
+	  to avoid conflicts with existing products.
+
 config EFI_CAPSULE_FIRMWARE
 	bool
 
 config EFI_CAPSULE_FIRMWARE_MANAGEMENT
diff --git a/lib/efi_loader/efi_capsule.c b/lib/efi_loader/efi_capsule.c
index 0937800e588f..ac02e79ae7d8 100644
--- a/lib/efi_loader/efi_capsule.c
+++ b/lib/efi_loader/efi_capsule.c
@@ -19,8 +19,9 @@ 
 #include <mapmem.h>
 #include <sort.h>
 #include <sysreset.h>
 #include <asm/global_data.h>
+#include <uuid.h>
 
 #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)))
+		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;
+	}
+
+	for (i = 0; i < update_info.num_images; i++) {
+		gen_uuid_v5(&namespace,
+			    (struct uuid *)&fw_array[i].image_type_id,
+			    compatible, strlen(compatible),
+			    fw_array[i].fw_name, u16_strsize(fw_array[i].fw_name)
+				- sizeof(uint16_t),
+			    NULL);
+
+		log_debug("Image %ls UUID %pUs\n", fw_array[i].fw_name,
+			  &fw_array[i].image_type_id);
+	}
+
+	return EFI_SUCCESS;
+}
+#else
+static efi_status_t efi_capsule_update_info_gen_ids(void)
+{
+	return EFI_SUCCESS;
+}
+#endif
+
 /**
  * efi_fill_image_desc_array - populate image descriptor array
  * @image_info_size:		Size of @image_info
  * @image_info:			Image information
@@ -282,8 +345,11 @@  static efi_status_t efi_fill_image_desc_array(
 		return EFI_BUFFER_TOO_SMALL;
 	}
 	*image_info_size = total_size;
 
+	if (efi_capsule_update_info_gen_ids() != EFI_SUCCESS)
+		return EFI_UNSUPPORTED;
+
 	fw_array = update_info.images;
 	*descriptor_count = update_info.num_images;
 	*descriptor_version = EFI_FIRMWARE_IMAGE_DESCRIPTOR_VERSION;
 	*descriptor_size = sizeof(*image_info);