Re: [RFC PATCH 0/4] RFC: increased memory protection

["Yao, Jiewen" <jiewen.yao@intel.com>]

HI Ard
In X86 CPU driver - UefiCpuPkg\CpuDxe, we use a global variable – mIsFlushingGCD.

In RefreshGcdMemoryAttributes(), we set mIsFlushingGCD=TRUE.
In CpuSetMemoryAttributes(), we check mIsFlushingGCD. If mIsFlushingGCD is TRUE, CpuSetMemoryAttributes() returns immediately without touching cache attribute or memory attribute.

The reason is that RefreshGcdMemoryAttributes() just sync current CPU hardware setting to GCD software record.
No real need to set cache again.

Previous we purposely skip GCD setting on RO/XP, the reason is still compatibility concern.
We do not want to provide a different memory map to 3rd part code, just in case there is hidden assumption on memory map attributes.


Maybe ARM can use similar way in SyncCacheConfig() and do a simple check in CpuSetMemoryAttributes().

Thank you
Yao Jiewen

From: Ard Biesheuvel [mailto:ard.biesheuvel@linaro.org]
Sent: Friday, February 24, 2017 3:33 AM
To: Yao, Jiewen <jiewen.yao@intel.com>
Cc: edk2-devel@lists.01.org; afish@apple.com; leif.lindholm@linaro.org; Kinney, Michael D <michael.d.kinney@intel.com>; Gao, Liming <liming.gao@intel.com>; lersek@redhat.com; Tian, Feng <feng.tian@intel.com>; Zeng, Star <star.zeng@intel.com>
Subject: Re: [RFC PATCH 0/4] RFC: increased memory protection

On 23 February 2017 at 11:45, Yao, Jiewen <jiewen.yao@intel.com<mailto:jiewen.yao@intel.com>> wrote:
> Sounds great.
>
> I look forward to your V2.
>

Hello Jiewen,

What I am currently struggling with is the fact that we don't use the
GCD RO/XP permissions at all. This means that
RefreshGcdMemoryAttributes () (or SyncCacheConfig() on ARM) will
remove non-exec attributes if we add them in the CPU arch protocol
installation notifier callback.

So there are two approaches imo:
- introduce a way to call into the DXE core to mark all non-code
regions non-exec after RefreshGcdMemoryAttributes () has been called,
or
- add the RO/XP attributes to the GCD memory space map, and enable
them in the attributes.

Option #2 will require a change to CoreAddRange to prevent those RO/XP
attributes to leak into the UEFI memory map, because that results in
all regions have to RO/XP attributes set by default, which is
obviously not what we want.

Any thoughts?

Thanks,
Ard.