On Thu, Jan 14, 2016 at 11:45 PM, Konstantin Serebryany <[email protected]> wrote: > > > On Thu, Jan 14, 2016 at 12:17 PM, Yuri Gribov <[email protected]> wrote: >> >> Hi Martin, >> >> On Thu, Jan 14, 2016 at 3:17 PM, Martin Liška <[email protected]> >> wrote: >> > Hello. >> > >> > Let me firstly give you big compliment for address sanitizer >> > infrastructure! >> > I've been currently working as GCC developer and I find two potential >> > interesting use-cases for the sanitizer: >> > >> > 1) use of uninitialized memory >> > >> > GCC started to annotate all class ctors and dtors with memory clobber >> > store: >> > -flifetime-store >> > >> > https://gcc.gnu.org/onlinedocs/gcc-5.3.0/gcc/Optimize-Options.html#Optimize-Options >> > >> > Looks Firefox suffers from many of these errors and segfaults during >> > startup >> > with latest GCC and link-time optimization. >> > Let's assume following example: >> > >> > #include <stdio.h> >> > #include <stdlib.h> >> > #include <string.h> >> > #include <new> >> > >> > struct A >> > { >> > char i[100]; >> > A() {} >> > ~A() {} >> > }; >> > >> > int get_value (int *a, int i) >> > { >> > return a[i]; >> > } >> > >> > int main() >> > { >> > int *ar = (int *)malloc (111); >> > *ar = 123; >> > fprintf (stderr, "memory: %p\n", ar); >> > >> > A* ap = new(ar) A; >> > ap->~A(); >> > >> > get_value (ar, 10); // use of uninitialized memory >> > >> > return 0; >> > } >> > >> > As you can see, calling get_value is an invalid operation. However, e.g. >> > valgrind tool is unable to catch these as the value is initialized. >> > I came with an experimental patch to address sanitizer that annotates >> > every >> > memory clobber to call memory poisoning. On the other hand, >> > every memory store must unpoison the memory. >> >> This has been brought up before in >> https://github.com/google/sanitizers/issues/73 but guys decided to >> firstly do this in MSan. I guess adding this to ASan as an option >> would be cool because it's much easier to integrate (I'm not an owner >> or maintainer though so speaking for myself here). > > > Correct. In msan this feature is already implemented: > http://clang.llvm.org/docs/MemorySanitizer.html#use-after-destruction-detection > I frankly don't know how to correctly implement it in asan > because asan's shadow does not distinguish between reads and writes. > >> >> >> > Sample output from patched GCC & sanitizer: >> > >> > ==12358==ERROR: AddressSanitizer: heap-clobbered-memory-read on address >> > 0x60b00000afb8 at pc 0x000000400ad1 bp 0x7fffffffdca0 sp 0x7fffffffdc98 >> > READ of size 4 at 0x60b00000afb8 thread T0 >> > #0 0x400ad0 in get_value(int*, int) (/tmp/a.out+0x400ad0) >> > #1 0x400bca in main (/tmp/a.out+0x400bca) >> > #2 0x7ffff621960f in __libc_start_main (/lib64/libc.so.6+0x2060f) >> > #3 0x4009a8 in _start (/tmp/a.out+0x4009a8) >> > >> > 0x60b00000afb8 is located 40 bytes inside of 111-byte region >> > [0x60b00000af90,0x60b00000afff) >> > allocated by thread T0 here: >> > #0 0x7ffff6f02128 in __interceptor_malloc >> > ../../../../libsanitizer/asan/asan_malloc_linux.cc:38 >> > #1 0x400ae9 in main (/tmp/a.out+0x400ae9) >> > #2 0x7ffff621960f in __libc_start_main (/lib64/libc.so.6+0x2060f) >> > >> > SUMMARY: AddressSanitizer: heap-clobbered-memory-read >> > (/tmp/a.out+0x400ad0) >> > in get_value(int*, int) >> > Shadow bytes around the buggy address: >> > 0x0c167fff95a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff95b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff95c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff95d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff95e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > =>0x0c167fff95f0: fa fa f0 f0 f0 f0 f0[f0]f0 f0 f0 f0 f0 f0 00 07 >> > 0x0c167fff9600: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff9610: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff9620: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff9630: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 0x0c167fff9640: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa >> > 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 >> > Heap clobbered memory f0 >> > >> > Well, the implementation currently has granularity equal to 8 bytes and >> > every clobber/store must be considered conservatively. Ideally, I would >> > like >> > to be able to clobber (mark store) >> > of every byte. Another issues is that I am unable to clear clobber >> > shadow >> > byte for stack memory after a frame is unwinded, not sure where to find >> > a >> > place where stacks are prepared? >> >> Current implementation only unpoisons stack redzones on exit from >> function (see calls to asan_clear_shadow in >> asan_emit_stack_protection). You'll probably have to change this to >> unpoison the entire frame (if it contains C++ objects with non-trivial >> dtors). >> >> > 2) memory access checks based on TBAA >> > >> > Every memory access in compiler is connected to an alias set and these >> > sets >> > can be compared in runtime if the access is valid. > > > alias-sanitizer... yes, we thought about this before, but never implemented. > This will probably be a different tool... > >> >> > Such check would require to assign 4B information (alias set number) for >> > every byte (I know huge memory footprint), but it can provide >> > interesting >> > informations. >> >> Sounds very interesting. Wouldn't it be possible to round objects to 8 >> bytes (same way as ASan does) to decrease the overhead? BTW note that >> you'll have to take care of generating unique alias set numbers across >> all compilation units which may not be easy (unless you plan to use >> LTO). >> >> > Well, as I've briefly described my use-cases, I have couple of >> > questions: >> > a) Are these optimizations interesting enough to be eventually merged as >> > part of sanitizer? >> > b) I see need to create a separate shadow regions for different usages, >> > maybe current shadow memory implementation can be generalized? >> >> You mean you'd want to have multiple distinct shadow areas for >> different checks? I think the strategy until now was to have dedicated >> tools (each with it's own shadow if at all) which you apply >> separately. > > > Correct. Mixing several tools in one process is painful. > (We do mix asan and ubsan, but that's because ubsan is simple).
And because it's an easy way to promite UBSan) >> > Thank you for any suggestions, ides. >> > Martin >> > >> > -- >> > You received this message because you are subscribed to the Google >> > Groups >> > "address-sanitizer" group. >> > To unsubscribe from this group and stop receiving emails from it, send >> > an >> > email to [email protected]. >> > For more options, visit https://groups.google.com/d/optout. >> >> -- >> You received this message because you are subscribed to the Google Groups >> "address-sanitizer" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> For more options, visit https://groups.google.com/d/optout. > > > -- > You received this message because you are subscribed to the Google Groups > "address-sanitizer" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "address-sanitizer" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
