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 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 * On Behalf Of *Pedro > Falcato > *Sent:* Saturday, March 26, 2022 4:48 AM > *To:* edk2-devel-groups-io ; Andrew Fish < > afish@apple.com> > *Cc:* Marvin Häuser > *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 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 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 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