This is an archive of the discontinued LLVM Phabricator instance.

[hwasan] support hwasan-match-all-tag flag for hwasan meminstrinsic calls
ClosedPublic

Authored by Enna1 on May 5 2023, 2:31 AM.

Details

Summary

This patch implements __hwasan_memset_match_all, __hwasan_memcpy_match_all and __hwasan_memmove_match_all, making hwasan-match-all-tag flag working for hwasan versions of memset, memcpy and memmove.

Diff Detail

Event Timeline

Enna1 created this revision.May 5 2023, 2:31 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 5 2023, 2:31 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
Enna1 added a comment.May 5 2023, 3:12 AM

D149580 and this patch implement hwasan-match-all-tag flag support for hwasan callback memaccess instrumentation and memset, memcpy, memmove meminstrinsic calls.
And with D149580 and this patch, if hwasan-match-all-tag is set, calls to __hwasan_load, __hwasan_store, ____hwasan_memset, __hwasan_memcpy and __hwasan_memmove will be replaced with their match_all versions: __hwasan_load_match_all, __hwasan_store_match_all, __hwasan_memset_match_all, __hwasan_memcpy_match_all and __hwasan_memmove_match_all.
When HWAddressSanitizer compiling in kernel mode, hwasan-match-all-tag is implicitly set to 0xFF.
I'm not familiar with kernel mode HWASAN.
But from https://github.com/torvalds/linux/blob/master/mm/kasan/sw_tags.c#L88, I see pointers tagged with 0xff is already ignored in __hwasan_load, __hwasan_store.
So should we distinguish kernel mode HWASAN and user-space HWASAN for hwasan-match-all-tag flag ? :

  • for user-space HWASAN, when hwasan-match-all-tag flag is set, we emit calls to __hwasan_load_match_all, __hwasan_memset_match_all, etc.
  • for kernel mode HWASAN, hwasan-match-all-tag is implicitly set to 0xFF, we emit calls to __hwasan_load, __hwasan_memset, etc.

Or if hwasan-match-all-tag is set, no matter user-space HWASAN or kernel mode HWASAN, we always emit calls to __hwasan_load_match_all,__hwasan_memset_match_all, and implement __hwasan_load_match_all, __hwasan_memset_match_all in kernel HWASAN runtime ?

llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
690–691

Introduced in https://reviews.llvm.org/D122724, for supporting HWAddressSanitizer compiling in kernel mode

llvm/test/Instrumentation/HWAddressSanitizer/mem-intrinsics.ll
18–21

-hwasan-kernel implicit set match-all-tag to 0xff.
With this change, opt -S -passes=hwasan -hwasan-kernel will emit:

call ptr @__hwasan_memset_match_all
call ptr @memset_match_all

Just happen to pass these checks.

Enna1 published this revision for review.May 5 2023, 3:13 AM
Enna1 added reviewers: vitalybuka, eugenis, pcc, melver.
Enna1 added a subscriber: MTC.
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptMay 5 2023, 3:13 AM
Herald added subscribers: llvm-commits, Restricted Project. · View Herald Transcript
Enna1 added a comment.May 9 2023, 6:31 PM

gentle ping :)

Enna1 updated this revision to Diff 521895.May 13 2023, 3:25 AM

do not use match-all memset/memcpy/memmove for kernel mode HWASAN

vitalybuka added inline comments.May 16 2023, 3:48 PM
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
696

Can you make use of this constructor

template <size_t N>
    /*implicit*/ constexpr ArrayRef(const std::array<T, N> &Arr)
FunctionType *HWAsanMemTransferFnTy = FunctionType::get(
      Int8PtrTy,       ArrayRef(HWAsanMemTransferArgTys), false);
696

Can you make use of this constructor

template <size_t N>
    /*implicit*/ constexpr ArrayRef(const std::array<T, N> &Arr)

sorry, copied wrong one

template <size_t N>
    /*implicit*/ constexpr ArrayRef(const T (&Arr)[N]) : Data(Arr), Length(N) {}
FunctionType *HWAsanMemTransferFnTy = FunctionType::get(
      Int8PtrTy,       ArrayRef(HWAsanMemTransferArgTys), false);
Enna1 added inline comments.May 17 2023, 12:24 AM
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
696

Sorry, I'm not sure if this is exactly what you meant:

FunctionType *HWAsanMemTransferFnTy;
if (UseMatchAllCallback) {
  HWAsanMemTransferFnTy = FunctionType::get(
    Int8PtrTy, {Int8PtrTy, Int8PtrTy, IntptrTy, Int8Ty}, false);
} else {
  HWAsanMemTransferFnTy = FunctionType::get(
    Int8PtrTy, {Int8PtrTy, Int8PtrTy, IntptrTy}, false);
}

gentle ping

vitalybuka accepted this revision.May 23 2023, 12:25 PM
vitalybuka added inline comments.
llvm/lib/Transforms/Instrumentation/HWAddressSanitizer.cpp
696

yes, this also looks good

if (UseMatchAllCallback) {
     HWAsanMemTransferFnTy = FunctionType::get(
        Int8PtrTy, {Int8PtrTy, Int8PtrTy, IntptrTy, Int8Ty}, false);
     HWAsanMemsetFnTy = FunctionType::get(
        Int8PtrTy, {Int8PtrTy, Int32Ty, IntptrTy, Int8Ty}, false);
  } else {

  }
This revision is now accepted and ready to land.May 23 2023, 12:25 PM
Enna1 updated this revision to Diff 525122.May 24 2023, 5:29 AM

update from comments