Bug 61547

Summary: Invalid sanitization of trailing byte in __builtin_strlen
Product: gcc Reporter: Yury Gribov <y.gribov>
Component: sanitizerAssignee: 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

Description Yury Gribov 2014-06-18 09:52:14 UTC
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
Comment 1 Yury Gribov 2014-06-18 09:54:44 UTC
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.
Comment 2 Kostya Serebryany 2014-06-18 10:37:51 UTC
Note that in clang we chose not to instrument any builtins in compiler, 
but instead fully rely on interceptors.
Comment 3 Richard Biener 2014-06-18 11:18:57 UTC
(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?
Comment 4 Kostya Serebryany 2014-06-18 11:33:02 UTC
(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)
Comment 5 Yuri Gribov 2014-06-19 20:09:29 UTC
Yeah, GCC is different in this regard.
Comment 6 Yury Gribov 2014-10-16 13:47:10 UTC
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
Comment 7 Yury Gribov 2015-04-17 07:45:42 UTC
.