Sounds great. I look forward to your V2. Thank you Yao Jiewen
From: Ard Biesheuvel [mailto:ard.biesheu...@linaro.org] Sent: Thursday, February 23, 2017 7:39 PM To: Yao, Jiewen <jiewen....@intel.com> Cc: edk2-devel@lists.01.org; af...@apple.com; leif.lindh...@linaro.org; Kinney, Michael D <michael.d.kin...@intel.com>; Gao, Liming <liming....@intel.com>; ler...@redhat.com; Tian, Feng <feng.t...@intel.com>; Zeng, Star <star.z...@intel.com> Subject: Re: [RFC PATCH 0/4] RFC: increased memory protection On 23 February 2017 at 08:52, Yao, Jiewen <jiewen....@intel.com<mailto:jiewen....@intel.com>> wrote: > HI Ard > > Thanks to protect more. :-) > Of course! This is a very important topic for me. > We did consider the idea to remove EXEC attribute for Data page before. But > we got compatibility issue. > > > > We documented some gaps in white paper - > > https://github.com/tianocore-docs/Docs/raw/master/White_Papers/A_Tour_Beyond_BIOS_Memory_Map_And_Practices_in_UEFI_BIOS_V2.pdf > Thanks for the link. > I am glad that some limitation is already resolved or we have solution to > mitigate it. But there is still some other need consideration. > > > > 1) We observe some 3rd part code allocating data page for code. – That > is hardest part. (OS limitation #12) > > We might also need consider ReservedMemory/AcpiNvs. There might be code > there, too. (Firmware limitation #6 and #7). > OK > If we want to apply the protection, we might need define a new PCD to > disable the data protection for compatibility consideration. > > > > 2) About DxeCore in data page. (Firmware limitation #4) > > I am thinking if we can fix LoadFile implementation in PeiCore. > > > > MdeModulePkg\Core\Pei\Image\Image.c: > > LoadAndRelocatePeCoffImage() > > { > > ImageContext.ImageAddress = (EFI_PHYSICAL_ADDRESS)(UINTN) AllocatePages > (EFI_SIZE_TO_PAGES ((UINT32) AlignImageSize)); > > } > > > > AllocatePages means to allocate data page. > > I think we should use PeiAllocatePages(EfiBootServicesCode, > EFI_SIZE_TO_PAGES ((UINT32) AlignImageSize), &ImageContext.ImageAddress); > > > > Does that fix the problem? > Using PeiServicesAllocatePage() in the way that you describe does indeed remove the problem, so I will use that instead. > 3) I am not worried about EBC. That can be handled separately. > OK > 4) I did not find patch 4/4 in my mail box. Maybe it is lost due to > some unknown reason. Would you please send it again? > https://lists.01.org/pipermail/edk2-devel/2017-February/007685.html I will send out a v2 shortly which, as you suggest, moves the handling to DXE core. The only problem is that ARM's SyncCacheConfig() removes the noexec attributes again, so I need to fix that first. Then, the arch CPU protocol installation event can iterate over the memory map to set the permissions according to a policy PCD Thanks, Ard. _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
