From: "Ard Biesheuvel" <ardb@kernel.org>
To: "Marvin Häuser" <mhaeuser@posteo.de>
Cc: devel@edk2.groups.io, t@taylorbeebe.com
Subject: Re: [edk2-devel] [PATCH 4/4] ArmPkg/CpuDxe: Implement EFI memory attributes protocol
Date: Tue, 7 Feb 2023 18:56:43 +0100 [thread overview]
Message-ID: <CAMj1kXEJkeGRCDbTnA+7Tv9rL_fj-vR37_YLq3MimPmLFr-FBw@mail.gmail.com> (raw)
In-Reply-To: <A151225E-4A06-4E14-BAA0-C0BB8169E204@posteo.de>
On Tue, 7 Feb 2023 at 11:13, Marvin Häuser <mhaeuser@posteo.de> wrote:
>
>
> On 7. Feb 2023, at 11:01, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> Actually, it seems UnprotectUefiImage () is corrent under the
> assumption that all code regions have EFI_MEMORY_XP cleared by
> default.
>
> However, if you redefine the policy to set EFI_MEMORY_XP on code
> regions by default, and only permit execution after remapping the code
> read-only explicitly, and only then clearing EFI_MEMORY_XP, that
> routine should revert the region to EFI_MEMORY_XP. But given the
> existing ASSERT()s on having EFI_MEMORY_XP cleared for all code
> regions, the code as it is currently is not incorrect.
>
>
> Right. My main issue is, it’s nowhere documented that manually changed permissions must be restored to their default before freeing. Within DxeCore, this is easily done using the PCDs, but outside (say you allocate a trampoline buffer and then free it), you would need to manually query the permissions, store them, and restore later.
>
Agreed. However, I'd still prefer to only call
SetMemorySpaceAttributes() if needed, and setting the same attributes
on the entire image allocation at least permits us to double check
whether the new attributes are already set on a region, and avoid the
call if that is the case.
Perhaps we should just set EFI_MEMORY_XP on all images regardless of
the policy - the memory should no longer be executable anyway, and
what we currently do is remap the entire region RWX after it has
executed, which is kind of nasty.
> I did *not* look into the implementation code in detail, but does the new memory permission protocol impose the same constraint implementation-wise and if so, is this documented anywhere?
>
Not sure I follow you: which constraint is that?
> PS: Fetched the wrong link in my last mail: https://lkml.org/lkml/2022/12/15/352
>
Yeah saw that. I'm hoping to get that in for v6.4 as we missed v6.3 by
now. (I did take his patch that adds the definition of the EFI memory
attribute protocol only, as I need it for EFI zboot)
next prev parent reply other threads:[~2023-02-07 17:56 UTC|newest]
Thread overview: 24+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-01-31 22:35 [PATCH 0/4] ArmPkg: implement EFI memory attributes protocol Ard Biesheuvel
2023-01-31 22:35 ` [PATCH 1/4] MdePkg: Add Memory Attribute Protocol definition Ard Biesheuvel
2023-02-02 3:19 ` 回复: " gaoliming
2023-02-02 9:25 ` [edk2-devel] " Ard Biesheuvel
2023-01-31 22:35 ` [PATCH 2/4] MdePkg: Bump implemented UEFI version to v2.10 Ard Biesheuvel
2023-02-01 0:10 ` Michael D Kinney
2023-02-01 7:54 ` Ard Biesheuvel
2023-01-31 22:35 ` [PATCH 3/4] ArmPkg/CpuDxe: Unify PageAttributeToGcdAttribute helper Ard Biesheuvel
2023-01-31 22:35 ` [PATCH 4/4] ArmPkg/CpuDxe: Implement EFI memory attributes protocol Ard Biesheuvel
2023-02-01 18:41 ` [edk2-devel] " Taylor Beebe
2023-02-02 9:43 ` Ard Biesheuvel
2023-02-03 19:08 ` Taylor Beebe
2023-02-03 19:58 ` Marvin Häuser
2023-02-07 1:18 ` Taylor Beebe
2023-02-07 8:29 ` Ard Biesheuvel
2023-02-07 8:56 ` Marvin Häuser
2023-02-07 9:16 ` Ard Biesheuvel
2023-02-07 10:00 ` Marvin Häuser
2023-02-07 10:01 ` Ard Biesheuvel
2023-02-07 10:13 ` Marvin Häuser
2023-02-07 17:56 ` Ard Biesheuvel [this message]
2023-02-07 18:19 ` Taylor Beebe
2023-02-07 18:50 ` Marvin Häuser
2023-02-07 18:19 ` Marvin Häuser
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=CAMj1kXEJkeGRCDbTnA+7Tv9rL_fj-vR37_YLq3MimPmLFr-FBw@mail.gmail.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