From: msalter@redhat.com (Mark Salter)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v2] arm64/efi: efistub: jump to 'stext' directly, not through the header
Date: Thu, 17 Jul 2014 10:09:01 -0400 [thread overview]
Message-ID: <1405606141.25580.108.camel@deneb.redhat.com> (raw)
In-Reply-To: <CAKv+Gu8kvU8ndeG2vF5HqRhjK+_F=8D52eabHzy++nTHt_pAYQ@mail.gmail.com>
On Wed, 2014-07-16 at 23:13 +0200, Ard Biesheuvel wrote:
> On 16 July 2014 23:03, Mark Salter <msalter@redhat.com> wrote:
> > On Wed, 2014-07-16 at 22:38 +0200, Ard Biesheuvel wrote:
> >> On 16 July 2014 21:45, Mark Salter <msalter@redhat.com> wrote:
> >> > On Wed, 2014-07-16 at 16:53 +0100, Mark Rutland wrote:
> >> >> On Wed, Jul 16, 2014 at 03:51:37PM +0100, Mark Salter wrote:
> >> >> > On Tue, 2014-07-15 at 12:58 +0200, Ard Biesheuvel wrote:
> >> >> > > After the EFI stub has done its business, it jumps into the kernel by branching
> >> >> > > to offset #0 of the loaded Image, which is where it expects to find the header
> >> >> > > containing a 'branch to stext' instruction.
> >> >> > >
> >> >> > > However, the header is not covered by any PE/COFF section, so the header may
> >> >> > > not actually be loaded at the expected offset. So instead, jump to 'stext'
> >> >> > > directly, which is at the base of the PE/COFF .text section, by supplying a
> >> >> > > symbol 'stext_offset' to efi-entry.o which contains the relative offset of
> >> >> > > stext into the Image. Also replace other open coded calculations of the same
> >> >> > > value with a reference to 'stext_offset'
> >> >> >
> >> >> > Have you actually seen a situation where the header isn't there?
> >> >> > Isn't the kernel header actually part of the pe/coff file and
> >> >> > firmware loads the whole file into RAM?
> >> >>
> >> >> From my understanding of Ard's earlier comments, this part isn't
> >> >> guaranteed per the UEFI spec.
> >> >>
> >> >> I would rather we weren't relying on implementation details.
> >> >>
> >> >
> >> > Could be. I didn't see anything about it in the UEFI spec, but I
> >> > probably wasn't exhaustive in my search. In any case, there's at
> >> > least one other place broken if the kernel header isn't included
> >> > in the loaded image.
> >> >
> >>
> >> I have not been able to find anything in the PE/COFF documents that
> >> tells you what to put in memory areas that are not covered by a
> >> section. Expecting the header to be there is indeed relying on an
> >> implementation detail, which seems risky.
> >> And indeed, if there are any other (non EFI related) uses of header
> >> fields in the kernel, it would be good to have a look at those well,
> >
> > I think we need to come up with a loader which does load an image
> > without kernel header so that we can test. Otherwise, we'll probably
> > end up with buggy code anyway. The stub code assumes the the loaded
> > image pointed to by the system table is the whole image. Seems like
> > we'd need to add code to determine if it is whole kernel image or
> > image without initial header. Stub would have to handle both cases.
> > For instance, one case would want image placed at 2MiB+TEXT_OFFSET,
> > other case would want 2MiB+TEXT_OFFSET+sizeof(kernel header).
> >
>
> No, this has nothing to do with misaligned data.
>
> The PE/COFF .text section does not start at virtual offset #0 but at
> virtual offset 'stext - efi_head'.
> In other words, there is a hole in the virtual image where the header
> is supposed to be.
> So if there is no PE/COFF section describing what data should be put
> at offset #0 by the loader, we can't assume the header is there, even
> if ImageBase does start at #0
I get that. You're supposing UEFI will always allocate memory for the
full image, but only sometimes copy the PE/COFF headers. I can see your
point from a PE/COFF perspective, but not so much from the UEFI spec
perspective where the language leads me to think it treats the PE/COFF
images as one unit wrt loading. In any case, it really isn't worth
arguing about. I don't have any objection to the patch since it won't
break anything from my perspective and it'll protect against breakage
which could possibly occur with some firmware implementations.
next prev parent reply other threads:[~2014-07-17 14:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-15 10:58 [PATCH v2] arm64/efi: efistub: jump to 'stext' directly, not through the header Ard Biesheuvel
2014-07-16 14:51 ` Mark Salter
2014-07-16 15:53 ` Mark Rutland
2014-07-16 19:45 ` Mark Salter
2014-07-16 20:38 ` Ard Biesheuvel
2014-07-16 21:03 ` Mark Salter
2014-07-16 21:13 ` Ard Biesheuvel
2014-07-17 14:09 ` Mark Salter [this message]
2014-07-21 16:32 ` Ard Biesheuvel
2014-10-06 18:13 ` Ard Biesheuvel
2014-10-06 19:33 ` Peter Jones
2014-10-07 7:49 ` Ard Biesheuvel
2014-07-16 21:03 ` Roy Franz
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1405606141.25580.108.camel@deneb.redhat.com \
--to=msalter@redhat.com \
--cc=linux-arm-kernel@lists.infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).