workaround for issue #100

[bennofs]

Using link-args, we force the load address for the executable to be higher than
the shadow memory area required by address sanitizer.

[frewsxcv]

@Manishearth @nagisa Are either of you familiar with what's going on here? I'm not the best reviewer for this

[Manishearth]

not entirely. Makes some sense looking at google/sanitizers#837 but I'm not entirely sure this is the right fix.

[bennofs]

What this commit does is the following. Usually, the memory map of a rust exe at runtime would looks like this:

[0x00007fff7000-0x10007fff7fff]  shadow memory region for asan
ELF_ET_DYN_BASE + random offset: text segment (contains the executable code)
directly afterwards: data segment (contains global variables etc)
... lots of free address space ...
dynamic libraries (mmap region)

The problem now is that ELF_ET_DYN_BASE is so low that i in most cases (unless you get lucky with the ASLR offset) is lower than the top end of the shadow memory region, so it collides. The "fix" forces GCC to place the text segment at a fixed, higher address so that it cannot collide with the shadow memory region. With the fix, the memory map would look like this:

[0x00007fff7000-0x10007fff7fff]  shadow memory region for asan
0x200000000000 (not random): text segment (contains the executable code)
directly afterwards: data segment (contains global variables etc)
... lots of free address space ...
dynamic libraries (mmap region)

Note that this means that the code is always loaded at the same address (this makes exploiting buffer overflows etc easier, but it should not be a problem since fuzzing is not run against user data?), but it guarantees that it won't collid with the shadow memory space.

Possible questions:

• I didn't find any option to control the data segment. It seems (?) that the data segment is always loaded directly following the text segment, so we are safe if the text segment is placed high enough
• Ideally, we would only apply this linker option to the final executable, as I am not sure how it interacts with a dylib crates (they of course cannot be placed at a fixed location). But I couldn't get dylib crates to work with ASan even without this fix, so I cannot test this.

Other possible solutions:

• Another solution would be to disable position independent executables, and hope that the default load address of the executable is high enough (which appears to be the case right now). It seems that you can do that with -C relocation-model=dynamic-no-pic. I did not go for this solution though as I think it is less reliable than simply specifying the load address thus ensuring that it can never collide.

Of course, ideally this would be fixed in rustc itself, so that it passes the correct args when you use -Z sanitize=address. But I'm not sure what the correct way to do this is, as people may want to run address sanitizer in production (?) where PIE would be relevant.

[nagisa]

I’m generally against such workarounds for several reasons:

1. It is super easy to forget to remove a workaround when it is not necessary anymore;
2. I generally consider it to be a great idea to fix issues like these upstream – either in the kernel, if it is a kernel bug (which seems unlikely), or the sanitizer (why is it assuming anything about any addresses??);
3. It is non-portable for the eventual future when all this stuff works on platforms other than Linux. It also assumes a particular linker which supports this particular operation/flag.

OTOH this comes with benefits as well, such as disablement of ASLR which results in more reproducible runs, so I’m only slightly against.

[bennofs]

@nagisa I'm not sure that this can be fixed in the sanitizer / kernel. ASan needs a bit of memory for book keeping, and it appears that hardcoding this location is necessary for performance (upstream mentioned that they did not want to make it configurable because performance would suffer too much). So the only "upstream" that could fix this would be rustc.

[bennofs]

The platform-specificness is definitely an issue though. Perhaps we should go with -C relocation-model=dynamic-no-pic? That is a little bit less linker specific at least and should work (or else, no other executable would ever work).

[frewsxcv]

Anyone opposed to closing this?

[bennofs]

Has the bug been fixed? It's been a long time, I have not kept up with the latest developments in these areas.

However, I guess this particular PR can be closed anyway, since that's not the approach anybody seems to want to take.