Re: [PATCH v2 4/5] MdeModulePkg/Core: add freed-memory guard feature

[Laszlo Ersek <lersek@redhat.com>]

On 10/23/18 16:53, Jian J Wang wrote:
>> v2 changes:
>> a. Change prototype and implementation of IsHeapGuardEnabled()
>>    to allow it to check freed-memory guard feature.
>> b. Drop IsUafEnabled() because of a.
>> c. Move the sanity check of freed-memory guard and heap guard
>>    into HeapGuardCpuArchProtocolNotify()
>> d. Add GuardFreedPagesChecked() to avoid duplicate feature check
>> e. Coding style cleanup
> 
> Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
> which is illegal access to memory which has been freed. The principle
> behind is similar to heap guard feature, that is we'll turn all pool
> memory allocation to page allocation and mark them to be not-present
> once they are freed.
> 
> This also implies that, once a page is allocated and freed, it cannot
> be re-allocated. This will bring another issue, which is that there's
> risk that memory space will be used out. To address it, the memory
> service add logic to put part (at most 64 pages a time) of freed pages
> back into page pool, so that the memory service can still have memory
> to allocate, when all memory space have been allocated once. This is
> called memory promotion. The promoted pages are always from the eldest
> pages which haven been freed.
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Ruiyu Ni <ruiyu.ni@intel.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409 +++++++++++++++++++++++++++++++++-
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  63 +++++-
>  MdeModulePkg/Core/Dxe/Mem/Page.c      |  41 +++-
>  MdeModulePkg/Core/Dxe/Mem/Pool.c      |  21 +-
>  4 files changed, 513 insertions(+), 21 deletions(-)

I don't know when I will find the time to review this patch. Please make
sure that with BIT4 clear in the PCD, the changes are a no-op.

I'd prefer if you could regression-test the changes on OVMF as well, not
just on physical platforms.

Other than that, until I find the time, please proceed with the normal
review workflow -- feel free to submit further versions, according to
the MdeModulePkg maintainers' comments, and/or even push the final
version, should I prove unable to comment on this patch in time.

Thanks!
Laszlo