Re: [edk2-devel] Question about UEFI, AddressSanitizer and MMU mappings

["Steven Shi" <steven.shi@intel.com>]

[-- Attachment #1: Type: text/plain, Size: 9065 bytes --]

Yes, MPX is dead. Thank you to remove the MPX suggestion. And we could focus on PEI/DXE firstly.


Thanks
Steven Shi

From: Pedro Falcato <pedro.falcato@gmail.com>
Sent: Tuesday, March 29, 2022 12:31 AM
To: edk2-devel-groups-io <devel@edk2.groups.io>; Shi, Steven <steven.shi@intel.com>
Cc: Andrew Fish <afish@apple.com>; vit9696@protonmail.com; Marvin Häuser <mhaeuser@posteo.de>; Kinney, Michael D <michael.d.kinney@intel.com>; Zimmer, Vincent <vincent.zimmer@intel.com>; Schulz, Steffen <steffen.schulz@intel.com>; Tarral, Mathieu <mathieu.tarral@intel.com>; Morgan, Benoit <benoit.morgan@intel.com>; Xu, Min <min.xu@intel.com>; Liu, Wei <wei.liu@intel.com>
Subject: Re: [edk2-devel] Question about UEFI, AddressSanitizer and MMU mappings

Hi Steven!

Good to know you already have something. I removed your LLVM Optimizations suggestion that was about MPX, as Intel MPX is pretty dead (Intel is dropping it, compilers don't support it) as far as I know, and added
a new suggestion for UBSan, ASAN, and possibly MSAN (https://github.com/tianocore/tianocore.github.io/wiki/Tasks#LLVM_Sanitizer_support), mentioning your branch; note that I still left you in the "suggested by:".

I briefly looked at your code, and it seems that you had a different idea for shadow memory allocation. My idea (custom shadow mappings) uses up less memory and is probably way faster to boot, although I don't think it's possible to use it in runtime
services/SMM. Is it even important to instrument these with ASAN? I was thinking that most of the need was in PEI/DXE, not those.

Best regards,
Pedro

On Mon, Mar 28, 2022 at 12:32 PM Steven Shi <steven.shi@intel.com<mailto:steven.shi@intel.com>> wrote:
We enabled Asan and UBsan on edk2 DXE in 2017 after we introduced the CLANG38 build toolchain in edk2. It was quite useful to find dozens of code bugs. It is not difficult as it sounds, but we never finished all the scope, e.g., PEI, SMM. There are many limitations in current implementation, e.g., not cover page memory service. I’m glad if some people can continue to enhance it and finish it.

The edk2 sanitizer branch:
https://github.com/shijunjing/edk2/tree/sanitizer2
Edk2 sanitizer slides:
https://github.com/shijunjing/edk2/blob/sanitizer2/Edk2ASan.pptx
Usage readme:
https://github.com/shijunjing/edk2/blob/sanitizer2/readme_sanitizer.txt


  *   OvmfPkgIa32X64 build with sanitizers on edk2 and run:

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ git remote -v

origin  https://github.com/shijunjing/edk2.git (fetch)

origin  https://github.com/shijunjing/edk2.git (push)

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ git status

On branch sanitizer2

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ export CLANGSAN40_BIN=~/llvm/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/bin/export CLANGSAN40_BIN=~/llvm/clang+llvm-11.0.0-x86_64-linux-gnu-ubuntu-20.04/bin/

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ rm Conf/tools_def.txt

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ rm Conf/build_rule.txt

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ rm Conf/target.txt

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ source edksetup.sh

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ make -C BaseTools/

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ build -p OvmfPkg/OvmfPkgIa32X64.dsc -t CLANGSAN40 -a IA32 -a X64 -b NOOPT -n 5 -DDEBUG_ON_SERIAL_PORT

jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ qemu-system-x86_64 -m 5120 -smp 1 -bios ~/wksp_efi/edk2-fork4/Build/Ovmf3264/NOOPT_CLANGSAN40/FV/OVMF.fd -global e1000.romfile=""  -machine q35 -serial mon:stdio -display none --net none



  *   To see the enabling code:
jshi19@ub2-uefi-b01:~/wksp_efi/edk2-fork4$ git diff 4adc364c --name-only


  *   Asan Shadow Memory setup:
https://github.com/shijunjing/edk2/blob/sanitizer2/OvmfPkg/PlatformPei/MemDetect.c#L1133


  *   The compiler instrumentation routines for AddressSanitizer(ASan)
https://github.com/shijunjing/edk2/blob/sanitizer2/MdeModulePkg/Library/AsanLib/Asan.c

This Asan branch was synced to latest edk2 early this month by some people’s fuzz test requirement. But I didn’t really test it. I would like to help if there is something wrong in it. Let me know.


Thanks
Steven Shi


From: devel@edk2.groups.io<mailto:devel@edk2.groups.io> <devel@edk2.groups.io<mailto:devel@edk2.groups.io>> On Behalf Of Pedro Falcato
Sent: Saturday, March 26, 2022 4:48 AM
To: edk2-devel-groups-io <devel@edk2.groups.io<mailto:devel@edk2.groups.io>>; Andrew Fish <afish@apple.com<mailto:afish@apple.com>>
Cc: Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>>
Subject: Re: [edk2-devel] Question about UEFI, AddressSanitizer and MMU mappings

Andrew, Marvin,

Thanks for the quick responses.

I'll give you a rundown of asan/kasan: You create a big (16TB in PML5-less x86) virtual mapping for ASAN, each byte in the shadow map represents 8 bytes of address space, and you poison/unpoison memory as you go and allocate chunks of the address space (usually through malloc, but in our case, AllocatePool()/AllocatePages(), I imagine). Since the only thing you have is a large contiguous virtual mapping, you need to either take a page fault and create mappings on the address space as you go along (very possible in user-space, usually not possible in kernel space and I assume UEFI), or you need to do fun stuff w/ page tables; usually, this means that you set up some page tables pointing to a zero page and remap those same page tables all over the virtual mapping; after taking a look at all our available memory, we allocate shadow pages for those (so you can RW to them).

Note that going a different route (with some data structure instead of the big mapping) is possible but, if you do, you can't use the faster inline ASAN that clang/gcc can generate for you (which do these same memory accesses, but inlined instead of doing e.g call __asan_load_8).

So yeah, if SetMemoryAttributes is the only thing we have, we're going to need some support MMU code for each architecture.

Since adding AddressSanitizer support is pretty involved (build system + actual ASAN code + MMU support code for each arch), I feel like it would be a good large project for this year. I also feel tempted to throw UBSan into the mix and just call it "Add LLVM Sanitizer support to EDK2", but I don't know if that's too much for a GSoC student. Would love some feedback on this.

Note: I would like to work on this, but since I'll be a mentor this year I prefer to first see if a student is interested in this project.

Best regards,
Pedro

On Fri, Mar 25, 2022 at 6:42 PM Andrew Fish via groups.io<http://groups.io> <afish=apple.com@groups.io<mailto:apple.com@groups.io>> wrote:
From an UEFI point of view if you own the memory you can do what you want with it. The UEFI Spec does not deal with paging but the PI Spec does have abstractions for how the CPU operates via the CPU ARCH Protocol [1].

So for example if you want to write protect the page tables, add guard page, or add a stack guard all that is OK and exists today [2].
PcdNullPointerDetectionPropertyMask
PcdInitValueInTempStack
PcdHeapGuardPageType
PcdHeapGuardPoolType
PcdHeapGuardPropertyMask
PcdHeapGuardPageType
PcdHeapGuardPropertyMask
PcdCpuStackGuard

Does Asan just need to force page faults? Or does it want to make virtual address mappings?

If someone wants to work on ASan (or any of the other sanitizers) I’m happy to volunteer to consult.

[1] https://github.com/tianocore/edk2/blob/master/MdePkg/Include/Protocol/Cpu.h#L221
[2] https://github.com/tianocore/edk2/blob/master/MdeModulePkg/MdeModulePkg.dec#L979

Thanks,

Andrew Fish

On Mar 25, 2022, at 2:07 AM, Marvin Häuser <mhaeuser@posteo.de<mailto:mhaeuser@posteo.de>> wrote:

Hey Pedro,

ASan is somewhat listed for „LLVM Optimizations“.
A quick and dirty reference for UEFI UBSan can be found here: https://github.com/acidanthera/OpenCorePkg/tree/master/Library/OcGuardLib

I don’t think you need to strictly adhere to the UEFI spec for debug tooling. I cannot check the code now, but I can imagine things like ConvertPointer() will not be happy about non-identity-mapping OOTB. But the issues I can think of should be fairly easy to resolve.

Best regards,
Marvin

On 24. Mar 2022, at 23:32, Pedro Falcato <pedro.falcato@gmail.com<mailto:pedro.falcato@gmail.com>> wrote:

Hi!

I've been thinking about adding sanitizer support (UBSan and KASAN), like coreboot already has, to the wiki's Tasks for the upcoming GSoC, but I'm a bit confused by something.
Is there anything in the UEFI spec that stops us from doing non-identity memory mappings? I know it specifies the need for the identity mappings (in the architectures where it requires the MMU being enabled), but nowhere do I see anything about the other parts of the address space.
Of course, UEFI supporting AddressSanitizer would be kind of dependent on fancier memory mappings.

Thanks,
Pedro



--
Pedro Falcato



--
Pedro Falcato

[-- Attachment #2: Type: text/html, Size: 37542 bytes --]