[edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

I've been debugging an assert failure when using HeapGuard on AArch64. A 
call to FreePages in SmbiosDxe is failing because the memory is aligned 
to 0x1000 instead of 0x10000 as defined by 
RUNTIME_PAGE_ALLOCATION_GRANULARITY.

I'm enabling HeapGuard by setting the PCDs to the following values:

gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC00000000000FFFF
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC00000000000FFFF

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#113995): https://edk2.groups.io/g/devel/message/113995
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/18/2024 7:27 AM, Rebecca Cran via groups.io wrote:
> I've been debugging an assert failure when using HeapGuard on AArch64. A 
> call to FreePages in SmbiosDxe is failing because the memory is aligned 
> to 0x1000 instead of 0x10000 as defined by 
> RUNTIME_PAGE_ALLOCATION_GRANULARITY.
> 
> I'm enabling HeapGuard by setting the PCDs to the following values:
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC00000000000FFFF
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC00000000000FFFF
> 

Hi Rebecca,

Are you including this commit: 
https://github.com/tianocore/edk2/commit/00b51e0d78a547dd78119ec44fcc74a01b6f79c8?

Can you share some more details on where this is failing? I.e. what
assert is getting tripped? Presumably without HeapGuard enabled, you
aren't seeing the failure?

Are you hitting this case:
https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Core/Dxe/Mem/Page.c#L1570-L1573?

Does this repro on ArmVirtPkg?

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114001): https://edk2.groups.io/g/devel/message/114001
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/18/2024 9:48 AM, Oliver Smith-Denny via groups.io wrote:

> Are you including this commit: 
> https://github.com/tianocore/edk2/commit/00b51e0d78a547dd78119ec44fcc74a01b6f79c8?
> 
> Can you share some more details on where this is failing? I.e. what
> assert is getting tripped? Presumably without HeapGuard enabled, you
> aren't seeing the failure?
> 
> Are you hitting this case:
> https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Core/Dxe/Mem/Page.c#L1570-L1573?
> 
> Does this repro on ArmVirtPkg?

Yes, I have that commit in my tree.

I'm hitting this assert in FreePages: 
https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Library/DxeCoreMemoryAllocationLib/MemoryAllocationLib.c#L190

It's called by SmbiosCreate64BitTable: 
https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c#L1342

And yes, that's the case I'm hitting.

I'm having trouble getting ArmVirtPkg to run. Would it be useful testing 
using SbsaQemu instead?

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114002): https://edk2.groups.io/g/devel/message/114002
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/18/2024 9:42 AM, Rebecca Cran via groups.io wrote:
> On 1/18/2024 9:48 AM, Oliver Smith-Denny via groups.io wrote:
> 
>> Are you including this commit: 
>> https://github.com/tianocore/edk2/commit/00b51e0d78a547dd78119ec44fcc74a01b6f79c8?
>>
>> Can you share some more details on where this is failing? I.e. what
>> assert is getting tripped? Presumably without HeapGuard enabled, you
>> aren't seeing the failure?
>>
>> Are you hitting this case:
>> https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Core/Dxe/Mem/Page.c#L1570-L1573?
>>
>> Does this repro on ArmVirtPkg?
> 
> Yes, I have that commit in my tree.
> 
> I'm hitting this assert in FreePages: 
> https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Library/DxeCoreMemoryAllocationLib/MemoryAllocationLib.c#L190
> 
> It's called by SmbiosCreate64BitTable: 
> https://github.com/tianocore/edk2/blob/59f024c76ee57c2bec84794536302fc770cd6ec2/MdeModulePkg/Universal/SmbiosDxe/SmbiosDxe.c#L1342
> 
> And yes, that's the case I'm hitting.
> 
> I'm having trouble getting ArmVirtPkg to run. Would it be useful testing 
> using SbsaQemu instead?
> 

Yeah, if you can get it running there, that would be a good data point.
I assume you mean the Project Mu QemuSbsaPkg? If so that is great, but
you will need to update the RUNTIME_PAGE_ALLOCATION_GRANULARITY back to
0x10000. It was set to 0x1000 for a historical issue that we are
working on reconciling with what edk2 has.

If you can get an open source repro, I'm happy to take a look at the
failure.

To clarify, if you turn off pool guard, does the assert go away?

I am suspicious that we are checking for the alignment before we
adjust the memory for the guard. I'm wondering if we actually
should do AdjustMemoryF (recalling function from memory) before
we check the alignment.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114003): https://edk2.groups.io/g/devel/message/114003
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/18/2024 11:38 AM, Oliver Smith-Denny wrote:

> Yeah, if you can get it running there, that would be a good data point.
> I assume you mean the Project Mu QemuSbsaPkg? If so that is great, but
> you will need to update the RUNTIME_PAGE_ALLOCATION_GRANULARITY back to
> 0x10000. It was set to 0x1000 for a historical issue that we are
> working on reconciling with what edk2 has.

No, I mean SbsaQemu from edk2-platforms: 
https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu

> To clarify, if you turn off pool guard, does the assert go away?

Yes.

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114004): https://edk2.groups.io/g/devel/message/114004
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/18/2024 10:45 AM, Rebecca Cran via groups.io wrote:
> No, I mean SbsaQemu from edk2-platforms: 
> https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu
> 

Sure, if you can repro there that is helpful.

Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114006): https://edk2.groups.io/g/devel/message/114006
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/18/2024 12:04 PM, Oliver Smith-Denny wrote:
> On 1/18/2024 10:45 AM, Rebecca Cran via groups.io wrote:
>> No, I mean SbsaQemu from edk2-platforms: 
>> https://github.com/tianocore/edk2-platforms/tree/master/Platform/Qemu/SbsaQemu
>>
> 
> Sure, if you can repro there that is helpful.

I've realized it won't repro there or other virtual platforms because it 
requires that SmbiosDxe run the reallocation code, which I believe is 
only done on systems with a larger number of SMBIOS tables - for example 
where RAS etc. is supported.

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114007): https://edk2.groups.io/g/devel/message/114007
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/18/2024 10:38 AM, Oliver Smith-Denny wrote:
> I am suspicious that we are checking for the alignment before we
> adjust the memory for the guard. I'm wondering if we actually
> should do AdjustMemoryF (recalling function from memory) before
> we check the alignment.
> 

Following up on this, this is very suspicious to me. If you change
the CoreInternalFreePages alignment check to something like:

EFI_PHYSICAL_ADDRESS CheckMemory = Memory;
UINTN		     CheckPages   = NumberOfPages;

AdjustMemoryF (&CheckMemory, &CheckPages);

if ((CheckMemory & (Alignment - 1)) != 0) {
     Status = EFI_INVALID_PARAMETER;
     goto Done;
}

Does this solve your issue? I have to run to a meeting, but I can
write this in actual patch form (and give it a quick test) later.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114008): https://edk2.groups.io/g/devel/message/114008
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/18/2024 12:26 PM, Oliver Smith-Denny wrote:

> Does this solve your issue? I have to run to a meeting, but I can
> write this in actual patch form (and give it a quick test) later.

Unfortunately that didn't work: I still get the assert.

...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table

ASSERT_EFI_ERROR (Status = Invalid Parameter)
ASSERT [SmbiosDxe] 
/local-data/src/ampereone/edk2/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c(145): 
!(((INTN)(RETURN_STATUS)(Status)) < 0)

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114090): https://edk2.groups.io/g/devel/message/114090
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/19/2024 8:34 AM, Rebecca Cran wrote:
> On 1/18/2024 12:26 PM, Oliver Smith-Denny wrote:
> 
>> Does this solve your issue? I have to run to a meeting, but I can
>> write this in actual patch form (and give it a quick test) later.
> 
> Unfortunately that didn't work: I still get the assert.
> 
> ...
> SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
> 
> ASSERT_EFI_ERROR (Status = Invalid Parameter)
> ASSERT [SmbiosDxe] 
> /local-data/src/ampereone/edk2/MdePkg/Library/UefiMemoryAllocationLib/MemoryAllocationLib.c(145): !(((INTN)(RETURN_STATUS)(Status)) < 0)
> 

Thanks for trying. In lieu of being able to test myself, all I can offer
is adding some more prints, when the memory gets allocated, making sure
it is 64k aligned then. I'd be curious to see what the address is that
is attempting to be freed.

My guess (as it was earlier) is that it is going to be aligned to
64k but + 4k. I.e the guard page at the front is throwing it off. There
may have just been an error in my attempt to fix the check for that.

If however that address is not 64k + 4k aligned, then something else is
afoot.

Happy to look at some more data if you get it or can engineer an example
on an open source system (can you force the system to call this function
twice even without the extra SMBIOS entries, etc.).

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114098): https://edk2.groups.io/g/devel/message/114098
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/19/2024 1:03 PM, Oliver Smith-Denny wrote:

> Thanks for trying. In lieu of being able to test myself, all I can offer
> is adding some more prints, when the memory gets allocated, making sure
> it is 64k aligned then. I'd be curious to see what the address is that
> is attempting to be freed.
> 
> My guess (as it was earlier) is that it is going to be aligned to
> 64k but + 4k. I.e the guard page at the front is throwing it off. There
> may have just been an error in my attempt to fix the check for that.
> 
> If however that address is not 64k + 4k aligned, then something else is
> afoot.
> 
> Happy to look at some more data if you get it or can engineer an example
> on an open source system (can you force the system to call this function
> twice even without the extra SMBIOS entries, etc.).

These are the addresses it's allocating with and without HeapGuard (with 
the original, upstream Page.c code).


Without HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0x00000000EF110000 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0x00000000FB7C0000)
...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0x00000000EF110000, 1)
Allocated 0x00000000EF110000 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (4153), 0x00000000FB8AEC8E)

------------------------------------------------------

WITH HeapGuard:

SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
Allocated 0x00000000ED36F000 with gBS->AllocatePages (AllocateAnyPages, 
EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0x00000000ED38F000)
...
SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
SmbiosCreate64BitTable: calling FreePages (0x00000000ED36F000, 1)
<ASSERT>

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114150): https://edk2.groups.io/g/devel/message/114150
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/22/2024 2:06 PM, Rebecca Cran via groups.io wrote:
> On 1/19/2024 1:03 PM, Oliver Smith-Denny wrote:
> 
>> Thanks for trying. In lieu of being able to test myself, all I can offer
>> is adding some more prints, when the memory gets allocated, making sure
>> it is 64k aligned then. I'd be curious to see what the address is that
>> is attempting to be freed.
>>
>> My guess (as it was earlier) is that it is going to be aligned to
>> 64k but + 4k. I.e the guard page at the front is throwing it off. There
>> may have just been an error in my attempt to fix the check for that.
>>
>> If however that address is not 64k + 4k aligned, then something else is
>> afoot.
>>
>> Happy to look at some more data if you get it or can engineer an example
>> on an open source system (can you force the system to call this function
>> twice even without the extra SMBIOS entries, etc.).
> 
> These are the addresses it's allocating with and without HeapGuard (with 
> the original, upstream Page.c code).
> 
> 
> Without HeapGuard:
> 
> SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
> Allocated 0x00000000EF110000 with gBS->AllocatePages (AllocateAnyPages, 
> EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0x00000000FB7C0000)
> ...
> SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
> SmbiosCreate64BitTable: calling FreePages (0x00000000EF110000, 1)
> Allocated 0x00000000EF110000 with gBS->AllocatePages (AllocateAnyPages, 
> EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (4153), 0x00000000FB8AEC8E)
> 
> ------------------------------------------------------
> 
> WITH HeapGuard:
> 
> SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
> Allocated 0x00000000ED36F000 with gBS->AllocatePages (AllocateAnyPages, 
> EfiRuntimeServicesData, EFI_SIZE_TO_PAGES (55), 0x00000000ED38F000)
> ...
> SmbiosCreate64BitTable() re-allocate SMBIOS 64-bit table
> SmbiosCreate64BitTable: calling FreePages (0x00000000ED36F000, 1)
> <ASSERT>
> 

I was able to repro your bug (by just turning on page guards on
ArmVirtQemu, allocating runtime mem and freeing it). I think you
are the first person to free runtime mem on ARM64 with page guards
enabled (and to care when it failed :).

