[Bug sanitizer/58543] New: Invalid unpoisoning of stack redzones on ARM

[Bug sanitizer/58543] New: Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

            Bug ID: 58543
           Summary: Invalid unpoisoning of stack redzones on ARM
           Product: gcc
           Version: 4.9.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: y.gribov at samsung dot com
                CC: dodji at gcc dot gnu.org, dvyukov at gcc dot gnu.org,
                    jakub at gcc dot gnu.org, kcc at gcc dot gnu.org

Created attachment 30902
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30902&action=edit
Repro

Gcc seems to generate mismatched prologue/epilogue code to poison/unpoison
stack frame for the attached source code when I compile it with

 $ ~/install/gcc-master-arm/bin/arm-v7a15v3r2-linux-gnueabi-gcc bad.i -O0 -S
-fsanitize=address

Prologue seems to poison words at frame_shadow_base + { 0, 4, 8, 12, 16, 24,
28}:

  add  r4, r3, #536870912
  ldr  r3, .L3+8
  str  r3, [r4]
  ldr  r3, .L3+12
  str  r3, [r4, #4]
  ldr  r3, .L3+16
  str  r3, [r4, #8]
  ldr  r3, .L3+20
  str  r3, [r4, #12]
  ldr  r3, .L3+16
  str  r3, [r4, #16]
  ldr  r3, .L3+20
  str  r3, [r4, #24]
  ldr  r3, .L3+24
  str  r3, [r4, #28]

Whereas epilogue poisons frame_shadow_base + { 0, 4, 8, 12, 16, 40, 44}:

  add  r3, r4, #20
.L1:
  mov  r2, #0
  str  r2, [r4]
  add  r4, r4, #4
  cmp  r4, r3
  bcc  .L1
  add  r3, r4, #24   ; r3 gets invalid value here
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]
  add  r3, r3, #1
  mov  r2, #0
  strb  r2, [r3]

This causes some shadow bytes to remain set after function returns. Those may
trigger incorrect Asan errors later.

-Y


PS: my configuration:

* GCC version: trunk, September 24

* OS: Ubuntu 12.04

* GCC configuration string: ~/gcc/gcc-master/configure
--target=arm-v7a15v3r2-linux-gnueabi
--prefix=/home/ygribov/install/gcc-master-arm --disable-libmudflap
--disable-libssp --disable-nls --enable-long-long --enable-languages=c,c++
--disable-bootstrap --disable-multilib --disable-libstdcxx --disable-libgcc
--disable-libgomp --disable-libatomic --disable-libquadmath
--disable-libstdc++-v3 --disable-libsanitizer --disable-libitm

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #1 from Yury Gribov <y.gribov at samsung dot com> ---
Created attachment 30903
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30903&action=edit
Proposed patch

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #2 from Yury Gribov <y.gribov at samsung dot com> ---
I've attached a simple patch which causes compiler new register to allocate new
temp address register. I'll only be able to test it tomorrow though.

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #3 from Yury Gribov <y.gribov at samsung dot com> ---
Created attachment 30908
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30908&action=edit
Test results

Tests seem to pass both on x86_64 and on ARM (attached).

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[dodji at seketeli dot org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #4 from dodji at seketeli dot org <dodji at seketeli dot org> ---
Thank you for reporting this bug.

Please find my comments below,


"y.gribov at samsung dot com" <gcc-bugzilla@gcc.gnu.org> a écrit:

> Prologue seems to poison words at frame_shadow_base + { 0, 4, 8, 12, 16, 24,
> 28}:

Right.

>
>   add  r4, r3, #536870912
>   ldr  r3, .L3+8
>   str  r3, [r4]
>   ldr  r3, .L3+12
>   str  r3, [r4, #4]
>   ldr  r3, .L3+16
>   str  r3, [r4, #8]
>   ldr  r3, .L3+20
>   str  r3, [r4, #12]
>   ldr  r3, .L3+16
>   str  r3, [r4, #16]
>   ldr  r3, .L3+20
>   str  r3, [r4, #24]
>   ldr  r3, .L3+24
>   str  r3, [r4, #28]
>
> Whereas epilogue poisons frame_shadow_base + { 0, 4, 8, 12, 16, 40, 44}:

I guess you mean *un*poison here.

>   add  r3, r4, #20
> .L1:
>   mov  r2, #0
>   str  r2, [r4]
>   add  r4, r4, #4
>   cmp  r4, r3
>   bcc  .L1

My understanding is that in the loop above, we are setting the memory
pointed to by frame_shadow_base + { 0, 4, 8, 12, 16} to zero.

And in the code below, we are preparing to set the memory pointed to by
frame_shadow_base + {24, 28} to zero.

>   add  r3, r4, #24   ; r3 gets invalid value here

Why is r3 invalid?  It's being set to #24, so that the strb r2, [r3]
below writes a zero byte to [r4,#24].

Or what am I missing?

>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>   add  r3, r3, #1
>   mov  r2, #0
>   strb  r2, [r3]
>
> This causes some shadow bytes to remain set after function returns. Those may
> trigger incorrect Asan errors later.

I am guessing that you have a short and self contained example of an
asan error that is caused by a wrong epilogue.  Would it be possible
that you file it so that I can understand better what is going on?

Thanks.
>From gcc-bugs-return-430800-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org Mon Sep 30 10:15:16 2013
Return-Path: <gcc-bugs-return-430800-listarch-gcc-bugs=gcc.gnu.org@gcc.gnu.org>
Delivered-To: listarch-gcc-bugs@gcc.gnu.org
Received: (qmail 15047 invoked by alias); 30 Sep 2013 10:15:16 -0000
Mailing-List: contact gcc-bugs-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-bugs.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-bugs/>
List-Post: <mailto:gcc-bugs@gcc.gnu.org>
List-Help: <mailto:gcc-bugs-help@gcc.gnu.org>
Sender: gcc-bugs-owner@gcc.gnu.org
Delivered-To: mailing list gcc-bugs@gcc.gnu.org
Received: (qmail 14979 invoked by uid 48); 30 Sep 2013 10:15:13 -0000
From: "rguenth at gcc dot gnu.org" <gcc-bugzilla@gcc.gnu.org>
To: gcc-bugs@gcc.gnu.org
Subject: [Bug c++/58568] [4.8/4.9 Regression] [c++11] ICE with lambda in invalid template variable definition
Date: Mon, 30 Sep 2013 10:15:00 -0000
X-Bugzilla-Reason: CC
X-Bugzilla-Type: changed
X-Bugzilla-Watch-Reason: None
X-Bugzilla-Product: gcc
X-Bugzilla-Component: c++
X-Bugzilla-Version: 4.9.0
X-Bugzilla-Keywords: ice-checking, ice-on-invalid-code
X-Bugzilla-Severity: normal
X-Bugzilla-Who: rguenth at gcc dot gnu.org
X-Bugzilla-Status: NEW
X-Bugzilla-Priority: P3
X-Bugzilla-Assigned-To: unassigned at gcc dot gnu.org
X-Bugzilla-Target-Milestone: 4.8.2
X-Bugzilla-Flags:
X-Bugzilla-Changed-Fields: target_milestone
Message-ID: <bug-58568-4-ffFiZ7LZn9@http.gcc.gnu.org/bugzilla/>
In-Reply-To: <bug-58568-4@http.gcc.gnu.org/bugzilla/>
References: <bug-58568-4@http.gcc.gnu.org/bugzilla/>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
X-Bugzilla-URL: http://gcc.gnu.org/bugzilla/
Auto-Submitted: auto-generated
MIME-Version: 1.0
X-SW-Source: 2013-09/txt/msg02040.txt.bz2
Content-length: 293

http://gcc.gnu.org/bugzilla/show_bug.cgi?idX568

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.8.2

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #5 from Yury Gribov <y.gribov at samsung dot com> ---
> I guess you mean *un*poison here.

Right you are!

> My understanding is that in the loop above, we are setting the memory
> pointed to by frame_shadow_base + { 0, 4, 8, 12, 16} to zero.
>
> And in the code below, we are preparing to set the memory pointed to by
> frame_shadow_base + {24, 28} to zero.

Exactly.

>   add  r3, r4, #24   ; r3 gets invalid value here

> Why is r3 invalid?  It's being set to #24,
> so that the strb r2, [r3] below writes a zero byte to [r4,#24].

Not really - it's set to #40 because r4 was changed to #16 inside the loop

> Would it be possible that you file it
> so that I can understand better what is going on?

Sure - simply compile the attached repro with `-O0 -fsanitize-address'. But
note that you need to use ARM target because x86 does not trigger this bug
(most probably because it uses different code path in asan_clear_shadow).

-Y

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #6 from Yury Gribov <y.gribov at samsung dot com> ---
Dodji,

Let me know if I can provide additional info which may help to debug/fix this.

-Y

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #7 from Yury Gribov <y.gribov at samsung dot com> ---
Created attachment 30946
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=30946&action=edit
Standalone repro

Dodji,

It has just occured to me that you probably want an executable repro with nice
runtime error. Here is the first try (mainly a harness around the previous
repro).

The testcase firstly runs function bad() which fails to unpoison frame shadow.
It then tries to accesses bad()'s stack which triggers Asan error.

Simply compile the attached file with `-fsanitize=address -O2' and run it on
ARM target. As I mentioned before, the stack shadow gets borked so by default
you'll get an abort from Asan runtime:

=================================================================
==3366== ERROR: AddressSanitizer: stack-buffer-overflow on address 0xbefff797
at pc 0x8670 bp 0xbefff7ac sp 0xbefff7a4
READ of size 1 at 0xbefff797 thread T0
    #0 0x866f (/dtv/usb/sda1/a.out+0x866f)
    #1 0x41632bcb (/mtd_exe/lib/libc-2.14.1.so+0x17bcb)
==3366== AddressSanitizer CHECK failed:
/home/ygribov/gcc/gcc-master/libsanitizer/asan/asan_report.cc:250 "((name_end))
!= (0)" (0x0, 0x0)
    #0 0x4003ace7 (/dtv/usb/sda1/libasan.so.0+0x12ce7)
    #1 0x400436fb (/dtv/usb/sda1/libasan.so.0+0x1b6fb)
    #2 0x4004121b (/dtv/usb/sda1/libasan.so.0+0x1921b)
    #3 0x4004136f (/dtv/usb/sda1/libasan.so.0+0x1936f)
    #4 0x40041feb (/dtv/usb/sda1/libasan.so.0+0x19feb)
    #5 0x4003b18b (/dtv/usb/sda1/libasan.so.0+0x1318b)
    #6 0x866f (/dtv/usb/sda1/a.out+0x866f)
    #7 0x41632bcb (/mtd_exe/lib/libc-2.14.1.so+0x17bcb)

This is already a sign of invalid instrumentation. To get a more informative
output, replace
 CHECK(name_end);
with
 if(!name_end) return true;
in libsanitizer/asan/asan_report.cc:
=================================================================
==3468== ERROR: AddressSanitizer: stack-buffer-overflow on address 0xbefff797
at pc 0x8670 bp 0xbefff7ac sp 0xbefff7a4
READ of size 1 at 0xbefff797 thread T0
    #0 0x866f (/dtv/usb/sda1/a.out+0x866f)
    #1 0x41632bcb (/mtd_exe/lib/libc-2.14.1.so+0x17bcb)
Shadow bytes around the buggy address:
  0x37dffea0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dffeb0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dffec0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dffed0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dffee0: 00 00 00 00 00 00 00 00 00 00 00 00 f4 f4 f4 f3
=>0x37dffef0: f3 f3[f3]00 00 00 f1 f1 f1 f1 01 f4 f4 f4 f3 00
  0x37dfff00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dfff10: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dfff20: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dfff30: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x37dfff40: 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 righ 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
  ASan internal:         fe
==3468== ABORTING

You can see that bad() function has not cleared the poisoned bytes.

-Y

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #8 from Yury Gribov <y.gribov at samsung dot com> ---
Created attachment 31032
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=31032&action=edit
Changelog entry

Adding Changelog entry.

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[ramana at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

Ramana Radhakrishnan <ramana at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Keywords|                            |wrong-code
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2013-10-31
                 CC|                            |ramana at gcc dot gnu.org
     Ever confirmed|0                           |1
      Known to fail|                            |4.9.0

--- Comment #9 from Ramana Radhakrishnan <ramana at gcc dot gnu.org> ---
Thread on lists. 

http://gcc.gnu.org/ml/gcc-patches/2013-10/msg00380.html

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[ygribov at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

--- Comment #10 from ygribov at gcc dot gnu.org ---
Author: ygribov
Date: Thu Oct 31 12:10:01 2013
New Revision: 204251

URL: http://gcc.gnu.org/viewcvs?rev=204251&root=gcc&view=rev
Log:
2013-10-31  Richard Sandiford  <rsandifo@linux.vnet.ibm.com>
        Yury Gribov  <y.gribov@samsung.com>

    PR sanitizer/58543
    * asan.c (asan_clear_shadow): Allocate a new vreg for temporary
    shadow pointer to avoid clobbering the main one.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/asan.c

[Bug sanitizer/58543] Invalid unpoisoning of stack redzones on ARM

[y.gribov at samsung dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=58543

Yury Gribov <y.gribov at samsung dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|---                         |FIXED

--- Comment #11 from Yury Gribov <y.gribov at samsung dot com> ---
Fixed by afore-mentioned commit.