| Message ID | 20190125114516.12127-3-agraf@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: Support HP Envy X2 | expand |
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 <agraf@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... > +#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... > #define GRUB_PE32_FILE_ALIGNMENT GRUB_PE32_SECTION_ALIGNMENT Daniel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
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 <agraf@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. > >> +#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? Alex _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
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 <agraf@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
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. */ +#define GRUB_PE32_SECTION_ALIGNMENT GRUB_EFI_PAGE_SIZE #define GRUB_PE32_FILE_ALIGNMENT GRUB_PE32_SECTION_ALIGNMENT struct grub_pe32_coff_header
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 <agraf@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(-) -- 2.12.3 _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel