From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1go6ZI-0001ix-JH for mharc-grub-devel@gnu.org; Mon, 28 Jan 2019 08:03:16 -0500 Received: from eggs.gnu.org ([209.51.188.92]:52533) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1go6ZF-0001im-MM for grub-devel@gnu.org; Mon, 28 Jan 2019 08:03:15 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1go6ZE-0001Z0-OK for grub-devel@gnu.org; Mon, 28 Jan 2019 08:03:13 -0500 Received: from dibed.net-space.pl ([84.10.22.86]:51709) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_3DES_EDE_CBC_SHA1:24) (Exim 4.71) (envelope-from ) id 1go6ZE-0001Yg-DC for grub-devel@gnu.org; Mon, 28 Jan 2019 08:03:12 -0500 Received: from router-fw.i.net-space.pl ([192.168.52.1]:52694 "EHLO tomti.i.net-space.pl") by router-fw-old.i.net-space.pl with ESMTP id S904307AbfA1NDK (ORCPT ); Mon, 28 Jan 2019 14:03:10 +0100 X-Comment: RFC 2476 MSA function at dibed.net-space.pl logged sender identity as: dkiper Date: Mon, 28 Jan 2019 14:03:07 +0100 From: Daniel Kiper To: Alexander Graf Cc: grub-devel@gnu.org, Leif Lindholm , Peter Jones , Jon Masters Subject: Re: [PATCH v5 2/3] mkimage: Align efi sections on 4k boundary Message-ID: <20190128130307.qtkpmzx5aemlv4fi@tomti.i.net-space.pl> References: <20190125114516.12127-1-agraf@suse.de> <20190125114516.12127-3-agraf@suse.de> <20190128122215.px3ws2iijevnglf6@tomti.i.net-space.pl> <56849721-2276-5477-9493-146b75f06706@suse.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56849721-2276-5477-9493-146b75f06706@suse.de> User-Agent: NeoMutt/20170113 (1.7.2) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 84.10.22.86 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 28 Jan 2019 13:03:15 -0000 On Mon, Jan 28, 2019 at 01:33:33PM +0100, Alexander Graf wrote: > On 28.01.19 13:22, Daniel Kiper wrote: > > On Fri, Jan 25, 2019 at 12:45:15PM +0100, Alexander Graf wrote: > >> There is UEFI firmware popping up in the wild now that implements stricter > >> permission checks using NX and write protect page table entry bits. > >> > >> This means that firmware now may fail to load binaries if its individual > >> sections are not page aligned, as otherwise it can not ensure permission > >> boundaries. > >> > >> So let's bump all efi section alignments up to 4k (EFI page size). That way > >> we will stay compatible going forward. > >> > >> Unfortunately our internals can't deal very well with a mismatch of alignment > >> between the virtual and file offsets, so we have to also pad our target > >> binary a bit. > >> > >> Signed-off-by: Alexander Graf > >> > >> --- > >> > >> v4 -> v5: > >> > >> - Use GRUB_EFI_PAGE_SIZE > >> - Add include to have above const defined > >> --- > >> include/grub/efi/pe32.h | 10 ++++++++-- > >> 1 file changed, 8 insertions(+), 2 deletions(-) > >> > >> diff --git a/include/grub/efi/pe32.h b/include/grub/efi/pe32.h > >> index 7d44732d2..fe8a85ce6 100644 > >> --- a/include/grub/efi/pe32.h > >> +++ b/include/grub/efi/pe32.h > >> @@ -20,6 +20,7 @@ > >> #define GRUB_EFI_PE32_HEADER 1 > >> > >> #include > >> +#include > >> > >> /* The MSDOS compatibility stub. This was copied from the output of > >> objcopy, and it is not necessary to care about what this means. */ > >> @@ -50,8 +51,13 @@ > >> /* According to the spec, the minimal alignment is 512 bytes... > >> But some examples (such as EFI drivers in the Intel > >> Sample Implementation) use 32 bytes (0x20) instead, and it seems > >> - to be working. For now, GRUB uses 512 bytes for safety. */ > >> -#define GRUB_PE32_SECTION_ALIGNMENT 0x200 > >> + to be working. > >> + > >> + However, there is firmware showing up in the field now with > >> + page alignment constraints to guarantee that page protection > >> + bits take effect. Because we can not easily distinguish between > >> + in-memory and in-file layout, let's bump all alignment to 4k. */ > > > > s/4k/GRUB_EFI_PAGE_SIZE/ By chance GRUB_EFI_PAGE_SIZE is equal to 4k but > > there is no requirement for that... > > There is, it's defined in the spec. It is for currently existing platforms. There is no guarantee that it will be the same for new ones. So, I tend to change it to GRUB_EFI_PAGE_SIZE. Even if today GRUB_EFI_PAGE_SIZE always equals 4k. > >> +#define GRUB_PE32_SECTION_ALIGNMENT GRUB_EFI_PAGE_SIZE > > > > I am still missing an explanation here why GRUB_PE32_FILE_ALIGNMENT has > > to be equal GRUB_PE32_SECTION_ALIGNMENT. And IIRC I have asked about > > that in my earlier emails... > > It is in the comment above exactly those 2 lines. What more do you want? Ahhh... Sorry, I have missed that. Though I would change s/Because we/Because currently existing GRUB code/ Daniel