Page MenuHomePhabricator

cryptoad (Kostya Kortchinsky)
User

Projects

User does not belong to any projects.

User Details

User Since
May 5 2016, 2:57 PM (140 w, 5 d)

Recent Activity

Wed, Dec 19

cryptoad committed rL349666: [sanitizer] Remove spurious semi-colon.
[sanitizer] Remove spurious semi-colon
Wed, Dec 19, 11:08 AM
cryptoad committed rCRT349666: [sanitizer] Remove spurious semi-colon.
[sanitizer] Remove spurious semi-colon
Wed, Dec 19, 11:08 AM
cryptoad closed D55849: [sanitizer] Remove spurious semi-colon.
Wed, Dec 19, 11:08 AM

Tue, Dec 18

cryptoad created D55849: [sanitizer] Remove spurious semi-colon.
Tue, Dec 18, 12:37 PM

Dec 10 2018

cryptoad added a comment to D54904: Introduce `AddressSpaceView` template parameter to `SizeClassAllocator32`, `FlatByteMap`, and `TwoLevelByteMap`..

LGTM

Dec 10 2018, 8:02 AM
cryptoad added a comment to D54951: Implement a small subset of the C++ `type_traits` header inside sanitizer_common so we can avoid depending on system C++ headers..

I don't see it in D54904

Dec 10 2018, 7:47 AM

Oct 31 2018

cryptoad added a comment to D53910: [Ubsan] link minimal Ubsan into full Ubsan.

FYI, regarding -fsanitize-minimal-runtime it's also used for Scudo to link the lib that doesn't include RTUBsan & the symbolizer & stacktraces RT.

Oct 31 2018, 7:45 AM

Oct 23 2018

cryptoad accepted D53528: [sanitizer] Avoid calling a nullptr in MonotonicNanoTime if interceptors are not yet initialized.

So, it's okay to land this?

Oct 23 2018, 1:16 PM · Restricted Project
cryptoad added a comment to D53528: [sanitizer] Avoid calling a nullptr in MonotonicNanoTime if interceptors are not yet initialized.

The problem I'm seeing is when real_clock_gettime does exist, but it's not yet initialized, see the backtrace posted earlier. I'm not sure how performance sensitive this area is: Is it used elsewhere other than the memory allocator?

Oct 23 2018, 9:11 AM · Restricted Project
cryptoad added a comment to D53528: [sanitizer] Avoid calling a nullptr in MonotonicNanoTime if interceptors are not yet initialized.

Yes, but it would need to be on the if (CanUseVDSO) line otherwise we still get a crash. I'll update the patch.

Oct 23 2018, 8:49 AM · Restricted Project
cryptoad added a comment to D53528: [sanitizer] Avoid calling a nullptr in MonotonicNanoTime if interceptors are not yet initialized.

There is a check for real_clock_gettime (as an extern "C", not part of the interception namespace) in MonotonicNanoTime, as defined by sanitizer_common_interceptors.inc, can this check be moved there? eg: if real_clock_gettime exists & the interception function is not null?

Oct 23 2018, 8:39 AM · Restricted Project

Oct 2 2018

cryptoad accepted D52770: [sanitizer] Use -Wl,-z,global on Android for sanitizers except UBsan.

Thanks!

Oct 2 2018, 7:39 AM

Sep 27 2018