The heap guard code is not written with ARM64 in mind (nor is much of
the codebase, of course). Specifically in this case the heap guard code
only wishes to preserve 4 KB alignment, it knows nothing of ARM64's
runtime page granularity required.

Let me take a look at this, I'm working on a solution here, but I want
to test this out further. I'll try to send a patch later this week or
next.

Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114162): https://edk2.groups.io/g/devel/message/114162
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 1/22/2024 6:53 PM, Oliver Smith-Denny wrote:

> I was able to repro your bug (by just turning on page guards on
> ArmVirtQemu, allocating runtime mem and freeing it). I think you
> are the first person to free runtime mem on ARM64 with page guards
> enabled (and to care when it failed :).
> 
> The heap guard code is not written with ARM64 in mind (nor is much of
> the codebase, of course). Specifically in this case the heap guard code
> only wishes to preserve 4 KB alignment, it knows nothing of ARM64's
> runtime page granularity required.
> 
> Let me take a look at this, I'm working on a solution here, but I want
> to test this out further. I'll try to send a patch later this week or
> next.

Thanks! I wonder if the same problem occurs on LoongArch64, which also 
defines the runtime page allocation granularity to be 0x10000?


MdePkg/Include/X64/ProcessorBind.h
261:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/LoongArch64/ProcessorBind.h
89:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x10000)

MdePkg/Include/RiscV64/ProcessorBind.h
120:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/Ia32/ProcessorBind.h
262:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/AArch64/ProcessorBind.h
164:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x10000)

MdePkg/Include/Arm/ProcessorBind.h
170:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

MdePkg/Include/Ebc/ProcessorBind.h
125:#define RUNTIME_PAGE_ALLOCATION_GRANULARITY  (0x1000)

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114164): https://edk2.groups.io/g/devel/message/114164
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/22/2024 6:14 PM, Rebecca Cran wrote:
> Thanks! I wonder if the same problem occurs on LoongArch64, which also 
> defines the runtime page allocation granularity to be 0x10000?
> 

Yes, it should have exactly the same problem


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#114348): https://edk2.groups.io/g/devel/message/114348
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 1/22/2024 5:53 PM, Oliver Smith-Denny wrote:> I was able to repro 
your bug (by just turning on page guards on
> ArmVirtQemu, allocating runtime mem and freeing it). I think you
> are the first person to free runtime mem on ARM64 with page guards
> enabled (and to care when it failed :).
> 
> The heap guard code is not written with ARM64 in mind (nor is much of
> the codebase, of course). Specifically in this case the heap guard code
> only wishes to preserve 4 KB alignment, it knows nothing of ARM64's
> runtime page granularity required.
> 
> Let me take a look at this, I'm working on a solution here, but I want
> to test this out further. I'll try to send a patch later this week or
> next.

Ok, got sidetracked, but got back to this. Rebecca, can you create a
bugzilla for this and try this patch:

https://github.com/tianocore/edk2/pull/5339.

I want to get some feedback on it and your testing before I send out a
patch for it. However, it did fix my repro case where I was failing to
free runtime memory with page guard enabled, so I believe it should fix
your case.

I need to fix some minor things like patch check, etc. I also did my
testing on a slightly old version of master, so may need a minor touch
up.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115073): https://edk2.groups.io/g/devel/message/115073
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 2/2/2024 5:59 PM, Oliver Smith-Denny wrote:

> Ok, got sidetracked, but got back to this. Rebecca, can you create a
> bugzilla for this and try this patch:
> 
> https://github.com/tianocore/edk2/pull/5339.
> 
> I want to get some feedback on it and your testing before I send out a
> patch for it. However, it did fix my repro case where I was failing to
> free runtime memory with page guard enabled, so I believe it should fix
> your case.
> 
> I need to fix some minor things like patch check, etc. I also did my
> testing on a slightly old version of master, so may need a minor touch
> up.

I've created https://bugzilla.tianocore.org/show_bug.cgi?id=4674 .

I cherry-picked your commit into my tree but unfortunately I now get a 
crash earlier:

