On 21 August 2018 at 07:50, Sughosh Ganu <[email protected]> wrote: > hi Ard, > > On Tue July 23, 2018 at 11:03PM +0530, Supreeth Venkatesh wrote: >> >> On Sat, 2018-07-21 at 20:06 +0900, Ard Biesheuvel wrote: >> > On 20 July 2018 at 21:38, Sughosh Ganu <[email protected]> wrote: >> > > >> > > From: Achin Gupta <[email protected]> >> > > >> > > The Standalone MM drivers runs in S-EL0 in AArch64 on ARM Standard >> > > Platforms and is deployed during SEC phase. The memory allocated to >> > > the Standalone MM drivers should be marked as RO+X. >> > > >> > > During PE/COFF Image section parsing, this patch implements extra >> > > action "UpdatePeCoffPermissions" to request the privileged firmware >> > > in >> > > EL3 to update the permissions. >> > > >> > > Contributed-under: TianoCore Contribution Agreement 1.1 >> > > Signed-off-by: Sughosh Ganu <[email protected]> >> > Apologies for bringing this up only now, but I don't think I was ever >> > cc'ed on these patches. >> > >> Apologies if you have missed it. But I am pretty sure it was part of >> earlier large patch-set on which you and leif were copied, as it was >> part of ArmPkg. >> > >> > We are relying on a debug hook in the PE/COFF loader to ensure that >> > we >> > don't end up with memory that is both writable and executable in the >> > secure world. Do we really think that is a good idea? >> > >> > (I know this code was derived from a proof of concept that I did >> > years >> > ago, but that was just a PoC) >> I think we need a little bit more details on what is your suggestion? >> >> A little bit background here: This code runs in S-EL0 and Request gets >> sent to secure world SPM to ensure that the region permissions are >> updated correctly via the "ArmMmuStandaloneMmCoreLib" SVC - >> ARM_SVC_ID_SP_SET_MEM_ATTRIBUTES_AARCH64. >> >> DebugPeCoffExtraActionLib is just used to extract image region >> information, but the region permission >> update request is sent to secure world for validation. >> >> With the above explanation, can you provide an insight into what was >> your thinking? >> Do you want us to create a separate library and call it >> as PeCoffExtraActionLib to avoid the "Debug" word though it is a hook >> to PeCoffExtraActionLib in MdePkg or do we want to create this library >> in a separate package (may be in MdePkg?) or something totally >> different. > > Supreeth had replied to your comments on the patch. Can you please > check this. If you feel that this needs to be implemented differently, > can you please suggest it to us. Thanks. >
My point is that such a fundamental action that needs to occur while loading the PE/COFF image should not be hooked into the loader this way. _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
