This is an archive of the discontinued LLVM Phabricator instance.

[tsan] re-exec when ASLR is enabled for x86_64 as well
ClosedPublic

Authored by ZijunZhao on Oct 27 2022, 4:10 PM.

Diff Detail

Event Timeline

ZijunZhao created this revision.Oct 27 2022, 4:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2022, 4:10 PM
Herald added a subscriber: Enna1. · View Herald Transcript
ZijunZhao requested review of this revision.Oct 27 2022, 4:10 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 27 2022, 4:10 PM
Herald added a subscriber: Restricted Project. · View Herald Transcript
enh added inline comments.Oct 27 2022, 4:18 PM
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
302

do we know the corresponding x86-64 patch?

pirama added inline comments.Oct 28 2022, 10:06 AM
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
302

https://android.googlesource.com/platform/system/core/+/master/init/security.cpp#100 points to 9e08f57d684a x86: mm: support ARCH_MMAP_RND_BITS as the equivalent for x86.

319

This function seems to be needed only for aarch64 [1]? Add a separate #if guard around this call with the old check #if SANITIZER_LINUX && defined(__aarch64__).

[1] https://github.com/llvm/llvm-project/blob/main/compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp#L432

ZijunZhao updated this revision to Diff 471656.Oct 28 2022, 2:28 PM
ZijunZhao marked an inline comment as done.
pirama added inline comments.Oct 28 2022, 2:49 PM
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
319

For clarity, I'd duplicate the SANITIZER_LINUX here instead of nesting the pre-processor checks - because the ASLR personality check is unrelated to initializing the longjmp key.

ZijunZhao updated this revision to Diff 472134.Oct 31 2022, 2:27 PM
ZijunZhao retitled this revision from Extend the personality way to x86_64 to [tsan] re-exec when ASLR is enabled for x86_64 as well.
ZijunZhao marked an inline comment as done.
fmayer added a subscriber: fmayer.Oct 31 2022, 3:00 PM
fmayer added inline comments.
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
306

nit

vitalybuka requested changes to this revision.Dec 7 2022, 1:58 PM
vitalybuka added inline comments.
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
302

Is this 39-bit VA specific?
Can you please update comment to cover x86

I guess it's android only?
So it should be SANITIZER_ANDROID.

I was recently notoced internal that ASLR is not working on TSAN arm64 linux, and want to undo this reexec

This revision now requires changes to proceed.Dec 7 2022, 1:58 PM
pirama added inline comments.Dec 7 2022, 2:37 PM
compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
302

I was recently notoced internal that ASLR is not working on TSAN arm64 linux, and want to undo this reexec

If I'm reading this correctly, TSAN for arm64-linux doesn't need the re-exec? It's possible the address mappings here are for a different VA address space. Can you confirm if it's ok to relax the re-exec for all linux systems?

ZijunZhao updated this revision to Diff 481112.Dec 7 2022, 5:09 PM

Use SANITIZER_ANDROID in line 293 instead of SANITIZER_LINUX and fix the nit.

ZijunZhao marked an inline comment as done.Dec 7 2022, 5:09 PM
vitalybuka accepted this revision.Dec 7 2022, 5:53 PM
This revision is now accepted and ready to land.Dec 7 2022, 5:53 PM
This revision was landed with ongoing or failed builds.Dec 8 2022, 11:38 AM
This revision was automatically updated to reflect the committed changes.