[PATCH] D18003: [tsan] Disable randomized address space on linux when necessary.

[Dmitry Vyukov via llvm-commits]

dvyukov added inline comments.

================
Comment at: lib/tsan/rtl/tsan_platform_linux.cc:267
@@ -265,1 +266,3 @@
 
+static uptr GetHeapEnd() {
+  MemoryMappingLayout proc_maps(true);
----------------
yabinc wrote:
> dvyukov wrote:
> > Please call it somehow differently from "Heap". Heap commonly means "malloc heap" throughout the runtime, as is HeapMemEnd below.
> > I think we are mainly looking for mmaped modules here, so "ModulesEnd" would be fine. "MmapEnd" would be fine as well.
> > 
> I am confused what is kHeapMem is tsan_platform.h for? In my current understanding, if the malloc map is allocated using brk(), the space is just above bss; if the malloc map is allocated using mmap(), the space should have no difference than other mmaped areas. like the one in my x86_64 linux pc:
> 006f0000-006f9000 rw-p 000f0000 fc:00 1838842                            /bin/bash
> 006f9000-006ff000 rw-p 00000000 00:00 0 
> 00f8e000-01401000 rw-p 00000000 00:00 0                                  [heap]
> 
> To make me more confusing, there is a gap between kHeapMemEnd and kHiAppMemBeg in x86_64, how is it decided?
> 
> For the code here, I am looking for mmaped modules, but I think it is probably represented by kHeapMem in tsan_platform.h, that's why I compare GetHeapEnd() < HeapMemEnd() below.
Tsan substitutes malloc, so the libc [heap] mapping is pretty much irrelevant. Tsan malloc manually mmaps heap at kHeapMem-kHeapMemEnd.

> To make me more confusing, there is a gap between kHeapMemEnd and kHiAppMemBeg in x86_64, how is it decided?

These holes should be mprotected to prevent app from mapping anything there. See CheckAndProtect in tsan_platform_posix.cc.

> For the code here, I am looking for mmaped modules, but I think it is probably represented by kHeapMem in tsan_platform.h, that's why I compare GetHeapEnd() < HeapMemEnd() below.

No, kHeapMem is tsan mapping for heap. mmap for heap uses fixed address, so kHeapMem can be arbitrary is not related to any other mappings.

================
Comment at: lib/tsan/rtl/tsan_platform_linux.cc:311
@@ +310,3 @@
+    if (old_personality != -1 && (old_personality & ADDR_NO_RANDOMIZE) == 0) {
+      if (GetHeapEnd() < HeapMemEnd()) {
+        Report("WARNING: Program is run with randomized virtual address space,"
----------------
yabinc wrote:
> dvyukov wrote:
> > I am trying to convince myself that it won't break any of linux setups. We can have pie/non-pie binary. COMPAT mapping (setarch -L) is not supported now on x86 (modules mapped at 0x2a). Also disabled randomization is _not_ supported on x86 (modules mapped at 0x55).
> > So you have a good explanation as to why it all will continue to work?
> > I guess that we always have at least 1 dynamic library (static libc linking is not supported), so on x86 glibc should be mapped at 0x7f in supported configuration (randomization + no compat mapping). So GetHeapEnd should return 0x7f, and that's larger than HeapMemEnd.
> > 
> > But I think it will make failure mode for at least x86+COMPAT mapping much worse (infinite exec recursion instead of a readable message). Potentially there are other cases on other platforms (e.g. power/mips) when that will happen as well. Or maybe they legally have modules below heap?
> > 
> > I think it makes sense to tread more gradually and enable it just for aarch64 with an explanation as to why we are doing this. Other platforms will be enable to enable this by just altering ifdef condition later if necessary.
> > 
> > Or am I missing something?
> > 
> > 
> Frankly I don't understand what is COMPAT mapping and try `setarch -L` fails on my x86_64 pc. I'd like to learn if you can give me more contexts or instructions about how to test.
> As I can see, on x86_64, mmap base is always at HiAppMem range, thus larger than HeapMemEnd.
> x86+COMPAT mapping, how to experiment that? setarch -L seems don't work.
> I don't think there will a infinite loop of reexec, because if CHECK(personality()) fails, it will exit. In consideration of not breaking existing behavior, I remove the CHECK() and don't reexec if failed to set personality. I am not sure if it is proper.
> 
> I just realize that the check of GetHeapEnd() < HeapMemEnd() doesn't work on aarch64 39-bit.  Because there is a hole between kHeapMemEnd and kHiAppMemBeg, whose shadow mem space is taken by kMidAppMem. It means even if GetHeapEnd() is bigger than HeapMemEnd(), it can be like 0x7d20000000, and takes the same shadow mem as kMidAppMem. Currently I think we have two choices, one is making it android specific and reexec without checking real mmap space, another is keep the checking and change Mapping39 of aarch64 (but I am not confident as I don't know what kHeapMem stands for).
> I don't think there will a infinite loop of reexec, because if CHECK(personality()) fails, it will exit.

I mean the following situation.
Randomization is already disabled, but we still have a bad mapping. We try to disable randomization and it succeeds (as randomization is already disable). Then we reexec, but nothing is changed, we still have the same bad mapping. And here we get into infinite loop.

It probably would be safer to never try to reexec more than once. If first reexec does not help, we can just continue and CheckAndProtect later will catch and properly report the bad mapping.

Re COMPAT mapping, I can't find any definitive docs. It probably requires some kernel CONFIG. What it does is it causes modules to be mapped at 0x2a range on x86_64 linux. It is not supported now, but we must not go into infinite reexec loop, if user enables it.



http://reviews.llvm.org/D18003