On 11/7/19 7:37 PM, Matthew Malcomson wrote:
I have rebased this series onto Martin Liska's patches that take the most
recent libhwasan from upstream LLVM.
https://gcc.gnu.org/ml/gcc-patches/2019-11/msg00340.html
I've also cleared up some nomenclature (I had previously used the word 'colour'
a few times instead of the word 'tag' and that clashes with other descriptions)
and based the patch series off a more recent GCC revision (r277678).
There's an ongoing discussion on whether to have __SANITIZER_ADDRESS__, or
__SANITIZER_HWADDRESS__, but I'm keeping that discussion to the existing
thread.
Similarly there's still the question around C++ exceptions that I'm keeping to
the existing thread (on the first patch series).
NOTE:
Unfortunately, there's a bug in the more recent version of GCC I rebased
onto.
Hwasan catches this when bootstrapping, which means bootstrapping with hwasan
fails.
I'm working on tracking the bug down now, but sending this series upstream
for visibility while that happens.
Bugzilla link:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92410
Entire patch series attached to cover letter.=
Hello.
Thank you very much for the patch set, I've just wrote some inline replies
and I have also some questions/suggestions that I'll summarize here. One caveat,
I'm not maintainer in any of the ideas and I must confirm that I haven't made
a deep review of the part which relates to new RTL hooks and stack expansion.
I expect Jakub can help us here.
Questions/notes:
a) For ASAN we do have bunch of parameters:
gcc --help=param | grep asan
asan-stack Enable asan stack protection.
asan-instrument-allocas Enable asan allocas/VLAs protection.
asan-globals Enable asan globals protection.
asan-instrument-writes Enable asan store operations protection.
asan-instrument-reads Enable asan load operations protection.
asan-memintrin Enable asan builtin functions protection.
asan-use-after-return Enable asan detection of use-after-return bugs.
asan-instrumentation-with-call-threshold Use callbacks instead of inline code
if number of accesses in function becomes greater or equal to this number.
We can probably use some of these in order to drive hwaddress in a similar way.
Most of them makes sense for hwaddress as well
b) Apparently clang prints also value of all registers. Do we want to do the
same?
$ clang
/home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/hwasan/stack-tagging-basic-1.c
-fsanitize=hwaddress && ./a.out
...
Tags for short granules around the buggy address (one tag corresponds to 16
bytes):
.. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
=> .. .. .. .. .. .. .. .. e9 .. [..] .. .. .. .. .. <=
.. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
See
https://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html#short-granules
for a description of short granule tags
Registers where the failure occurred (pc 0xaaaab2976d60):
x0 0000000000000063 x1 0000fffff763249c x2 e900fffff76324a4 x3
0000aaaab2976d94
x4 0000000000000000 x5 165a043ab8eb3734 x6 0000000000000000 x7
0000aaaab299f99c
x8 0000000000000011 x9 0200efff00000000 x10 0000000000000000 x11
0000000000000063
x12 0200fffeff763241 x13 0000000000000011 x14 0000000000000008 x15
00000000c000e9e9
x16 0000aaaab2959b88 x17 0000000000000007 x18 0000000000000040 x19
0000aaaab2977148
x20 0000000000000000 x21 0000aaaab2950388 x22 0000000000000000 x23
0000000000000000
x24 0000000000000000 x25 0000000000000000 x26 0000000000000000 x27
0000000000000000
x28 0000000000000000 x29 0000fffff76324a0 x30 0000aaaab2976d60
SUMMARY: HWAddressSanitizer: tag-mismatch
(/home/marxin/Programming/gcc/a.out+0x2cd5c) in using_stack
c) I'm a bit confused by the pointer tags, where clang does:
$ clang
/home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/hwasan/stack-tagging-basic-1.c
-fsanitize=hwaddress && ./a.out
==30028==ERROR: HWAddressSanitizer: tag-mismatch on address 0xfffff76324a4 at
pc 0xaaaab2976d60
READ of size 4 at 0xfffff76324a4 tags: e9/00 (ptr/mem) in thread T0
#0 0xaaaab2976d5c in using_stack
(/home/marxin/Programming/gcc/a.out+0x2cd5c)
#1 0xaaaab2976e08 in main (/home/marxin/Programming/gcc/a.out+0x2ce08)
#2 0xffff83ca73e8 in __libc_start_main (/lib64/libc.so.6+0x243e8)
#3 0xaaaab29503b8 in _start
/home/abuild/rpmbuild/BUILD/glibc-2.30/csu/../sysdeps/aarch64/start.S:92
Address 0xfffff76324a4 is located in stack of thread T0
Thread: T0 0xeffe00002000 stack: [0xfffff6e33000,0xfffff7633000) sz: 8388608
tls: [0xffff83f82d20,0xffff83f83470)
Previously allocated frames:
record_addr:0xffff83c68748 record:0x324aaaaab2976c8c in using_stack
(/home/marxin/Programming/gcc/a.out+0x2cc8c)
Memory tags around the buggy address (one tag corresponds to 16 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=> 00 69 69 69 69 69 e9 e9 08 00 [00] 00 00 00 00 00 <=
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Tags for short granules around the buggy address (one tag corresponds to 16
bytes):
.. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
=> .. .. .. .. .. .. .. .. e9 .. [..] .. .. .. .. .. <=
.. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
where I can see 'e9' in the dump. But for the GCC there's no:
$ gcc /home/marxin/Programming/gcc/gcc/testsuite/gcc.dg/hwasan/stack-tagging-basic-1.c
-fsanitize=hwaddress && ./a.out
==30037==ERROR: HWAddressSanitizer: tag-mismatch on address 0xfffffd2bf574 at
pc 0xffff81e3ad70
READ of size 4 at 0xfffffd2bf574 tags: df/de (ptr/mem) in thread T0
#0 0xffff81e3ad6c in SigTrap<2>
../../../../libsanitizer/hwasan/hwasan_checks.h:27
#1 0xffff81e3ad6c in CheckAddress<(__hwasan::ErrorAction)0,
(__hwasan::AccessType)0, 2> ../../../../libsanitizer/hwasan/hwasan_checks.h:88
#2 0xffff81e3ad6c in __hwasan_load4
../../../../libsanitizer/hwasan/hwasan.cpp:475
#3 0x4008bc in using_stack (/home/marxin/Programming/gcc/a.out+0x4008bc)
#4 0x400910 in main (/home/marxin/Programming/gcc/a.out+0x400910)
#5 0xffff81cdd3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8)
#6 0x4006b0 (/home/marxin/Programming/gcc/a.out+0x4006b0)
0xfffffd2bf574 is located to the right of a global variable in ([stack]+0x20550)
Address 0xfffffd2bf574 is located in stack of thread T0
Thread: T0 0xeffe00002000 stack: [0xfffffcac0000,0xfffffd2c0000) sz: 8388608
tls: [0xffff827a4020,0xffff827a4790)
Previously allocated frames:
Memory tags around the buggy address (one tag corresponds to 16 bytes):
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
=> 00 00 00 df df df de [de] de de de 00 00 00 00 00 <=
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Tags for short granules around the buggy address (one tag corresponds to 16
bytes):
.. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
=> .. .. .. .. .. .. .. [..] .. .. .. .. .. .. .. .. <=
.. .. .. .. .. .. .. .. .. .. .. .. .. .. .. ..
It's bit confusing. May I please ask for explanation?
d) Are you planning to come up with inline stack variable tagging/untagging for
GCC 10?
e) In hwasan_increment_tag, shouldn't you skip HWASAN_STACK_BACKGROUND color?
f) I would rename ASAN_MARK in tree dumps to HWASAN_MARK, similarly to what you
did
for HWASAN_CHECK?
g) I noticed quite some GNU coding style violations: 'if (! my_cond)', should
be 'if (!my_cond)',
but it's a nit.
h) I maybe found a bug for:
$ cat malloc.c
#include <stdlib.h>
int main(int argc, char **argv)
{
char foo[3];
char *ptr = &foo[2];
__builtin_printf ("ptr: %p\n", ptr);
return *(ptr+10);
}
$ gcc malloc.c -fsanitize=hwaddress && ./a.out
ptr: 0x3c00ffffc86ac092
$ gcc malloc.c -fsanitize=address && ./a.out
ptr: 0xffffc41e0c32
=================================================================
==30102==ERROR: AddressSanitizer: stack-buffer-overflow on address
0xffffc41e0c3c at pc 0x0000004009a4 bp 0xffffc41e0ba0 sp 0xffffc41e0bb8
...
$ clang malloc.c -fsanitize=hwaddress && ./a.out
==30107==ERROR: HWAddressSanitizer: tag-mismatch on address 0xfffff12f5e2c at
pc 0xaaaaad97eaa4
READ of size 1 at 0xfffff12f5e2c tags: e4/03 (ptr/mem) in thread T0
i) As mentioned, we would appreciate a comment before each newly added function
:)
As said, thanks for working on that.
Martin
