Re: Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
On 24/11/2020 12:30, Hongtao Liu wrote: Hi: I'm learning about this patch, and I see one place that might be slighted improved. + poly_int64 size = (top - bot); + + /* Assert the edge of each variable is aligned to the HWASAN tag granule +size. */ + gcc_assert (multiple_p (top, HWASAN_TAG_GRANULE_SIZE)); + gcc_assert (multiple_p (bot, HWASAN_TAG_GRANULE_SIZE)); + gcc_assert (multiple_p (size, HWASAN_TAG_GRANULE_SIZE)); + The last gcc_assert looks redundant? Hi I think you're right. Just FYI I'm planning on making that change as an obvious fix after the patchset as it is now goes in. That way I can say I ran all my tests on the patch series that I applied without going through the all the tests again. Thanks for the catch! Matthew On Sat, Nov 21, 2020 at 2:48 AM Matthew Malcomson via Gcc-patches wrote:
Re: Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Hi: I'm learning about this patch, and I see one place that might be slighted improved. + poly_int64 size = (top - bot); + + /* Assert the edge of each variable is aligned to the HWASAN tag granule +size. */ + gcc_assert (multiple_p (top, HWASAN_TAG_GRANULE_SIZE)); + gcc_assert (multiple_p (bot, HWASAN_TAG_GRANULE_SIZE)); + gcc_assert (multiple_p (size, HWASAN_TAG_GRANULE_SIZE)); + The last gcc_assert looks redundant? On Sat, Nov 21, 2020 at 2:48 AM Matthew Malcomson via Gcc-patches wrote: > > > > Hi there, > > I was just doing some double-checks and noticed I'd placed the > documentation in the wrong section of tm.texi. The `MEMTAG` hooks were > documented in the `Register Classes` section, so I've now moved it to > the `Misc` section. > > That's the only change, Ok for trunk? > > Matthew > > > > > > > Handling stack variables has three features. > > 1) Ensure HWASAN required alignment for stack variables > > When tagging shadow memory, we need to ensure that each tag granule is > only used by one variable at a time. > > This is done by ensuring that each tagged variable is aligned to the tag > granule representation size and also ensure that the end of each > object is aligned to ensure the start of any other data stored on the > stack is in a different granule. > > This patch ensures the above by forcing the stack pointer to be aligned > before and after allocating any stack objects. Since we are forcing > alignment we also use `align_local_variable` to ensure this new alignment > is advertised properly through SET_DECL_ALIGN. > > 2) Put tags into each stack variable pointer > > Make sure that every pointer to a stack variable includes a tag of some > sort on it. > > The way tagging works is: > 1) For every new stack frame, a random tag is generated. > 2) A base register is formed from the stack pointer value and this > random tag. > 3) References to stack variables are now formed with RTL describing an > offset from this base in both tag and value. > > The random tag generation is handled by a backend hook. This hook > decides whether to introduce a random tag or use the stack background > based on the parameter hwasan-random-frame-tag. Using the stack > background is necessary for testing and bootstrap. It is necessary > during bootstrap to avoid breaking the `configure` test program for > determining stack direction. > > Using the stack background means that every stack frame has the initial > tag of zero and variables are tagged with incrementing tags from 1, > which also makes debugging a bit easier. > > Backend hooks define the size of a tag, the layout of the HWASAN shadow > memory, and handle emitting the code that inserts and extracts tags from a > pointer. > > 3) For each stack variable, tag and untag the shadow stack on function >prologue and epilogue. > > On entry to each function we tag the relevant shadow stack region for > each stack variable. This stack region is tagged to match the tag added to > each pointer to that variable. > > This is the first patch where we use the HWASAN shadow space, so we need > to add in the libhwasan initialisation code that creates this shadow > memory region into the binary we produce. This instrumentation is done > in `compile_file`. > > When exiting a function we need to ensure the shadow stack for this > function has no remaining tags. Without clearing the shadow stack area > for this stack frame, later function calls could get false positives > when those later function calls check untagged areas (such as parameters > passed on the stack) against a shadow stack area with left-over tag. > > Hence we ensure that the entire stack frame is cleared on function exit. > > config/ChangeLog: > > * bootstrap-hwasan.mk: Disable random frame tags for stack-tagging > during bootstrap. > > ChangeLog: > > * gcc/asan.c (struct hwasan_stack_var): New. > (hwasan_sanitize_p): New. > (hwasan_sanitize_stack_p): New. > (hwasan_sanitize_allocas_p): New. > (initialize_sanitizer_builtins): Define new builtins. > (ATTR_NOTHROW_LIST): New macro. > (hwasan_current_frame_tag): New. > (hwasan_frame_base): New. > (stack_vars_base_reg_p): New. > (hwasan_maybe_init_frame_base_init): New. > (hwasan_record_stack_var): New. > (hwasan_get_frame_extent): New. > (hwasan_increment_frame_tag): New. > (hwasan_record_frame_init): New. > (hwasan_emit_prologue): New. > (hwasan_emit_untag_frame): New. > (hwasan_finish_file): New. > (hwasan_truncate_to_tag_size): New. > * gcc/asan.h (hwasan_record_frame_init): New declaration. > (hwasan_record_stack_var): New declaration. > (hwasan_emit_prologue): New declaration. > (hwasan_emit_untag_frame): New declaration. >
Re: Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Sorry for the earlier OK, was replying to the wrong message…
Matthew Malcomson writes:
> @@ -1216,6 +1255,24 @@ expand_stack_vars (bool (*pred) (size_t), class
> stack_vars_data *data)
> {
> offset = alloc_stack_frame_space (stack_vars[i].size, alignb);
> base_align = crtl->max_used_stack_slot_alignment;
> +
> + if (hwasan_sanitize_stack_p ())
> + {
> + /* Align again since the point of this alignment is to handle
> + the "end" of the object (i.e. smallest address after the
> + stack object). For FRAME_GROWS_DOWNWARD that requires
> + aligning the stack before allocating, but for a frame that
> + grows upwards that requires aligning the stack after
> + allocation.
> +
> + Use `frame_offset` to record the offset value rather than
> + offset since the `frame_offset` describes the extent
What I meant here was to quote “offset”, i.e.:
Use `frame_offset` to record the offset value rather than
`offset` since `frame_offset` describes the extent
Without the quoting, “the offset value rather than offset” was quite
hard to parse.
OK with that change, thanks.
Richard
> + allocated for this particular variable while `offset`
> + describes the address that this variable starts at. */
> + align_frame_offset (HWASAN_TAG_GRANULE_SIZE);
> + hwasan_record_stack_var (virtual_stack_vars_rtx, base,
> +hwasan_orig_offset, frame_offset);
> + }
> }
> }
>else
Re: Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Matthew Malcomson writes: > Hi there, > > I was just doing some double-checks and noticed I'd placed the > documentation in the wrong section of tm.texi. The `MEMTAG` hooks were > documented in the `Register Classes` section, so I've now moved it to > the `Misc` section. > > That's the only change, Ok for trunk? OK, thanks. Richard
Re: Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Hi there, I was just doing some double-checks and noticed I'd placed the documentation in the wrong section of tm.texi. The `MEMTAG` hooks were documented in the `Register Classes` section, so I've now moved it to the `Misc` section. That's the only change, Ok for trunk? Matthew Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each object is aligned to ensure the start of any other data stored on the stack is in a different granule. This patch ensures the above by forcing the stack pointer to be aligned before and after allocating any stack objects. Since we are forcing alignment we also use `align_local_variable` to ensure this new alignment is advertised properly through SET_DECL_ALIGN. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. Backend hooks define the size of a tag, the layout of the HWASAN shadow memory, and handle emitting the code that inserts and extracts tags from a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable. This stack region is tagged to match the tag added to each pointer to that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tags. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. config/ChangeLog: * bootstrap-hwasan.mk: Disable random frame tags for stack-tagging during bootstrap. ChangeLog: * gcc/asan.c (struct hwasan_stack_var): New. (hwasan_sanitize_p): New. (hwasan_sanitize_stack_p): New. (hwasan_sanitize_allocas_p): New. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_frame_tag): New. (hwasan_frame_base): New. (stack_vars_base_reg_p): New. (hwasan_maybe_init_frame_base_init): New. (hwasan_record_stack_var): New. (hwasan_get_frame_extent): New. (hwasan_increment_frame_tag): New. (hwasan_record_frame_init): New. (hwasan_emit_prologue): New. (hwasan_emit_untag_frame): New. (hwasan_finish_file): New. (hwasan_truncate_to_tag_size): New. * gcc/asan.h (hwasan_record_frame_init): New declaration. (hwasan_record_stack_var): New declaration. (hwasan_emit_prologue): New declaration. (hwasan_emit_untag_frame): New declaration. (hwasan_get_frame_extent): New declaration. (hwasan_maybe_enit_frame_base_init): New declaration. (hwasan_frame_base): New declaration. (stack_vars_base_reg_p): New declaration. (hwasan_current_frame_tag): New declaration. (hwasan_increment_frame_tag): New declaration. (hwasan_truncate_to_tag_size): New declaration. (hwasan_finish_file): New declaration. (hwasan_sanitize_p): New declaration. (hwasan_sanitize_stack_p): New declaration. (hwasan_sanitize_allocas_p): New declaration. (HWASAN_TAG_SIZE): New macro. (HWASAN_TAG_GRANULE_SIZE): New macro. (HWASAN_STACK_BACKGROUND): New macro. * gcc/builtin-types.def
Re: Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Matthew Malcomson writes:
> […]
> +/* hwasan_frame_base_init_seq is the sequence of RTL insns that will
> initialize
> + the hwasan_frame_base_ptr. When the hwasan_frame_base_ptr is requested,
> we
> + generate this sequence but do not emit it. If the sequence was created it
> + is emitted once the function body has been expanded.
> +
> + This delay is because the frame base pointer may be needed anywhere in the
> + function body, or needed by the expand_used_vars function. Emitting once
> in
> + a known place is simpler than requiring the emition of the instructions to
s/emition/emission/
> + be know where it should go depending on the first place the hwasan frame
> + base is needed. */
> +static GTY(()) rtx_insn *hwasan_frame_base_init_seq = NULL;
> […]
> +/* For stack tagging:
> +
> + Return the 'base pointer' for this function. If that base pointer has not
> + yet been created then we create a register to hold it and record the insns
> + to initialize the register in `hwasan_frame_base_init_seq` for later
> + emission. */
> +rtx
> +hwasan_frame_base ()
> +{
> + if (! hwasan_frame_base_ptr)
> +{
> + start_sequence ();
> + hwasan_frame_base_ptr =
> + force_reg (Pmode,
> +targetm.memtag.insert_random_tag (virtual_stack_vars_rtx,
> + NULL_RTX));
Nit: should be formatted as:
hwasan_frame_base_ptr
= force_reg (Pmode,
targetm.memtag.insert_random_tag (virtual_stack_vars_rtx,
NULL_RTX));
> […]
> + size_t length = hwasan_tagged_stack_vars.length ();
> + hwasan_stack_var *vars = hwasan_tagged_stack_vars.address ();
> +
> + poly_int64 bot = 0, top = 0;
> + size_t i = 0;
> + for (i = 0; i < length; i++)
> +{
> + hwasan_stack_var& cur = vars[i];
Simpler as:
poly_int64 bot = 0, top = 0;
for (hwasan_stack_var : hwasan_tagged_stack_vars)
(GCC style is to add a space before “&”, as for “*”)
> + poly_int64 nearest = cur.nearest_offset;
> + poly_int64 farthest = cur.farthest_offset;
> +
> + if (known_ge (nearest, farthest))
> + {
> + top = nearest;
> + bot = farthest;
> + }
> + else
> + {
> + /* Given how these values are calculated, one must be known greater
> + than the other. */
> + gcc_assert (known_le (nearest, farthest));
> + top = farthest;
> + bot = nearest;
> + }
> + poly_int64 size = (top - bot);
> +
> + /* Assert the edge of each variable is aligned to the HWASAN tag
> granule
> + size. */
> + gcc_assert (multiple_p (top, HWASAN_TAG_GRANULE_SIZE));
> + gcc_assert (multiple_p (bot, HWASAN_TAG_GRANULE_SIZE));
> + gcc_assert (multiple_p (size, HWASAN_TAG_GRANULE_SIZE));
> +
> + rtx ret = init_one_libfunc ("__hwasan_tag_memory");
> + rtx base_tag = targetm.memtag.extract_tag (cur.tagged_base, NULL_RTX);
> + rtx tag = plus_constant (QImode, base_tag, cur.tag_offset);
> + tag = hwasan_truncate_to_tag_size (tag, NULL_RTX);
> +
> + rtx bottom = convert_memory_address (ptr_mode,
> +plus_constant (Pmode,
> + cur.untagged_base,
> + bot));
> + emit_library_call (ret, LCT_NORMAL, VOIDmode,
> + bottom, ptr_mode,
> + tag, QImode,
> + gen_int_mode (size, ptr_mode), ptr_mode);
> +}
> + /* Clear the stack vars, we've emitted the prologue for them all now. */
> + hwasan_tagged_stack_vars.truncate (0);
> +}
> +
> +/* For stack tagging:
> +
> + Return RTL insns to clear the tags between DYNAMIC and VARS pointers
> + into the stack. These instructions should be emitted at the end of
> + every function.
> +
> + If `dynamic` is NULL_RTX then no insns are returned. */
> +rtx_insn *
> +hwasan_emit_untag_frame (rtx dynamic, rtx vars)
> +{
> + if (! dynamic)
> +return NULL;
> +
> + start_sequence ();
> +
> + dynamic = convert_memory_address (ptr_mode, dynamic);
> + vars = convert_memory_address (ptr_mode, vars);
> +
> + rtx top_rtx;
> + rtx bot_rtx;
> + if (FRAME_GROWS_DOWNWARD)
> +{
> + top_rtx = vars;
> + bot_rtx = dynamic;
> +}
> + else
> +{
> + top_rtx = dynamic;
> + bot_rtx = vars;
> +}
> +
> + rtx size_rtx = expand_simple_binop (ptr_mode, MINUS, top_rtx, bot_rtx,
> + NULL_RTX, /* unsignedp = */0,
> + OPTAB_DIRECT);
> +
> + rtx ret = init_one_libfunc ("__hwasan_tag_memory");
> + emit_library_call (ret, LCT_NORMAL, VOIDmode,
> + bot_rtx, ptr_mode,
> + HWASAN_STACK_BACKGROUND, QImode,
> + size_rtx, ptr_mode);
Nit: “ret” seems like a strange name for this variable, since
Update: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Hi there, After offline discussion with Richard I've modified the way in which the initialisation for the hwasan base pointer is emitted. Originally it was getting emitted during `expand_used_vars`, and requiring `handle_builtin_alloca` to register a need for it to be emitted so that `expand_HWASAN_CHOOSE_TAG` can use an initialised base pointer. Now we go through the entire expansion of the function body, and then if `hwasan_frame_base_ptr` was used anywhere we emit the initialisation just before `parm_birth_insn`. This is the updated patch for the stack variable handling. (Testing underway) MM --- Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each object is aligned to ensure the start of any other data stored on the stack is in a different granule. This patch ensures the above by forcing the stack pointer to be aligned before and after allocating any stack objects. Since we are forcing alignment we also use `align_local_variable` to ensure this new alignment is advertised properly through SET_DECL_ALIGN. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. Backend hooks define the size of a tag, the layout of the HWASAN shadow memory, and handle emitting the code that inserts and extracts tags from a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable. This stack region is tagged to match the tag added to each pointer to that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tags. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. config/ChangeLog: * bootstrap-hwasan.mk: Disable random frame tags for stack-tagging during bootstrap. ChangeLog: * gcc/asan.c (struct hwasan_stack_var): New. (hwasan_sanitize_p): New. (hwasan_sanitize_stack_p): New. (hwasan_sanitize_allocas_p): New. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_frame_tag): New. (hwasan_frame_base): New. (stack_vars_base_reg_p): New. (hwasan_maybe_emit_frame_base_init): New. (hwasan_record_stack_var): New. (hwasan_get_frame_extent): New. (hwasan_increment_frame_tag): New. (hwasan_record_frame_init): New. (hwasan_emit_prologue): New. (hwasan_emit_untag_frame): New. (hwasan_finish_file): New. (hwasan_truncate_to_tag_size): New. * gcc/asan.h (hwasan_record_frame_init): New declaration. (hwasan_record_stack_var): New declaration. (hwasan_emit_prologue): New declaration. (hwasan_emit_untag_frame): New declaration. (hwasan_get_frame_extent): New declaration. (hwasan_maybe_emit_frame_base_init): New declaration. (hwasan_frame_base): New declaration. (stack_vars_base_reg_p): New declaration. (hwasan_current_frame_tag): New declaration. (hwasan_increment_frame_tag): New declaration. (hwasan_truncate_to_tag_size): New declaration. (hwasan_finish_file): New declaration. (hwasan_sanitize_p): New declaration.
[PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each object is aligned to ensure the start of any other data stored on the stack is in a different granule. This patch ensures the above by forcing the stack pointer to be aligned before and after allocating any stack objects. Since we are forcing alignment we also use `align_local_variable` to ensure this new alignment is advertised properly through SET_DECL_ALIGN. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. Backend hooks define the size of a tag, the layout of the HWASAN shadow memory, and handle emitting the code that inserts and extracts tags from a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable. This stack region is tagged to match the tag added to each pointer to that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tags. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. config/ChangeLog: * bootstrap-hwasan.mk: Disable random frame tags for stack-tagging during bootstrap. ChangeLog: * gcc/asan.c (struct hwasan_stack_var): New. (hwasan_sanitize_p): New. (hwasan_sanitize_stack_p): New. (hwasan_sanitize_allocas_p): New. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_frame_tag): New. (hwasan_frame_base): New. (stack_vars_base_reg_p): New. (hwasan_maybe_init_frame_base): New. (hwasan_record_frame_base_needed): New. (hwasan_record_stack_var): New. (hwasan_get_frame_extent): New. (hwasan_increment_frame_tag): New. (hwasan_record_frame_init): New. (hwasan_emit_prologue): New. (hwasan_emit_untag_frame): New. (hwasan_finish_file): New. (hwasan_truncate_to_tag_size): New. * gcc/asan.h (hwasan_record_frame_init): New declaration. (hwasan_record_stack_var): New declaration. (hwasan_emit_prologue): New declaration. (hwasan_emit_untag_frame): New declaration. (hwasan_get_frame_extent): New declaration. (hwasan_maybe_init_frame_base): New declaration. (hwasan_frame_base): New declaration. (stack_vars_base_reg_p): New declaration. (hwasan_record_frame_base_needed): New declaration. (hwasan_current_frame_tag): New declaration. (hwasan_increment_frame_tag): New declaration. (hwasan_truncate_to_tag_size): New declaration. (hwasan_finish_file): New declaration. (hwasan_sanitize_p): New declaration. (hwasan_sanitize_stack_p): New declaration. (hwasan_sanitize_allocas_p): New declaration. (HWASAN_TAG_SIZE): New macro. (HWASAN_TAG_GRANULE_SIZE): New macro. (HWASAN_STACK_BACKGROUND): New macro. * gcc/builtin-types.def (BT_FN_VOID_PTR_UINT8_PTRMODE): New. * gcc/builtins.def (DEF_SANITIZER_BUILTIN): Enable for HWASAN. * gcc/cfgexpand.c (align_local_variable): When using hwasan ensure alignment to tag granule. (align_frame_offset): New.
Re: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Matthew Malcomson writes:
> Hi Richard,
>
> I'm sending up the revised patch 5 (introducing stack variable handling)
> without the other changes to other patches.
>
> I figure there's been quite a lot of changes to this patch and I wanted
> to give you time to review them while I worked on finishing the less
> widespread changes in patch 6 and before I ran the more exhaustive (and
> time-consuming) tests in case you didn't like the changes and those
> exhaustive tests would just have to get repeated.
Thanks, the new approach looks good to me. Most of the comments below
are just minor.
> […]
> @@ -75,6 +89,26 @@ extern hash_set *asan_used_labels;
>
> #define ASAN_USE_AFTER_SCOPE_ATTRIBUTE "use after scope memory"
>
> +/* NOTE: The values below and the hooks under targetm.memtag define an ABI
> and
> + are hard-coded to these values in libhwasan, hence they can't be changed
> + independently here. */
> +/* How many bits are used to store a tag in a pointer.
> + The default version uses the entire top byte of a pointer (i.e. 8 bits).
> */
> +#define HWASAN_TAG_SIZE targetm.memtag.tag_size ()
> +/* Tag Granule of HWASAN shadow stack.
> + This is the size in real memory that each byte in the shadow memory refers
> + to. I.e. if a variable is X bytes long in memory then it's tag in shadow
s/it's/its/
> + memory will span X / HWASAN_TAG_GRANULE_SIZE bytes.
> + Most variables will need to be aligned to this amount since two variables
> + that are neighbours in memory and share a tag granule would need to share
s/neighbours/neighbors/
> + the same tag (the shared tag granule can only store one tag). */
> +#define HWASAN_TAG_GRANULE_SIZE targetm.memtag.granule_size ()
> +/* Define the tag for the stack background.
> + This defines what tag the stack pointer will be and hence what tag all
> + variables that are not given special tags are (e.g. spilled registers,
> + and parameters passed on the stack). */
> +#define HWASAN_STACK_BACKGROUND gen_int_mode (0, QImode)
> +
> /* Various flags for Asan builtins. */
> enum asan_check_flags
> {
> […]
> @@ -1352,6 +1393,28 @@ asan_redzone_buffer::flush_if_full (void)
> flush_redzone_payload ();
> }
>
> +/* Returns whether we are tagging pointers and checking those tags on memory
> + access. */
> +bool
> +hwasan_sanitize_p ()
> +{
> + return sanitize_flags_p (SANITIZE_HWADDRESS);
> +}
> +
> +/* Are we tagging the stack? */
> +bool
> +hwasan_sanitize_stack_p ()
> +{
> + return (hwasan_sanitize_p () && param_hwasan_instrument_stack);
> +}
> +
> +/* Are we protecting alloca objects? */
Same comment as before about avoiding the word “protect”, both in the
comment and the option name. Maybe s/protect/sanitize/ or s/protect/tag/.
> +bool
> +hwasan_sanitize_allocas_p (void)
> +{
> + return (hwasan_sanitize_stack_p () && param_hwasan_protect_allocas);
> +}
> +
> /* Insert code to protect stack vars. The prologue sequence should be
> emitted
> directly, epilogue sequence returned. BASE is the register holding the
> stack base, against which OFFSETS array offsets are relative to, OFFSETS
> […]
> @@ -3702,4 +3772,330 @@ make_pass_asan_O0 (gcc::context *ctxt)
>return new pass_asan_O0 (ctxt);
> }
>
> +/* For stack tagging:
> +
> + Return the offset from the frame base tag that the "next" expanded object
> + should have. */
> +uint8_t
> +hwasan_current_frame_tag ()
> +{
> + return hwasan_frame_tag_offset;
> +}
> +
> +/* For stack tagging:
> +
> + Return the 'base pointer' for this function. If that base pointer has not
> + yet been created then we create a register to hold it and initialise that
> + value with a possibly random tag and the value of the
> + virtual_stack_vars_rtx. */
As discussed offline, I think the old approach of generating the
initialisation in hwasan_emit_prologue was safer, although I agree
there doesn't seem to be a specific problem with doing things this way.
> +rtx
> +hwasan_frame_base ()
> +{
> + if (! hwasan_frame_base_ptr)
> +{
> + hwasan_frame_base_ptr
> + = targetm.memtag.insert_random_tag (virtual_stack_vars_rtx);
> +}
Nit: should be no braces around single statements, even if they span
multiple lines.
> +
> + return hwasan_frame_base_ptr;
> +}
> +
> +/* Record a compile-time constant size stack variable that HWASAN will need
> to
> + tag. This record of the range of a stack variable will be used by
> + `hwasan_emit_prologue` to emit the RTL at the start of each frame which
> will
> + set tags in the shadow memory according to the assigned tag for each
> object.
> +
> + The range that the object spans in stack space should be described by the
> + bounds `untagged_base + nearest` and `untagged_base + farthest`.
> + `tagged_base` is the base address which contains the "base frame tag" for
> + this frame, and from which the value to address this object with will be
> + calculated.
> +
> + We record the
[PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Hi Richard, I'm sending up the revised patch 5 (introducing stack variable handling) without the other changes to other patches. I figure there's been quite a lot of changes to this patch and I wanted to give you time to review them while I worked on finishing the less widespread changes in patch 6 and before I ran the more exhaustive (and time-consuming) tests in case you didn't like the changes and those exhaustive tests would just have to get repeated. The big differences between this and the last version are: - I moved all helper functions which rely on how the tag is encoded into hooks. This allows backends to choose a different ABI for hwasan tagging. That said, any ABI which doesn't ensure the entire the tag is stored as a top byte is unsupported as the library doesn't handle anything else. - I no longer delay emitting RTL to initialise the hwasan_base_pointer. It's now emitted as soon as the pointer is used anywhere. - No longer delay allocating stack variables for hwasan to work. Instead we record stack variables when allocating through expand_stack_var_1 *and* when allocating through expand_stack_vars. - Use `frame_offset` in more places to avoid having to manually handle the case of `!FRAME_GROWS_DOWNWARDS`. Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each object is aligned to ensure the start of any other data stored on the stack is in a different granule. This patch ensures the above by adding alignment requirements in `align_local_variable` and forcing the stack pointer to be aligned before allocating any stack objects. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. The tag offsets are also handled by a backend hook. This patch also adds some macros defining how the HWASAN shadow memory is stored and how a tag is stored in a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable the tag to match the tag added to each pointer for that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tag. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. ChangeLog: * config/bootstrap-hwasan.mk: Disable random frame tags for stack-tagging during bootstrap. * gcc/asan.c (struct hwasan_stack_var): New. (hwasan_sanitize_p): New. (hwasan_sanitize_stack_p): New. (hwasan_sanitize_allocas_p): New. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_frame_tag): New. (hwasan_frame_base): New. (hwasan_record_stack_var): New. (hwasan_get_frame_extent): New. (hwasan_increment_frame_tag): New. (hwasan_record_frame_init): New. (hwasan_emit_prologue): New. (hwasan_emit_untag_frame): New. (hwasan_finish_file): New. (hwasan_truncate_to_tag_size): New. * gcc/asan.h (hwasan_record_frame_init): New declaration. (hwasan_record_stack_var): New declaration. (hwasan_emit_prologue): New declaration.
Re: [PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Matthew Malcomson writes:
> @@ -75,6 +89,31 @@ extern hash_set *asan_used_labels;
>
> #define ASAN_USE_AFTER_SCOPE_ATTRIBUTE "use after scope memory"
>
> +/* NOTE: The values below define an ABI and are hard-coded to these values in
> + libhwasan, hence they can't be changed independently here. */
> +/* How many bits are used to store a tag in a pointer.
> + HWASAN uses the entire top byte of a pointer (i.e. 8 bits). */
> +#define HWASAN_TAG_SIZE 8
> +/* Tag Granule of HWASAN shadow stack.
> + This is the size in real memory that each byte in the shadow memory refers
> + to. I.e. if a variable is X bytes long in memory then it's tag in shadow
s/it's/its/
> + memory will span X / HWASAN_TAG_GRANULE_SIZE bytes.
> + Most variables will need to be aligned to this amount since two variables
> + that are neighbours in memory and share a tag granule would need to share
“neighbors” (alas)
> + the same tag (the shared tag granule can only store one tag). */
> +#define HWASAN_TAG_SHIFT_SIZE 4
> +#define HWASAN_TAG_GRANULE_SIZE (1ULL << HWASAN_TAG_SHIFT_SIZE)
> +/* Define the tag for the stack background.
> + This defines what tag the stack pointer will be and hence what tag all
> + variables that are not given special tags are (e.g. spilled registers,
> + and parameters passed on the stack). */
> +#define HWASAN_STACK_BACKGROUND 0
> +/* How many bits to shift in order to access the tag bits.
> + The tag is stored in the top 8 bits of a pointer hence shifting 56 bits
> will
> + leave just the tag. */
> +#define HWASAN_SHIFT 56
> +#define HWASAN_SHIFT_RTX const_int_rtx[MAX_SAVED_CONST_INT + HWASAN_SHIFT]
> +
> /* Various flags for Asan builtins. */
> enum asan_check_flags
> {
> diff --git a/gcc/asan.c b/gcc/asan.c
> index
> 9c9aa4cae35832c1534a2cffac1d3d13eed0e687..f755a3290f1091be14fbe4c51d9579389e5eb245
> 100644
> --- a/gcc/asan.c
> +++ b/gcc/asan.c
> @@ -257,6 +257,15 @@ hash_set *asan_handled_variables = NULL;
>
> hash_set *asan_used_labels = NULL;
>
> +/* Global variables for HWASAN stack tagging. */
> +/* tag_offset records the offset from the frame base tag that the next object
> + should have. */
> +static uint8_t tag_offset = 0;
> +/* hwasan_base_ptr is a pointer with the same address as
> + `virtual_stack_vars_rtx` for the current frame, and with the frame base
> tag
> + stored in it. */
> +static rtx hwasan_base_ptr = NULL_RTX;
Was initially surprised that this didn't need to be GTY, but I guess
there are no ggc_collect calls during the relevant parts of cfgexpand.
It's normally easy to tell when GTY is needed if all the users of the
state are in the same file, but for cases like this it's a little harder.
Might be worth a comment.
Might also be worth having “stack” or “frame” in these names, and similarly
for the public functions.
> @@ -1352,6 +1361,28 @@ asan_redzone_buffer::flush_if_full (void)
> flush_redzone_payload ();
> }
>
> +/* Returns whether we are tagging pointers and checking those tags on memory
> + access. */
> +bool
> +hwasan_sanitize_p ()
> +{
> +return sanitize_flags_p (SANITIZE_HWADDRESS);
Nit: excess indentation.
> @@ -2901,6 +2932,11 @@ initialize_sanitizer_builtins (void)
> = build_function_type_list (void_type_node, uint64_type_node,
> ptr_type_node, NULL_TREE);
>
> + tree BT_FN_VOID_PTR_UINT8_SIZE
> += build_function_type_list (void_type_node, ptr_type_node,
> + unsigned_char_type_node, size_type_node,
> + NULL_TREE);
The function prototype seems to be:
void __hwasan_tag_memory(uptr p, u8 tag, uptr sz)
and size_t doesn't have the same precision as pointers on all targets.
Maybe pointer_sized_int_node would be more accurate. (Despite its name,
it's unsigned rather than signed.)
> @@ -3702,4 +3740,269 @@ make_pass_asan_O0 (gcc::context *ctxt)
>return new pass_asan_O0 (ctxt);
> }
>
> +/* For stack tagging:
> + Initialise tag of the base register.
“Initialize”
Very minor, but I've not seen this style of comment elsewhere in GCC,
where the main comment is indented by two extra spaces. Think a blank
line and no extra indentation is more usual.
> + This has to be done as soon as the stack is getting expanded to ensure
> + anything emitted with `get_dynamic_stack_base` will use the value set
> here
> + instead of using a register without a tag.
> + Especially note that RTL expansion of large aligned values does that.
> */
> +void
> +hwasan_record_base (rtx base)
> +{
> + targetm.memtag.gentag (base, virtual_stack_vars_rtx);
> + hwasan_base_ptr = base;
> +}
> +
> +/* For stack tagging:
> + Return the offset from the frame base tag that the "next" expanded
> object
> + should have. */
> +uint8_t
> +hwasan_current_tag ()
> +{
> + return tag_offset;
> +}
> +
> +/* For stack tagging:
> + Increment the tag offset modulo the size a tag can represent.
[PATCH 5/X] libsanitizer: mid-end: Introduce stack variable handling for HWASAN
Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each object is aligned to ensure the start of any other data stored on the stack is in a different granule. This patch ensures the above by adding alignment requirements in `align_local_variable` and forcing all stack variable allocation to be deferred so that `expand_stack_vars` can ensure the stack pointer is aligned before allocating any variable for the current frame. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. The tag offsets are also handled by a backend hook. This patch also adds some macros defining how the HWASAN shadow memory is stored and how a tag is stored in a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable the tag to match the tag added to each pointer for that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tag. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. config/ChangeLog: * bootstrap-hwasan.mk: Disable random frame tags for stack-tagging during bootstrap. gcc/ChangeLog: * asan.c (hwasan_record_base): New function. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New function. (hwasan_with_tag): New function. (hwasan_tag_init): New function. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_tag): New. (hwasan_extract_tag): New. (hwasan_emit_prologue): New. (hwasan_create_untagged_base): New. (hwasan_finish_file): New. (hwasan_ctor_statements): New variable. (hwasan_sanitize_stack_p): New. (hwasan_sanitize_p): New. (hwasan_sanitize_allocas_p): New. * asan.h (hwasan_record_base): New declaration. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New declaration. (hwasan_with_tag): New declaration. (hwasan_sanitize_stack_p): New declaration. (hwasan_sanitize_allocas_p): New declaration. (hwasan_tag_init): New declaration. (hwasan_sanitize_p): New declaration. (HWASAN_TAG_SIZE): New macro. (HWASAN_TAG_GRANULE_SIZE): New macro. (HWASAN_TAG_SHIFT_SIZE): New macro. (HWASAN_SHIFT): New macro. (HWASAN_SHIFT_RTX): New macro. (HWASAN_STACK_BACKGROUND): New macro. (hwasan_finish_file): New declaration. (hwasan_current_tag): New declaration. (hwasan_create_untagged_base): New declaration. (hwasan_extract_tag): New declaration. (hwasan_emit_prologue): New declaration. * cfgexpand.c (struct stack_vars_data): Add information to record hwasan variable stack offsets. (expand_stack_vars): Ensure variables are offset from a tagged base. Record offsets for hwasan. Ensure alignment. (expand_used_vars): Call function to emit prologue, and get untagging instructions for function exit. (align_local_variable): Ensure alignment. (defer_stack_allocation): Ensure all variables are deferred so they can be handled by
[PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each variable as an alignment boundary between the end and the start of any other data stored on the stack. This patch ensures that by adding alignment requirements in `align_local_variable` and forcing all stack variable allocation to be deferred so that `expand_stack_vars` can ensure the stack pointer is aligned before allocating any variable for the current frame. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. The tag offsets are also handled by a backend hook. This patch also adds some macros defining how the HWASAN shadow memory is stored and how a tag is stored in a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable the tag to match the tag added to each pointer for that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tag. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. gcc/ChangeLog: 2019-12-12 Matthew Malcomson * asan.c (hwasan_record_base): New function. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New function. (hwasan_with_tag): New function. (hwasan_tag_init): New function. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_tag): New. (hwasan_emit_prologue): New. (hwasan_create_untagged_base): New. (hwasan_finish_file): New. (hwasan_sanitize_stack_p): New. (hwasan_sanitize_p): New. * asan.h (hwasan_record_base): New declaration. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New declaration. (hwasan_with_tag): New declaration. (hwasan_sanitize_stack_p): New declaration. (hwasan_tag_init): New declaration. (hwasan_sanitize_p): New declaration. (HWASAN_TAG_SIZE): New macro. (HWASAN_TAG_GRANULE_SIZE):New macro. (HWASAN_SHIFT):New macro. (HWASAN_SHIFT_RTX):New macro. (HWASAN_STACK_BACKGROUND):New macro. (hwasan_finish_file): New. (hwasan_current_tag): New. (hwasan_create_untagged_base): New. (hwasan_emit_prologue): New. * cfgexpand.c (struct stack_vars_data): Add information to record hwasan variable stack offsets. (expand_stack_vars): Ensure variables are offset from a tagged base. Record offsets for hwasan. Ensure alignment. (expand_used_vars): Call function to emit prologue, and get untagging instructions for function exit. (align_local_variable): Ensure alignment. (defer_stack_allocation): Ensure all variables are deferred so they can be handled by `expand_stack_vars`. (expand_one_stack_var_at): Account for tags in variables when using HWASAN. (expand_one_stack_var_1): Pass new argument to expand_one_stack_var_at. (init_vars_expansion): Initialise hwasan internal variables when starting variable expansion. * doc/tm.texi (TARGET_MEMTAG_GENTAG): Document. * doc/tm.texi.in (TARGET_MEMTAG_GENTAG): Document.
Re: [PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
On Wed, 20 Nov 2019, Matthew Malcomson wrote: > I don't have much of a plan. > > The most promising lead I have is that libiberty/alloca.c has a similar > functionality but with macros to account for a special case. The comment in libiberty/aclocal.m4 is: # We always want a C version of alloca() compiled into libiberty, # because native-compiler support for the real alloca is so !@#$% # unreliable that GCC has decided to use it only when being compiled # by GCC. This is the part of AC_FUNC_ALLOCA that calculates the # information alloca.c needs. This is the sort of thing that was relevant when GCC was built on lots of proprietary Unix systems with their system C compilers. Most of those proprietary Unix systems are long obsolete and are no longer supported by GCC. On the limited remaining set of host systems supported by GCC, there are a limited number of C++ compilers used to build most of the host code in GCC that is C++, and presumably a limited number of accompanying C compilers used to build libiberty. Now, libiberty is used by binutils more of which is written in C, but I doubt that expands the range of relevant host C compilers; the set of host OSes used nowadays is simply much smaller than it was when this code was written. So I'd suggest either completely eliminating C alloca from libiberty, or at least not building it at all when building with a compiler that defines __GNUC__. -- Joseph S. Myers jos...@codesourcery.com
Re: [PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
On 20/11/2019 14:46, Martin Liška wrote:
> On 11/20/19 3:37 PM, Matthew Malcomson wrote:
>> Hi Martin,
>>
>> Thanks for the review,
>
> You're welcome.
>
>> I'll get working on your comments now, but since I really enjoyed
>> finding this bug in ./configure when I hit it I thought I'd answer this
>> right away.
>
> Heh :)
>
>>
>>
>> On 20/11/2019 14:02, Martin Liška wrote:
>>> On 11/7/19 7:37 PM, Matthew Malcomson wrote:
diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk
index
4f60bed3fd6e98b47a3a38aea6eba2a7c320da25..91989f4bb1db6ccff56438357b896645e541
100644
--- a/config/bootstrap-hwasan.mk
+++ b/config/bootstrap-hwasan.mk
@@ -1,7 +1,11 @@
# This option enables -fsanitize=hwaddress for stage2 and stage3.
+# We need to disable random frame tags for bootstrap since the
autoconf check
+# for which direction the stack is growing has UB that a random frame
tag
+# breaks. Running with a random frame tag gives approx. 50% chance of
+# bootstrap comparison diff in libiberty/alloca.c.
>>>
>>> Here I would like to see what's exactly the problem. I would expect ASAN
>>> will
>>> have exactly the same problem? Can you please isolate it and file a bug.
>>> I bet
>>> a configure script should not expose an undefined behavior.
>>>
>>
>> The configure problem is this snippet below:
>>
>>
>> find_stack_direction ()
>> {
>> static char *addr = 0;
>> auto char dummy;
>> if (addr == 0)
>> {
>> addr =
>> return find_stack_direction ();
>> }
>> else
>> return ( > addr) ? 1 : -1;
>> }
>> main ()
>> {
>> exit (find_stack_direction() < 0);
>> }
>>
>>
>> configure uses this to determine the direction that the stack grows.
>>
>> `find_stack_direction` compares the address of two different objects and
>> uses that to make a decision.
>>
>> With HWASAN random frame tags the answer to the comparison is mostly
>> determined by what random tag was assigned to the object in each frame,
>> rather than the memory layout of the stack -- which means this configure
>> test program can end up getting different answers on different runs.
>>
>> This is not a problem for ASAN since ASAN does not store tags in the
>> pointers of variables.
>>
>>
>> You're right -- I should file a bug on that for configure.
>>
>> For reference the UB clause in the standard is 6.5.8 #5 (relational
>> operators) where there's a sentence at the end saying "In all other
>> cases, the behaviour is undefined". Essentially, this program is
>> comparing the address of two different objects on the stack, and that's
>> not allowed.
>
> Well, to be honest, this is quite cute violation of the standard. I would
> have written exactly the same code for the stack direction direction. I
> understand
> that a top byte will (a.k.a. tag) make the randomness.
>
> Do you have an idea how can we rewrite the check?
> Thanks,
> Martin
>
I don't have much of a plan.
The most promising lead I have is that libiberty/alloca.c has a similar
functionality but with macros to account for a special case.
Instead of just using '&' it uses a macro `ADDRESS_FUNCTION`.
I can use that macro to ensure the libiberty/alloca.c function could
handle tags, but I'm not sure that architecture specific conditions will
neatly fit into autoconf.
Re: [PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
On 11/20/19 3:37 PM, Matthew Malcomson wrote:
Hi Martin,
Thanks for the review,
You're welcome.
I'll get working on your comments now, but since I really enjoyed
finding this bug in ./configure when I hit it I thought I'd answer this
right away.
Heh :)
On 20/11/2019 14:02, Martin Liška wrote:
On 11/7/19 7:37 PM, Matthew Malcomson wrote:
diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk
index
4f60bed3fd6e98b47a3a38aea6eba2a7c320da25..91989f4bb1db6ccff56438357b896645e541
100644
--- a/config/bootstrap-hwasan.mk
+++ b/config/bootstrap-hwasan.mk
@@ -1,7 +1,11 @@
# This option enables -fsanitize=hwaddress for stage2 and stage3.
+# We need to disable random frame tags for bootstrap since the
autoconf check
+# for which direction the stack is growing has UB that a random frame
tag
+# breaks. Running with a random frame tag gives approx. 50% chance of
+# bootstrap comparison diff in libiberty/alloca.c.
Here I would like to see what's exactly the problem. I would expect ASAN
will
have exactly the same problem? Can you please isolate it and file a bug.
I bet
a configure script should not expose an undefined behavior.
The configure problem is this snippet below:
find_stack_direction ()
{
static char *addr = 0;
auto char dummy;
if (addr == 0)
{
addr =
return find_stack_direction ();
}
else
return ( > addr) ? 1 : -1;
}
main ()
{
exit (find_stack_direction() < 0);
}
configure uses this to determine the direction that the stack grows.
`find_stack_direction` compares the address of two different objects and
uses that to make a decision.
With HWASAN random frame tags the answer to the comparison is mostly
determined by what random tag was assigned to the object in each frame,
rather than the memory layout of the stack -- which means this configure
test program can end up getting different answers on different runs.
This is not a problem for ASAN since ASAN does not store tags in the
pointers of variables.
You're right -- I should file a bug on that for configure.
For reference the UB clause in the standard is 6.5.8 #5 (relational
operators) where there's a sentence at the end saying "In all other
cases, the behaviour is undefined". Essentially, this program is
comparing the address of two different objects on the stack, and that's
not allowed.
Well, to be honest, this is quite cute violation of the standard. I would
have written exactly the same code for the stack direction direction. I
understand
that a top byte will (a.k.a. tag) make the randomness.
Do you have an idea how can we rewrite the check?
Thanks,
Martin
Re: [PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
Hi Martin,
Thanks for the review,
I'll get working on your comments now, but since I really enjoyed
finding this bug in ./configure when I hit it I thought I'd answer this
right away.
On 20/11/2019 14:02, Martin Liška wrote:
> On 11/7/19 7:37 PM, Matthew Malcomson wrote:
>>
>> diff --git a/config/bootstrap-hwasan.mk b/config/bootstrap-hwasan.mk
>> index
>> 4f60bed3fd6e98b47a3a38aea6eba2a7c320da25..91989f4bb1db6ccff56438357b896645e541
>>
>> 100644
>> --- a/config/bootstrap-hwasan.mk
>> +++ b/config/bootstrap-hwasan.mk
>> @@ -1,7 +1,11 @@
>> # This option enables -fsanitize=hwaddress for stage2 and stage3.
>> +# We need to disable random frame tags for bootstrap since the
>> autoconf check
>> +# for which direction the stack is growing has UB that a random frame
>> tag
>> +# breaks. Running with a random frame tag gives approx. 50% chance of
>> +# bootstrap comparison diff in libiberty/alloca.c.
>
> Here I would like to see what's exactly the problem. I would expect ASAN
> will
> have exactly the same problem? Can you please isolate it and file a bug.
> I bet
> a configure script should not expose an undefined behavior.
>
The configure problem is this snippet below:
find_stack_direction ()
{
static char *addr = 0;
auto char dummy;
if (addr == 0)
{
addr =
return find_stack_direction ();
}
else
return ( > addr) ? 1 : -1;
}
main ()
{
exit (find_stack_direction() < 0);
}
configure uses this to determine the direction that the stack grows.
`find_stack_direction` compares the address of two different objects and
uses that to make a decision.
With HWASAN random frame tags the answer to the comparison is mostly
determined by what random tag was assigned to the object in each frame,
rather than the memory layout of the stack -- which means this configure
test program can end up getting different answers on different runs.
This is not a problem for ASAN since ASAN does not store tags in the
pointers of variables.
You're right -- I should file a bug on that for configure.
For reference the UB clause in the standard is 6.5.8 #5 (relational
operators) where there's a sentence at the end saying "In all other
cases, the behaviour is undefined". Essentially, this program is
comparing the address of two different objects on the stack, and that's
not allowed.
Re: [PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
On 11/7/19 7:37 PM, Matthew Malcomson wrote: Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each variable as an alignment boundary between the end and the start of any other data stored on the stack. This patch ensures that by adding alignment requirements in `align_local_variable` and forcing all stack variable allocation to be deferred so that `expand_stack_vars` can ensure the stack pointer is aligned before allocating any variable for the current frame. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. The tag offsets are also handled by a backend hook. This patch also adds some macros defining how the HWASAN shadow memory is stored and how a tag is stored in a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable the tag to match the tag added to each pointer for that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tag. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. gcc/ChangeLog: 2019-11-07 Matthew Malcomson * asan.c (hwasan_record_base): New function. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New function. (hwasan_with_tag): New function. (hwasan_tag_init): New function. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_tag): New. (hwasan_emit_prologue): New. (hwasan_create_untagged_base): New. (hwasan_finish_file): New. (hwasan_sanitize_stack_p): New. (memory_tagging_p): New. * asan.h (hwasan_record_base): New declaration. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New declaration. (hwasan_with_tag): New declaration. (hwasan_sanitize_stack_p): New declaration. (hwasan_tag_init): New declaration. (memory_tagging_p): New declaration. (HWASAN_TAG_SIZE): New macro. (HWASAN_TAG_GRANULE_SIZE):New macro. (HWASAN_SHIFT):New macro. (HWASAN_SHIFT_RTX):New macro. (HWASAN_STACK_BACKGROUND):New macro. (hwasan_finish_file): New. (hwasan_current_tag): New. (hwasan_create_untagged_base): New. (hwasan_emit_prologue): New. * cfgexpand.c (struct stack_vars_data): Add information to record hwasan variable stack offsets. (expand_stack_vars): Ensure variables are offset from a tagged base. Record offsets for hwasan. Ensure alignment. (expand_used_vars): Call function to emit prologue, and get untagging instructions for function exit. (align_local_variable): Ensure alignment. (defer_stack_allocation): Ensure all variables are deferred so they can be handled by `expand_stack_vars`. (expand_one_stack_var_at): Account for tags in variables when using HWASAN. (expand_one_stack_var_1): Pass new argument to expand_one_stack_var_at. (init_vars_expansion): Initialise hwasan internal variables when starting variable expansion. * doc/tm.texi (TARGET_MEMTAG_GENTAG): Document.
[PATCH 5/X] [libsanitizer][mid-end] Introduce stack variable handling for HWASAN
Handling stack variables has three features. 1) Ensure HWASAN required alignment for stack variables When tagging shadow memory, we need to ensure that each tag granule is only used by one variable at a time. This is done by ensuring that each tagged variable is aligned to the tag granule representation size and also ensure that the end of each variable as an alignment boundary between the end and the start of any other data stored on the stack. This patch ensures that by adding alignment requirements in `align_local_variable` and forcing all stack variable allocation to be deferred so that `expand_stack_vars` can ensure the stack pointer is aligned before allocating any variable for the current frame. 2) Put tags into each stack variable pointer Make sure that every pointer to a stack variable includes a tag of some sort on it. The way tagging works is: 1) For every new stack frame, a random tag is generated. 2) A base register is formed from the stack pointer value and this random tag. 3) References to stack variables are now formed with RTL describing an offset from this base in both tag and value. The random tag generation is handled by a backend hook. This hook decides whether to introduce a random tag or use the stack background based on the parameter hwasan-random-frame-tag. Using the stack background is necessary for testing and bootstrap. It is necessary during bootstrap to avoid breaking the `configure` test program for determining stack direction. Using the stack background means that every stack frame has the initial tag of zero and variables are tagged with incrementing tags from 1, which also makes debugging a bit easier. The tag offsets are also handled by a backend hook. This patch also adds some macros defining how the HWASAN shadow memory is stored and how a tag is stored in a pointer. 3) For each stack variable, tag and untag the shadow stack on function prologue and epilogue. On entry to each function we tag the relevant shadow stack region for each stack variable the tag to match the tag added to each pointer for that variable. This is the first patch where we use the HWASAN shadow space, so we need to add in the libhwasan initialisation code that creates this shadow memory region into the binary we produce. This instrumentation is done in `compile_file`. When exiting a function we need to ensure the shadow stack for this function has no remaining tag. Without clearing the shadow stack area for this stack frame, later function calls could get false positives when those later function calls check untagged areas (such as parameters passed on the stack) against a shadow stack area with left-over tag. Hence we ensure that the entire stack frame is cleared on function exit. gcc/ChangeLog: 2019-11-07 Matthew Malcomson * asan.c (hwasan_record_base): New function. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New function. (hwasan_with_tag): New function. (hwasan_tag_init): New function. (initialize_sanitizer_builtins): Define new builtins. (ATTR_NOTHROW_LIST): New macro. (hwasan_current_tag): New. (hwasan_emit_prologue): New. (hwasan_create_untagged_base): New. (hwasan_finish_file): New. (hwasan_sanitize_stack_p): New. (memory_tagging_p): New. * asan.h (hwasan_record_base): New declaration. (hwasan_emit_untag_frame): New. (hwasan_increment_tag): New declaration. (hwasan_with_tag): New declaration. (hwasan_sanitize_stack_p): New declaration. (hwasan_tag_init): New declaration. (memory_tagging_p): New declaration. (HWASAN_TAG_SIZE): New macro. (HWASAN_TAG_GRANULE_SIZE):New macro. (HWASAN_SHIFT):New macro. (HWASAN_SHIFT_RTX):New macro. (HWASAN_STACK_BACKGROUND):New macro. (hwasan_finish_file): New. (hwasan_current_tag): New. (hwasan_create_untagged_base): New. (hwasan_emit_prologue): New. * cfgexpand.c (struct stack_vars_data): Add information to record hwasan variable stack offsets. (expand_stack_vars): Ensure variables are offset from a tagged base. Record offsets for hwasan. Ensure alignment. (expand_used_vars): Call function to emit prologue, and get untagging instructions for function exit. (align_local_variable): Ensure alignment. (defer_stack_allocation): Ensure all variables are deferred so they can be handled by `expand_stack_vars`. (expand_one_stack_var_at): Account for tags in variables when using HWASAN. (expand_one_stack_var_1): Pass new argument to expand_one_stack_var_at. (init_vars_expansion): Initialise hwasan internal variables when starting variable expansion. * doc/tm.texi (TARGET_MEMTAG_GENTAG): Document. * doc/tm.texi.in (TARGET_MEMTAG_GENTAG): Document.
