On 29 June 2015 at 18:21, Carsey, Jaben <jaben.car...@intel.com> wrote: > >>-----Original Message----- >>From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>Sent: Monday, June 29, 2015 8:35 AM >>To: Yao, Jiewen >>Cc: Fleming, Matt; edk2-devel@lists.sourceforge.net >>Subject: Re: [edk2] BUG in properties table feature implementation >> >>On 29 June 2015 at 16:08, Yao, Jiewen <jiewen....@intel.com> wrote: >>> Good to know. Thanks! >>> >> >>Another question from my side: is it guaranteed the the memory map >>returned by GetMemoryMap() is sorted? >>Otherwise, it becomes non-trivial for the OS to ensure that all >>Runtime regions are mapped adjacently. >> > > I have seen lots of odd maps with the memmap shell command. I think that the > answer is no as a result, but I'd like to know if someone knows for sure. >
Thanks. Obvious next question: how are those OSes supposed to ensure that adjacent regions are mapped adjacently in the virtual mapping then? AFAICT the only way to do this 100% reliably is to sort the memory map first, and then go and find adjacent meta-regions, and keep those together. -- Ard. >> >>> -----Original Message----- >>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>> Sent: Monday, June 29, 2015 10:06 PM >>> To: Yao, Jiewen >>> Cc: edk2-devel@lists.sourceforge.net; Gao, Liming; Laszlo Ersek; Justen, >>Jordan L; Kinney, Michael D; Zeng, Star; Zimmer, Vincent; Fleming, Matt >>> Subject: Re: BUG in properties table feature implementation >>> >>> On 29 June 2015 at 15:58, Yao, Jiewen <jiewen....@intel.com> wrote: >>>> Yes. It seems we need some special handling for AArch64. >>>> If 64KiB is minimal paging unit, I think we need check 64KiB PE section >>alignment for AArch64. >>>> >>>> I confess that I only validated IA32 and X64, and I did not take AArch64 >>>> into >>account. >>>> >>>> BTW: There is no 64KiB alignment requirement for AArch32, right? I cannot >>find the word. So I just want to double confirm. >>>> >>> >>> No there is not. Only 64-bit ARM can execute with 64 KB page size. >>> >>> -- >>> Ard. >>> >>> >>>> -----Original Message----- >>>> From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] >>>> Sent: Monday, June 29, 2015 9:42 PM >>>> To: Yao, Jiewen >>>> Cc: edk2-devel@lists.sourceforge.net; Gao, Liming; Laszlo Ersek; Justen, >>Jordan L; Kinney, Michael D; Zeng, Star; Zimmer, Vincent; Fleming, Matt >>>> Subject: Re: BUG in properties table feature implementation >>>> >>>> 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 >>|0x00 >>>>> 00006e >>>>> >>>>> 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 > > ------------------------------------------------------------------------------ > 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 ------------------------------------------------------------------------------ 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
