| Summary: | Invalid sanitization of trailing byte in __builtin_strlen | ||
|---|---|---|---|
| Product: | gcc | Reporter: | Yury Gribov <y.gribov> |
| Component: | sanitizer | Assignee: | Not yet assigned to anyone <unassigned> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | dodji, dvyukov, jakub, kcc |
| Priority: | P3 | ||
| Version: | 5.0 | ||
| Target Milestone: | --- | ||
| Host: | Target: | ||
| Build: | Known to work: | ||
| Known to fail: | Last reconfirmed: | ||
| Attachments: |
Reprocase
Proposed patch |
||
Created attachment 32964 [details]
Proposed patch
Adding draft patch. I only ran Asan regression tests though (leaving for long vacation today). Perhaps someone will have time to have a look at this.
Note that in clang we chose not to instrument any builtins in compiler, but instead fully rely on interceptors. (In reply to Kostya Serebryany from comment #2) > Note that in clang we chose not to instrument any builtins in compiler, > but instead fully rely on interceptors. So you never expand such builtins inline? (In reply to Richard Biener from comment #3) > (In reply to Kostya Serebryany from comment #2) > > Note that in clang we chose not to instrument any builtins in compiler, > > but instead fully rely on interceptors. > > So you never expand such builtins inline? Not that I know of. LLVM has only 3 builtins like this (memset/memcpy/memmove): http://llvm.org/docs/LangRef.html#standard-c-library-intrinsics And asan replaces these builtins with asan's own call (e.g. __asan_memset) Yeah, GCC is different in this regard. Author: ygribov Date: Thu Oct 16 13:46:39 2014 New Revision: 216326 URL: https://gcc.gnu.org/viewcvs?rev=216326&root=gcc&view=rev Log: New asan-instrumentation-with-call-threshold parameter. 2014-10-16 Yury Gribov <y.gribov@samsung.com> Backport from mainline 2014-06-16 Yury Gribov <y.gribov@samsung.com> * asan.c (check_func): New function. (maybe_create_ssa_name): Likewise. (build_check_stmt_with_calls): Likewise. (use_calls_p): Likewise. (report_error_func): Change interface. (build_check_stmt): Allow non-integer lengths; add support for new parameter. (asan_instrument): Likewise. (instrument_mem_region_access): Moved code to build_check_stmt. (instrument_derefs): Likewise. (instrument_strlen_call): Likewise. * cfgcleanup.c (old_insns_match_p): Add support for new functions. * doc/invoke.texi: Describe new parameter. * params.def: Define new parameter. * params.h: Likewise. * sanitizer.def: Describe new builtins. * c-c++-common/asan/instrument-with-calls-1.c: New test. * c-c++-common/asan/instrument-with-calls-2.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-1.c: Update test patterns. * c-c++-common/asan/no-redundant-instrumentation-2.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-4.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-5.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-6.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-7.c: Likewise. * c-c++-common/asan/no-redundant-instrumentation-8.c: Likewise. Backport from mainline 2014-06-16 Yury Gribov <y.gribov@samsung.com> * asan.c (build_check_stmt): Fix maybe-uninitialized warning. Backport from mainline 2014-06-18 Yury Gribov <y.gribov@samsung.com> PR sanitizer/61530 * asan.c (build_check_stmt): Add condition. * c-c++-common/asan/pr61530.c: New test. Backport from mainline 2014-06-18 Yury Gribov <y.gribov@samsung.com> PR sanitizer/61547 * asan.c (instrument_strlen_call): Fixed instrumentation of trailing byte. * c-c++-common/asan/strlen-overflow-1.c: New test. Added: branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/instrument-with-calls-1.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/instrument-with-calls-2.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/pr61530.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/strlen-overflow-1.c Modified: branches/gcc-4_9-branch/gcc/ChangeLog branches/gcc-4_9-branch/gcc/asan.c branches/gcc-4_9-branch/gcc/cfgcleanup.c branches/gcc-4_9-branch/gcc/doc/invoke.texi branches/gcc-4_9-branch/gcc/params.def branches/gcc-4_9-branch/gcc/params.h branches/gcc-4_9-branch/gcc/sanitizer.def branches/gcc-4_9-branch/gcc/testsuite/ChangeLog branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-1.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-2.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-4.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-5.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-6.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-7.c branches/gcc-4_9-branch/gcc/testsuite/c-c++-common/asan/no-redundant-instrumentation-8.c . |
Created attachment 32963 [details] Reprocase The attached testcase performs an overflow in strlen call. Current GCC fails to detect it because of invalid instrumentation of trailing byte: $ gcc repro.c -fsanitize=address -O1 -static-libasan $ ./a.out $ echo $? 0