User Details
- User Since
- Oct 3 2012, 3:00 AM (573 w, 11 h)
Aug 3 2023
LGTM, thank you.
Aug 2 2023
Jul 20 2023
Hmm, this is not 100% correct. CFI by design respects caller's settings for -fsanitize-recover and -fsanitize-trap, communicated through the DiagData argument to __cfi_check. A better default implementation would call __cfi_check_fail instead of trap unconditionally.
Jun 15 2023
Jun 14 2023
Jun 12 2023
This is long overdue, I think. Thank you!
May 24 2023
May 18 2023
I'm far from a C or C++ expert, but FWIW I'm not entirely convinced that this example breaks language semantics.
May 11 2023
LGTM
I do not understand the bionic issue. __hwasan_thread_exit is called very late (as the last thing in that function), while the signals are blocked. Why do thread cleanup functions matter?
May 1 2023
I'm fine with it in general. Is asan_abi.cpp meant as a temporary stub? It's not even link anywhere in the current version.
Apr 28 2023
Apr 26 2023
Apr 20 2023
LGTM
LGTM, thanks!
Apr 12 2023
Sorry, I do not remember why this requirement is there. Must be related to shadow / allocator placement and kernel mapping conflicts, but hwasan is using dynamic shadow so that should not be an issue... LGTM as long as it works.
Apr 4 2023
Mar 29 2023
Mar 24 2023
Mar 3 2023
Not convinced that this is right. The original fix is for a false postive, not a false negative - i.e., we want to prevent speculation of a memory access that is not provably safe.
Feb 9 2023
I think this should generally work.
Feb 8 2023
LGTM
LGTM but please make a better test case.
Jan 19 2023
LGTM, thank you!
Jan 17 2023
I've uploaded D141978 with a revert. I've opted against introducing a new subtarget feature because that would mean keeping dead code in the repo, without a way of testing it.
LGTM
LGTM with a comment
Jan 12 2023
Perfect, LGTM
Hmm do we allocate the ring buffer when tracking is not required? This would be a waste of a page.
Jan 11 2023
I think using the environment part of the triple is the right choice. We could also just go ahead and flip the current behavior until there is evidence of a platform that configures SCTRL_EL1.BT0 differently, at which point they can implement the triple logic.
LGTM
Jan 10 2023
This change claims that PACIASP has an implicit BTI JC. Is that correct? From my reading of the spec (and I have about 50% certainty that I'm right), the behavior is actually the same as of BTI C. At least this is true on Linux, where SCTRL_EL1.BT0 is set to 1.
LGTM
LGTM
Dec 20 2022
Do you have a stress test to exhaust all gwp-asan slots?
Dec 16 2022
Dec 15 2022
It's a single mprotect across the gwp-asan region. Also no need to bother with recursive guards as no new allocations can reach gwp-asan.
It seems like this could become much simpler if we disable gwp-asan on the first report instead of killing slots one-by-one.
Dec 9 2022
LGTM
Nov 23 2022
If there are no concerns about deprecating 39 and 42 bit VMA support in MSan, then I'm fine with this. I'd love to see performance comparison of static vs dynamic shadow, but this is a separate issue.
Nov 21 2022
Nov 18 2022
addressed Florian's comment
ping
Nov 17 2022
Nov 16 2022
expanded comments
Nov 10 2022
If we are changing the mapping + the ABI, we should be 100% confident the new one covers all ASLR possibilities. Why not just run some binary a lot of times and collect the range of addresses, or even inspect the kernel source for possible executable locations? Also, make it a large binary.
ping
ping
Ah, so is the problem with ASLR randomizing the initial executable mappings over a region larger than 64Gb? We do not care about app allocating memory, heap placement is defined by msan. Could you confirm that the new mapping covers all possible locations? Even with ex. a huge executable binary.
Sorry, I don't follow. What is limited to 64Gb (the sum of all app regions? why does it matter?) and why can't an "invalid" region be mapped?
Nov 7 2022
As explained in D117635, kPageSizeBits is part of the compiler contract and must be a compile-time constant.
Nov 3 2022
Oct 24 2022
LGTM, but I'd merge the two changes. I don't really see the point of submitting a test for the broken behavior first.
Oct 17 2022
LGTM
I like the direction of this change and agree that it needs a test.
Oct 14 2022
I think this is a fix for LazyInitialize in the tsan_rtl.h - the logic there assumes that when preinit_array is always used whenever it can be used, and no further initialization is necessary. This assumption does not hold under -shared-libsan. IMO this should be fixed in LazyInitialize, not here.
Oct 12 2022
LGTM
LGTM
Sep 28 2022
LGTM
Sep 20 2022
stage2 is fine with me, too
LGTM
Sep 12 2022
I think I'd rather keep left/right as internal concepts and only translate them to before/after in the output.
Sep 9 2022
Change description says that the new pass "marks them as tagged". That's not what is happening.
Sep 7 2022
Well compile-time may be hard. Hwasan does some runtime checks in InitShadow, that's ok too
Maybe add a few compile-time checks that mem->shadow->mem returns the original value? At least the sparc equations are complicated enough.
Sep 6 2022
Could you also fix asan and msan for consistency?
LGTM
LGTM
Aug 30 2022
Aug 26 2022
LGTM
LGTM
Aug 24 2022
I think this is a pretty minor change - we are not touching the general structure of the report, nor introducing a new error type (even though the latter happens from time to time).
Aug 23 2022
LGTM
LGTM
Aug 10 2022
Sorry but I had to revert this because of the conflicts with another revert: https://reviews.llvm.org/D131065