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

["Laszlo Ersek" <lersek@redhat.com>]

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