From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by spool.mail.gandi.net (Postfix) with ESMTPS id EB77074003D for ; Wed, 6 Dec 2023 20:00:49 +0000 (UTC) DKIM-Signature: a=rsa-sha256; bh=U/eJUSeUSP0YVxACjN/eh6Hs4p4tXSbuP1U5h5j8hg8=; c=relaxed/simple; d=groups.io; h=Message-ID:Date:MIME-Version:User-Agent:Subject:To:Cc:References:From:In-Reply-To:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type:Content-Language; s=20140610; t=1701892848; v=1; b=fSZy3mgT3GikNjE6VuJtMFiZI/Hmw6fTIXxstLh4rtGhz+2QKap942mkcdKOVUXQe+BJ0GMN hOOcu+ds84mzRl93Z33zHPYCL/1XJH8oPuT4+OBNNeGqzZWK/UAknbhFXFS2ZQriK+2AIN4TuQ+ Vh/a5UBXGtZTUpn5yiEklOuE= X-Received: by 127.0.0.2 with SMTP id oRK9YY7687511xYdTtpaEzxV; Wed, 06 Dec 2023 12:00:48 -0800 X-Received: from mail-pl1-f170.google.com (mail-pl1-f170.google.com [209.85.214.170]) by mx.groups.io with SMTP id smtpd.web10.43419.1701892847896686758 for ; Wed, 06 Dec 2023 12:00:47 -0800 X-Received: by mail-pl1-f170.google.com with SMTP id d9443c01a7336-1d03bcf27e9so1361875ad.0 for ; Wed, 06 Dec 2023 12:00:47 -0800 (PST) X-Gm-Message-State: QXrviwG4wAxJ58jCN2YSR31Ix7686176AA= X-Google-Smtp-Source: AGHT+IEVjzAtoLj47UCgBl+lAX+joDafD8nTxwlop3mEj5LQwcWzfX5hjHELoVq58kyul00A+WRb9w== X-Received: by 2002:a17:903:2448:b0:1d0:b16a:b259 with SMTP id l8-20020a170903244800b001d0b16ab259mr1328525pls.74.1701892847101; Wed, 06 Dec 2023 12:00:47 -0800 (PST) X-Received: from [192.168.0.125] ([50.46.253.1]) by smtp.gmail.com with ESMTPSA id b2-20020a170902ed0200b001d1c9e0c12csm212770pld.56.2023.12.06.12.00.45 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 06 Dec 2023 12:00:46 -0800 (PST) Message-ID: Date: Wed, 6 Dec 2023 12:00:44 -0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Subject: Re: [edk2-devel] [PATCH] ArmVirtPkg: Allow EFI memory attributes protocol to be disabled To: Gerd Hoffmann , devel@edk2.groups.io, ardb@kernel.org Cc: Oliver Smith-Denny , Peter Jones , Ard Biesheuvel , =?UTF-8?B?TO+/vXN6bO+/vSDvv71yc2Vr?= , Oliver Steffen , Alexander Graf , Leif Lindholm References: <20231204095215.1053032-1-ardb@google.com> From: "Taylor Beebe" In-Reply-To: Precedence: Bulk List-Subscribe: List-Help: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,taylor.d.beebe@gmail.com List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: Content-Type: multipart/alternative; boundary="------------p0gLioO1cdy490SzEF8RPW60" Content-Language: en-US X-GND-Status: LEGIT Authentication-Results: spool.mail.gandi.net; dkim=pass header.d=groups.io header.s=20140610 header.b=fSZy3mgT; dmarc=fail reason="SPF not aligned (relaxed), DKIM not aligned (relaxed)" header.from=gmail.com (policy=none); spf=pass (spool.mail.gandi.net: domain of bounce@groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce@groups.io --------------p0gLioO1cdy490SzEF8RPW60 Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: quoted-printable >> But what we might do is invent a way to avoid setting the XP attribute >> on the entire region based on some heuristic. Given that the main >> purpose of the EFI memory attribute protocol is to provide the ability >> to remove XP (and set RO instead), perhaps we can avoid the set >> entirely? Just brainstorming here. > Can the fault handler deal with this? Set a flag somewhere, print a > big'n'fat error message, wait 5 secs, reset machine? After reset the > firmware will see the flag and come up in 'compat' instead of 'strict' > mode. > > Not sure what a good place for such a flag would be. Do we have other > options than a non-volatile efi variable? When using a efi variable we > probably should add an 'expires' timestamp, so the machine doesn't stay > in 'compat' mode forever. This is what we do in Project Mu currently and is what we would like to=20 push into EDK2. For x86 platforms, we use CMOS to communicate to the next boot=20 that the system needs to enter compatibility mode. Of course this doesn't work on AR= M platforms, so we'll have to come up with a more permanent mechanism to=20 support this functionality. >> (cc'ing Taylor and Oliver given that this is related to the memory >> policy work as well) Perhaps we can use the fact that the active image >> is non-NX compat to make some tweaks? > Memory policies would be another candidate which could possibly use a > less strict profile in 'compat' mode. I'd love to see memory policies > land for the February stable tag. I don't think the policy can change how the SHIM sets attributes using the protocol, but you can hook the installation of the Memory Attribute Protocol into the policy system so it's not installed in compat mode. I have not revisited the memory protection policy interface update since Lazlo's feedback in October, but I'd be happy to return to it if=20 there's motivation to get it in over the finish line. Note that there are more=20 changes that will need to be made to add testing, compat mode switching, support for the nx_compat flag, etc. The patch series that's currently in flight is just meant to be a lateral move to a runtime=20 configurable interface. >> What I really want to avoid is derail our effort to tighten things >> down and comply with the NX compat related policies, by adding some >> build time control that the distros will enable now and never disable >> again, citing backward compat concerns. > Sure, I want that too. Having an runtime switch is already an > improvement over having a compile time switch. Having this working > fully automatic would be even better of course. If a fix for this issue is needed immediately, I'm fine with Ard's=20 solution as a stop-gap. Assuming we can make progress on committing the memory protection updates, I can update the CpuDxe drivers to check the memory protection policy before installing the Memory Attribute Protocol. When adding this policy=20 config, I would revert the change made here to uninstall the protocol. Thanks :) -Taylor -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#112138): https://edk2.groups.io/g/devel/message/112138 Mute This Topic: https://groups.io/mt/102967690/7686176 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- --------------p0gLioO1cdy490SzEF8RPW60 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable 
But what we might do is inve=
nt a way to avoid setting the XP attribute
on the entire region based on some heuristic. Given that the main
purpose of the EFI memory attribute protocol is to provide the ability
to remove XP (and set RO instead), perhaps we can avoid the set
entirely? Just brainstorming here.

