Re: [PATCH v2] efi: use 32-bit alignment for efi_guid_t literals

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Mar 18, 2021 at 07:20:46PM +0100, Ard Biesheuvel wrote:
> Commit 494c704f9af0 ("efi: Use 32-bit alignment for efi_guid_t") updated
> the type definition of efi_guid_t to ensure that it always appears
> sufficiently aligned (the UEFI spec is ambiguous about this, but given
> the fact that its EFI_GUID type is defined in terms of a struct carrying
> a uint32_t, the natural alignment is definitely >= 32 bits).
> 
> However, we missed the EFI_GUID() macro which is used to instantiate
> efi_guid_t literals: that macro is still based on the guid_t type,
> which does not have a minimum alignment at all. This results in warnings
> such as
> 
>   In file included from drivers/firmware/efi/mokvar-table.c:35:
>   include/linux/efi.h:1093:34: warning: passing 1-byte aligned argument to
>       4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer
>       access [-Walign-mismatch]
>           status = get_var(L"SecureBoot", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size,
>                                           ^
>   include/linux/efi.h:1101:24: warning: passing 1-byte aligned argument to
>       4-byte aligned parameter 2 of 'get_var' may result in an unaligned pointer
>       access [-Walign-mismatch]
>           get_var(L"SetupMode", &EFI_GLOBAL_VARIABLE_GUID, NULL, &size, &setupmode);
> 
> The distinction only matters on CPUs that do not support misaligned loads
> fully, but 32-bit ARM's load-multiple instructions fall into that category,
> and these are likely to be emitted by the compiler that built the firmware
> for loading word-aligned 128-bit GUIDs from memory
> 
> So re-implement the initializer in terms of our own efi_guid_t type, so that
> the alignment becomes a property of the literal's type.
> 
> Reported-by: Nathan Chancellor <nathan@xxxxxxxxxx>
> Signed-off-by: Ard Biesheuvel <ardb@xxxxxxxxxx>

Reviewed-by: Nathan Chancellor <nathan@xxxxxxxxxx>
Tested-by: Nathan Chancellor <nathan@xxxxxxxxxx>

> ---
>  include/linux/efi.h | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/efi.h b/include/linux/efi.h
> index 8710f5710c1d..6b5d36babfcc 100644
> --- a/include/linux/efi.h
> +++ b/include/linux/efi.h
> @@ -72,8 +72,10 @@ typedef void *efi_handle_t;
>   */
>  typedef guid_t efi_guid_t __aligned(__alignof__(u32));
>  
> -#define EFI_GUID(a,b,c,d0,d1,d2,d3,d4,d5,d6,d7) \
> -	GUID_INIT(a, b, c, d0, d1, d2, d3, d4, d5, d6, d7)
> +#define EFI_GUID(a, b, c, d...) (efi_guid_t){ {					\
> +	(a) & 0xff, ((a) >> 8) & 0xff, ((a) >> 16) & 0xff, ((a) >> 24) & 0xff,	\
> +	(b) & 0xff, ((b) >> 8) & 0xff,						\
> +	(c) & 0xff, ((c) >> 8) & 0xff, d } }
>  
>  /*
>   * Generic EFI table header
> -- 
> 2.30.2
> 



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux