From nobody Tue May 21 00:26:04 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+112179+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+112179+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=kernel.org ARC-Seal: i=1; a=rsa-sha256; t=1701943568; cv=none; d=zohomail.com; s=zohoarc; b=BMBiyHnqr+go3PFfM2b3ekPjgMkmbv+a+U+pW9+z9DMYc0iNPK//jTl5hSeRrEADhR5F/H3+/HFnTjrHQqfB3xBsTr3wOKv1GxAsqW9KqEchyL4eRPhdEKqdpuYj1qjOmYLCfDj+EofdQt5FiOOK+KjYtI4sgShk/8IuXREvdrs= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1701943568; h=Content-Type:Cc:Cc:Date:Date:From:From:List-Subscribe:List-Id:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:Reply-To:Sender:Subject:Subject:To:To:Message-Id; bh=Z7yCDbDwY1EZd62BpUOSTDGaMRSkXfYfaHOH/PFk5kc=; b=aPlRZmg2nSLx3ofg7tttIk8KeLGV00HRZrk3bbm4Vyv5IaJVAprxAdXM4oB9+V+IXK1qVhaLXmgyIcF4NKwi7wPoEKSgZqlgclyT1bgjXIEZVeflaNAt6AekTaQKQPuAwsNN7+m8GQ5OuEIkRzXSJhve9prHwyC0PNDhvaFj0s8= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+112179+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 1701943568868824.3045013141273; Thu, 7 Dec 2023 02:06:08 -0800 (PST) Return-Path: DKIM-Signature: a=rsa-sha256; bh=LhyV9pQlUTAOJAA8UyxXIgnOMV+jOTNY8MEbO8ql82A=; c=relaxed/simple; d=groups.io; h=Date:Mime-Version:Message-ID:Subject:From:To:Cc:Precedence:List-Subscribe:List-Help:Sender:List-Id:Mailing-List:Delivered-To:Reply-To:List-Unsubscribe-Post:List-Unsubscribe:Content-Type; s=20140610; t=1701943568; v=1; b=IGTtfZgReTiMfidVSyZ0snOj13i946X7WvGobmLtBDattAOPDWYX6NuUo0grFsp0ZDs7+TtO 049+vQ+L2D6aHzvdRqGj/+WhcCv7DSYr60BJV+R8RlYF1PNWQEDH1p3rbVXrd99j33/yqwADRxG 3sM2BzYdwrG7KWbMBR+jI2Y4= X-Received: by 127.0.0.2 with SMTP id 7SfvYY1788612xHSQ0AeNXdh; Thu, 07 Dec 2023 02:06:08 -0800 X-Received: from mail-yw1-f201.google.com (mail-yw1-f201.google.com [209.85.128.201]) by mx.groups.io with SMTP id smtpd.web10.80701.1701943567800505772 for ; Thu, 07 Dec 2023 02:06:07 -0800 X-Received: by mail-yw1-f201.google.com with SMTP id 00721157ae682-5d42c43d8daso3494257b3.0 for ; Thu, 07 Dec 2023 02:06:07 -0800 (PST) X-Gm-Message-State: mwgosJNVMxcYHSZLBxkX9Ftzx1787277AA= X-Google-Smtp-Source: AGHT+IE7Nckb2loEczJ+U4t3wvfEPPmQVpr9VV1xjnUXRkHYXhEqQnJo8yeJas1nEPO/xucn/C0hwety X-Received: from palermo.c.googlers.com ([fda3:e722:ac3:cc00:28:9cb1:c0a8:118a]) (user=ardb job=sendgmr) by 2002:a05:690c:3382:b0:5d3:985c:800c with SMTP id fl2-20020a05690c338200b005d3985c800cmr149657ywb.3.1701943566548; Thu, 07 Dec 2023 02:06:06 -0800 (PST) Date: Thu, 7 Dec 2023 11:06:03 +0100 Mime-Version: 1.0 Message-ID: <20231207100603.2654084-1-ardb@google.com> Subject: [edk2-devel] [PATCH v2] ArmVirt: Allow memory attributes protocol to be disabled on first boot From: "Ard Biesheuvel" To: devel@edk2.groups.io Cc: Ard Biesheuvel , Laszlo Ersek , Gerd Hoffmann , Oliver Steffen , Alexander Graf , Oliver Smith-Denny , Taylor Beebe , Peter Jones , Leif Lindholm 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,ardb@kernel.org List-Unsubscribe-Post: List-Unsubscribe=One-Click List-Unsubscribe: X-ZohoMail-DKIM: pass (identity @groups.io) X-ZM-MESSAGEID: 1701943569335100001 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" From: Ard Biesheuvel Shim's PE loader uses the EFI memory attributes protocol in a way that results in an immediate crash when invoking the loaded image, unless the base and size of its executable segment are both aligned to 4k. If this is not the case, it will strip the memory allocation of its executable permissions, but fail to add them back for the executable region, resulting in non-executable code. Unfortunately, the PE loader does not even bother invoking the protocol in this case (as it notices the misalignment), making it very hard for system firmware to work around this by attempting to infer the intent of the caller. So let's introduce a QEMU command line option to indicate that the protocol should not be exposed at all on the first boot, which is when the issue is triggered. (fbaa64.efi is broken but grubaa64.efi boots fine) -fw_cfg opt/org.tianocore/UninstallMemAttrProtocolOnFirstBoot,string=3Dy Also introduce a fixed boolean PCD that sets the default. Cc: Laszlo Ersek Cc: Gerd Hoffmann Cc: Oliver Steffen Cc: Alexander Graf Cc: Oliver Smith-Denny Cc: Taylor Beebe Cc: Peter Jones Cc: Leif Lindholm Link: https://gitlab.com/qemu-project/qemu/-/issues/1990 Signed-off-by: Ard Biesheuvel --- ArmVirtPkg/ArmVirtPkg.dec | 6 = ++ ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf | 7 = ++ ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c | 85 = ++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/ArmVirtPkg/ArmVirtPkg.dec b/ArmVirtPkg/ArmVirtPkg.dec index 0f2d7873279f..c55978f75c19 100644 --- a/ArmVirtPkg/ArmVirtPkg.dec +++ b/ArmVirtPkg/ArmVirtPkg.dec @@ -68,3 +68,9 @@ [PcdsFixedAtBuild, PcdsPatchableInModule] # Cloud Hypervisor has no other way to pass Rsdp address to the guest ex= cept use a PCD. # gArmVirtTokenSpaceGuid.PcdCloudHvAcpiRsdpBaseAddress|0x0|UINT64|0x000000= 05 + + ## + # Whether the EFI memory attribus protocol should be uninstalled before + # invoking the OS loader on the first boot. This may be needed to work a= round + # problematic builds of shim that use the protocol incorrectly. + gArmVirtTokenSpaceGuid.PcdUninstallMemAttrProtocolOnFirstBoot|FALSE|BOOL= EAN|0x00000006 diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerL= ib.inf b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.i= nf index 997eb1a4429f..5d119af6a3b3 100644 --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBootManagerLib.inf @@ -16,6 +16,7 @@ [Defines] MODULE_TYPE =3D DXE_DRIVER VERSION_STRING =3D 1.0 LIBRARY_CLASS =3D PlatformBootManagerLib|DXE_DRIVER + CONSTRUCTOR =3D PlatformBootManagerLibConstructor =20 # # The following information is for reference only and not required by the = build tools. @@ -46,6 +47,7 @@ [LibraryClasses] PcdLib PlatformBmPrintScLib QemuBootOrderLib + QemuFwCfgSimpleParserLib QemuLoadImageLib ReportStatusCodeLib TpmPlatformHierarchyLib @@ -55,6 +57,7 @@ [LibraryClasses] UefiRuntimeServicesTableLib =20 [FixedPcd] + gArmVirtTokenSpaceGuid.PcdUninstallMemAttrProtocolOnFirstBoot gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity @@ -73,5 +76,9 @@ [Guids] [Protocols] gEfiFirmwareVolume2ProtocolGuid gEfiGraphicsOutputProtocolGuid + gEfiMemoryAttributeProtocolGuid gEfiPciRootBridgeIoProtocolGuid gVirtioDeviceProtocolGuid + +[Depex] + gEfiVariableArchProtocolGuid diff --git a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c b/ArmVi= rtPkg/Library/PlatformBootManagerLib/PlatformBm.c index 85c01351b09d..5306d9ea0a05 100644 --- a/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c +++ b/ArmVirtPkg/Library/PlatformBootManagerLib/PlatformBm.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -1274,3 +1275,87 @@ PlatformBootManagerUnableToBoot ( EfiBootManagerBoot (&BootManagerMenu); } } + +/** + Uninstall the EFI memory attribute protocol if it exists. +**/ +STATIC +VOID +UninstallEfiMemoryAttributesProtocol ( + VOID + ) +{ + EFI_STATUS Status; + EFI_HANDLE Handle; + UINTN Size; + VOID *MemoryAttributeProtocol; + + Size =3D sizeof (Handle); + Status =3D gBS->LocateHandle ( + ByProtocol, + &gEfiMemoryAttributeProtocolGuid, + NULL, + &Size, + &Handle + ); + + if (EFI_ERROR (Status)) { + ASSERT (Status =3D=3D EFI_NOT_FOUND); + return; + } + + Status =3D gBS->HandleProtocol ( + Handle, + &gEfiMemoryAttributeProtocolGuid, + &MemoryAttributeProtocol + ); + ASSERT_EFI_ERROR (Status); + + Status =3D gBS->UninstallProtocolInterface ( + Handle, + &gEfiMemoryAttributeProtocolGuid, + MemoryAttributeProtocol + ); + ASSERT_EFI_ERROR (Status); +} + +EFI_STATUS +EFIAPI +PlatformBootManagerLibConstructor ( + IN EFI_HANDLE ImageHandle, + IN EFI_SYSTEM_TABLE *SystemTable + ) +{ + BOOLEAN Uninstall; + UINTN VarSize; + UINT32 Attr; + + // + // Work around shim's terminally broken use of the EFI memory attributes + // protocol, by uninstalling it if requested on the QEMU command line. + // + // E.g., + // -fw_cfg opt/org.tianocore/UninstallMemAttrProtocolOnFirstBoot,s= tring=3Dy + // + // This is only needed on the first boot, when fbaa64.efi is being invok= ed to + // set the boot order variables. Subsequent boots involving GRUB are not + // affected. + // + VarSize =3D 0; + if (gRT->GetVariable ( + L"BootOrder", + &gEfiGlobalVariableGuid, + &Attr, + &VarSize, + NULL + ) =3D=3D EFI_NOT_FOUND) + { + Uninstall =3D FixedPcdGetBool (PcdUninstallMemAttrProtocolOnFirstBoot); + QemuFwCfgParseBool ("opt/org.tianocore/UninstallMemAttrProtocolOnFirst= Boot", &Uninstall); + if (Uninstall) { + UninstallEfiMemoryAttributesProtocol (); + } + } + + return EFI_SUCCESS; +} -- 2.43.0.rc2.451.g8631bc7472-goog -=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 (#112179): https://edk2.groups.io/g/devel/message/112179 Mute This Topic: https://groups.io/mt/103031504/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-