From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.2 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,INCLUDES_CR_TRAILER,INCLUDES_PATCH,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1AB0FC433DB for ; Wed, 10 Mar 2021 08:34:26 +0000 (UTC) Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id A5DF764FDE for ; Wed, 10 Mar 2021 08:34:25 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org A5DF764FDE Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding :Content-Type:List-Subscribe:List-Help:List-Post:List-Archive: List-Unsubscribe:List-Id:Cc:To:Subject:Message-ID:Date:From:In-Reply-To: References:MIME-Version:Reply-To:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Owner; bh=MbekuEWC5CLABOBL40EjsGjqIkhbsr58eRN7meLNaZk=; b=flhx/jZrXhjJ65R1/s9VmOQSX qAt5fqLGXJEPrJfQp3WyoZuhgXYlHLFX69H/ObOsmm4w9T3Ceb+vwypLl3vO7M7lfnMe6s9nLI3KQ xDHiyWuNdS2KS8/ZpCP9r9bv0/DWmbgo+elSCsRtc9CG+U2NdrqwwaUIPDkOyMGuzEo4iB8XzScFd TM4oH1c07fkb/UJQk8OiuOZPmy97Lq0RnP/PP0xqso7no0pM5fywA5a0K8Z4M5S9k1yLFil/g021b 7XMi5eTTCxEYRpcq4Lm8WGmjAejBbrw7G8RyZOHHsMevMHwPY3TAoStBahJNZt4yq5yBXAqXGZD/F iTXun6+Ww==; Received: from localhost ([::1] helo=desiato.infradead.org) by desiato.infradead.org with esmtp (Exim 4.94 #2 (Red Hat Linux)) id 1lJuGo-006Is1-Gd; Wed, 10 Mar 2021 08:32:42 +0000 Received: from mail.kernel.org ([198.145.29.99]) by desiato.infradead.org with esmtps (Exim 4.94 #2 (Red Hat Linux)) id 1lJuGj-006Iqk-7k for linux-arm-kernel@lists.infradead.org; Wed, 10 Mar 2021 08:32:40 +0000 Received: by mail.kernel.org (Postfix) with ESMTPSA id D4E9D64FDE for ; Wed, 10 Mar 2021 08:32:35 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1615365155; bh=PNdAS/mTlUGP0BENBQF2q15QeFNfRijmwR2vqiJm5z4=; h=References:In-Reply-To:From:Date:Subject:To:Cc:From; b=CX1JeVkN4MlMvrvNzojX+4YYdcnpW6n9wd9RhJg7I8TpVH8tWdY+mKnyBKlRC1Ir+ Yh5fQKc3qyBzrTrpF5In0w193g0l0Hx72KZnL8cWQDTlKF182GmPQZ8nXL1/KshE6N OwsKKWw0DNuNdSuk3O98151hwEAs0falTrxkGfzQ/Losfu/OferQRKe07ARKA7hz8S +jH90gP27HeP2s/P9mxku6miWVLEYj+XqT5MsPa6T+Boi2O6hyVQKpi3rnl3Fx3SWy svc0g0AAttVk7PnfTN0s4FIO6rNkGSe7JAIW/M87PxC0wANTrSaS4oNUMNNlAGrlBC 06SFyUv+xpE3w== Received: by mail-ot1-f44.google.com with SMTP id e45so15614479ote.9 for ; Wed, 10 Mar 2021 00:32:35 -0800 (PST) X-Gm-Message-State: AOAM530GQXKI9wkLjtjVrWMioKANNfSesXcqbim/E5TUZLKpL+gUaELG BBSPn5pj7gT+1zevtaaROPpV5zfgEITlQFjXgVI= X-Google-Smtp-Source: ABdhPJzZoYFuw1NY6109ap7T6MERy5G5rAEatPGD9Bb5a9+KXLnOEixF0vJu8xDxbfCVcBko4AOiWpw8GKybl2LpQWk= X-Received: by 2002:a9d:7512:: with SMTP id r18mr1812027otk.90.1615365155174; Wed, 10 Mar 2021 00:32:35 -0800 (PST) MIME-Version: 1.0 References: <20210310081210.95147-1-ardb@kernel.org> In-Reply-To: <20210310081210.95147-1-ardb@kernel.org> From: Ard Biesheuvel Date: Wed, 10 Mar 2021 09:32:24 +0100 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] efi: use 32-bit alignment for efi_guid_t literals To: linux-efi Cc: Linux ARM , Nathan Chancellor X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210310_083238_457530_23E0F7D0 X-CRM114-Status: GOOD ( 35.00 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, 10 Mar 2021 at 09:12, 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 > > Instead of bodging this further, let's simply switch to our own definition > of efi_guid_t that carries a uint32_t as well. Since efi_guid_t is used as > an opaque type everywhere in the EFI code, this is only a minor code change. > > Reported-by: Nathan Chancellor > Signed-off-by: Ard Biesheuvel > --- > > I am currently testing this change via my for-kernelci branch. Please give > this some soak time in the other CIs that we have access to. > Note: efivarfs needs a tweak as well: --- a/fs/efivarfs/inode.c +++ b/fs/efivarfs/inode.c @@ -84,7 +84,7 @@ static int efivarfs_create(struct user_namespace *mnt_userns, struct inode *dir, /* length of the variable name itself: remove GUID and separator */ namelen = dentry->d_name.len - EFI_VARIABLE_GUID_LEN - 1; - err = guid_parse(dentry->d_name.name + namelen + 1, &var->var.VendorGuid); + err = guid_parse(dentry->d_name.name + namelen + 1, (guid_t *)&var->var.VendorGuid); if (err) goto out; > include/linux/efi.h | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/include/linux/efi.h b/include/linux/efi.h > index 8710f5710c1d..f39e9ec7485f 100644 > --- a/include/linux/efi.h > +++ b/include/linux/efi.h > @@ -63,17 +63,22 @@ typedef void *efi_handle_t; > * is 32 bits not 8 bits like our guid_t. In some cases (i.e., on 32-bit ARM), > * this means that firmware services invoked by the kernel may assume that > * efi_guid_t* arguments are 32-bit aligned, and use memory accessors that > - * do not tolerate misalignment. So let's set the minimum alignment to 32 bits. > + * do not tolerate misalignment. > * > * Note that the UEFI spec as well as some comments in the EDK2 code base > * suggest that EFI_GUID should be 64-bit aligned, but this appears to be > * a mistake, given that no code seems to exist that actually enforces that > * or relies on it. > */ > -typedef guid_t efi_guid_t __aligned(__alignof__(u32)); > +typedef struct { > + u32 a; > + u16 b; > + u16 c; > + u8 d[8]; > +} efi_guid_t; > > #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) > + (efi_guid_t){ a, b, c, { d0,d1,d2,d3,d4,d5,d6,d7 }} > > /* > * Generic EFI table header > @@ -598,8 +603,8 @@ efi_guidcmp (efi_guid_t left, efi_guid_t right) > static inline char * > efi_guid_to_str(efi_guid_t *guid, char *out) > { > - sprintf(out, "%pUl", guid->b); > - return out; > + sprintf(out, "%pUl", guid); > + return out; > } > > extern void efi_init (void); > -- > 2.30.1 > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel