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

["Pedro Falcato" <pedro.falcato@gmail.com>]

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

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> 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 <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>; Andrew Fish <
> afish@apple.com>
> *Cc:* Marvin Häuser <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 <afish=
> 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> 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> 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: 20638 bytes --]