public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0
@ 2022-04-26 12:43 shaohua.li at inf dot ethz.ch
  2022-04-26 13:40 ` [Bug sanitizer/105396] " marxin at gcc dot gnu.org
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: shaohua.li at inf dot ethz.ch @ 2022-04-26 12:43 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

            Bug ID: 105396
           Summary: missed stack-buffer-overflow by -O0
           Product: gcc
           Version: 12.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: sanitizer
          Assignee: unassigned at gcc dot gnu.org
          Reporter: shaohua.li at inf dot ethz.ch
                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, marxin at gcc dot gnu.org
  Target Milestone: ---

For the following code, `gcc -fsanitize=address -O0` misses the
buffer-overflow, while `gcc -fsanitize=address -O3` catches it.

$cat a.c
int main() {
  int a;
  int *b[1];
  int c[10];
  int d[1][1];
  for (a = 0; a < 1; a++)
    d[1][a] = 0;
}
$
$gcc -fsanitize=address -O0;./a.out
$
$gcc -fsanitize=address -O3;./a.out
==1==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7fffda35c0d4
at pc 0x000000401104 bp 0x7fffda35c0a0 sp 0x7fffda35c098
WRITE of size 4 at 0x7fffda35c0d4 thread T0
    #0 0x401103 in main /app/example.c:8
    #1 0x7fc3351380b2 in __libc_start_main
(/lib/x86_64-linux-gnu/libc.so.6+0x240b2)
    #2 0x40118d in _start (/app/output.s+0x40118d)

Address 0x7fffda35c0d4 is located in stack of thread T0 at offset 36 in frame
    #0 0x40107f in main /app/example.c:2

  This frame has 1 object(s):
    [32, 36) 'd' (line 6) <== Memory access at offset 36 overflows this
variable
HINT: this may be a false positive if your program uses some custom stack
unwind mechanism, swapcontext or vfork
      (longjmp and C++ exceptions *are* supported)
SUMMARY: AddressSanitizer: stack-buffer-overflow /app/example.c:8 in main
Shadow bytes around the buggy address:
  0x10007b4637c0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b4637d0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b4637e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b4637f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b463800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=>0x10007b463810: 00 00 00 00 00 00 f1 f1 f1 f1[04]f3 f3 f3 00 00
  0x10007b463820: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b463830: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b463840: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b463850: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x10007b463860: 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
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  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
==1==ABORTING

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
@ 2022-04-26 13:40 ` marxin at gcc dot gnu.org
  2022-04-26 13:47 ` jakub at gcc dot gnu.org
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: marxin at gcc dot gnu.org @ 2022-04-26 13:40 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

Martin Liška <marxin at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Last reconfirmed|                            |2022-04-26
           Assignee|unassigned at gcc dot gnu.org      |marxin at gcc dot gnu.org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever confirmed|0                           |1

--- Comment #1 from Martin Liška <marxin at gcc dot gnu.org> ---
Started with my revision r9-4503-g6e644a50045f8032, I'm going to take a look.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
  2022-04-26 13:40 ` [Bug sanitizer/105396] " marxin at gcc dot gnu.org
@ 2022-04-26 13:47 ` jakub at gcc dot gnu.org
  2022-04-26 14:39 ` jakub at gcc dot gnu.org
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-26 13:47 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #2 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
I think the bug is visible in -fdump-rtl-expand-details dump:
Partition 2: size 40 align 16
        c
Partition 1: size 8 align 8
        b
Partition 0: size 4 align 4
        a_1
Partition 3: size 4 align 8
        d
Flushing rzbuffer at offset -160 with: f1 f1 f1 f1
Flushing rzbuffer at offset -128 with: 04 f2 00 00
Flushing rzbuffer at offset -128 with: 00 00 00 f2
Flushing rzbuffer at offset -96 with: f2 f2 00 00
Flushing rzbuffer at offset -64 with: 00 00 00 f3
Flushing rzbuffer at offset -32 with: f3 f3 f3 f3

