On Mon, 8 Apr 2019 at 02:56, Leif Lindholm <leif.lindh...@linaro.org> wrote: > > On Mon, Apr 08, 2019 at 12:19:05PM +0300, Alexander Graf wrote: > > On 05.04.19 06:06, Leif Lindholm wrote: > > > This does bring to mind the clunkiness of the above. Marking > > > *everything* executable bypasses the improved security provided by the > > > firmware. Should I register a bug on Savannah to address this? > > > (blatantly not for the upcoming release) > > > > I quite frankly don't understand why we need to mark the PE binary as > > CODE in the first place. I thought the whole point of invoking the UEFI > > loader protocol was to ensure that the placement of sections from that > > binary into CODE/DATA happens properly? > > You have a point, but I don't think Jeffrey found this through code > review. >
Whether a region is R-X, RW- or RWX and whether a region is carved out of Efi*Code or Efi*Data memory are two different things. The sections of a PE/COFF image need to be contiguous in memory, and you cannot expect the heap allocator to return contiguous regions for separate Efi*Code and Efi*Data allocations, so the memory holding a PE/COFF image should be allocated in a single call. So usually, we allocate Efi*Code regions for executable images (wbich are guaranteed to be RWX when created) and rely on the SetMemoryAttributes() DXE services to remove either the W or the X bit depending on the nature of the section. This obviously leaves a small window where RWX memory exists, but this is the best we can do without violating the UEFI spec. > It is possible that my belt-and-braces approach of both adding a > memory mapped device path and setting SourceBuffer breaks assumptions > in the UEFI implementation. > > Jeffrey - could you try changing > status = b->load_image (0, grub_efi_image_handle, > (grub_efi_device_path_t *) mempath, > (void *) addr, > size, &image_handle); > to > status = b->load_image (0, grub_efi_image_handle, > NULL, > (void *) addr, > size, &image_handle); > and see if that makes the problem go away without changing the > allocation type? > > > Or are we not calling LoadImage? > > We are. > > / > Leif _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
