Re: [PATCH, RFC] Introduce -fsanitize=use-after-scope

["Martin Liška" <mliska@suse.cz>]

On 05/06/2016 01:48 PM, Yury Gribov wrote:
> On 05/06/2016 02:04 PM, Martin Liška wrote:
>> Hello.
>>
>> I've started working on the patch couple of month go, basically after
>> a brief discussion with Jakub on IRC.
>>
>> I'm sending the initial version which can successfully run instrumented
>> tramp3d, postgresql server and Inkscape. It catches the basic set of
>> examples which are added in following patch.
>>
>> The implementation is quite straightforward as works in following steps:
>>
>> 1) Every local variable stack slot is poisoned at the very beginning of a function (RTL emission)
>> 2) In gimplifier, once we spot a DECL_EXPR, a variable is unpoisoned (by emitting ASAN_MARK builtin)
>> and the variable is marked as addressable
>> 3) Similarly, BIND_EXPR is the place where we poison the variable (scope exit)
>> 4) At the very end of the function, we clean up the poisoned memory
>> 5) The builtins are expanded to call to libsanitizer run-time library (__asan_poison_stack_memory, __asan_unpoison_stack_memory)
> 
> Can we inline these?

Currently not as libasan is a shared library that an instrumented executable is linked with.
Possible solution would be to directly emit gimple instruction that would poison/unpoison the memory.
But it's not a trivial job which is done in the poisoning code (ALWAYS_INLINE void FastPoisonShadow(uptr aligned_beg, uptr aligned_size, u8 value)

> 
>> 6) As the use-after-scope stuff is already included in libsanitizer, no change is needed for the library
> 
> Note that upstream seems to use a different cmdline interface. They don't have a dedicated -fsanitize=use-after-scope and instead consider it to be a part of -fsanitize=address (disabled by default, enabled via -mllvm -asan-use-after-scope=1). I'd suggest to keep this interface (or at least discuss with them) and use GCC's --param.
> 
> FTR here's the upstream work on this: http://reviews.llvm.org/D19347

Thanks for the link, I will adapt part of the test to our test-suite.
Some of them are really interesting.

Martin

> 
>> Example:
>>
>> int
>> main (void)
>> {
>>    char *ptr;
>>    {
>>      char my_char[9];
>>      ptr = &my_char[0];
>>    }
>>
>>    *(ptr+9) = 'c';
>> }
>>
>> ./a.out
>> =================================================================
>> ==12811==ERROR: AddressSanitizer: stack-use-after-scope on address 0x7ffec9bcff69 at pc 0x000000400a73 bp 0x7ffec9bcfef0 sp 0x7ffec9bcfee8
>> WRITE of size 1 at 0x7ffec9bcff69 thread T0
>>      #0 0x400a72 in main (/tmp/a.out+0x400a72)
>>      #1 0x7f100824860f in __libc_start_main (/lib64/libc.so.6+0x2060f)
>>      #2 0x400868 in _start (/tmp/a.out+0x400868)
>>
>> Address 0x7ffec9bcff69 is located in stack of thread T0 at offset 105 in frame
>>      #0 0x400945 in main (/tmp/a.out+0x400945)
>>
>>    This frame has 2 object(s):
>>      [32, 40) 'ptr'
>>      [96, 105) 'my_char' <== Memory access at offset 105 overflows this variable
>> HINT: this may be a false positive if your program uses some custom stack unwind mechanism or swapcontext
>>        (longjmp and C++ exceptions *are* supported)
>> SUMMARY: AddressSanitizer: stack-use-after-scope (/tmp/a.out+0x400a72) in main
>> Shadow bytes around the buggy address:
>>    0x100059371f90: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059371fa0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059371fb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059371fc0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059371fd0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> =>0x100059371fe0: f1 f1 f1 f1 00 f4 f4 f4 f2 f2 f2 f2 f8[f8]f4 f4
>>    0x100059371ff0: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059372000: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059372010: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059372020: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>>    0x100059372030: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
>> Shadow byte legend (one shadow byte represents 8 application bytes):
>>    Addressable:           00
>>    Partially addressable: 01 02 03 04 05 06 07
>>    Heap left redzone:       fa
>>    Heap right redzone:      fb
>>    Freed heap region:       fd
>>    Stack left redzone:      f1
>>    Stack mid redzone:       f2
>>    Stack right redzone:     f3
>>    Stack partial redzone:   f4
>>    Stack after return:      f5
>>    Stack use after scope:   f8
>>    Global redzone:          f9
>>    Global init order:       f6
>>    Poisoned by user:        f7
>>    Container overflow:      fc
>>    Array cookie:            ac
>>    Intra object redzone:    bb
>>    ASan internal:           fe
>>    Left alloca redzone:     ca
>>    Right alloca redzone:    cb
>> ==12811==ABORTING
>>
>> As mentioned, it's request for comment as it still has couple of limitations:
>> a) VLA are not supported, which should make sense as we are unable to allocate a stack slot for that
> 
> Note that we plan some work on VLA sanitization later this year (upstream ASan now sanitizes dynamic allocas and VLAs).
> 
>> b) we can possibly strip some instrumentation in situations where a variable is introduced in a very first BB (RTL poisoning is superfluous).
>> Similarly for a very last BB of a function, we can strip end of scope poisoning (and RTL unpoisoning). I'll do that incrementally.
>> c) We require -fstack-reuse=none option, maybe it worth to warn a user if -fsanitize=use-after-scope is provided without the option?
> 
> As a user, I'd prefer it to be automatically disabled when use-after-scope is on (unless it has been set explicitly in cmdline in which case we should probably issue error).
> 
>> d) An instrumented binary is quite slow (~20x for tramp3d) as every function call produces many memory read/writes. I'm wondering whether
>> we should provide a faster alternative (like instrument just variables that have address taken) ?
> 
> Can this due to -fstack-reuse or to outline poisoning? The latter sounds particularly heavy.
> 
>> Patch can survive bootstrap and regression tests on x86_64-linux-gnu.
> 
> That's bootstrap-asan?
> 
>> Thanks for feedback.
>> Martin
>>
>