| Message ID | HE1PR0802MB2251D0E75E8367F311B36775E0780@HE1PR0802MB2251.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [3/X,libsanitizer] Add option to bootstrap using HWASAN | expand |
On 11/7/19 7:37 PM, Matthew Malcomson wrote: > +@item @samp{bootstrap-hwasan} > +Compiles GCC itself using HWAddress Sanitization in order to catch invalid > +memory accesses within the GCC code. This option is only available on AArch64 > +targets with a very recent linux kernel (5.4 or later). > + Apparently, I see this hunk being applied: diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index 2cb8a342a2c..ed47796e052 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -2671,6 +2671,11 @@ the build tree. @end table +@item @samp{bootstrap-hwasan} +Compiles GCC itself using HWAddress Sanitization in order to catch invalid +memory accesses within the GCC code. This option is only available on AArch64 +targets with a very recent linux kernel (5.4 or later). + @section Building a cross compiler When building a cross compiler, it is not generally possible to do a and then I see: make[2]: Entering directory '/dev/shm/objdir/gcc' if [ xinfo = xinfo ]; then \ makeinfo --split-size=5000000 --split-size=5000000 --no-split -I /home/marxin/Programming/gcc/gcc/doc \ -I /home/marxin/Programming/gcc/gcc/doc/include -o doc/gccinstall.info /home/marxin/Programming/gcc/gcc/doc/install.texi; \ fi /home/marxin/Programming/gcc/gcc/doc/install.texi:2674: @item outside of table or list make[2]: *** [Makefile:3279: doc/gccinstall.info] Error 1 make[2]: Leaving directory '/dev/shm/objdir/gcc' Can you please check it? Thanks, Martin
On 11/11/2019 14:30, Martin Liška wrote: > On 11/7/19 7:37 PM, Matthew Malcomson wrote: >> +@item @samp{bootstrap-hwasan} >> +Compiles GCC itself using HWAddress Sanitization in order to catch >> invalid >> +memory accesses within the GCC code. This option is only available >> on AArch64 >> +targets with a very recent linux kernel (5.4 or later). >> + > > Apparently, I see this hunk being applied: > > diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi > index 2cb8a342a2c..ed47796e052 100644 > --- a/gcc/doc/install.texi > +++ b/gcc/doc/install.texi > @@ -2671,6 +2671,11 @@ the build tree. > > @end table > > +@item @samp{bootstrap-hwasan} > +Compiles GCC itself using HWAddress Sanitization in order to catch invalid > +memory accesses within the GCC code. This option is only available on > AArch64 > +targets with a very recent linux kernel (5.4 or later). > + > @section Building a cross compiler > > When building a cross compiler, it is not generally possible to do a > > and then I see: > > make[2]: Entering directory '/dev/shm/objdir/gcc' > if [ xinfo = xinfo ]; then \ > makeinfo --split-size=5000000 --split-size=5000000 --no-split -I > /home/marxin/Programming/gcc/gcc/doc \ > -I /home/marxin/Programming/gcc/gcc/doc/include -o > doc/gccinstall.info /home/marxin/Programming/gcc/gcc/doc/install.texi; \ > fi > /home/marxin/Programming/gcc/gcc/doc/install.texi:2674: @item outside of > table or list > make[2]: *** [Makefile:3279: doc/gccinstall.info] Error 1 > make[2]: Leaving directory '/dev/shm/objdir/gcc' > > Can you please check it? > Thanks, > Martin Ah! My apologies -- I sent up a series with a few documentation mistakes. (the others were wording problems so less noticeable) I'm attaching the entire updated patch series (with the other documentation fixes in it too) and the fixed patch for just this part in case you just want to compile and test right now. Regards, Matthew
On 11/11/19 5:03 PM, Matthew Malcomson wrote: > Ah! > My apologies -- I sent up a series with a few documentation mistakes. > (the others were wording problems so less noticeable) That's fine, I fixed that very easily. Right now, I can confirm using a aarch64 KVM with the following linux kernel: 5.4.0-rc6-3.g7068448-default works. I haven't tried HWASAN bootstrap, but I can run almost all hwasan.exp tests. There are 2 exceptions: FAIL: gcc.dg/hwasan/stack-tagging-basic-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test FAIL: gcc.dg/hwasan/large-aligned-1.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects execution test These fail due to unused value of a function that returns int. The attached patch fixes that. I'm planning to make a proper comments about the series starting next week. For the meantime, I have some libsanitizer upstream suggestions that you can may be discuss. It's mostly about shadow memory dump differences in between ASAN and HWASAN: Let's consider one example: $ cat malloc.c #include <stdlib.h> int main(int argc, char **argv) { char *ptr = malloc (argc); return ptr[1]; } $ gcc malloc.c -fsanitize=address && ./a.out ================================================================= ==7319==ERROR: AddressSanitizer: heap-buffer-overflow on address 0xffffaca007b1 at pc 0x0000004007a0 bp 0xfffff26df150 sp 0xfffff26df168 READ of size 1 at 0xffffaca007b1 thread T0 #0 0x40079c in main (/home/marxin/Programming/gcc/a.out+0x40079c) #1 0xffffb0d3d3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8) #2 0x400670 (/home/marxin/Programming/gcc/a.out+0x400670) 0xffffaca007b1 is located 0 bytes to the right of 1-byte region [0xffffaca007b0,0xffffaca007b1) allocated by thread T0 here: #0 0xffffb0f2bdbc in __interceptor_malloc ../../../../libsanitizer/asan/asan_malloc_linux.cpp:145 #1 0x400748 in main (/home/marxin/Programming/gcc/a.out+0x400748) #2 0xffffb0d3d3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8) #3 0x400670 (/home/marxin/Programming/gcc/a.out+0x400670) SUMMARY: AddressSanitizer: heap-buffer-overflow (/home/marxin/Programming/gcc/a.out+0x40079c) in main Shadow bytes around the buggy address: 0x200ff59400a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff59400b0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff59400c0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff59400d0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff59400e0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa =>0x200ff59400f0: fa fa fa fa fa fa[01]fa fa fa fa fa fa fa fa fa 0x200ff5940100: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff5940110: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff5940120: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff5940130: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 0x200ff5940140: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa 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 Shadow gap: cc ==7319==ABORTING $ gcc malloc.c -fsanitize=hwaddress && ./a.out ==7329==ERROR: HWAddressSanitizer: tag-mismatch on address 0xefdeffffe001 at pc 0xffff804bbcd0 READ of size 1 at 0xefdeffffe001 tags: 03/01 (ptr/mem) in thread T0 #0 0xffff804bbccc in SigTrap<0> ../../../../libsanitizer/hwasan/hwasan_checks.h:27 #1 0xffff804bbccc in CheckAddress<(__hwasan::ErrorAction)0, (__hwasan::AccessType)0, 0> ../../../../libsanitizer/hwasan/hwasan_checks.h:88 #2 0xffff804bbccc in __hwasan_load1 ../../../../libsanitizer/hwasan/hwasan.cpp:469 #3 0x4007d4 in main (/home/marxin/Programming/gcc/a.out+0x4007d4) #4 0xffff8035e3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8) #5 0x4006b0 (/home/marxin/Programming/gcc/a.out+0x4006b0) [0xefdeffffe000,0xefdeffffe020) is a small allocated heap chunk; size: 32 offset: 1 0xefdeffffe001 is located 0 bytes to the right of 1-byte region [0xefdeffffe000,0xefdeffffe001) allocated here: #0 0xffff804bd81c in __sanitizer_malloc ../../../../libsanitizer/hwasan/hwasan_interceptors.cpp:169 #1 0x4007b8 in main (/home/marxin/Programming/gcc/a.out+0x4007b8) #2 0xffff8035e3e8 in __libc_start_main (/lib64/libc.so.6+0x243e8) #3 0x4006b0 (/home/marxin/Programming/gcc/a.out+0x4006b0) Thread: T0 0xeffe00002000 stack: [0xffffd63c2000,0xffffd6bc2000) sz: 8388608 tls: [0xffff80e25020,0xffff80e25790) 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 =>[01] 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 <= 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 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): .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. =>[03] .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. <= .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. .. See https://clang.llvm.org/docs/HardwareAssistedAddressSanitizerDesign.html#short-granules for a description of short granule tags SUMMARY: HWAddressSanitizer: tag-mismatch ../../../../libsanitizer/hwasan/hwasan_checks.h:27 in SigTrap<0> Improvements I see: a) HWASAN uses less compact dump (2 spaces compared to one) b) HWASAN is not using colors and it would be handy to know which color is used for "uninitialized" tags and I would mark the 2 compares tags in dumps (ptr/mem) c) "Tags for short granules around the buggy address" dump is using a dot notation which seems a bit misleading d) For HWASAN address offset is missing for each line in both shadow memory and the pointer Thanks, Martin > > I'm attaching the entire updated patch series (with the other > documentation fixes in it too) and the fixed patch for just this part in > case you just want to compile and test right now.
On 12/11/2019 12:08, Martin Liška wrote: > On 11/11/19 5:03 PM, Matthew Malcomson wrote: >> Ah! >> My apologies -- I sent up a series with a few documentation mistakes. >> (the others were wording problems so less noticeable) > > That's fine, I fixed that very easily. > > Right now, I can confirm using a aarch64 KVM with the following linux > kernel: > 5.4.0-rc6-3.g7068448-default works. I haven't tried HWASAN bootstrap, > but I can > run almost all hwasan.exp tests. > > There are 2 exceptions: > > FAIL: gcc.dg/hwasan/stack-tagging-basic-1.c -O2 -flto > -fuse-linker-plugin -fno-fat-lto-objects execution test > FAIL: gcc.dg/hwasan/large-aligned-1.c -O2 -flto -fuse-linker-plugin > -fno-fat-lto-objects execution test Wow, I have no idea how I missed that but thanks for the catch! > > These fail due to unused value of a function that returns int. The > attached patch fixes that. > I'm planning to make a proper comments about the series starting next week. > > For the meantime, I have some libsanitizer upstream suggestions > that you can may be discuss. It's mostly about > shadow memory dump differences in between ASAN and HWASAN: > > > Improvements I see: > a) HWASAN uses less compact dump (2 spaces compared to one) > b) HWASAN is not using colors and it would be handy to know which color > is used for "uninitialized" tags > and I would mark the 2 compares tags in dumps (ptr/mem) > c) "Tags for short granules around the buggy address" dump is using a > dot notation which seems a bit misleading > d) For HWASAN address offset is missing for each line in both shadow > memory and the pointer To mention my thoughts in turn: a) The dump of HWASAN takes more space, but represents more application bytes for a given space on the console. Each byte of HWASAN shadow space represents 16 application bytes while each byte in ASAN shadow space represents 8 application bytes. I personally appreciate the extra spacing, and figure that the 8|16 byte difference allows for this. b) Marking 'ptr' and 'mem' in the dump sounds like a good idea to me. Exactly how I'm not sure -- maybe with a colourscheme? Do you have a marking in mind? Uninitialised shadow space has the zero tag, however, there are a few extra details that help understanding these traces: On the stack, zero is both uninitialized and "the background" (i.e. the tag for anything not specially instrumented, like register spills and parameters passed on the stack). However, accessible tagged objects can be given a zero tag. We allow this to avoid runtime checks when incrementing the random frame tag to get the tag for a new local variable. We can easily avoid the zero tag at compile-time if we don't use a random tag for each frame. I had this in development at one point and found it quite useful for verification. I already have an option to disable random tags for each frame that this ability could go under. I don't believe (but am not 100% certain) this option is in LLVM. On the heap uninitialised is tag zero, but memory that has been `free`d is given a random tag, so non-zero in a dump does not mean a valid object. c) Is there an alternate notation you have in mind? I would guess the "dots" are there to say "this granule has no short-tag information", and I'm not sure what would be a better way to demonstrate that. d) I agree, an address offset annotation on each line of the shadow memory sounds useful. Cheers, MM > > Thanks, > Martin > >> >> I'm attaching the entire updated patch series (with the other >> documentation fixes in it too) and the fixed patch for just this part in >> case you just want to compile and test right now. >
On 11/13/19 4:24 PM, Matthew Malcomson wrote: > On 12/11/2019 12:08, Martin Liška wrote: >> On 11/11/19 5:03 PM, Matthew Malcomson wrote: >>> Ah! >>> My apologies -- I sent up a series with a few documentation mistakes. >>> (the others were wording problems so less noticeable) >> >> That's fine, I fixed that very easily. >> >> Right now, I can confirm using a aarch64 KVM with the following linux >> kernel: >> 5.4.0-rc6-3.g7068448-default works. I haven't tried HWASAN bootstrap, >> but I can >> run almost all hwasan.exp tests. >> >> There are 2 exceptions: >> >> FAIL: gcc.dg/hwasan/stack-tagging-basic-1.c -O2 -flto >> -fuse-linker-plugin -fno-fat-lto-objects execution test >> FAIL: gcc.dg/hwasan/large-aligned-1.c -O2 -flto -fuse-linker-plugin >> -fno-fat-lto-objects execution test > > Wow, I have no idea how I missed that but thanks for the catch! > >> >> These fail due to unused value of a function that returns int. The >> attached patch fixes that. >> I'm planning to make a proper comments about the series starting next week. >> >> For the meantime, I have some libsanitizer upstream suggestions >> that you can may be discuss. It's mostly about >> shadow memory dump differences in between ASAN and HWASAN: >> > >> >> Improvements I see: >> a) HWASAN uses less compact dump (2 spaces compared to one) >> b) HWASAN is not using colors and it would be handy to know which color >> is used for "uninitialized" tags >> and I would mark the 2 compares tags in dumps (ptr/mem) >> c) "Tags for short granules around the buggy address" dump is using a >> dot notation which seems a bit misleading >> d) For HWASAN address offset is missing for each line in both shadow >> memory and the pointer Hi. > > To mention my thoughts in turn: > a) The dump of HWASAN takes more space, but represents more application > bytes for a given space on the console. > Each byte of HWASAN shadow space represents 16 application bytes > while each byte in ASAN shadow space represents 8 application bytes. > I personally appreciate the extra spacing, and figure that the 8|16 > byte difference allows for this. Thanks for explanation, it works for me. > > b) Marking 'ptr' and 'mem' in the dump sounds like a good idea to me. > Exactly how I'm not sure -- maybe with a colourscheme? Do you have a > marking in mind? Libsanitizer is capable of using colors for report printing. I can help with that and come up with a patch for upstream. > > Uninitialised shadow space has the zero tag, however, there are a few > extra details that help understanding these traces: > > On the stack, zero is both uninitialized and "the background" (i.e. > the tag for anything not specially instrumented, like register spills > and parameters passed on the stack). > However, accessible tagged objects can be given a zero tag. Question here would be if we should use non-zero tags here? Maybe related to my comment about skipping of HWASAN_STACK_BACKGROUND tag? > We allow this to avoid runtime checks when incrementing the random > frame tag to get the tag for a new local variable. > We can easily avoid the zero tag at compile-time if we don't use a > random tag for each frame. I had this in development at one point > and found it quite useful for verification. I already have an option > to disable random tags for each frame that this ability could go > under. > I don't believe (but am not 100% certain) this option is in LLVM. > > On the heap uninitialised is tag zero, but memory that has been > `free`d is given a random tag, so non-zero in a dump does not mean a > valid object. > > c) Is there an alternate notation you have in mind? > I would guess the "dots" are there to say "this granule has no > short-tag information", and I'm not sure what would be a better > way to demonstrate that. Now I've got it here. Dot means that top-byte of a pointer equals to zero. Right? > > d) I agree, an address offset annotation on each line of the shadow > memory sounds useful. I can come up with an upstream patch as well. Thank you, Martin > > Cheers, > MM > >> >> Thanks, >> Martin >> >>> >>> I'm attaching the entire updated patch series (with the other >>> documentation fixes in it too) and the fixed patch for just this part in >>> case you just want to compile and test right now. >> >
On 20/11/2019 14:33, Martin Liška wrote: > On 11/13/19 4:24 PM, Matthew Malcomson wrote: >> On 12/11/2019 12:08, Martin Liška wrote: >>> On 11/11/19 5:03 PM, Matthew Malcomson wrote: >>>> Ah! >>>> My apologies -- I sent up a series with a few documentation mistakes. > >> >> b) Marking 'ptr' and 'mem' in the dump sounds like a good idea to me. >> Exactly how I'm not sure -- maybe with a colourscheme? Do you >> have a >> marking in mind? > > Libsanitizer is capable of using colors for report printing. > I can help with that and come up with a patch for upstream. > >> >> Uninitialised shadow space has the zero tag, however, there are a >> few >> extra details that help understanding these traces: >> >> On the stack, zero is both uninitialized and "the background" (i.e. >> the tag for anything not specially instrumented, like register >> spills >> and parameters passed on the stack). >> However, accessible tagged objects can be given a zero tag. > > Question here would be if we should use non-zero tags here? Maybe related > to my comment about skipping of HWASAN_STACK_BACKGROUND tag? Unfortunately we can't skip non-zero tags at compile time when using a random frame tag. This is because we don't know at compile time what the random frame tag will be. On each entry to a frame a "base tag" is generated randomly at runtime. Each local object in the frame has a compile-time offset that's what gets calculated in `hwasan_increment_tag` -- the offset from this random tag. The tag assigned to a local object is the runtime random frame tag plus the compile-time constant offset. I could avoid HWASAN_STACK_BACKGROUND as a tag when the parameter `hwasan-random-frame-tag` is false, since then there is no runtime random base tag (instead I start with zero). I'll be happy to add that in if you'd like -- I decided against it since it would only matter when a function has 256 or more variables, but I flip-flopped on the decision a few times. > >> We allow this to avoid runtime checks when incrementing the random >> frame tag to get the tag for a new local variable. >> We can easily avoid the zero tag at compile-time if we don't use a >> random tag for each frame. I had this in development at one point >> and found it quite useful for verification. I already have an >> option >> to disable random tags for each frame that this ability could go >> under. >> I don't believe (but am not 100% certain) this option is in LLVM. >> >> On the heap uninitialised is tag zero, but memory that has been >> `free`d is given a random tag, so non-zero in a dump does not mean a >> valid object. >> >> c) Is there an alternate notation you have in mind? >> I would guess the "dots" are there to say "this granule has no >> short-tag information", and I'm not sure what would be a better >> way to demonstrate that. > > Now I've got it here. Dot means that top-byte of a pointer equals to zero. > Right? Ah! I think I never described the "short-tag" functionality, and the fact it's in the debug output is getting confusing. This will also be part of answering your question "c)", and question "h", in the other email https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01950.html . ---------- The main tagging behaviour as described has a natural limitation. Invalid accesses that do not cross a 16 byte boundary are not caught, since each shadow-memory tag applies to a 16 byte chunk. To account for this, HWASAN has a "short-tag" functionality. This functionality was introduced in llvm-svn revision 365551. Usually, a shadow-memory byte records the *tag* that is valid for access to the relevant 16 byte granule in normal memory. When using short-tags, if an object fills only part of a 16 byte granule in normal memory, the corresponding shadow-memory byte stores the *length (in bytes) into this granule that is valid*. The *tag* is then stored in the last byte of the 16 byte granule in normal memory. (We know that last byte is unused, since this is a "short" granule tag). Now, checking a memory access consists of two parts. 1) A normal tag comparison. 2) A fallback in the tag-mismatch case. This fallback checks if the accessing pointer is accessing less bytes into the granule than the length given in shadow-memory. Then if that's the case it also checks the pointers tag matches the last byte in this 16 byte granule. That is a little difficult to explain clearly in text, so I apologise if the above doesn't make sense. ---------- The hwasan error-reporting output lists both memory tags *and* short-tags. These are the two sections under the titles of "Memory tags ..." and "Tags for short granules ...". The first printed section shows what is stored in shadow-memory. This is usually the tag, but can be a length if using "short" tags. The second section contains the "last byte of a granule" for every granule whose shadow-memory byte could be a length. This is why the majority of the "Memory tags ..." section is zero (uninitialized). The majority of the "Tags for short granules ..." section is dots to represent that this granule can't be a "short" granule. Hwasan knows those granules can't be "short" granules since their corresponding byte in shadow memory is not a valid length for a short-granule interpretation. Valid lengths are in the range 1 to 15. It is up to the user to disambiguate the two possibilities in the output. ---------- I have not implemented setting up short-tags for the stack (and do not intend to for GCC 10). This explains your question "h)" -- the testcase you found accesses a stack-allocated buffer. The access is outside the buffer, but not outside the 16 byte granule that object is in. Hence without short-tags this can not be detected by hwasan. It also explains your question "c)" there are no granules that could be interpreted as "short" because GCC doesn't yet set any granules as "short". ---------- Note: You will likely see some stack error-reports that do include "short" tag information. This is not because the compiler has generated short tag information, but it's because the tags that have been generated could be interpreted as valid short tags. This could cause some rare false-passes, but hwasan is already a probabilistic sanitizer. Note 2: Adding short-tags later is backwards-compatible -- especially since I have not added inline tests yet. The compatibility story for adding short-tags is: - If you generate short-tags you must have short-tag checking. - Having short-tag checking without generating short-tags can add rare false-passes. Note 3: short-tags is not a feature in MTE. > >> >> d) I agree, an address offset annotation on each line of the shadow >> memory sounds useful. > > I can come up with an upstream patch as well. > > Thank you, > Martin > >> >> Cheers, >> MM >> >>> >>> Thanks, >>> Martin >>> >>>> >>>> I'm attaching the entire updated patch series (with the other >>>> documentation fixes in it too) and the fixed patch for just this >>>> part in >>>> case you just want to compile and test right now. >>> >> >
On 11/20/19 4:45 PM, Matthew Malcomson wrote: > On 20/11/2019 14:33, Martin Liška wrote: >> On 11/13/19 4:24 PM, Matthew Malcomson wrote: >>> On 12/11/2019 12:08, Martin Liška wrote: >>>> On 11/11/19 5:03 PM, Matthew Malcomson wrote: >>>>> Ah! >>>>> My apologies -- I sent up a series with a few documentation mistakes. >> >>> >>> b) Marking 'ptr' and 'mem' in the dump sounds like a good idea to me. >>> Exactly how I'm not sure -- maybe with a colourscheme? Do you >>> have a >>> marking in mind? >> >> Libsanitizer is capable of using colors for report printing. >> I can help with that and come up with a patch for upstream. >> >>> >>> Uninitialised shadow space has the zero tag, however, there are a >>> few >>> extra details that help understanding these traces: >>> >>> On the stack, zero is both uninitialized and "the background" (i.e. >>> the tag for anything not specially instrumented, like register >>> spills >>> and parameters passed on the stack). >>> However, accessible tagged objects can be given a zero tag. >> >> Question here would be if we should use non-zero tags here? Maybe related >> to my comment about skipping of HWASAN_STACK_BACKGROUND tag? > > Unfortunately we can't skip non-zero tags at compile time when using a > random frame tag. This is because we don't know at compile time what > the random frame tag will be. > > On each entry to a frame a "base tag" is generated randomly at runtime. > Each local object in the frame has a compile-time offset that's what > gets calculated in `hwasan_increment_tag` -- the offset from this random > tag. > The tag assigned to a local object is the runtime random frame tag plus > the compile-time constant offset. Ah, ok, I see. > > > I could avoid HWASAN_STACK_BACKGROUND as a tag when the parameter > `hwasan-random-frame-tag` is false, since then there is no runtime > random base tag (instead I start with zero). Yes, I would recommend that approach. > > I'll be happy to add that in if you'd like -- I decided against it since > it would only matter when a function has 256 or more variables, but I > flip-flopped on the decision a few times. > >> >>> We allow this to avoid runtime checks when incrementing the random >>> frame tag to get the tag for a new local variable. >>> We can easily avoid the zero tag at compile-time if we don't use a >>> random tag for each frame. I had this in development at one point >>> and found it quite useful for verification. I already have an >>> option >>> to disable random tags for each frame that this ability could go >>> under. >>> I don't believe (but am not 100% certain) this option is in LLVM. >>> >>> On the heap uninitialised is tag zero, but memory that has been >>> `free`d is given a random tag, so non-zero in a dump does not mean a >>> valid object. >>> >>> c) Is there an alternate notation you have in mind? >>> I would guess the "dots" are there to say "this granule has no >>> short-tag information", and I'm not sure what would be a better >>> way to demonstrate that. >> >> Now I've got it here. Dot means that top-byte of a pointer equals to zero. >> Right? > > > Ah! > I think I never described the "short-tag" functionality, and the fact > it's in the debug output is getting confusing. > > This will also be part of answering your question "c)", and question > "h", in the other email > https://gcc.gnu.org/ml/gcc-patches/2019-11/msg01950.html . > > > ---------- > > The main tagging behaviour as described has a natural limitation. > Invalid accesses that do not cross a 16 byte boundary are not caught, > since each shadow-memory tag applies to a 16 byte chunk. > > To account for this, HWASAN has a "short-tag" functionality. > This functionality was introduced in llvm-svn revision 365551. > > Usually, a shadow-memory byte records the *tag* that is valid for access > to the relevant 16 byte granule in normal memory. > When using short-tags, if an object fills only part of a 16 byte granule > in normal memory, the corresponding shadow-memory byte stores the > *length (in bytes) into this granule that is valid*. > The *tag* is then stored in the last byte of the 16 byte granule in > normal memory. > (We know that last byte is unused, since this is a "short" granule tag). > > > Now, checking a memory access consists of two parts. > 1) A normal tag comparison. > 2) A fallback in the tag-mismatch case. > This fallback checks if the accessing pointer is accessing less bytes > into the granule than the length given in shadow-memory. > Then if that's the case it also checks the pointers tag matches the > last byte in this 16 byte granule. > > > That is a little difficult to explain clearly in text, so I apologise if > the above doesn't make sense. I've got the technique, it's quite nice trick I would say. However, it makes the sanitizer even more complicated. > > > ---------- > > The hwasan error-reporting output lists both memory tags *and* short-tags. > These are the two sections under the titles of "Memory tags ..." and > "Tags for short granules ...". > > The first printed section shows what is stored in shadow-memory. > This is usually the tag, but can be a length if using "short" tags. > The second section contains the "last byte of a granule" for every > granule whose shadow-memory byte could be a length. > > This is why the majority of the "Memory tags ..." section is zero > (uninitialized). > The majority of the "Tags for short granules ..." section is dots to > represent that this granule can't be a "short" granule. > Hwasan knows those granules can't be "short" granules since their > corresponding byte in shadow memory is not a valid length for a > short-granule interpretation. > Valid lengths are in the range 1 to 15. > > It is up to the user to disambiguate the two possibilities in the output. I've got it. > > ---------- > > I have not implemented setting up short-tags for the stack (and do not > intend to for GCC 10). Which makes perfectly sense, it's an initial implementation. > > This explains your question "h)" -- the testcase you found accesses a > stack-allocated buffer. > The access is outside the buffer, but not outside the 16 byte granule > that object is in. Hence without short-tags this can not be detected by > hwasan. > > It also explains your question "c)" there are no granules that could be > interpreted as "short" because GCC doesn't yet set any granules as "short". > > ---------- > > Note: You will likely see some stack error-reports that do include > "short" tag information. This is not because the compiler has generated > short tag information, but it's because the tags that have been > generated could be interpreted as valid short tags. > This could cause some rare false-passes, but hwasan is already a > probabilistic sanitizer. > > Note 2: Adding short-tags later is backwards-compatible -- especially > since I have not added inline tests yet. > The compatibility story for adding short-tags is: > - If you generate short-tags you must have short-tag checking. > - Having short-tag checking without generating short-tags can add > rare false-passes. Martin > > Note 3: short-tags is not a feature in MTE. > >> >>> >>> d) I agree, an address offset annotation on each line of the shadow >>> memory sounds useful. >> >> I can come up with an upstream patch as well. >> >> Thank you, >> Martin >> >>> >>> Cheers, >>> MM >>> >>>> >>>> Thanks, >>>> Martin >>>> >>>>> >>>>> I'm attaching the entire updated patch series (with the other >>>>> documentation fixes in it too) and the fixed patch for just this >>>>> part in >>>>> case you just want to compile and test right now. >>>> >>> >> >
On 11/20/19 3:33 PM, Martin Liška wrote:
> I can come up with an upstream patch as well.
Hello.
I've just made an upstream review request for that:
https://reviews.llvm.org/D70707
Martin
diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk new file mode 100644 index 0000000000000000000000000000000000000000..4f60bed3fd6e98b47a3a38aea6eba2a7c320da25 --- /dev/null +++ b/config/bootstrap-hwasan.mk @@ -0,0 +1,8 @@ +# This option enables -fsanitize=hwaddress for stage2 and stage3. + +STAGE2_CFLAGS += -fsanitize=hwaddress +STAGE3_CFLAGS += -fsanitize=hwaddress +POSTSTAGE1_LDFLAGS += -fsanitize=hwaddress -static-libhwasan \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/ \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/.libs diff --git a/configure b/configure index aec9186b2b0123d3088b69eb1ee541567654953e..6f71b111bd18ec053180beecf83dd4549e83c2b9 100755 --- a/configure +++ b/configure @@ -7270,7 +7270,7 @@ fi # or bootstrap-ubsan, bootstrap it. if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then case "$BUILD_CONFIG" in - *bootstrap-asan* | *bootstrap-ubsan* ) + *bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* ) bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer, bootstrap_fixincludes=yes ;; diff --git a/configure.ac b/configure.ac index b8ce2ad20b9d03e42731252a9ec2a8417c13e566..16bfdf164555dad94c789f17b6a63ba1a2e3e9f4 100644 --- a/configure.ac +++ b/configure.ac @@ -2775,7 +2775,7 @@ fi # or bootstrap-ubsan, bootstrap it. if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then case "$BUILD_CONFIG" in - *bootstrap-asan* | *bootstrap-ubsan* ) + *bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* ) bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer, bootstrap_fixincludes=yes ;; diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index 6c9579bfaff955eb43875b404fb7db1a667bf522..427a2f4e56b37e165b72cc166e1acb0732449a8b 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -2645,6 +2645,11 @@ Some examples of build configurations designed for developers of GCC are: Compiles GCC itself using Address Sanitization in order to catch invalid memory accesses within the GCC code. +@item @samp{bootstrap-hwasan} +Compiles GCC itself using HWAddress Sanitization in order to catch invalid +memory accesses within the GCC code. This option is only available on AArch64 +targets with a very recent linux kernel (5.4 or later). + @section Building a cross compiler When building a cross compiler, it is not generally possible to do a diff --git a/libiberty/configure b/libiberty/configure index 7a34dabec32b0b383bd33f07811757335f4dd39c..cb2dd4ff5295598343cc18b3a79a86a778f2261d 100755 --- a/libiberty/configure +++ b/libiberty/configure @@ -5261,6 +5261,7 @@ fi NOASANFLAG= case " ${CFLAGS} " in *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;; + *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;; esac diff --git a/libiberty/configure.ac b/libiberty/configure.ac index f1ce76010c9acde79c5dc46686a78b2e2f19244e..043237628b79cbf37d07359b59c5ffe17a7a22ef 100644 --- a/libiberty/configure.ac +++ b/libiberty/configure.ac @@ -240,6 +240,7 @@ AC_SUBST(PICFLAG) NOASANFLAG= case " ${CFLAGS} " in *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;; + *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;; esac AC_SUBST(NOASANFLAG) diff --git a/lto-plugin/Makefile.am b/lto-plugin/Makefile.am index 28dc21014b2e86988fa88adabd63ce6092e18e02..34aa397d785e3cc9b6975de460d065900364c3ff 100644 --- a/lto-plugin/Makefile.am +++ b/lto-plugin/Makefile.am @@ -11,8 +11,8 @@ AM_CPPFLAGS = -I$(top_srcdir)/../include $(DEFS) AM_CFLAGS = @ac_lto_plugin_warn_cflags@ AM_LDFLAGS = @ac_lto_plugin_ldflags@ AM_LIBTOOLFLAGS = --tag=disable-static -override CFLAGS := $(filter-out -fsanitize=address,$(CFLAGS)) -override LDFLAGS := $(filter-out -fsanitize=address,$(LDFLAGS)) +override CFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(CFLAGS)) +override LDFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(LDFLAGS)) libexecsub_LTLIBRARIES = liblto_plugin.la gcc_build_dir = @gcc_build_dir@ diff --git a/lto-plugin/Makefile.in b/lto-plugin/Makefile.in index 8dd6e40ac9dddab39fe1752f9a70e6834ab3c926..7acfc047eff6f86f8d38287e6ffb6533c4c13500 100644 --- a/lto-plugin/Makefile.in +++ b/lto-plugin/Makefile.in @@ -672,8 +672,8 @@ uninstall-am: uninstall-libexecsubLTLIBRARIES .PRECIOUS: Makefile -override CFLAGS := $(filter-out -fsanitize=address,$(CFLAGS)) -override LDFLAGS := $(filter-out -fsanitize=address,$(LDFLAGS)) +override CFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(CFLAGS)) +override LDFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(LDFLAGS)) all-local: $(in_gcc_libs)
This is an analogous option to --bootstrap-asan to configure. It allows bootstrapping GCC using HWASAN. For the same reasons as for ASAN we have to avoid using the HWASAN sanitizer when compiling libiberty and the lto-plugin. Also add a function to query whether -fsanitize=hwaddress has been passed. ChangeLog: 2019-08-29 Matthew Malcomson <matthew.malcomson@arm.com> * configure: Regenerate. * configure.ac: Add --bootstrap-hwasan option. config/ChangeLog: 2019-11-07 Matthew Malcomson <matthew.malcomson@arm.com> * bootstrap-hwasan.mk: New file. libiberty/ChangeLog: 2019-11-07 Matthew Malcomson <matthew.malcomson@arm.com> * configure: Regenerate. * configure.ac: Avoid using sanitizer. lto-plugin/ChangeLog: 2019-11-07 Matthew Malcomson <matthew.malcomson@arm.com> * Makefile.am: Avoid using sanitizer. * Makefile.in: Regenerate. ############### Attachment also inlined for ease of reply ############### diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk new file mode 100644 index 0000000000000000000000000000000000000000..4f60bed3fd6e98b47a3a38aea6eba2a7c320da25 --- /dev/null +++ b/config/bootstrap-hwasan.mk @@ -0,0 +1,8 @@ +# This option enables -fsanitize=hwaddress for stage2 and stage3. + +STAGE2_CFLAGS += -fsanitize=hwaddress +STAGE3_CFLAGS += -fsanitize=hwaddress +POSTSTAGE1_LDFLAGS += -fsanitize=hwaddress -static-libhwasan \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/ \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/ \ + -B$$r/prev-$(TARGET_SUBDIR)/libsanitizer/hwasan/.libs diff --git a/configure b/configure index aec9186b2b0123d3088b69eb1ee541567654953e..6f71b111bd18ec053180beecf83dd4549e83c2b9 100755 --- a/configure +++ b/configure @@ -7270,7 +7270,7 @@ fi # or bootstrap-ubsan, bootstrap it. if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then case "$BUILD_CONFIG" in - *bootstrap-asan* | *bootstrap-ubsan* ) + *bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* ) bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer, bootstrap_fixincludes=yes ;; diff --git a/configure.ac b/configure.ac index b8ce2ad20b9d03e42731252a9ec2a8417c13e566..16bfdf164555dad94c789f17b6a63ba1a2e3e9f4 100644 --- a/configure.ac +++ b/configure.ac @@ -2775,7 +2775,7 @@ fi # or bootstrap-ubsan, bootstrap it. if echo " ${target_configdirs} " | grep " libsanitizer " > /dev/null 2>&1; then case "$BUILD_CONFIG" in - *bootstrap-asan* | *bootstrap-ubsan* ) + *bootstrap-hwasan* | *bootstrap-asan* | *bootstrap-ubsan* ) bootstrap_target_libs=${bootstrap_target_libs}target-libsanitizer, bootstrap_fixincludes=yes ;; diff --git a/gcc/doc/install.texi b/gcc/doc/install.texi index 6c9579bfaff955eb43875b404fb7db1a667bf522..427a2f4e56b37e165b72cc166e1acb0732449a8b 100644 --- a/gcc/doc/install.texi +++ b/gcc/doc/install.texi @@ -2645,6 +2645,11 @@ Some examples of build configurations designed for developers of GCC are: Compiles GCC itself using Address Sanitization in order to catch invalid memory accesses within the GCC code. +@item @samp{bootstrap-hwasan} +Compiles GCC itself using HWAddress Sanitization in order to catch invalid +memory accesses within the GCC code. This option is only available on AArch64 +targets with a very recent linux kernel (5.4 or later). + @section Building a cross compiler When building a cross compiler, it is not generally possible to do a diff --git a/libiberty/configure b/libiberty/configure index 7a34dabec32b0b383bd33f07811757335f4dd39c..cb2dd4ff5295598343cc18b3a79a86a778f2261d 100755 --- a/libiberty/configure +++ b/libiberty/configure @@ -5261,6 +5261,7 @@ fi NOASANFLAG= case " ${CFLAGS} " in *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;; + *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;; esac diff --git a/libiberty/configure.ac b/libiberty/configure.ac index f1ce76010c9acde79c5dc46686a78b2e2f19244e..043237628b79cbf37d07359b59c5ffe17a7a22ef 100644 --- a/libiberty/configure.ac +++ b/libiberty/configure.ac @@ -240,6 +240,7 @@ AC_SUBST(PICFLAG) NOASANFLAG= case " ${CFLAGS} " in *\ -fsanitize=address\ *) NOASANFLAG=-fno-sanitize=address ;; + *\ -fsanitize=hwaddress\ *) NOASANFLAG=-fno-sanitize=hwaddress ;; esac AC_SUBST(NOASANFLAG) diff --git a/lto-plugin/Makefile.am b/lto-plugin/Makefile.am index 28dc21014b2e86988fa88adabd63ce6092e18e02..34aa397d785e3cc9b6975de460d065900364c3ff 100644 --- a/lto-plugin/Makefile.am +++ b/lto-plugin/Makefile.am @@ -11,8 +11,8 @@ AM_CPPFLAGS = -I$(top_srcdir)/../include $(DEFS) AM_CFLAGS = @ac_lto_plugin_warn_cflags@ AM_LDFLAGS = @ac_lto_plugin_ldflags@ AM_LIBTOOLFLAGS = --tag=disable-static -override CFLAGS := $(filter-out -fsanitize=address,$(CFLAGS)) -override LDFLAGS := $(filter-out -fsanitize=address,$(LDFLAGS)) +override CFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(CFLAGS)) +override LDFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(LDFLAGS)) libexecsub_LTLIBRARIES = liblto_plugin.la gcc_build_dir = @gcc_build_dir@ diff --git a/lto-plugin/Makefile.in b/lto-plugin/Makefile.in index 8dd6e40ac9dddab39fe1752f9a70e6834ab3c926..7acfc047eff6f86f8d38287e6ffb6533c4c13500 100644 --- a/lto-plugin/Makefile.in +++ b/lto-plugin/Makefile.in @@ -672,8 +672,8 @@ uninstall-am: uninstall-libexecsubLTLIBRARIES .PRECIOUS: Makefile -override CFLAGS := $(filter-out -fsanitize=address,$(CFLAGS)) -override LDFLAGS := $(filter-out -fsanitize=address,$(LDFLAGS)) +override CFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(CFLAGS)) +override LDFLAGS := $(filter-out -fsanitize=address -fsanitize=hwaddress,$(LDFLAGS)) all-local: $(in_gcc_libs)