Re: [RFC] arm64/linux/loader: Use EFI CODE allocations for the linux kernel

[Jeffrey Hugo]

On Mon, Apr 8, 2019 at 7:47 AM Jeffrey Hugo <address@hidden> wrote:
>
> On Mon, Apr 8, 2019 at 3:56 AM Leif Lindholm <address@hidden> 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.
> >
> > 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?
>
> I'll give that a shot and report back.

Hmm.  I apparently wasn't very methodical about my previous testing.
Mainline GRUB doesn't appear to have the original issue, thus my fix
would appear to be not needed.
However, downstream distro GRUB appears to be affected (I was
originally attempting to use Ubuntu).

I suspect this change, which appears to remove the load_image call
while adding Secure Boot support -
https://git.launchpad.net/~ubuntu-core-dev/grub/+git/ubuntu/commit/grub-core/loader/arm64/linux.c?h=ubuntu&id=98bc4f52034abb18e7bd028d424bde15375c14af

Sorry for the noise, although I would appreciate some pointers on how
this should be addressed.  Should I go file bugs against the distros?

>
> >
> > > Or are we not calling LoadImage?
> >
> > We are.
> >
> > /
> >     Leif