Re: [PATCH 13/X] [libsanitizer][options] Add hwasan flags and argument parsing
OK, let's keep the macros district then. In the kernel it doesn't give
you a lot, since you actually know which ASAN you're using based on
the kernel CONFIG_ values, but looks like it's important for
userspace. Thanks!
On Thu, Nov 7, 2019 at 7:01 PM Evgenii Stepanov wrote:
>
> Clang has a function level attribute,
> __attribute__((no_sanitize("hwaddress")))
> a feature macro
> #if __has_feature(hwaddress_sanitizer)
> and a blacklist section
> [hwaddress]
> https://clang.llvm.org/docs/SanitizerSpecialCaseList.html
>
> I think it makes sense for the compiler to err on the side of not losing
> information and provide distinct macros for these two sanitizers. If the
> kernel does not care about the difference, they can add a simple #ifdef. They
> would need to, anyway, because gcc does not have feature macros and clang
> does not define __SANITIZE_ADDRESS__.
>
>
> On Thu, Nov 7, 2019 at 7:51 AM Andrey Konovalov wrote:
>>
>> On Thu, Nov 7, 2019 at 1:48 PM Matthew Malcomson
>> wrote:
>> >
>> > On 05/11/2019 13:11, Andrey Konovalov wrote:
>> > > On Tue, Nov 5, 2019 at 12:34 PM Matthew Malcomson
>> > > wrote:
>> > >>
>> > >> NOTE:
>> > >> --
>> > >> I have defined a new macro of __SANITIZE_HWADDRESS__ that gets
>> > >> automatically defined when compiling with hwasan. This is analogous to
>> > >> __SANITIZE_ADDRESS__ which is defined when compiling with asan.
>> > >>
>> > >> Users in the kernel have expressed an interest in using
>> > >> __SANITIZE_ADDRESS__ for both
>> > >> (https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/690703.html).
>> > >>
>> > >> One approach to do this could be to define __SANITIZE_ADDRESS__ with
>> > >> different values depending on whether we are compiling with hwasan or
>> > >> asan.
>> > >>
>> > >> Using __SANITIZE_ADDRESS__ for both means that code like the kernel
>> > >> which wants to treat the two sanitizers as alternate implementations of
>> > >> the same thing gets that automatically.
>> > >>
>> > >> My preference is to use __SANITIZE_HWADDRESS__ since that means any
>> > >> existing code will not be predicated on this (and hence I guess less
>> > >> surprises), but would appreciate feedback on this given the point above.
>> > >
>> > > +Evgenii Stepanov
>> > >
>> > > (A repost from my answer from the mentioned thread):
>> > >
>> > >> Similarly, I'm thinking I'll add no_sanitize_hwaddress as the hwasan
>> > >> equivalent of no_sanitize_address, which will require an update in the
>> > >> kernel given it seems you want KASAN to be used the same whether using
>> > >> tags or not.
>> > >
>> > > We have intentionally reused the same macros to simplify things. Is
>> > > there any reason to use separate macros for GCC? Are there places
>> > > where we need to use specifically no_sanitize_hwaddress and
>> > > __SANITIZE_HWADDRESS__, but not no_sanitize_address and
>> > > __SANITIZE_ADDRESS__?
>> > >
>> > >
>> >
>> > I've just looked through some open source repositories (via github
>> > search) that used the existing __SANITIZE_ADDRESS__ macro.
>> >
>> > There are a few repos that would want to use a feature macro for hwasan
>> > or asan in the exact same way as each other, but of the 31 truly
>> > different uses I found, 11 look like they would need to distinguish
>> > between hwasan and asan (where 4 uses I found I couldn't easily tell)
>> >
>> > NOTE
>> > - This is a count of unique uses, ignoring those repos which use a file
>> > from another repo.
>> > - I'm just giving links to the first of the relevant kind that I found,
>> > not putting effort into finding the "canonical" source of each repository.
>> >
>> >
>> > Places that need distinction (and their reasons):
>> >
>> > There are quite a few that use the ASAN_POISON_MEMORY_REGION and
>> > ASAN_UNPOISON_MEMORY_REGION macros to poison/unpoison memory themselves.
>> > This abstraction doesn't quite make sense in a hwasan environment, as
>> > there is not really a "poisoned/unpoisoned" concept.
>> >
>> > https://github.com/laurynas-biveinis/unodb
>> > https://github.com/darktable-org/rawspeed
>> > https://github.com/MariaDB/server
>> > https://github.com/ralfbrown/framepac-ng
>> > https://github.com/peters/aom
>> > https://github.com/pspacek/knot-resolver-docker-fix
>> > https://github.com/harikrishnan94/sheap
>> >
>> >
>> > Some use it to record their compilation "type" as `-fsanitize=address`
>> > https://github.com/wallix/redemption
>> >
>> > Or to decide to set the environment variable ASAN_OPTIONS
>> > https://github.com/dephonatine/VBox5.2.18
>> >
>> > Others worry about stack space due to asan's redzones (hwasan has a much
>> > smaller stack memory overhead).
>> > https://github.com/fastbuild/fastbuild
>> > https://github.com/scylladb/seastar
>> > (n.b. seastar has a lot more conditioned code that would be the same
>> > between asan and hwasan).
>> >
>> >
>> > Each of these needs to know the difference between compiling with asan
>> > and hwasan, so I'm confident that having some
Re: [PATCH 13/X] [libsanitizer][options] Add hwasan flags and argument parsing
Clang has a function level attribute,
__attribute__((no_sanitize("hwaddress")))
a feature macro
#if __has_feature(hwaddress_sanitizer)
and a blacklist section
[hwaddress]
https://clang.llvm.org/docs/SanitizerSpecialCaseList.html
I think it makes sense for the compiler to err on the side of not losing
information and provide distinct macros for these two sanitizers. If the
kernel does not care about the difference, they can add a simple #ifdef.
They would need to, anyway, because gcc does not have feature macros and
clang does not define __SANITIZE_ADDRESS__.
On Thu, Nov 7, 2019 at 7:51 AM Andrey Konovalov
wrote:
> On Thu, Nov 7, 2019 at 1:48 PM Matthew Malcomson
> wrote:
> >
> > On 05/11/2019 13:11, Andrey Konovalov wrote:
> > > On Tue, Nov 5, 2019 at 12:34 PM Matthew Malcomson
> > > wrote:
> > >>
> > >> NOTE:
> > >> --
> > >> I have defined a new macro of __SANITIZE_HWADDRESS__ that gets
> > >> automatically defined when compiling with hwasan. This is analogous
> to
> > >> __SANITIZE_ADDRESS__ which is defined when compiling with asan.
> > >>
> > >> Users in the kernel have expressed an interest in using
> > >> __SANITIZE_ADDRESS__ for both
> > >> (
> https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/690703.html
> ).
> > >>
> > >> One approach to do this could be to define __SANITIZE_ADDRESS__ with
> > >> different values depending on whether we are compiling with hwasan or
> > >> asan.
> > >>
> > >> Using __SANITIZE_ADDRESS__ for both means that code like the kernel
> > >> which wants to treat the two sanitizers as alternate implementations
> of
> > >> the same thing gets that automatically.
> > >>
> > >> My preference is to use __SANITIZE_HWADDRESS__ since that means any
> > >> existing code will not be predicated on this (and hence I guess less
> > >> surprises), but would appreciate feedback on this given the point
> above.
> > >
> > > +Evgenii Stepanov
> > >
> > > (A repost from my answer from the mentioned thread):
> > >
> > >> Similarly, I'm thinking I'll add no_sanitize_hwaddress as the hwasan
> > >> equivalent of no_sanitize_address, which will require an update in the
> > >> kernel given it seems you want KASAN to be used the same whether using
> > >> tags or not.
> > >
> > > We have intentionally reused the same macros to simplify things. Is
> > > there any reason to use separate macros for GCC? Are there places
> > > where we need to use specifically no_sanitize_hwaddress and
> > > __SANITIZE_HWADDRESS__, but not no_sanitize_address and
> > > __SANITIZE_ADDRESS__?
> > >
> > >
> >
> > I've just looked through some open source repositories (via github
> > search) that used the existing __SANITIZE_ADDRESS__ macro.
> >
> > There are a few repos that would want to use a feature macro for hwasan
> > or asan in the exact same way as each other, but of the 31 truly
> > different uses I found, 11 look like they would need to distinguish
> > between hwasan and asan (where 4 uses I found I couldn't easily tell)
> >
> > NOTE
> > - This is a count of unique uses, ignoring those repos which use a file
> > from another repo.
> > - I'm just giving links to the first of the relevant kind that I found,
> > not putting effort into finding the "canonical" source of each
> repository.
> >
> >
> > Places that need distinction (and their reasons):
> >
> > There are quite a few that use the ASAN_POISON_MEMORY_REGION and
> > ASAN_UNPOISON_MEMORY_REGION macros to poison/unpoison memory themselves.
> > This abstraction doesn't quite make sense in a hwasan environment, as
> > there is not really a "poisoned/unpoisoned" concept.
> >
> > https://github.com/laurynas-biveinis/unodb
> > https://github.com/darktable-org/rawspeed
> > https://github.com/MariaDB/server
> > https://github.com/ralfbrown/framepac-ng
> > https://github.com/peters/aom
> > https://github.com/pspacek/knot-resolver-docker-fix
> > https://github.com/harikrishnan94/sheap
> >
> >
> > Some use it to record their compilation "type" as `-fsanitize=address`
> > https://github.com/wallix/redemption
> >
> > Or to decide to set the environment variable ASAN_OPTIONS
> > https://github.com/dephonatine/VBox5.2.18
> >
> > Others worry about stack space due to asan's redzones (hwasan has a much
> > smaller stack memory overhead).
> > https://github.com/fastbuild/fastbuild
> > https://github.com/scylladb/seastar
> > (n.b. seastar has a lot more conditioned code that would be the same
> > between asan and hwasan).
> >
> >
> > Each of these needs to know the difference between compiling with asan
> > and hwasan, so I'm confident that having some way to determine that in
> > the source code is a good idea.
> >
> >
> > I also believe there could be code in the wild that would need to
> > distinguish between hwasan and asan where the existence of tags could be
> > problematic:
> >
> > - code already using the top-byte-ignore feature may be able to be used
> > with asan but not hwasan.
> > - Code that makes assumptions about pointer
Re: [PATCH 13/X] [libsanitizer][options] Add hwasan flags and argument parsing
On Thu, Nov 7, 2019 at 1:48 PM Matthew Malcomson wrote: > > On 05/11/2019 13:11, Andrey Konovalov wrote: > > On Tue, Nov 5, 2019 at 12:34 PM Matthew Malcomson > > wrote: > >> > >> NOTE: > >> -- > >> I have defined a new macro of __SANITIZE_HWADDRESS__ that gets > >> automatically defined when compiling with hwasan. This is analogous to > >> __SANITIZE_ADDRESS__ which is defined when compiling with asan. > >> > >> Users in the kernel have expressed an interest in using > >> __SANITIZE_ADDRESS__ for both > >> (https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/690703.html). > >> > >> One approach to do this could be to define __SANITIZE_ADDRESS__ with > >> different values depending on whether we are compiling with hwasan or > >> asan. > >> > >> Using __SANITIZE_ADDRESS__ for both means that code like the kernel > >> which wants to treat the two sanitizers as alternate implementations of > >> the same thing gets that automatically. > >> > >> My preference is to use __SANITIZE_HWADDRESS__ since that means any > >> existing code will not be predicated on this (and hence I guess less > >> surprises), but would appreciate feedback on this given the point above. > > > > +Evgenii Stepanov > > > > (A repost from my answer from the mentioned thread): > > > >> Similarly, I'm thinking I'll add no_sanitize_hwaddress as the hwasan > >> equivalent of no_sanitize_address, which will require an update in the > >> kernel given it seems you want KASAN to be used the same whether using > >> tags or not. > > > > We have intentionally reused the same macros to simplify things. Is > > there any reason to use separate macros for GCC? Are there places > > where we need to use specifically no_sanitize_hwaddress and > > __SANITIZE_HWADDRESS__, but not no_sanitize_address and > > __SANITIZE_ADDRESS__? > > > > > > I've just looked through some open source repositories (via github > search) that used the existing __SANITIZE_ADDRESS__ macro. > > There are a few repos that would want to use a feature macro for hwasan > or asan in the exact same way as each other, but of the 31 truly > different uses I found, 11 look like they would need to distinguish > between hwasan and asan (where 4 uses I found I couldn't easily tell) > > NOTE > - This is a count of unique uses, ignoring those repos which use a file > from another repo. > - I'm just giving links to the first of the relevant kind that I found, > not putting effort into finding the "canonical" source of each repository. > > > Places that need distinction (and their reasons): > > There are quite a few that use the ASAN_POISON_MEMORY_REGION and > ASAN_UNPOISON_MEMORY_REGION macros to poison/unpoison memory themselves. > This abstraction doesn't quite make sense in a hwasan environment, as > there is not really a "poisoned/unpoisoned" concept. > > https://github.com/laurynas-biveinis/unodb > https://github.com/darktable-org/rawspeed > https://github.com/MariaDB/server > https://github.com/ralfbrown/framepac-ng > https://github.com/peters/aom > https://github.com/pspacek/knot-resolver-docker-fix > https://github.com/harikrishnan94/sheap > > > Some use it to record their compilation "type" as `-fsanitize=address` > https://github.com/wallix/redemption > > Or to decide to set the environment variable ASAN_OPTIONS > https://github.com/dephonatine/VBox5.2.18 > > Others worry about stack space due to asan's redzones (hwasan has a much > smaller stack memory overhead). > https://github.com/fastbuild/fastbuild > https://github.com/scylladb/seastar > (n.b. seastar has a lot more conditioned code that would be the same > between asan and hwasan). > > > Each of these needs to know the difference between compiling with asan > and hwasan, so I'm confident that having some way to determine that in > the source code is a good idea. > > > I also believe there could be code in the wild that would need to > distinguish between hwasan and asan where the existence of tags could be > problematic: > > - code already using the top-byte-ignore feature may be able to be used > with asan but not hwasan. > - Code that makes assumptions about pointer ordering (e.g. the autoconf > program that looks for stack growth direction) could break on hwasan but > not on asan. > - Code looking for the distance between two objects in memory would need > to account for tags in pointers. > > > Hence I think this distinction is needed. Evgenii, how does clang-compiled code dististinguishes whether it's being compiled with ASAN or HWASAN?
Re: [PATCH 13/X] [libsanitizer][options] Add hwasan flags and argument parsing
On 05/11/2019 13:11, Andrey Konovalov wrote: > On Tue, Nov 5, 2019 at 12:34 PM Matthew Malcomson > wrote: >> >> NOTE: >> -- >> I have defined a new macro of __SANITIZE_HWADDRESS__ that gets >> automatically defined when compiling with hwasan. This is analogous to >> __SANITIZE_ADDRESS__ which is defined when compiling with asan. >> >> Users in the kernel have expressed an interest in using >> __SANITIZE_ADDRESS__ for both >> (https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/690703.html). >> >> One approach to do this could be to define __SANITIZE_ADDRESS__ with >> different values depending on whether we are compiling with hwasan or >> asan. >> >> Using __SANITIZE_ADDRESS__ for both means that code like the kernel >> which wants to treat the two sanitizers as alternate implementations of >> the same thing gets that automatically. >> >> My preference is to use __SANITIZE_HWADDRESS__ since that means any >> existing code will not be predicated on this (and hence I guess less >> surprises), but would appreciate feedback on this given the point above. > > +Evgenii Stepanov > > (A repost from my answer from the mentioned thread): > >> Similarly, I'm thinking I'll add no_sanitize_hwaddress as the hwasan >> equivalent of no_sanitize_address, which will require an update in the >> kernel given it seems you want KASAN to be used the same whether using >> tags or not. > > We have intentionally reused the same macros to simplify things. Is > there any reason to use separate macros for GCC? Are there places > where we need to use specifically no_sanitize_hwaddress and > __SANITIZE_HWADDRESS__, but not no_sanitize_address and > __SANITIZE_ADDRESS__? > > I've just looked through some open source repositories (via github search) that used the existing __SANITIZE_ADDRESS__ macro. There are a few repos that would want to use a feature macro for hwasan or asan in the exact same way as each other, but of the 31 truly different uses I found, 11 look like they would need to distinguish between hwasan and asan (where 4 uses I found I couldn't easily tell) NOTE - This is a count of unique uses, ignoring those repos which use a file from another repo. - I'm just giving links to the first of the relevant kind that I found, not putting effort into finding the "canonical" source of each repository. Places that need distinction (and their reasons): There are quite a few that use the ASAN_POISON_MEMORY_REGION and ASAN_UNPOISON_MEMORY_REGION macros to poison/unpoison memory themselves. This abstraction doesn't quite make sense in a hwasan environment, as there is not really a "poisoned/unpoisoned" concept. https://github.com/laurynas-biveinis/unodb https://github.com/darktable-org/rawspeed https://github.com/MariaDB/server https://github.com/ralfbrown/framepac-ng https://github.com/peters/aom https://github.com/pspacek/knot-resolver-docker-fix https://github.com/harikrishnan94/sheap Some use it to record their compilation "type" as `-fsanitize=address` https://github.com/wallix/redemption Or to decide to set the environment variable ASAN_OPTIONS https://github.com/dephonatine/VBox5.2.18 Others worry about stack space due to asan's redzones (hwasan has a much smaller stack memory overhead). https://github.com/fastbuild/fastbuild https://github.com/scylladb/seastar (n.b. seastar has a lot more conditioned code that would be the same between asan and hwasan). Each of these needs to know the difference between compiling with asan and hwasan, so I'm confident that having some way to determine that in the source code is a good idea. I also believe there could be code in the wild that would need to distinguish between hwasan and asan where the existence of tags could be problematic: - code already using the top-byte-ignore feature may be able to be used with asan but not hwasan. - Code that makes assumptions about pointer ordering (e.g. the autoconf program that looks for stack growth direction) could break on hwasan but not on asan. - Code looking for the distance between two objects in memory would need to account for tags in pointers. Hence I think this distinction is needed. Matthew
Re: [PATCH 13/X] [libsanitizer][options] Add hwasan flags and argument parsing
On Tue, 5 Nov 2019, Matthew Malcomson wrote:
> +DEFHOOK
> +(can_tag_addresses,
> + "True if backend architecture naturally supports ignoring the top byte of\
> + pointers. This feature means that -fsanitize=hwaddress can work.",
> + bool, (), default_memtag_can_tag_addresses)
@option{-fsanitize=hwaddress} (and then regenerate tm.texi).
--
Joseph S. Myers
jos...@codesourcery.com
Re: [PATCH 13/X] [libsanitizer][options] Add hwasan flags and argument parsing
On Tue, Nov 5, 2019 at 12:34 PM Matthew Malcomson wrote: > > These flags can't be used at the same time as any of the other > sanitizers. > We add an equivalent flag to -static-libasan in -static-libhwasan to > ensure static linking. > > The -fsanitize=kernel-hwaddress option is for compiling targeting the > kernel. This flag has defaults that allow compiling KASAN with tags as > it is currently implemented. > These defaults are that we do not sanitize variables on the stack and > always recover from a detected bug. > Stack tagging in the kernel is a future aim, stack instrumentation has > not yet been enabled for the kernel for clang either > (https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/687121.html). > > We introduce a backend hook `targetm.memtag.can_tag_addresses` that > indicates to the mid-end whether a target has a feature like AArch64 TBI > where the top byte of an address is ignored. > Without this feature hwasan sanitization is not done. > > NOTE: > -- > I have defined a new macro of __SANITIZE_HWADDRESS__ that gets > automatically defined when compiling with hwasan. This is analogous to > __SANITIZE_ADDRESS__ which is defined when compiling with asan. > > Users in the kernel have expressed an interest in using > __SANITIZE_ADDRESS__ for both > (https://lists.infradead.org/pipermail/linux-arm-kernel/2019-October/690703.html). > > One approach to do this could be to define __SANITIZE_ADDRESS__ with > different values depending on whether we are compiling with hwasan or > asan. > > Using __SANITIZE_ADDRESS__ for both means that code like the kernel > which wants to treat the two sanitizers as alternate implementations of > the same thing gets that automatically. > > My preference is to use __SANITIZE_HWADDRESS__ since that means any > existing code will not be predicated on this (and hence I guess less > surprises), but would appreciate feedback on this given the point above. +Evgenii Stepanov (A repost from my answer from the mentioned thread): > Similarly, I'm thinking I'll add no_sanitize_hwaddress as the hwasan > equivalent of no_sanitize_address, which will require an update in the > kernel given it seems you want KASAN to be used the same whether using > tags or not. We have intentionally reused the same macros to simplify things. Is there any reason to use separate macros for GCC? Are there places where we need to use specifically no_sanitize_hwaddress and __SANITIZE_HWADDRESS__, but not no_sanitize_address and __SANITIZE_ADDRESS__? > -- > > gcc/ChangeLog: > > 2019-11-05 Matthew Malcomson > > * asan.c (memory_tagging_p): New. > * asan.h (memory_tagging_p): New. > * common.opt (flag_sanitize_recover): Default for kernel > hwaddress. > (static-libhwasan): New cli option. > * config/aarch64/aarch64.c (aarch64_can_tag_addresses): New. > (TARGET_MEMTAG_CAN_TAG_ADDRESSES): New. > * config/gnu-user.h (LIBHWASAN_EARLY_SPEC): hwasan equivalent of > asan command line flags. > * cppbuiltin.c (define_builtin_macros_for_compilation_flags): > Add hwasan equivalent of __SANITIZE_ADDRESS__. > * doc/tm.texi: Document new hook. > * doc/tm.texi.in: Document new hook. > * flag-types.h (enum sanitize_code): New sanitizer values. > * gcc.c (STATIC_LIBHWASAN_LIBS): New macro. > (LIBHWASAN_SPEC): New macro. > (LIBHWASAN_EARLY_SPEC): New macro. > (SANITIZER_EARLY_SPEC): Update to include hwasan. > (SANITIZER_SPEC): Update to include hwasan. > (sanitize_spec_function): Use hwasan options. > * opts.c (finish_options): Describe conflicts between address > sanitizers. > (sanitizer_opts): Introduce new sanitizer flags. > (common_handle_option): Add defaults for kernel sanitizer. > * params.def (PARAM_HWASAN_RANDOM_FRAME_TAG): New. > (PARAM_HWASAN_STACK): New. > * params.h (HWASAN_STACK): New. > (HWASAN_RANDOM_FRAME_TAG): New. > * target.def (HOOK_PREFIX): Add new hook. > * targhooks.c (default_memtag_can_tag_addresses): New. > * toplev.c (process_options): Ensure hwasan only on TBI > architectures. > > gcc/c-family/ChangeLog: > > 2019-11-05 Matthew Malcomson > > * c-attribs.c (handle_no_sanitize_hwaddress_attribute): New > attribute. > > > > ### Attachment also inlined for ease of reply > ### > > > diff --git a/gcc/c-family/c-attribs.c b/gcc/c-family/c-attribs.c > index > 6500b998321419a1d8d57062534206c5909adb7a..2de94815f91da5a0fd06c30d0044f866084121b8 > 100644 > --- a/gcc/c-family/c-attribs.c > +++ b/gcc/c-family/c-attribs.c > @@ -54,6 +54,8 @@ static tree handle_cold_attribute (tree *, tree, tree, int, > bool *); > static tree handle_no_sanitize_attribute (tree *, tree, tree, int, bool *); > static tree handle_no_sanitize_address_attribute (tree *, tree, tree, >
