Le mar. 15 août 2023, 14:00, Laszlo Ersek <ler...@redhat.com> a écrit :
> On 8/15/23 10:17, Ard Biesheuvel wrote:
> > On Tue, 15 Aug 2023 at 05:42, Vladimir 'phcoder' Serbinenko
> > <phco...@gmail.com> wrote:
> >>>
> >>> .
> >>>
> >>> Ard thinks that a 64 bit alignment for EFI GUIDs is a mistake - see [1]
> >>> and [2]. Hence Linux uses `__aligned(__alignof__(u32))` for
> "efi_guid_t"
> >>> since 2019.
> >>
> >>
> >> My patch presents a reliability paradigm: accept all inputs, produce
> outputs that are always aligned to 8-byte. It can make sense even though I
> agree that requiring 8-byte alignment for UUID is weird. Yet it's difficult
> to know if some obscure firmware does rely on it in some one in a thousand
> edge case
> >
> > +1
> >
> > The Linux side change ensures that the OS does not present misaligned
> > GUIDs to the firmware.
> >
> > The problem with 8-byte alignment is that it deviates from EDK2, and
> > could result in struct layout changes due to padding.
See my audit in previous message
> > Given that EDK2
> > uses 32-bit alignment only, some of the struct types it defines might
> > suddenly need the __packed attribute on 32-bit builds if the alignment
> > of the GUID type is increased to 64 bits.
> >
> > E.g.,
> >
> > struct {
> > void *
> > guid
> > }
> >
> > will have no padding in 32-bit EDK2 based firmware builds, even if the
> > spec claims that GUIDs are 64-bit aligned.
> >
Yes, this is weird but in case of reference implementation contradicting
spec, then reference implementation takes priority in practice.
>
> > So the lowest risk change for Linux was to increase to 32-bit
> > alignment, as it fixes any potential issues with misaligned
> > load-multiple instructions on ARM, and other architectures don't care
> > that much about alignment anyway.
> >
>
> This is terrible.
>
> First, the Itanium firmware in question clearly enforces the 8-byte
> alignment. (BTW, have we checked Itanium with a 4-byte but not 8-byte
> alignment?)
No, we didn't AFAIK
> specifies a binding for Itanium. And, we're having this discussion in
> the first place because of Itanium. So we need to make up our minds
> about adhering to the 8-byte alignment or not.
>
I'm biased towards allocating at 8-byte alignment but accepting any
alignment
>
> Second, the fact that edk2 does not align EFI_GUID to 8 bytes, contrary
> to the spec, is absolutely terrible. I don't understand then how it
> worked on Itanium before Itanium support was killed.
>
No members in guid have uint64_t type so most likely only 32-bit alignment
was used in most cases. 64-bit alignment allows accessing guid as 2
uint64_t's but I doubt it was ever widely used. A rare use might still
happen though.
>
> What I can imagine is the following situation:
> - the UEFI spec does not match reality (the reference implementation),
> and the actually required alignment is 4 bytes only
> - Itanium is happy with the 4 bytes alignment too (IIUC grub's malloc
> aligns at 1 byte by default, so the 4-byte alignment could still be
> violated in practice)
>
grub-malloc is 16-byte aligned.
The problem is that stack and static allocations are aligned only to type
alignment.
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