There is a wrong flush such that the store of what I think should be
04 f2 00 f2 is done in 2 separate overlapping stores.
Because the first var is just 4 bytes, after f1 f1 f1 f1 there should be
04 to indicate that only first 4 bytes of the 8 are valid, then f2 for the
inter-var gap, then 00 for the 8 byte b variable, then 3 f2 bytes for 24 bytes
of gap and then 5 00 bytes for 40 bytes of c and then f3s till end of the
frame.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
  2022-04-26 13:40 ` [Bug sanitizer/105396] " marxin at gcc dot gnu.org
  2022-04-26 13:47 ` jakub at gcc dot gnu.org
@ 2022-04-26 14:39 ` jakub at gcc dot gnu.org
  2022-04-26 14:41 ` jakub at gcc dot gnu.org
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-26 14:39 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #3 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
So, the bug is clearly in asan_redzone_buffer::emit_redzone_byte.
The off == offset case is handled correctly, but the other case is valid only
if the gap is bigger such that we need to flush in between.

--- gcc/asan.cc.jj      2022-02-19 09:03:50.000000000 +0100
+++ gcc/asan.cc 2022-04-26 16:36:55.717551793 +0200
@@ -1502,6 +1502,15 @@ asan_redzone_buffer::emit_redzone_byte (
       m_shadow_bytes.safe_push (value);
       flush_if_full ();
     }
