All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
To: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Cc: "pjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org"
	<pjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org"
	<msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>,
	"leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	"matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org"
	<matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>,
	"linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org"
	<roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>,
	Catalin Marinas <Catalin.Marinas-5wv7dgnIgG8@public.gmane.org>
Subject: Re: [PATCH v3] arm64/efi: efistub: jump to 'stext' directly, not through the header
Date: Fri, 10 Oct 2014 15:02:33 +0100	[thread overview]
Message-ID: <20141010140233.GD6004@leverpostej> (raw)
In-Reply-To: <CAKv+Gu-OJ+byitcYSsajgEhgPOwP2ooF4Ro1QgJzXw1Z_VX1Ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>

On Fri, Oct 10, 2014 at 02:27:46PM +0100, Ard Biesheuvel wrote:
> On 10 October 2014 15:03, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> > [...]
> >
> >> >> > But if the EFI loader is allowed to load stext at the precise start of
> >> >> > RAM (or anywhere not in the idmap), in attempting the copy we'd try to
> >> >> > access unmapped addresses.
> >> >> >
> >> >> > So if that's a possibility, we need to shrink the copy to cover stext
> >> >> > to _edata rather than _text to edata.
> >> >> >
> >> >> > Does that make sense?
> >> >> >
> >> >>
> >> >> That cannot happen. The PE/COFF .text section's positive relative
> >> >> virtual offset ensures that the memory image has room for the header,
> >> >> it's just not guaranteed that anything gets copied there.
> >> >
> >> > Ok. If we're guaranteed to have some space there, we're fine.
> >> >
> >> > I'm probably being a bit thick here, but where is the "positive relative
> >> > virtual offset" in the header? Which field defines that?
> >> >
> >>
> >> The fields VirtualSize, VirtualAddress (the field I was referring to),
> >> SizeOfRawData and PointerToRawData define the relation between the
> >> file layout and the memory layout of the .text section (line 219 and
> >> up in head.S)
> >
> > I guess my confusion is over the semantics of the VirtualAddress field.
> > If it's treated as an offset, what is that offset relative to in memory?
> > And what defines that the space covered by that offset is accessible?
> >
> 
> The PE/COFF spec 8.3 describes VirtualAddress as
> 
> """
> For executable images, the address of the first byte of the section
> relative to the image base when the section is loaded into memory.
> """
> 
> ImageBase is a field itself in the PE/COFF header, described as
> 
> """
> The preferred address of the first byte of image when loaded into
> memory; must be a multiple of 64 K.
> """

Thanks for the info, this now makes a lot more sense to me.

So that means the .text section is not the start of the image, but is
offset by (stext - efi_head) bytes from the start, covering the header
(regardless of whether it is actually present in the loaded image).

> The SizeOfImage field is described as
> 
> """
> The size (in bytes) of the image, including all headers, as the image
> is loaded in memory.
> """
> 
> My interpretation is that memory needs to be allocated for the header
> as well as all sections that have a VirtualSize (including sections
> like BSS which don't have a payload in the file)

That would match what I understand from reading the above, though it
strikes me as odd that space needs to be allocated for the headers if
they aren't guaranteed to be copied -- it's not defined where they would
live in the image.

> >> In our current definition, the memory offset and the file offset are
> >> identical (which this patch redefines as 'stext_offset'). The virtual
> >> size covers the entire static memory footprint of Image (minus the
> >> header). whereas the SizeOfRawData contains the size of the payload in
> >> the file (again, minus the header). The balance is zero initialized by
> >> the loader.
> >
> > I can see why this guarantees there is space for stext to _end, but I
> > don't understand how this guarantees there is a valid mapping for the
> > region that would otherwise be _head to stext.
> >
> 
> The allocation itself is defined in terms of ImageBase/SizeOfImage
> (although ImageBase is only a preferred offset)
> How this allocation is populated with data (and where the holes are)
> is described by the sections.

Ok. Thanks for putting this information together (it's remarkably
useful), and thanks for putting up with my ignorance of PE/COFF.

Cheers,
Mark.

WARNING: multiple messages have this Message-ID (diff)
From: mark.rutland@arm.com (Mark Rutland)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v3] arm64/efi: efistub: jump to 'stext' directly, not through the header
Date: Fri, 10 Oct 2014 15:02:33 +0100	[thread overview]
Message-ID: <20141010140233.GD6004@leverpostej> (raw)
In-Reply-To: <CAKv+Gu-OJ+byitcYSsajgEhgPOwP2ooF4Ro1QgJzXw1Z_VX1Ow@mail.gmail.com>

