回复: [PATCH] MdeModulePkg: Fix runtime panic in ValidateSetVariable()

["gaoliming" <gaoliming@byosoft.com.cn>]

Laszlo and James:
  Thanks for your root cause. The fix is clear. It works on runtime version and smm version. Reviewed-by: Liming Gao <gaoliming@byosoft.com.cn>
  
  I agree to merge this hot fix for this stable tag 202011. I will add Laszlo tag and update subject title when I merge this patch. 

Thanks
Liming
> -----邮件原件-----
> 发件人: Laszlo Ersek <lersek@redhat.com>
> 发送时间: 2020年11月26日 5:01
> 收件人: jejb@linux.ibm.com; Liming Gao (Byosoft address)
> <gaoliming@byosoft.com.cn>
> 抄送: devel@edk2.groups.io; Bret Barkelew <brbarkel@microsoft.com>; Ard
> Biesheuvel (ARM address) <ard.biesheuvel@arm.com>; Hao A Wu
> <hao.a.wu@intel.com>; Jian J Wang <jian.j.wang@intel.com>
> 主题: Re: [PATCH] MdeModulePkg: Fix runtime panic in ValidateSetVariable()
> 
> On 11/25/20 21:13, James Bottomley wrote:
> > The current variable policy is allocated by AllocatePool(), which is
> > boot time only.  This means that if you do any variable setting in the
> > runtime, the policy has been freed.  Ordinarily this isn't detected
> > because freed memory is still there, but when you boot the Linux
> > kernel, it's been remapped so the actual memory no longer exists in
> > the memory map causing a page fault.
> >
> > Fix this by making it AllocateRuntimePool().  For SMM drivers, the
> > platform DSC is responsible for resolving the MemoryAllocationLib
> > class to the SmmMemoryAllocationLib instance. In the
> > SmmMemoryAllocationLib instance, AllocatePool() and
> > AllocateRuntimePool() are implemented identically. Therefore this
> > change is a no-op when the RegisterVariablePolicy() function is built
> > into an SMM driver. The fix affects runtime DXE drivers only.
> >
> > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3092
> > Signed-off-by: James Bottomley <jejb@linux.ibm.com>
> > ---
> >  MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> > index 5029ddb96adb..12944ac7ea81 100644
> > --- a/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> > +++ b/MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> > @@ -411,7 +411,7 @@ RegisterVariablePolicy (
> >      }
> >
> >      // Reallocate and copy the table.
> > -    NewTable = AllocatePool( NewSize );
> > +    NewTable = AllocateRuntimePool( NewSize );
> >      if (NewTable == NULL) {
> >        return EFI_OUT_OF_RESOURCES;
> >      }
> >
> 
> (1) CC'ing Jian and Hao:
> 
> $ python BaseTools/Scripts/GetMaintainer.py \
>     -l MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.c
> 
>   Jian J Wang <jian.j.wang@intel.com>
>   Hao A Wu <hao.a.wu@intel.com>
>   Liming Gao <gaoliming@byosoft.com.cn>
>   devel@edk2.groups.io
> 
> 
> (2) My feedback:
> 
> Fixes: 355b181f74050cdf2f09b1755c1a5ee4affb1faf
> Reviewed-by: Laszlo Ersek <lersek@redhat.com>
> Tested-by: Laszlo Ersek <lersek@redhat.com>
> 
> (I tested the actual bugfix with SMM-less OVMF.  I also
> regression-tested the patch, namely with SMM OVMF, and ArmVirtQemu too.)
> 
> 
> (3) I suggest updating the subject line as follows:
> 
>   MdeModulePkg/VariablePolicyLib: Fix runtime panic in
> ValidateSetVariable()
> 
> 74 characters, so it's not overlong.
> 
> No need to repost because of this.
> 
> 
> Liming, can you please pick up my feedback tags from (2), in addition to
> your own review, and refresh the subject as requested in (3), and then
> merge this patch -- before releasing edk2-stable202011?
> 
> Thank you all,
> Laszlo