On Sat, Nov 03, 2018 at 02:42:21PM +0800, Jian J Wang wrote: > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 > > This issue originates from following patch which allows to enable > paging if PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy > (in addition to PcdSetNxForStack) are set to enable related features. > > 5267926134d17e86672b84fd57b438f05ffa68e1 > > Due to above change, PcdImageProtectionPolicy will be set to 0 by > default in many platforms, which, in turn, cause following code in > MdeModulePkg\Core\Dxe\Misc\MemoryProtection.c fail the creation of > notify event of CpuArchProtocol. > > 1138: if (mImageProtectionPolicy != 0 || > PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { > 1139: Status = CoreCreateEvent ( > ... > 1142: MemoryProtectionCpuArchProtocolNotify, > ... > 1145: ); > > Then following call flow won't be done and Guard pages will not be > set as not-present in SetAllGuardPages() eventually. > > MemoryProtectionCpuArchProtocolNotify() > => HeapGuardCpuArchProtocolNotify() > => SetAllGuardPages() > > The solution is removing the if(...) statement so that the notify > event will always be created and handler be registered. This won't > cause unnecessary code execution because, in the notify event handler, > the related PCDs like > > PcdImageProtectionPolicy and > PcdDxeNxMemoryProtectionPolicy > > will be checked again to do its job. > > Cc: Star Zeng <star.z...@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Ruiyu Ni <ruiyu...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.1 > Signed-off-by: Jian J Wang <jian.j.w...@intel.com> > --- > MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > index 30e5c5153c..30798b05b9 100644 > --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c > @@ -1135,7 +1135,6 @@ CoreInitializeMemoryProtection ( > ASSERT (GetPermissionAttributeForMemoryType (EfiBootServicesData) == > GetPermissionAttributeForMemoryType (EfiConventionalMemory)); > > - if (mImageProtectionPolicy != 0 || PcdGet64 > (PcdDxeNxMemoryProtectionPolicy) != 0) { > Status = CoreCreateEvent ( > EVT_NOTIFY_SIGNAL, > TPL_CALLBACK, > @@ -1154,7 +1153,6 @@ CoreInitializeMemoryProtection ( > &Registration > ); > ASSERT_EFI_ERROR(Status); > - }
And here we see why. The indentation changes need to be part of this patch, not 1/2. / Leif > > // > // Register a callback to disable NULL pointer detection at EndOfDxe > -- > 2.16.2.windows.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel