[PATCH v3 0/7] efi: CapsuleUpdate: support for dynamic UUIDs

[Vincent Stehlé]

On Fri, Jun 21, 2024 at 07:11:28PM +0200, Caleb Connolly wrote:
> On Fri, 21 Jun 2024, 19:06 Vincent Stehlé, <vincent.stehle at arm.com> wrote:
> 
> > On Fri, Jun 21, 2024 at 01:00:51PM +0200, Heinrich Schuchardt wrote:
> > (..)
> > > The current specification is in RFC 9562, 4.1, "Variant field"
> > >
> > > "The variant field consists of a variable number of the most significant
> > > bits of octet 8 of the UUID.
> > >
> > > ...
> > >
> > > Specifically for UUIDs in this document, bits 64 and 65 of the UUID
> > > (bits 0 and 1 of octet 8) MUST be set to 1 and 0 as specified in row 2
> > > of Table 1."
> > >
> > > This reference to byte 8 does not depend on endianness.
> >
> > Hi Heinrich,
> >
> > Agreed, variant is not concerned by the endianness.
> >
> > >
> > > U-Boot's include/uuid.h has:
> > >
> > >     /* This is structure is in big-endian */
> > >     struct uuid {
> > >
> > > The field time_hi_and_version needs to be stored in big-endian fashion.
> >
> > Thanks! I thought this structure was used to hold either a big-endian UUID
> > or a
> > little-endian GUID, but now you have convinced me.
> >
> > This confirms that the generation of the dynamic GUID is missing something:
> >
> >     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);
> >
> > It is not possible to cast the little-endian efi_guid_t .image_type_id as
> > the
> > big-endian struct uuid output of gen_uuid_v5() like this; we need to
> > convert the
> > three time fields from big to little endianness.
> >
> 
> I'm inclined to disagree, the comment above struct uuid in include/uuid.h
> state clearly that the format in memory is always big endian, but that a
> GUID when printed has some fields converted to little endian.

Hi Caleb,

I read the comments above struct uuid differently: the GUID has some fields
little-endian when stored in memory (and can thus not be stored in a struct
uuid after Heinrich's comments).

This is consistent with how it is done in U-Boot in various locations; for
example, the EFI_GUID() macro stores a GUID with its time fields little-endian
in memory. Similarly, the callers of uuid_str_to_bin() or uuid_bin_to_str() with
format UUID_STR_FORMAT_GUID have indeed a little-endian GUID in memory (most
often an efi_guid_t). This is also consistent with the UEFI specification's
16-byte buffer format. [1]

When you have a big-endian UUID in memory, the version field is stored in byte
6, which is consistent with the RFC 9562. [2]
If you convert this big-endian UUID with uuid_bin_to_str() and
UUID_STR_FORMAT_GUID as in genguid, the "time high and version" field's bytes
will be printed with byte 7 first and then byte 6, as per guid_char_order[].

This is in contradiction with the RFC, which shows that the version field ("M")
should be printed first.

If you print the UUID with format 'STD, you will indeed have byte 6 containing
the version field printed first as it should (uuid_char_order[]).

This is confirmed by "uuid -d".

Best regards,
Vincent.

[1] https://uefi.org/specs/UEFI/2.10/Apx_A_GUID_and_Time_Formats.html
[2] https://www.rfc-editor.org/rfc/rfc9562

> 
> 
> > >
> > >
> > > tools/genguid uses UUID_STR_FORMAT_GUID which prints low-endian which is
> > > typical in the EFI context but not understood by 'uuid -d'. Maybe we
> > > should add a parameter for the output format.
> >
> > My understanding is that there is a single universal string format for both
> > UUIDs and GUIDs, which uuid -d understands, and which has no notion of
> > endianness. (Only the structures in memory have an endianness.)
> > This means we do not need an output format parameter.
> >
> > genguid is calling the new gen_uuid_v5() function, which outputs a
> > big-endian
> > struct uuid. Therefore, genguid should print it with 'STD format, not
> > 'GUID.
> >
> > Best regards,
> > Vincent.
> >
> > >
> > > Best regards
> > >
> > > Heinrich
> >