Can the fault handler deal with this?  Set a flag somewhere, print a
big'n'fat error message, wait 5 secs, reset machine?  After reset the
firmware will see the flag and come up in 'compat' instead of 'strict'
mode.

Not sure what a good place for such a flag would be.  Do we have other
options than a non-volatile efi variable?  When using a efi variable we
probably should add an 'expires' timestamp, so the machine doesn't stay
in 'compat' mode forever.

This is what we do in Project = Mu currently and is what we would like to push into

EDK2. For x86 platforms, we us= e CMOS to communicate to the next boot that the

system needs to enter compatib= ility mode. Of course this doesn't work on ARM

platforms, so we'll have to co= me up with a more permanent mechanism to support

this functionality. 

(cc'ing Taylor and Oliver gi=
ven that this is related to the memory
policy work as well) Perhaps we can use the fact that the active image
is non-NX compat to make some tweaks?

Memory policies would be another candidate which could possibly use a
less strict profile in 'compat' mode.  I'd love to see memory policies
land for the February stable tag.

I don't think the policy can c= hange how the SHIM sets attributes using the

protocol, but you can hook the= installation of the Memory Attribute

Protocol into the policy syste= m so it's not installed in compat mode.

I have not revisited the memor= y protection policy interface update

since Lazlo's feedback in Octo= ber, but I'd be happy to return to it if there's

motivation to get it in over t= he finish line. Note that there are more changes

that will need to be made to a= dd testing, compat mode

switching, support for the nx_= compat flag, etc. The patch series that's

currently in flight is just me= ant to be a lateral move to a runtime configurable

interface. 

What I really want to avoid =
is derail our effort to tighten things
down and comply with the NX compat related policies, by adding some
build time control that the distros will enable now and never disable
again, citing backward compat concerns.

Sure, I want that too.  Having an runtime switch is already an
improvement over having a compile time switch.  Having this working
fully automatic would be even better of course.

If a fix for this issue is nee= ded immediately, I'm fine with Ard's solution as a stop-gap.

Assuming we can make progress = on committing the memory protection updates,

I can update the CpuDxe driver= s to check the memory protection policy

before installing the Memory A= ttribute Protocol. When adding this policy config,

I would revert the change made= here to uninstall the protocol.


Thanks :)

-Taylor

_._,_._,_
Groups.io Links:

=20 You receive all messages sent to this group. =20 =20

View/Reply Online (#112138) | =20 | Mute= This Topic | New Topic
Your Subscriptio= n | Contact Group Owner | Unsubscribe [rebecca@openfw.io]

_._,_._,_
--------------p0gLioO1cdy490SzEF8RPW60--