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 <ag...@suse.de> > >> > >> --- > >> > >> 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 <grub/types.h> > >> +#include <grub/efi/memory.h> > >> > >> /* 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 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
