public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Zeng, Star" <star.zeng@intel.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Laszlo Ersek <lersek@redhat.com>
Cc: "Wang, Jian J" <jian.j.wang@intel.com>,
	Matt Fleming <matt@codeblueprint.co.uk>,
	"edk2-devel@lists.01.org" <edk2-devel@lists.01.org>,
	"Yao, Jiewen" <jiewen.yao@intel.com>,
	"Dong, Eric" <eric.dong@intel.com>,
	"Zeng, Star" <star.zeng@intel.com>
Subject: Re: [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map
Date: Thu, 16 Nov 2017 02:46:39 +0000	[thread overview]
Message-ID: <0C09AFA07DD0434D9E2A0C6AEB0483103B9B5ED0@shsmsx102.ccr.corp.intel.com> (raw)
In-Reply-To: <CAKv+Gu-9bW7+36r12gCoFivqBqVrHPT3sHLM4eS3wb1hOZyQwA@mail.gmail.com>

Seemingly, the Memory Attributes Table should be consumed for memory protection.

UEFI spec: "The Memory Attributes Table is currently used to describe memory protections that may be applied to the EFI Runtime code and data by an operating system or hypervisor."
Someone (Jiewen?) familiar with the table could help introduce the background.

And seemingly, the capabilities in UEFI memory map could not have paging related as it will break the compatibility to OS.


Thanks,
Star
-----Original Message-----
From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org] 
Sent: Wednesday, November 15, 2017 11:59 PM
To: Laszlo Ersek <lersek@redhat.com>
Cc: Wang, Jian J <jian.j.wang@intel.com>; Zeng, Star <star.zeng@intel.com>; Matt Fleming <matt@codeblueprint.co.uk>; edk2-devel@lists.01.org; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>
Subject: Re: [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map

On 15 November 2017 at 15:48, Laszlo Ersek <lersek@redhat.com> wrote:
> Hi Jian,
>
> On 11/15/17 10:27, Wang, Jian J wrote:
>> I tried this workaround and there're no failure in booting Fedora 26 
>> and Windows server 2016 now. If no objection, I'll merge it into new version of this patch.
>
> I'm not too familiar with the Linux kernel's EFI pieces myself; that's 
> why I added Ard and Matt earlier to the thread (when I responded with 
> the regression report) -- Ard and Matt maintain EFI in the Linux kernel.
>
> So, if you think there's a bug in Linux (i.e., a mis-interpretation of 
> the UEFI spec), can you guys please discuss that together? Below you wrote:
>
> - "I think it must be that the kernel will mark the memory block to be
>    RO/XP/RP memory according to its capabilities but not its current
>    attributes"
>

The UEFI spec does not distinguish between capabilities and attributes, so how on earth should the OS be able to make this distinction?

For instance, the UEFI spec defines EFI_MEMORY_RO as

"""
Physical memory protection attribute: The memory region supports making this memory range read-only by system hardware.
"""

which is essentially a capability not an attribute. However, EFI_MEMORY_RO/XP are also used to convey information about the nature of the *contents* of the memory region, i.e., is it .text, .rodata or .data/.bss

So given that the OS only has the UEFI memory map to go on, how exactly should it figure out what these attributes are meant to imply?

> - "there's real gap between UEFI and OS on how to interpret the memory
>    capabilities"
>

Yes. There is also a gap between how GCD and the UEFI memory map interpret these attributes, which is why nobody bothers to set the protection capabilities for GCD: it gets copied into the UEFI memory map, and nobody has a clue how the OS should treat it.

When the [short lived]
EFI_PROPERTIES_RUNTIME_MEMORY_PROTECTION_NON_EXECUTABLE_PE_DATA
feature was introduced, it was essentially decided that RO and XP may be used to annotate the nature of the contents of memory regions, where code was mapped RO and data mapped XP.

> Why is it wrong for the OS kernel, according to the UEFI spec, to 
> change the attributes / mappings of a memory area, as long as it stays 
> compliant with the advertised capabilities? How is an OS expected to 
> work, upon seeing those new "paging capabilities from UEFI memory map"
> (in Star's words)?
>
> Apparently, it's not just Linux that's confused; see the Win2016 crash 
> too. Is the UEFI spec detailed enough on those capabilities? (Recently 
> I tried to review the paging capabilities myself in the spec, and I 
> ended up totally confused...)
>

I think it is simply impossible at this point to interpret those flags as attributes, i.e., RO means code that may be mapped read-only, and XP means data that may be mapped non-executable. Anything beyond that opens a can of worms that is bound to break stuff all over the place.

--
Ard.

  reply	other threads:[~2017-11-16 16:35 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-10  1:02 [PATCH v5] UefiCpuPkg/CpuDxe: Fix multiple entries of RT_CODE in memory map Jian J Wang
2017-11-10 12:23 ` Laszlo Ersek
2017-11-13  3:29   ` Wang, Jian J
2017-11-14 14:36   ` Wang, Jian J
2017-11-15  6:52     ` Zeng, Star
2017-11-15  7:36       ` Wang, Jian J
2017-11-15  9:27       ` Wang, Jian J
2017-11-15 15:48         ` Laszlo Ersek
2017-11-15 15:59           ` Ard Biesheuvel
2017-11-16  2:46             ` Zeng, Star [this message]
2017-11-16  3:03               ` Yao, Jiewen

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-list 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=0C09AFA07DD0434D9E2A0C6AEB0483103B9B5ED0@shsmsx102.ccr.corp.intel.com \
    --to=devel@edk2.groups.io \
    /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