+  else if (offset < m_prev_offset + ASAN_SHADOW_GRANULARITY * RZ_BUFFER_SIZE
+          && !m_shadow_bytes.is_empty ())
+    {
+      /* Shadow memory byte with a small gap.  */
+      for (; off < offset; off += ASAN_SHADOW_GRANULARITY)
+       m_shadow_bytes.safe_push (0);
+      m_shadow_bytes.safe_push (value);
+      flush_if_full ();
+    }
   else
     {
       if (!m_shadow_bytes.is_empty ())

fixes this.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (2 preceding siblings ...)
  2022-04-26 14:39 ` jakub at gcc dot gnu.org
@ 2022-04-26 14:41 ` jakub at gcc dot gnu.org
  2022-04-26 15:05 ` jakub at gcc dot gnu.org
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-26 14:41 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #4 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Oops, sorry Martin, missed you ASSIGNED this to yourself.
If you have your fix for this, go ahead with it, if you don't, I can test mine.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (3 preceding siblings ...)
  2022-04-26 14:41 ` jakub at gcc dot gnu.org
@ 2022-04-26 15:05 ` jakub at gcc dot gnu.org
  2022-04-27  6:35 ` cvs-commit at gcc dot gnu.org
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-26 15:05 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Assignee|marxin at gcc dot gnu.org          |jakub at gcc dot gnu.org

--- Comment #5 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Created attachment 52885
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=52885&action=edit
gcc12-pr105396.patch

Full untested patch.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (4 preceding siblings ...)
  2022-04-26 15:05 ` jakub at gcc dot gnu.org
@ 2022-04-27  6:35 ` cvs-commit at gcc dot gnu.org
  2022-04-27  6:36 ` [Bug sanitizer/105396] [9/10/11 Regression] " jakub at gcc dot gnu.org
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-04-27  6:35 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #6 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jakub Jelinek <jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:9715f10c0651c9549b479b69d67be50ac4bd98a6

commit r12-8276-g9715f10c0651c9549b479b69d67be50ac4bd98a6
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 27 08:34:18 2022 +0200

    asan: Fix up asan_redzone_buffer::emit_redzone_byte [PR105396]

    On the following testcase, we have in main's frame 3 variables,
    some red zone padding, 4 byte d, followed by 12 bytes of red zone padding,
then
    8 byte b followed by 24 bytes of red zone padding, then 40 bytes c followed
    by some red zone padding.
    The intended content of shadow memory for that is (note, each byte
describes
    8 bytes of memory):
    f1 f1 f1 f1 04 f2 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    left red    d  mr b  middle r c              right red zone

    f1 is left red zone magic
    f2 is middle red zone magic
    f3 is right red zone magic
    00 when all 8 bytes are accessible
    01-07 when only 1 to 7 bytes are accessible followed by inaccessible bytes

    The -fdump-rtl-expand-details dump makes it clear that it misbehaves:
    Flushing rzbuffer at offset -160 with: f1 f1 f1 f1
    Flushing rzbuffer at offset -128 with: 04 f2 00 00
    Flushing rzbuffer at offset -128 with: 00 00 00 f2
    Flushing rzbuffer at offset -96 with: f2 f2 00 00
    Flushing rzbuffer at offset -64 with: 00 00 00 f3
    Flushing rzbuffer at offset -32 with: f3 f3 f3 f3
    In the end we end up with
    f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    shadow bytes because at offset -128 there are 2 overlapping stores
    as asan_redzone_buffer::emit_redzone_byte has flushed the temporary 4 byte
    buffer in the middle.

    The function is called with an offset and value.  If the passed offset is
    consecutive with the prev_offset + buffer size (off == offset), then
    we handle it correctly, similarly if the new offset is far enough from the
    old one (we then flush whatever was in the buffer and if needed add up to 3
    bytes of 00 before actually pushing value.

    But what isn't handled correctly is when the offset isn't consecutive to
    what has been added last time, but it is in the same 4 byte word of shadow
    memory (32 bytes of actual memory), like the above case where
    we have consecutive 04 f2 and then skip one shadow memory byte (aka 8 bytes
    of real memory) and then want to emit f2.  Emitting that as a store
    of little-endian 0x0000f204 followed by a store of 0xf2000000 to the same
    address doesn't work, we want to emit 0xf200f204.

    The following patch does that by pushing 1 or 2 00 bytes.
    Additionally, as a small cleanup, instead of using
          m_shadow_bytes.safe_push (value);
          flush_if_full ();
    in all of if, else if and else bodies it sinks those 2 stmts to the end
    of function as all do the same thing.

    2022-04-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105396
            * asan.cc (asan_redzone_buffer::emit_redzone_byte): Handle the case
            where offset is bigger than off but smaller than m_prev_offset + 32
            bits by pushing one or more 0 bytes.  Sink the
            m_shadow_bytes.safe_push (value); flush_if_full (); statements from
            all cases to the end of the function.

            * gcc.dg/asan/pr105396.c: New test.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] [9/10/11 Regression] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (5 preceding siblings ...)
  2022-04-27  6:35 ` cvs-commit at gcc dot gnu.org
@ 2022-04-27  6:36 ` jakub at gcc dot gnu.org
  2022-05-06 23:44 ` cvs-commit at gcc dot gnu.org
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-04-27  6:36 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P3                          |P2
            Summary|missed                      |[9/10/11 Regression] missed
                   |stack-buffer-overflow by    |stack-buffer-overflow by
                   |-O0                         |-O0
   Target Milestone|---                         |9.5

--- Comment #7 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed on the trunk so far.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] [9/10/11 Regression] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (6 preceding siblings ...)
  2022-04-27  6:36 ` [Bug sanitizer/105396] [9/10/11 Regression] " jakub at gcc dot gnu.org
@ 2022-05-06 23:44 ` cvs-commit at gcc dot gnu.org
  2022-05-09 11:26 ` [Bug sanitizer/105396] [9/10 " jakub at gcc dot gnu.org
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-06 23:44 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #8 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-11 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:79265f689bd831809d7d9a86f756d5dd3254d347

commit r11-9963-g79265f689bd831809d7d9a86f756d5dd3254d347
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 27 08:34:18 2022 +0200

    asan: Fix up asan_redzone_buffer::emit_redzone_byte [PR105396]

    On the following testcase, we have in main's frame 3 variables,
    some red zone padding, 4 byte d, followed by 12 bytes of red zone padding,
then
    8 byte b followed by 24 bytes of red zone padding, then 40 bytes c followed
    by some red zone padding.
    The intended content of shadow memory for that is (note, each byte
describes
    8 bytes of memory):
    f1 f1 f1 f1 04 f2 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    left red    d  mr b  middle r c              right red zone

    f1 is left red zone magic
    f2 is middle red zone magic
    f3 is right red zone magic
    00 when all 8 bytes are accessible
    01-07 when only 1 to 7 bytes are accessible followed by inaccessible bytes

    The -fdump-rtl-expand-details dump makes it clear that it misbehaves:
    Flushing rzbuffer at offset -160 with: f1 f1 f1 f1
    Flushing rzbuffer at offset -128 with: 04 f2 00 00
    Flushing rzbuffer at offset -128 with: 00 00 00 f2
    Flushing rzbuffer at offset -96 with: f2 f2 00 00
    Flushing rzbuffer at offset -64 with: 00 00 00 f3
    Flushing rzbuffer at offset -32 with: f3 f3 f3 f3
    In the end we end up with
    f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    shadow bytes because at offset -128 there are 2 overlapping stores
    as asan_redzone_buffer::emit_redzone_byte has flushed the temporary 4 byte
    buffer in the middle.

    The function is called with an offset and value.  If the passed offset is
    consecutive with the prev_offset + buffer size (off == offset), then
    we handle it correctly, similarly if the new offset is far enough from the
    old one (we then flush whatever was in the buffer and if needed add up to 3
    bytes of 00 before actually pushing value.

    But what isn't handled correctly is when the offset isn't consecutive to
    what has been added last time, but it is in the same 4 byte word of shadow
    memory (32 bytes of actual memory), like the above case where
    we have consecutive 04 f2 and then skip one shadow memory byte (aka 8 bytes
    of real memory) and then want to emit f2.  Emitting that as a store
    of little-endian 0x0000f204 followed by a store of 0xf2000000 to the same
    address doesn't work, we want to emit 0xf200f204.

    The following patch does that by pushing 1 or 2 00 bytes.
    Additionally, as a small cleanup, instead of using
          m_shadow_bytes.safe_push (value);
          flush_if_full ();
    in all of if, else if and else bodies it sinks those 2 stmts to the end
    of function as all do the same thing.

    2022-04-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105396
            * asan.c (asan_redzone_buffer::emit_redzone_byte): Handle the case
            where offset is bigger than off but smaller than m_prev_offset + 32
            bits by pushing one or more 0 bytes.  Sink the
            m_shadow_bytes.safe_push (value); flush_if_full (); statements from
            all cases to the end of the function.

            * gcc.dg/asan/pr105396.c: New test.

    (cherry picked from commit 9715f10c0651c9549b479b69d67be50ac4bd98a6)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] [9/10 Regression] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (7 preceding siblings ...)
  2022-05-06 23:44 ` cvs-commit at gcc dot gnu.org
@ 2022-05-09 11:26 ` jakub at gcc dot gnu.org
  2022-05-10  8:26 ` cvs-commit at gcc dot gnu.org
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-05-09 11:26 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
            Summary|[9/10/11 Regression] missed |[9/10 Regression] missed
                   |stack-buffer-overflow by    |stack-buffer-overflow by
                   |-O0                         |-O0

--- Comment #9 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed for 11.4+ too.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] [9/10 Regression] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (8 preceding siblings ...)
  2022-05-09 11:26 ` [Bug sanitizer/105396] [9/10 " jakub at gcc dot gnu.org
@ 2022-05-10  8:26 ` cvs-commit at gcc dot gnu.org
  2022-05-11  6:26 ` cvs-commit at gcc dot gnu.org
  2022-05-11  6:36 ` jakub at gcc dot gnu.org
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-10  8:26 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:e3df97908818aa0915166c91fda27febe6f19dde

commit r10-10712-ge3df97908818aa0915166c91fda27febe6f19dde
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 27 08:34:18 2022 +0200

    asan: Fix up asan_redzone_buffer::emit_redzone_byte [PR105396]

    On the following testcase, we have in main's frame 3 variables,
    some red zone padding, 4 byte d, followed by 12 bytes of red zone padding,
then
    8 byte b followed by 24 bytes of red zone padding, then 40 bytes c followed
    by some red zone padding.
    The intended content of shadow memory for that is (note, each byte
describes
    8 bytes of memory):
    f1 f1 f1 f1 04 f2 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    left red    d  mr b  middle r c              right red zone

    f1 is left red zone magic
    f2 is middle red zone magic
    f3 is right red zone magic
    00 when all 8 bytes are accessible
    01-07 when only 1 to 7 bytes are accessible followed by inaccessible bytes

    The -fdump-rtl-expand-details dump makes it clear that it misbehaves:
    Flushing rzbuffer at offset -160 with: f1 f1 f1 f1
    Flushing rzbuffer at offset -128 with: 04 f2 00 00
    Flushing rzbuffer at offset -128 with: 00 00 00 f2
    Flushing rzbuffer at offset -96 with: f2 f2 00 00
    Flushing rzbuffer at offset -64 with: 00 00 00 f3
    Flushing rzbuffer at offset -32 with: f3 f3 f3 f3
    In the end we end up with
    f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    shadow bytes because at offset -128 there are 2 overlapping stores
    as asan_redzone_buffer::emit_redzone_byte has flushed the temporary 4 byte
    buffer in the middle.

    The function is called with an offset and value.  If the passed offset is
    consecutive with the prev_offset + buffer size (off == offset), then
    we handle it correctly, similarly if the new offset is far enough from the
    old one (we then flush whatever was in the buffer and if needed add up to 3
    bytes of 00 before actually pushing value.

    But what isn't handled correctly is when the offset isn't consecutive to
    what has been added last time, but it is in the same 4 byte word of shadow
    memory (32 bytes of actual memory), like the above case where
    we have consecutive 04 f2 and then skip one shadow memory byte (aka 8 bytes
    of real memory) and then want to emit f2.  Emitting that as a store
    of little-endian 0x0000f204 followed by a store of 0xf2000000 to the same
    address doesn't work, we want to emit 0xf200f204.

    The following patch does that by pushing 1 or 2 00 bytes.
    Additionally, as a small cleanup, instead of using
          m_shadow_bytes.safe_push (value);
          flush_if_full ();
    in all of if, else if and else bodies it sinks those 2 stmts to the end
    of function as all do the same thing.

    2022-04-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105396
            * asan.c (asan_redzone_buffer::emit_redzone_byte): Handle the case
            where offset is bigger than off but smaller than m_prev_offset + 32
            bits by pushing one or more 0 bytes.  Sink the
            m_shadow_bytes.safe_push (value); flush_if_full (); statements from
            all cases to the end of the function.

            * gcc.dg/asan/pr105396.c: New test.

    (cherry picked from commit 9715f10c0651c9549b479b69d67be50ac4bd98a6)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] [9/10 Regression] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (9 preceding siblings ...)
  2022-05-10  8:26 ` cvs-commit at gcc dot gnu.org
@ 2022-05-11  6:26 ` cvs-commit at gcc dot gnu.org
  2022-05-11  6:36 ` jakub at gcc dot gnu.org
  11 siblings, 0 replies; 13+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2022-05-11  6:26 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

--- Comment #11 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-9 branch has been updated by Jakub Jelinek
<jakub@gcc.gnu.org>:

https://gcc.gnu.org/g:4431014dc2e905a427d1e9dd4f4d494ae2d7ab96

commit r9-10152-g4431014dc2e905a427d1e9dd4f4d494ae2d7ab96
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 27 08:34:18 2022 +0200

    asan: Fix up asan_redzone_buffer::emit_redzone_byte [PR105396]

    On the following testcase, we have in main's frame 3 variables,
    some red zone padding, 4 byte d, followed by 12 bytes of red zone padding,
then
    8 byte b followed by 24 bytes of red zone padding, then 40 bytes c followed
    by some red zone padding.
    The intended content of shadow memory for that is (note, each byte
describes
    8 bytes of memory):
    f1 f1 f1 f1 04 f2 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    left red    d  mr b  middle r c              right red zone

    f1 is left red zone magic
    f2 is middle red zone magic
    f3 is right red zone magic
    00 when all 8 bytes are accessible
    01-07 when only 1 to 7 bytes are accessible followed by inaccessible bytes

    The -fdump-rtl-expand-details dump makes it clear that it misbehaves:
    Flushing rzbuffer at offset -160 with: f1 f1 f1 f1
    Flushing rzbuffer at offset -128 with: 04 f2 00 00
    Flushing rzbuffer at offset -128 with: 00 00 00 f2
    Flushing rzbuffer at offset -96 with: f2 f2 00 00
    Flushing rzbuffer at offset -64 with: 00 00 00 f3
    Flushing rzbuffer at offset -32 with: f3 f3 f3 f3
    In the end we end up with
    f1 f1 f1 f1 00 00 00 f2 f2 f2 00 00 00 00 00 f3 f3 f3 f3 f3
    shadow bytes because at offset -128 there are 2 overlapping stores
    as asan_redzone_buffer::emit_redzone_byte has flushed the temporary 4 byte
    buffer in the middle.

    The function is called with an offset and value.  If the passed offset is
    consecutive with the prev_offset + buffer size (off == offset), then
    we handle it correctly, similarly if the new offset is far enough from the
    old one (we then flush whatever was in the buffer and if needed add up to 3
    bytes of 00 before actually pushing value.

    But what isn't handled correctly is when the offset isn't consecutive to
    what has been added last time, but it is in the same 4 byte word of shadow
    memory (32 bytes of actual memory), like the above case where
    we have consecutive 04 f2 and then skip one shadow memory byte (aka 8 bytes
    of real memory) and then want to emit f2.  Emitting that as a store
    of little-endian 0x0000f204 followed by a store of 0xf2000000 to the same
    address doesn't work, we want to emit 0xf200f204.

    The following patch does that by pushing 1 or 2 00 bytes.
    Additionally, as a small cleanup, instead of using
          m_shadow_bytes.safe_push (value);
          flush_if_full ();
    in all of if, else if and else bodies it sinks those 2 stmts to the end
    of function as all do the same thing.

    2022-04-27  Jakub Jelinek  <jakub@redhat.com>

            PR sanitizer/105396
            * asan.c (asan_redzone_buffer::emit_redzone_byte): Handle the case
            where offset is bigger than off but smaller than m_prev_offset + 32
            bits by pushing one or more 0 bytes.  Sink the
            m_shadow_bytes.safe_push (value); flush_if_full (); statements from
            all cases to the end of the function.

            * gcc.dg/asan/pr105396.c: New test.

    (cherry picked from commit 9715f10c0651c9549b479b69d67be50ac4bd98a6)

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [Bug sanitizer/105396] [9/10 Regression] missed stack-buffer-overflow by -O0
  2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
                   ` (10 preceding siblings ...)
  2022-05-11  6:26 ` cvs-commit at gcc dot gnu.org
@ 2022-05-11  6:36 ` jakub at gcc dot gnu.org
  11 siblings, 0 replies; 13+ messages in thread
From: jakub at gcc dot gnu.org @ 2022-05-11  6:36 UTC (permalink / raw)
  To: gcc-bugs

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=105396

Jakub Jelinek <jakub at gcc dot gnu.org> changed:

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

--- Comment #12 from Jakub Jelinek <jakub at gcc dot gnu.org> ---
Fixed.

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2022-05-11  6:36 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-26 12:43 [Bug sanitizer/105396] New: missed stack-buffer-overflow by -O0 shaohua.li at inf dot ethz.ch
2022-04-26 13:40 ` [Bug sanitizer/105396] " marxin at gcc dot gnu.org
2022-04-26 13:47 ` jakub at gcc dot gnu.org
2022-04-26 14:39 ` jakub at gcc dot gnu.org
2022-04-26 14:41 ` jakub at gcc dot gnu.org
2022-04-26 15:05 ` jakub at gcc dot gnu.org
2022-04-27  6:35 ` cvs-commit at gcc dot gnu.org
2022-04-27  6:36 ` [Bug sanitizer/105396] [9/10/11 Regression] " jakub at gcc dot gnu.org
2022-05-06 23:44 ` cvs-commit at gcc dot gnu.org
2022-05-09 11:26 ` [Bug sanitizer/105396] [9/10 " jakub at gcc dot gnu.org
2022-05-10  8:26 ` cvs-commit at gcc dot gnu.org
2022-05-11  6:26 ` cvs-commit at gcc dot gnu.org
2022-05-11  6:36 ` jakub at gcc dot gnu.org

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).