[edk2-devel] [PATCH] MdeModulePkg: Fix runtime panic in ValidateSetVariable()

[Laszlo Ersek]

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 at 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 at intel.com>
  Hao A Wu <hao.a.wu at intel.com>
  Liming Gao <gaoliming at byosoft.com.cn>
  devel at edk2.groups.io


(2) My feedback:

Fixes: 355b181f74050cdf2f09b1755c1a5ee4affb1faf
Reviewed-by: Laszlo Ersek <lersek at redhat.com>
Tested-by: Laszlo Ersek <lersek at 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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68004): https://edk2.groups.io/g/devel/message/68004
Mute This Topic: https://groups.io/mt/78508231/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-