On Fri, Oct 10, 2014 at 02:27:46PM +0100, Ard Biesheuvel wrote:
> On 10 October 2014 15:03, Mark Rutland <mark.rutland@arm.com> wrote:
> > [...]
> >
> >> >> > But if the EFI loader is allowed to load stext at the precise start of
> >> >> > RAM (or anywhere not in the idmap), in attempting the copy we'd try to
> >> >> > access unmapped addresses.
> >> >> >
> >> >> > So if that's a possibility, we need to shrink the copy to cover stext
> >> >> > to _edata rather than _text to edata.
> >> >> >
> >> >> > Does that make sense?
> >> >> >
> >> >>
> >> >> That cannot happen. The PE/COFF .text section's positive relative
> >> >> virtual offset ensures that the memory image has room for the header,
> >> >> it's just not guaranteed that anything gets copied there.
> >> >
> >> > Ok. If we're guaranteed to have some space there, we're fine.
> >> >
> >> > I'm probably being a bit thick here, but where is the "positive relative
> >> > virtual offset" in the header? Which field defines that?
> >> >
> >>
> >> The fields VirtualSize, VirtualAddress (the field I was referring to),
> >> SizeOfRawData and PointerToRawData define the relation between the
> >> file layout and the memory layout of the .text section (line 219 and
> >> up in head.S)
> >
> > I guess my confusion is over the semantics of the VirtualAddress field.
> > If it's treated as an offset, what is that offset relative to in memory?
> > And what defines that the space covered by that offset is accessible?
> >
> 
> The PE/COFF spec 8.3 describes VirtualAddress as
> 
> """
> For executable images, the address of the first byte of the section
> relative to the image base when the section is loaded into memory.
> """
> 
> ImageBase is a field itself in the PE/COFF header, described as
> 
> """
> The preferred address of the first byte of image when loaded into
> memory; must be a multiple of 64 K.
> """

Thanks for the info, this now makes a lot more sense to me.

So that means the .text section is not the start of the image, but is
offset by (stext - efi_head) bytes from the start, covering the header
(regardless of whether it is actually present in the loaded image).