Loading driver at 0x000F3AA0000 EntryPoint=0x000F3AB1514 
StatusCodeHandlerRuntimeDxe.efi
InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF F3BFEFD0
ProtectUefiImageCommon - 0xF3CA0E98
   - 0x00000000F3AA0000 - 0x0000000000030000
ConvertPages: range F3ADF000 - F3AF0FFF covers multiple entries
ConvertPages: range F3ADF000 - F3AEFFFF covers multiple entries
ConvertPages: range F3ADF000 - F3AEFFFF covers multiple entries


Synchronous Exception at 0x00000000FB8928E4
PC 0x0000FB8928E4 (0x0000FB886000+0x0000C8E4) [ 0] DxeCore.dll
PC 0x0000FB8A4798 (0x0000FB886000+0x0001E798) [ 0] DxeCore.dll
PC 0x0000FB898798 (0x0000FB886000+0x00012798) [ 0] DxeCore.dll
PC 0x0000F3AB1944 (0x0000F3AA0000+0x00011944) [ 1] 
StatusCodeHandlerRuntimeDxe.dll
PC 0x0000FB88D8F0 (0x0000FB886000+0x000078F0) [ 2] DxeCore.dll
PC 0x0000FB899EEC (0x0000FB886000+0x00013EEC) [ 2] DxeCore.dll
PC 0x0000FB89123C (0x0000FB886000+0x0000B23C) [ 2] DxeCore.dll

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115123): https://edk2.groups.io/g/devel/message/115123
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 2/5/2024 9:22 AM, Rebecca Cran via groups.io wrote:
> On 2/2/2024 5:59 PM, Oliver Smith-Denny wrote:
> 
>> Ok, got sidetracked, but got back to this. Rebecca, can you create a
>> bugzilla for this and try this patch:
>>
>> https://github.com/tianocore/edk2/pull/5339.
>>
>> I want to get some feedback on it and your testing before I send out a
>> patch for it. However, it did fix my repro case where I was failing to
>> free runtime memory with page guard enabled, so I believe it should fix
>> your case.
>>
>> I need to fix some minor things like patch check, etc. I also did my
>> testing on a slightly old version of master, so may need a minor touch
>> up.
> 
> I've created https://bugzilla.tianocore.org/show_bug.cgi?id=4674 .
> 
> I cherry-picked your commit into my tree but unfortunately I now get a 
> crash earlier:
> 
> Loading driver at 0x000F3AA0000 EntryPoint=0x000F3AB1514 
> StatusCodeHandlerRuntimeDxe.efi
> InstallProtocolInterface: BC62157E-3E33-4FEC-9920-2D3B36D750DF F3BFEFD0
> ProtectUefiImageCommon - 0xF3CA0E98
>    - 0x00000000F3AA0000 - 0x0000000000030000
> ConvertPages: range F3ADF000 - F3AF0FFF covers multiple entries
> ConvertPages: range F3ADF000 - F3AEFFFF covers multiple entries
> ConvertPages: range F3ADF000 - F3AEFFFF covers multiple entries
> 
> 
> Synchronous Exception at 0x00000000FB8928E4
> PC 0x0000FB8928E4 (0x0000FB886000+0x0000C8E4) [ 0] DxeCore.dll
> PC 0x0000FB8A4798 (0x0000FB886000+0x0001E798) [ 0] DxeCore.dll
> PC 0x0000FB898798 (0x0000FB886000+0x00012798) [ 0] DxeCore.dll
> PC 0x0000F3AB1944 (0x0000F3AA0000+0x00011944) [ 1] 
> StatusCodeHandlerRuntimeDxe.dll
> PC 0x0000FB88D8F0 (0x0000FB886000+0x000078F0) [ 2] DxeCore.dll
> PC 0x0000FB899EEC (0x0000FB886000+0x00013EEC) [ 2] DxeCore.dll
> PC 0x0000FB89123C (0x0000FB886000+0x0000B23C) [ 2] DxeCore.dll
> 

Well that's a bummer :). I moved to top of tree edk2 and added
StatusCodeHandlerRuntimeDxe to the ArmVirtQemu build and I am
not seeing your crash. Do you have any PCDs set for this driver?

