[edk2-devel] [PATCH v2 2/6] OvmfPkg/AmdSev: add Grub Firmware Volume Package

[Laszlo Ersek]

On 11/25/20 19:17, James Bottomley wrote:
> On Wed, 2020-11-25 at 09:09 -0800, James Bottomley wrote:
>> On Wed, 2020-11-25 at 08:02 -0800, James Bottomley wrote:
>>> On Wed, 2020-11-25 at 15:01 +0100, Laszlo Ersek wrote:
>>>> This upgrade gave me kernel 5.8.18-100.fc31.x86_64 in the guest
>>>> --
>>>> and this one does *not* crash. From your boot log below, I see
>>>> your guest kernel is 5.5.0; I suggest upgrading it.
>>>
>>> Heh, that's easier said than done ... I always make my encrypted
>>> images too small to upgrade a kernel easily.  Anyway, after doing
>>> the remove and add stuff dance, I finally got it upgraded to the
>>> latest debian testing linux-image-5.8.0-3 it's still crashing
>>> although with a slightly different traceback.  It looks like there
>>> might be something additional in the fedora 5.8 kernel that fixes
>>> this.  I'm going to try out upstream kernels next.
>>
>> I've got the upstream kernel booting through OVMF with a qemu -kernel
>> command line.  I also have a fix: it's not to delete the dummy
>> variable which was part of the ancient x86 anti bricking code (which
>> is also why arm64 doesn't have the problem).
> 
> OK, found the problem: the memory the variable policy itself is stored
> in doesn't survive into the linux runtime.  It looks to be pretty
> generic except you need a way of detecting the boot time memory is
> free.  Because Linux does a remapping, we see this.  I'm not sure how
> you'd construct a regression test for something like this, though.
> 
> I propose this as the fix ... at least it gets my SEV kernels booting
> again.
> 
> James
> 
> ---8>8>8><8<8<8----
> 
> From b33aa8bd8ded4c7cfec67c2d5aa5e23271c75f51 Mon Sep 17 00:00:00 2001
> From: James Bottomley <James.Bottomley at HansenPartnership.com>
> Date: Wed, 25 Nov 2020 10:10:30 -0800
> Subject: [PATCH 1/1] VariablePolicyLib: Fix Runtime panic in
>  ValidateSetVariable
> 
> 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.
> 
> 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;
>      }
> 

... yes. :)

Since you got to writing this up and testing it first, can you please
file a new TianoCore BZ about the issue (kernel crash log welcome), and
post this patch to edk2-devel stand-alone?

One request for the commit message: right after "Fix this by making it
AllocateRuntimePool", please add:

"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."

Thanks!
Laszlo



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