On 29 June 2015 at 14:54, Yao, Jiewen <jiewen....@intel.com> wrote: > Thanks for the sharing. Yes. I agree with you that this breaks backward > compatibility. > Right, I do not think we can boot Win7 or old Win8 with this feature enabled. > > Both UEFI OS and Firmware need support this UEFI2.5 Properties Table feature. > I believe it is known by UEFI forum, when it is added to UEFI spec, and it is > published. >
Yes, but I don't think it is documented anywhere that the OS needs to map all runtime regions adjacently even if it does not actually care about the EFI_MEMORY_RO attributes. Essentially, the OS always need to map adjacently to be compatible with UEFI 2.5, and the current AArch64 Linux kernel crashes badly on any UEFI 2,5 implementation that has this feature enabled. > UEFI 2.5 properties table is optional feature. It can be enabled or disabled. > The benefit of this feature is that: a UEFI2.5 OS can do memory protection > based on UEFI memory map. > But the old UEFI OS cannot get any benefit. > > So a platform can made judgment based on its need, as well as the UEFI 2.5 OS > availability. > Unfortunately, this makes it an opt-in setting, and system will ship with it disabled by default, in order to prevent boot failures on older OSes. That is not typically how you want to deploy a security feature. There is another issue I would like your opinion about: for AArch64, MdeModulePkg/Core/Dxe/Mem/Imem.h defines the following #define EFI_ACPI_RUNTIME_PAGE_ALLOCATION_ALIGNMENT (SIZE_64KB) and in CoreTerminateMemoryMap(), a final check is done that no Runtime regions are aligned incorrectly. Yet, with the Properties Table feature enabled, memory regions are split without taking this into account. Strangely enough, CoreTerminateMemoryMap () does not complain about this. This is documented in the UEFI spec 2.3.6: """ All 4KiB memory pages allocated for use by runtime services (of types EfiRuntimeServicesCode, EfiRuntimeServicesData and EfiACPIMemoryNVS) must use identical ARM Memory Page Attributes (as described in Table 8) within the same physical 64KiB page. Mixed attribute mappings within a larger page are not allowed. """ So I suppose the Properties table feature should use 64 KB not 4 KB when executing on AArch64. However, the documentation for the Properties Table feature mentions 4 KB explicitly, so that should go into the errata as well. Regards, Ard. > That is why we do not enable this PE/COFF 4K alignment in BaseTool as > standard configuration. > So in other word, it is disabled by tool by default. A platform may also > decide to not publish PropertiesTable by below PCD: > ## Publish PropertiesTable or not. > # > # If this PCD is TRUE, DxeCore publishs PropertiesTable. > # DxeCore evaluates if all runtime drivers has 4K aligned PE sections. If > all > # PE sections in runtime drivers are 4K aligned, DxeCore sets BIT0 in > # PropertiesTable. Or DxeCore clears BIT0 in PropertiesTable. > # If this PCD is FALSE, DxeCore does not publish PropertiesTable. > # > # If PropertiesTable has BIT0 set, DxeCore uses below policy in UEFI memory > map: > # 1) Use EfiRuntimeServicesCode for runtime driver PE image code section > and > # use EfiRuntimeServicesData for runtime driver PE image header and > other section. > # 2) Set EfiRuntimeServicesCode to be EFI_MEMORY_RO. > # 3) Set EfiRuntimeServicesData to be EFI_MEMORY_XP. > # 4) Set EfiMemoryMappedIO and EfiMemoryMappedIOPortSpace to be > EFI_MEMORY_XP. > # > # NOTE: Platform need gurantee this PCD is set correctly. Platform should > set > # this PCD to be TURE if and only if all runtime driver has seperated > Code/Data > # section. If PE code/data sections are merged, the result is unpredictable. > # > # @Prompt Publish UEFI PropertiesTable. > gEfiMdeModulePkgTokenSpaceGuid.PropertiesTableEnable|TRUE|BOOLEAN|0x0000006e > > Thank you > Yao Jiewen > > > -----Original Message----- > From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] > Sent: Monday, June 29, 2015 8:44 PM > To: Yao, Jiewen > Cc: edk2-devel@lists.sourceforge.net; Gao, Liming; Laszlo Ersek; Justen, > Jordan L; Kinney, Michael D; Zeng, Star > Subject: Re: BUG in properties table feature implementation > > On 29 June 2015 at 14:34, Yao, Jiewen <jiewen....@intel.com> wrote: >> Hi Ard >> Yes, your are right. We do observe similar odd behavior in old >> Win7/Win8/Win8.1, and we have to disable it. >> Win8 and Win8.1 have OS patch to set adjacent virtual memory map, to resolve >> this issue. > > I see this as a big problem with the feature, as it breaks backward > compatibility. IOW, you won't be able to boot Windows 7 on a UEFI 2.5 system > unless you manually disable the feature (in the setup screen, I > suppose?) > >> We also tested Win10, and Suse 11 GM. They are good. >> >> Would you please share the information on which OS you are using? >> > > I am using the upstream Linux kernel for AArch64. I am not using a > distribution. > >> All in all, if OS cannot guarantee the virtual memory map is adjacent, we >> have to disable this feature. >> > > This is backwards: the feature should be implemented such that it cannot be > trivially broken by a well-behaving OS. Note that nothing in the > SetVirtualAddressMap () implementation suggests that the regions should be > adjacent. > > Perhaps it would have been better to introduce a new memory region type > EfiRuntimeServicesPecoffData, so the OS at least knows which regions it > should keep adjacent, (i.e., Code and PecoffData regions should be kept > together) > > So does PE/COFF even allow the memory image to deviate from the file image? > If so, the correct way is to update the PE/COFF loader and the relocation > logic can deal with sections being laid out differently in memory than they > are in the file. > > -- > Ard. > >> -----Original Message----- >> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >> Sent: Monday, June 29, 2015 6:46 PM >> To: edk2-devel@lists.sourceforge.net; Yao, Jiewen; Gao, Liming; Laszlo >> Ersek; Justen, Jordan L; Kinney, Michael D; Zeng, Star >> Subject: BUG in properties table feature implementation >> >> Hello all, >> >> I am running into another problem with the implementation of the UEFI >> 2.5 Properties Table feature. It splits PE/COFF images into separate but >> adjacent memory regions, only to be able to assign different permissions to >> .text and .data sections. This is working fine at boot time. >> >> However, at runtime, after calling virtual address map, this breaks >> down completely. Since the virtual mapping supplied to >> SetVirtualAddressMap() does not have to guarantee adjacency between code and >> data regions (of which the OS does not know whether they belong together or >> not), reapplying the relocations corrupts the memory image and breaks the >> runtime services. >> >> For example, this region >> >> 0x00005eeb1000-0x00005eeb6fff [Runtime Code] >> 0x00005eeb7000-0x00005eec0fff [Runtime Data] >> >> is mapped on AARCH64 as >> >> EFI remap 0x000000005eeb1000 => 00000000440a1000 >> EFI remap 0x000000005eeb7000 => 00000000440b7000 >> >> which retains the relative alignment, but adds a 64 KB offset to the second >> regions so that the regions can still be mapped with different permissions >> when the OS is executing with a 64 KB page size. >> >> As far as I can tell, this is in accordance with the spec, and was working >> fine until I tried to enable the properties table feature. >> With that enabled, the two above regions could actually describe one single >> PE/COFF image in memory, and the 64 KB offset results in the relocations to >> be applied incorrectly. >> >> I looked at PeCoffLoaderRelocateImageForRuntime () but to me, it is >> not very obvious how to solve this. Obviously, our PE/COFF >> implementation is not complete since it assumes file offset == memory >> offset for sections, but this does not hold anymore for UEFI 2.5 >> >> I would also like to point out again that this is another result of the fact >> that this series was pushed through with any review or testing outside of >> the Intel firmware team. For features of this magnitude and complexity, more >> scrutiny and testing is obviously required. >> >> Kind regards, >> Ard. ------------------------------------------------------------------------------ Monitor 25 network devices or servers for free with OpManager! OpManager is web-based network management software that monitors network devices and physical & virtual servers, alerts via email & sms for fault. Monitor 25 devices for free with no restriction. Download now http://ad.doubleclick.net/ddm/clk/292181274;119417398;o _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
