On 2023-03-15 00:23, Andy Lutomirski wrote:
On Tue, Mar 14, 2023, at 3:13 AM, Evgeniy Baskov wrote:
This patchset is aimed
* to improve UEFI compatibility of compressed kernel code for x86_64
* to setup proper memory access attributes for code and rodata
sections
* to implement W^X protection policy throughout the whole execution
of compressed kernel for EFISTUB code path.
The overall code quality seems okay, but I have some questions as to
what this is for. The early boot environment is not exposed to most
sorts of attacks -- there's no userspace, there's no network, and
there is not a whole lot of input that isn't implicitly completely
trusted.
What parts of this series are actually needed to get these fancy new
bootloaders to boot Linux? And why?
Well, most of the series is needed, except for may be adding W^X
for the non-UEFI boot path (patches 3-9), but those add changes,
required for booting via UEFI, like memory protection call-backs.
And since the important callbacks are already in-place W^X for
non-UEFI won't be too undesired property.
The most part of this series (3-16,26,27) implements W^X, and
the remaining patches improves the compatibility of PE, which
includes:
* Removing W+X sections (which is now required as Gerd have already
mentioned or at least very desired)
* Aligning sections to the page size in memory and to minimal
file alignment in file.
* Aligning data structures on their natural alignment
(e.g. [2] requires it)
* Filling more PE header fields to their actual values.
* Removing alignment flags on sections, which according to
the spec, is only for object files.
* Filling in relocation data directory and its rounding the size
to 4 bytes.
Most of this work is done in the patch 24 "x86/build: Make generated
PE more spec compliant", but it also requires working W^X due to
the removal of W+X sections and some clean-up work from patches
17-23.
Kernel is made to be more compatible with PE image specification [3],
allowing it to be successfully loaded by stricter PE loader
implementations like the one from [2]. There is at least one
known implementation that uses that loader in production [4].
There are also ongoing efforts to upstream these changes.
Can you clarify
Also the patchset adds EFI_MEMORY_ATTTRIBUTE_PROTOCOL, included into
EFI specification since version 2.10, as a better alternative to
using DXE services for memory protection attributes manipulation,
since it is defined by the UEFI specification itself and not UEFI PI
specification. This protocol is not widely available so the code
using DXE services is kept in place as a fallback in case specific
implementation does not support the new protocol.
One of EFI implementations that already support
EFI_MEMORY_ATTTRIBUTE_PROTOCOL is Microsoft Project Mu [5].
Maybe make this a separate series?
This now is just one fairly straight forward patch, since the protocol
definitions are already got accepted and the protocol is used elsewhere
in the EFISTUB. This patch would also have to be replaced, rather than
removed if it's made a separate series, since it adds a warning about
W+X mappings.
Kernel image generation tool (tools/build.c) is refactored as a part
of changes that makes PE image more compatible.
The patchset implements memory protection for compressed kernel
code while executing both inside EFI boot services and outside of
them. For EFISTUB code path W^X protection policy is maintained
throughout the whole execution of compressed kernel. The latter
is achieved by extracting the kernel directly from EFI environment
and jumping to it's head immediately after exiting EFI boot services.
As a side effect of this change one page table rebuild and a copy of
the kernel image is removed.
I have no problem with this, but what's it needed for?
The one hard part that made the series more complicated is that
non-UEFI (or rather the only) boot path relocates the kernel, which
messes up the memory protection for sections set by the UEFI. I did not
want to remove the support of in-place extraction and relocation, when
loaded in inappropriate place, for the non-UEFI boot path, which is why
extraction from boot services was implemented. A proper W^X in EFISTUB
is a side effect, but the desired one.
The alternative would be to make the whole image RWX after the EFISTUB
execution. But the current approach is a lot nicer solution.
Memory protection inside EFI environment is controlled by the
CONFIG_DXE_MEM_ATTRIBUTES option, although with these patches this
option also control the use EFI_MEMORY_ATTTRIBUTE_PROTOCOL and memory
protection attributes of PE sections and not only DXE services as the
name might suggest.
[1]
https://lore.kernel.org/lkml/893da11995f93a7ea8f7485d17bf356a@xxxxxxxxx/
[2] https://github.com/acidanthera/audk/tree/secure_pe
Link is broken
Ah, sorry, the branch was merged into master since I've first posted
the series, so the working link is:
https://github.com/acidanthera/audk
The loader itself is here:
https://github.com/acidanthera/audk/tree/master/MdePkg/Library/BasePeCoffLib2
[3]
https://download.microsoft.com/download/9/c/5/9c5b2167-8017-4bae-9fde-d599bac8184a/pecoff_v83.docx
I skimmed this very briefly, and I have no idea what I'm supposed to
look at. This is the entire PE spec!
I gave some explanations above, which are mostly the duplicates of
the patch 24 "x86/build: Make generated PE more spec compliant"
commit message.
Thanks,
Evgeniy Baskov