I am running with the HeapGuard PCDs you sent before:

gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC00000000000FFFF
gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC00000000000FFFF

However, I am seeing a crash when the shell launches now, so I will
debug this. Something changed in the upstream, probably exposing a bug
in my code. Let's see if it is the same thing as you are seeing.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115124): https://edk2.groups.io/g/devel/message/115124
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Rebecca Cran via groups.io]

On 2/5/2024 10:51 AM, Oliver Smith-Denny wrote:
> Well that's a bummer :). I moved to top of tree edk2 and added
> StatusCodeHandlerRuntimeDxe to the ArmVirtQemu build and I am
> not seeing your crash. Do you have any PCDs set for this driver?

Our code is pretty heavily customized, so it's likely it's doing 
something differently. I can debug it further later this week.

> 
> I am running with the HeapGuard PCDs you sent before:
> 
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC00000000000FFFF
> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC00000000000FFFF
> 
> However, I am seeing a crash when the shell launches now, so I will
> debug this. Something changed in the upstream, probably exposing a bug
> in my code. Let's see if it is the same thing as you are seeing.

Could it be this bug you're running into? 
https://bugzilla.tianocore.org/show_bug.cgi?id=1463

-- 
Rebecca Cran


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115125): https://edk2.groups.io/g/devel/message/115125
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 2/5/2024 9:58 AM, Rebecca Cran wrote:
> On 2/5/2024 10:51 AM, Oliver Smith-Denny wrote:
>> Well that's a bummer :). I moved to top of tree edk2 and added
>> StatusCodeHandlerRuntimeDxe to the ArmVirtQemu build and I am
>> not seeing your crash. Do you have any PCDs set for this driver?
> 
> Our code is pretty heavily customized, so it's likely it's doing 
> something differently. I can debug it further later this week.
> 

Ok, it would be interesting to see what is happening here. Out of
curiousity, does this repro if you only have page or pool guards set,
not both? My shell bug only repros if both are set (which would seem
to point to a bug in shared guard pages).

>>
>> I am running with the HeapGuard PCDs you sent before:
>>
>> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask|0x0F
>> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPageType|0xC00000000000FFFF
>> gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0xC00000000000FFFF
>>
>> However, I am seeing a crash when the shell launches now, so I will
>> debug this. Something changed in the upstream, probably exposing a bug
>> in my code. Let's see if it is the same thing as you are seeing.
> 
> Could it be this bug you're running into? 
> https://bugzilla.tianocore.org/show_bug.cgi?id=1463
> 

Unfortunately the suggested patch does not fix the bug.

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115127): https://edk2.groups.io/g/devel/message/115127
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] AArch64 with HeapGuard: page allocations wrongly aligned

[Oliver Smith-Denny]

On 2/5/2024 12:36 PM, Oliver Smith-Denny wrote:
> On 2/5/2024 9:58 AM, Rebecca Cran wrote:
>> On 2/5/2024 10:51 AM, Oliver Smith-Denny wrote:
>>> Well that's a bummer :). I moved to top of tree edk2 and added
>>> StatusCodeHandlerRuntimeDxe to the ArmVirtQemu build and I am
>>> not seeing your crash. Do you have any PCDs set for this driver?
>>
>> Our code is pretty heavily customized, so it's likely it's doing 
>> something differently. I can debug it further later this week.
>>
> 
> Ok, it would be interesting to see what is happening here. Out of
> curiousity, does this repro if you only have page or pool guards set,
> not both? My shell bug only repros if both are set (which would seem
> to point to a bug in shared guard pages).
> 

Alright, I fixed the shell bug I saw. I had an edge case I previously
had covered, but lost in one of the reworkings. I force pushed my branch
here: https://github.com/tianocore/edk2/pull/5339. Can you see if that
resolves both the new and old issues you saw?

Thanks,
Oliver


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#115142): https://edk2.groups.io/g/devel/message/115142
Mute This Topic: https://groups.io/mt/103810212/7686176
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [rebecca@openfw.io]
-=-=-=-=-=-=-=-=-=-=-=-