cryptoad committed rL343252: [sanitizer] Disable failing Android test after D52371.
[sanitizer] Disable failing Android test after D52371
Sep 27 2018, 12:19 PM
cryptoad committed rCRT343252: [sanitizer] Disable failing Android test after D52371.
[sanitizer] Disable failing Android test after D52371
Sep 27 2018, 12:18 PM
cryptoad closed D52623: [sanitizer] Disable failing Android test after D52371.
Sep 27 2018, 12:18 PM
cryptoad created D52623: [sanitizer] Disable failing Android test after D52371.
Sep 27 2018, 12:09 PM
cryptoad committed rCRT343246: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.
[sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries
Sep 27 2018, 11:22 AM
cryptoad committed rL343246: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.
[sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries
Sep 27 2018, 11:22 AM
cryptoad closed D52371: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.
Sep 27 2018, 11:22 AM
cryptoad closed D52371: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.
Sep 27 2018, 11:22 AM
cryptoad requested review of D52371: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.

Alright let's go with this one for the sake of HWasan.

Sep 27 2018, 9:58 AM

Sep 26 2018

cryptoad accepted D52566: [asan] [windows] Don't use CheckFailed in dynamic runtime thunk mode.

LGTM
For reference, see comments in https://reviews.llvm.org/D52279#1246222

Sep 26 2018, 12:09 PM
cryptoad added a comment to D52279: [sanitizer] Make __sanitizer::CheckFailed not public.

Or maybe replace the CHECK there with a Trap?

Sep 26 2018, 8:03 AM
cryptoad added a comment to D52279: [sanitizer] Make __sanitizer::CheckFailed not public.

Should we revert this again, or readd the attribute for windows builds only?

Sep 26 2018, 7:53 AM

Sep 25 2018

cryptoad planned changes to D52371: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.

I am actually gonna have to work on the numbers again.
I ran into some issues with the Quarantine. If the Quarantine is low or off, then the numbers are good because we keep reusing the recently freed chunks.
But when the Quarantine is enabled, then have a low amount of cached pointers is detrimental.

Sep 25 2018, 8:53 AM

Sep 21 2018

cryptoad created D52371: [sanitizer] Introduce a new SizeClassMap with minimal amount of cached entries.
Sep 21 2018, 11:10 AM
cryptoad committed rCRT342747: [sanitizer] Move __sanitizer_set_death_callback to a more appropriate file.
[sanitizer] Move __sanitizer_set_death_callback to a more appropriate file
Sep 21 2018, 10:07 AM
cryptoad committed rL342747: [sanitizer] Move __sanitizer_set_death_callback to a more appropriate file.
[sanitizer] Move __sanitizer_set_death_callback to a more appropriate file
Sep 21 2018, 10:07 AM
cryptoad closed D52363: [sanitizer] Move __sanitizer_set_death_callback to a more appropriate file.
Sep 21 2018, 10:07 AM
cryptoad accepted D52361: [XRay][compiler-rt] Update use of internal_mmap.
Sep 21 2018, 8:47 AM
cryptoad created D52363: [sanitizer] Move __sanitizer_set_death_callback to a more appropriate file.
Sep 21 2018, 8:34 AM
cryptoad added inline comments to D52361: [XRay][compiler-rt] Update use of internal_mmap.
Sep 21 2018, 8:28 AM

Sep 20 2018

cryptoad committed rL342651: [sanitizer] Make __sanitizer::CheckFailed not public.
[sanitizer] Make __sanitizer::CheckFailed not public
Sep 20 2018, 7:54 AM
cryptoad committed rCRT342651: [sanitizer] Make __sanitizer::CheckFailed not public.
[sanitizer] Make __sanitizer::CheckFailed not public
Sep 20 2018, 7:54 AM
cryptoad closed D52279: [sanitizer] Make __sanitizer::CheckFailed not public.
Sep 20 2018, 7:54 AM

Sep 19 2018

cryptoad created D52279: [sanitizer] Make __sanitizer::CheckFailed not public.
Sep 19 2018, 2:08 PM
cryptoad committed rL342584: [sanitizer][fuchsia] Fix VMAR leak.
[sanitizer][fuchsia] Fix VMAR leak
Sep 19 2018, 12:52 PM
cryptoad committed rCRT342584: [sanitizer][fuchsia] Fix VMAR leak.
[sanitizer][fuchsia] Fix VMAR leak
Sep 19 2018, 12:52 PM
cryptoad closed D52242: [sanitizer][fuchsia] Fix VMAR leak.
Sep 19 2018, 12:51 PM
cryptoad retitled D52242: [sanitizer][fuchsia] Fix VMAR leak from [sanitizer] Destroy and close a range's vmar if all its memory was unmapped to [sanitizer][fuchsia] Fix VMAR leak.
Sep 19 2018, 12:05 PM

Sep 18 2018

cryptoad updated the diff for D52242: [sanitizer][fuchsia] Fix VMAR leak.

Do not update base_ & size_ to reflect the fact that the reserved range
remains unchanged. Adding a comment to clarify that partial unmapping still
leaves the memory reserved.

Sep 18 2018, 2:25 PM
cryptoad added inline comments to D52242: [sanitizer][fuchsia] Fix VMAR leak.
Sep 18 2018, 1:33 PM
cryptoad updated the diff for D52242: [sanitizer][fuchsia] Fix VMAR leak.

Correct a comment to reflect that it is the destruction of the vmar that is
responsible for the unmapping.

Sep 18 2018, 1:14 PM
cryptoad updated the diff for D52242: [sanitizer][fuchsia] Fix VMAR leak.

Skip the UnmapOrDieVmar call when unmapping the whole mapping, as
vmar_destroy will take care of this. We still have to do some bookkeeping
via DecreaseTotalMmap.

Sep 18 2018, 12:42 PM
cryptoad added a comment to D52242: [sanitizer][fuchsia] Fix VMAR leak.

Some before & after numbers for one of the benchmarks involved:

1552377     207294    1121659    2214385    1550481 nanoseconds              N/A Thread/CreateAndJoin
451883      35547     374475     984192     448331 nanoseconds              N/A Thread/CreateAndJoin
Sep 18 2018, 11:52 AM
cryptoad created D52242: [sanitizer][fuchsia] Fix VMAR leak.
Sep 18 2018, 11:51 AM

Aug 29 2018

cryptoad committed rL340957: [sanitizer] Revert D50940.
[sanitizer] Revert D50940
Aug 29 2018, 12:42 PM
cryptoad committed rCRT340957: [sanitizer] Revert D50940.
[sanitizer] Revert D50940
Aug 29 2018, 12:42 PM
cryptoad closed D51451: [sanitizer] Revert D50940.
Aug 29 2018, 12:42 PM
cryptoad abandoned D51364: [msan] Tentative fix for failing aarch64 test.

Reverting the initial patch with D51451.
Abandoning this.

Aug 29 2018, 12:35 PM
cryptoad created D51451: [sanitizer] Revert D50940.
Aug 29 2018, 12:34 PM
cryptoad added a comment to D51364: [msan] Tentative fix for failing aarch64 test.

I wonder if ProtectMemoryRange in msan_linux.cc needs updating?

Aug 29 2018, 12:25 PM
cryptoad added a comment to D51364: [msan] Tentative fix for failing aarch64 test.

Something's broken.

Aug 29 2018, 11:36 AM
cryptoad added a comment to D51364: [msan] Tentative fix for failing aarch64 test.

Could I please get a LGTM and/or other options so that I can try and fix the aarch64 bots?

Aug 29 2018, 11:21 AM

Aug 28 2018

cryptoad created D51364: [msan] Tentative fix for failing aarch64 test.
Aug 28 2018, 9:12 AM
cryptoad added a comment to D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.

Link to test source: https://github.com/llvm-mirror/compiler-rt/blob/master/test/msan/mmap.cc#L78
Last test output: 0xf00000000
Link to msan mmap interceptor: https://github.com/llvm-mirror/compiler-rt/blob/master/lib/msan/msan_interceptors.cc#L939

Aug 28 2018, 8:24 AM

Aug 24 2018

cryptoad committed rL340633: [scudo] Replace eraseHeader with compareExchangeHeader for Quarantined chunks.
[scudo] Replace eraseHeader with compareExchangeHeader for Quarantined chunks
Aug 24 2018, 11:22 AM
cryptoad committed rCRT340633: [scudo] Replace eraseHeader with compareExchangeHeader for Quarantined chunks.
[scudo] Replace eraseHeader with compareExchangeHeader for Quarantined chunks
Aug 24 2018, 11:22 AM
cryptoad closed D51224: [scudo] Replace eraseHeader with compareExchangeHeader for Quarantined chunks.
Aug 24 2018, 11:22 AM
cryptoad created D51224: [scudo] Replace eraseHeader with compareExchangeHeader for Quarantined chunks.
Aug 24 2018, 10:25 AM

Aug 23 2018

cryptoad committed rL340576: [sanitizer] Change Mmap*NoAccess to return nullptr on error.
[sanitizer] Change Mmap*NoAccess to return nullptr on error
Aug 23 2018, 2:14 PM
cryptoad committed rCRT340576: [sanitizer] Change Mmap*NoAccess to return nullptr on error.
[sanitizer] Change Mmap*NoAccess to return nullptr on error
Aug 23 2018, 2:14 PM
cryptoad closed D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.
Aug 23 2018, 2:14 PM
cryptoad accepted D51146: [scudo] Update documentation with Scudo's "permissive" mode..
Aug 23 2018, 1:47 PM
cryptoad accepted D51173: Fix the configuration of the Primary allocator for Darwin ARM64.

LGTM with a nit.
Additional question but that doesn't require changes to the CL: if you have a ByteMap it means you are using the SizeClassAllocator32 in 64-bit mode (it's gated by a define, look for SANITIZER_CAN_USE_ALLOCATOR64).
Have you tried using the SizeClassAllocator64?

Aug 23 2018, 10:21 AM
cryptoad added a comment to D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.

Ping!

Aug 23 2018, 7:52 AM
cryptoad added a comment to D51146: [scudo] Update documentation with Scudo's "permissive" mode..

You probably can add as well the couple others that default to true that could be set to false as well:

  • DeallocationTypeMismatch
  • DeleteSizeMismatch
Aug 23 2018, 7:48 AM

Aug 20 2018

cryptoad retitled D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error from [sanitizer] Change Mmap*NoAccess to return MMAP_FAILED (~(uptr)0) on error to [sanitizer] Change Mmap*NoAccess to return nullptr on error.
Aug 20 2018, 11:40 AM
cryptoad updated the diff for D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.

Updated proposal: make Mmap*NoAccess return nullptr on failure (like the
other Mmap functions).
Modify callers that were checking for ~(uptr)0 to now check for nullptr.

Aug 20 2018, 11:37 AM
cryptoad added a comment to D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.

Other wrong use:
hwasan::MapDynamicShadow: checks for failure with ~(uptr)0 (while it's the syscall return value)
asan::FindDynamicShadowStart: same as above
__asan::PremapShadow: same as above

Aug 20 2018, 10:58 AM
cryptoad added a comment to D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.

I guess after scavenging further, my initial patch is not correct, as other functions return nullptr on failure.
I am open to anything if we can get a consensus on what should be returned.

Aug 20 2018, 10:46 AM
cryptoad committed rL340178: [sanitizer] Use private futex operations for BlockingMutex.
[sanitizer] Use private futex operations for BlockingMutex
Aug 20 2018, 7:58 AM
cryptoad committed rCRT340178: [sanitizer] Use private futex operations for BlockingMutex.
[sanitizer] Use private futex operations for BlockingMutex
Aug 20 2018, 7:58 AM
cryptoad closed D50910: [sanitizer] Use private futex operations for BlockingMutex.
Aug 20 2018, 7:58 AM

Aug 18 2018

cryptoad created D50940: [sanitizer] Change Mmap*NoAccess to return nullptr on error.
Aug 18 2018, 5:00 PM
cryptoad added a comment to D50910: [sanitizer] Use private futex operations for BlockingMutex.

Have you noticed any difference in performance? I tried the same back in March I have not noticed any perf gain on the couple of benchmarks. I am merely curious, I think this is a right change to make anyways.

Aug 18 2018, 10:51 AM

Aug 17 2018

cryptoad created D50910: [sanitizer] Use private futex operations for BlockingMutex.
Aug 17 2018, 10:22 AM

Aug 14 2018

cryptoad committed rL339705: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.
[scudo] Fix race condition in deallocation path when Quarantine is bypassed
Aug 14 2018, 11:35 AM
cryptoad committed rCRT339705: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.
[scudo] Fix race condition in deallocation path when Quarantine is bypassed
Aug 14 2018, 11:35 AM
cryptoad closed D50655: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.
Aug 14 2018, 11:35 AM
cryptoad added inline comments to D50655: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.
Aug 14 2018, 11:10 AM

Aug 13 2018

cryptoad updated the diff for D50655: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.

Grammar/punctuation corrections in comments.

Aug 13 2018, 3:07 PM
cryptoad added inline comments to D50655: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.
Aug 13 2018, 2:30 PM
cryptoad updated the diff for D50655: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.

Account for 0 size, which is more common than one would expect.

Aug 13 2018, 1:53 PM
cryptoad created D50655: [scudo] Fix race condition in deallocation path when Quarantine is bypassed.
Aug 13 2018, 11:37 AM
cryptoad committed rL339575: [sanitizer] Remove st(X) from the clobber list in 32-bit x86 atomics.
[sanitizer] Remove st(X) from the clobber list in 32-bit x86 atomics
Aug 13 2018, 8:02 AM
cryptoad committed rCRT339575: [sanitizer] Remove st(X) from the clobber list in 32-bit x86 atomics.
[sanitizer] Remove st(X) from the clobber list in 32-bit x86 atomics
Aug 13 2018, 8:02 AM
cryptoad closed D50562: [sanitizer] Remove st(X) from the clobber list in 32-bit x86 atomics.
Aug 13 2018, 8:01 AM

Aug 10 2018

cryptoad created D50562: [sanitizer] Remove st(X) from the clobber list in 32-bit x86 atomics.
Aug 10 2018, 8:34 AM

Aug 9 2018

cryptoad committed rL339370: [sanitizer] Remove rsp from the clobber list in internal_clone.
[sanitizer] Remove rsp from the clobber list in internal_clone
Aug 9 2018, 1:01 PM
cryptoad committed rCRT339370: [sanitizer] Remove rsp from the clobber list in internal_clone.
[sanitizer] Remove rsp from the clobber list in internal_clone
Aug 9 2018, 1:01 PM
cryptoad closed D50519: [sanitizer] Remove rsp from the clobber list in internal_clone.
Aug 9 2018, 1:01 PM
cryptoad added a comment to D50406: SafeStack: Delay thread stack clean-up.

malloc() would only be called during thread destruction, so it doesn't seem like there should be an issue unless malloc() causes a thread to destruct and even then this function shouldn't re-enter because of the order of when pthread_setspecific() is called. Perhaps I'm just failing to imagine a situation under which this could be an issue?

Aug 9 2018, 12:33 PM
cryptoad added a comment to D50406: SafeStack: Delay thread stack clean-up.

Random thought: isn't the introduction of malloc here (as opposed to an OS backed alternative like mmap) gonna mess compatibility with other Sanitizers that intercept it? (thinking of Scudo which is currently compatible with SafeStack but I haven't tested).

Aug 9 2018, 12:05 PM
cryptoad created D50519: [sanitizer] Remove rsp from the clobber list in internal_clone.
Aug 9 2018, 10:18 AM

Jul 20 2018

cryptoad committed rL337574: [zorg] Set CHECK_SCUDO to 1 for ppc64.
[zorg] Set CHECK_SCUDO to 1 for ppc64
Jul 20 2018, 9:45 AM
cryptoad closed D49604: [zorg] Set CHECK_SCUDO to 1 for ppc64.
Jul 20 2018, 9:45 AM
cryptoad created D49604: [zorg] Set CHECK_SCUDO to 1 for ppc64.
Jul 20 2018, 8:46 AM
cryptoad committed rL337557: [scudo] Simplify internal names (NFC).
[scudo] Simplify internal names (NFC)
Jul 20 2018, 8:12 AM
cryptoad committed rCRT337557: [scudo] Simplify internal names (NFC).
[scudo] Simplify internal names (NFC)
Jul 20 2018, 8:12 AM