> The SizeOfImage field is described as
> 
> """
> The size (in bytes) of the image, including all headers, as the image
> is loaded in memory.
> """
> 
> My interpretation is that memory needs to be allocated for the header
> as well as all sections that have a VirtualSize (including sections
> like BSS which don't have a payload in the file)

That would match what I understand from reading the above, though it
strikes me as odd that space needs to be allocated for the headers if
they aren't guaranteed to be copied -- it's not defined where they would
live in the image.

> >> In our current definition, the memory offset and the file offset are
> >> identical (which this patch redefines as 'stext_offset'). The virtual
> >> size covers the entire static memory footprint of Image (minus the
> >> header). whereas the SizeOfRawData contains the size of the payload in
> >> the file (again, minus the header). The balance is zero initialized by
> >> the loader.
> >
> > I can see why this guarantees there is space for stext to _end, but I
> > don't understand how this guarantees there is a valid mapping for the
> > region that would otherwise be _head to stext.
> >
> 
> The allocation itself is defined in terms of ImageBase/SizeOfImage
> (although ImageBase is only a preferred offset)
> How this allocation is populated with data (and where the holes are)
> is described by the sections.

Ok. Thanks for putting this information together (it's remarkably
useful), and thanks for putting up with my ignorance of PE/COFF.

Cheers,
Mark.

  parent reply	other threads:[~2014-10-10 14:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-10-08 14:11 [PATCH v3] arm64/efi: efistub: jump to 'stext' directly, not through the header Ard Biesheuvel
2014-10-08 14:11 ` Ard Biesheuvel
     [not found] ` <1412777487-13636-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2014-10-09 17:23   ` Mark Rutland
2014-10-09 17:23     ` Mark Rutland
2014-10-09 19:03     ` Ard Biesheuvel
2014-10-09 19:03       ` Ard Biesheuvel
     [not found]       ` <CAKv+Gu-D812Rn32rm5wQJfSM+NzPXZWbxDcDkZ8mBzbi5RQJXQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-09 22:19         ` Mark Salter
2014-10-09 22:19           ` Mark Salter
     [not found]           ` <1412893179.29182.71.camel-PDpCo7skNiwAicBL8TP8PQ@public.gmane.org>
2014-10-09 23:20             ` Roy Franz
2014-10-09 23:20               ` Roy Franz
     [not found]               ` <CAFECyb9Tz2Fo2ix0VYiM7WPeie7C=woiVwyQtR4aH_HPcLwkOg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-10  6:30                 ` Ard Biesheuvel
2014-10-10  6:30                   ` Ard Biesheuvel
     [not found]                   ` <CAKv+Gu_6uww=W9M9hDfoSmY+zr2i75Y=JOxTLCya2rr=HiT6Bw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-10 14:14                     ` Mark Salter
2014-10-10 14:14                       ` Mark Salter
     [not found]                       ` <1412950468.29182.82.camel-PDpCo7skNiwAicBL8TP8PQ@public.gmane.org>
2014-10-10 14:28                         ` Ard Biesheuvel
2014-10-10 14:28                           ` Ard Biesheuvel
2014-10-10 13:53                 ` Peter Jones
2014-10-10 13:53                   ` Peter Jones
2014-10-10 10:49         ` Mark Rutland
2014-10-10 10:49           ` Mark Rutland
2014-10-10 11:52           ` Ard Biesheuvel
2014-10-10 11:52             ` Ard Biesheuvel
     [not found]             ` <CAKv+Gu_mx3uWqnRESAjqz6MdUJqBSjgeMCdnpzKiRQ+1FxUjCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-10 12:19               ` Mark Rutland
2014-10-10 12:19                 ` Mark Rutland
2014-10-10 12:31                 ` Ard Biesheuvel
2014-10-10 12:31                   ` Ard Biesheuvel
     [not found]                   ` <CAKv+Gu-Kk=hZ78T+_ePceTgb9m4KmvpLcYOwvxEWuUcT+ScK_Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-10 13:03                     ` Mark Rutland
2014-10-10 13:03                       ` Mark Rutland
2014-10-10 13:27                       ` Ard Biesheuvel
2014-10-10 13:27                         ` Ard Biesheuvel
     [not found]                         ` <CAKv+Gu-OJ+byitcYSsajgEhgPOwP2ooF4Ro1QgJzXw1Z_VX1Ow-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2014-10-10 14:02                           ` Mark Rutland [this message]
2014-10-10 14:02                             ` Mark Rutland
2014-10-10 15:38                           ` Roy Franz
2014-10-10 15:38                             ` Roy Franz
2014-10-10 15:52                             ` Ard Biesheuvel
2014-10-10 15:52                               ` Ard Biesheuvel

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=20141010140233.GD6004@leverpostej \
    --to=mark.rutland-5wv7dgnigg8@public.gmane.org \
    --cc=Catalin.Marinas-5wv7dgnIgG8@public.gmane.org \
    --cc=ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=matt.fleming-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org \
    --cc=msalter-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=pjones-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org \
    --cc=roy.franz-QSEj5FYQhm4dnm+yROfE0A@public.gmane.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.