diff mbox

Support asan-fixed-shadow-offset in GCC

Message ID CA+FTKhusLtf9frmh7VcNMWpwDdanHT3mgUh8PhduDY9Ff4sCww@mail.gmail.com
State New
Headers show

Commit Message

Alexey Preobrazhensky July 21, 2014, 7 p.m. UTC 
Hi all,

This patch adds support for non-fixed shadow in asan stack instrumentation.

It is required for Kernel AddressSanitizer, as the shadow offset is
not known at the compile time, and the shadow may not be allocated
during the early boot stages.

This option is intended to be triggered by -fsanitize=kernel-address
option, together with enabling instrumentation with calls.

Bootstrapped&regtested on x86_64.

Codereview: https://codereview.appspot.com/118040043/

--
Alexey
2014-07-21  Yury Gribov  <y.gribov@samsung.com>
            Alexey Preobrazhensky  <preobr@google.com>

	New asan-fixed-shadow-offset parameter.

	gcc/
	* asan.c (asan_emit_stack_protection): Add support for new parameter.
	* params.def: Define new parameter.
	* params.h: Likewise.

	gcc/testsuite/
	* c-c++-common/asan/no-fixed-shadow-offset.c: New.

Comments

Yury Gribov July 22, 2014, 5:17 a.m. UTC | #1 
On 07/21/2014 11:00 PM, Alexey Preobrazhensky wrote:
> This patch adds support for non-fixed shadow in asan stack instrumentation.

We probably also need to support non-fixed shadow in the middle-end
(the patch only implements it for RTL stack poisoner).

-Y
Andrey Ryabinin July 22, 2014, 10:18 a.m. UTC | #2 
On 07/21/14 23:00, Alexey Preobrazhensky wrote:
> Hi all,
> 
> This patch adds support for non-fixed shadow in asan stack instrumentation.
> 
> It is required for Kernel AddressSanitizer, as the shadow offset is
> not known at the compile time,

To get shadow offset this patch uses function __asan_get_shadow_ptr.
Wouldn't be more effective just to read variable instead of function call?

> and the shadow may not be allocated
> during the early boot stages.
> 

It's true for now, but at some future point I want to make shadow's allocation very early,
before running any instrumented code, so check for __asan_get_shadow_ptr() == 0 will be useless.


> This option is intended to be triggered by -fsanitize=kernel-address
> option, together with enabling instrumentation with calls.
> 
> Bootstrapped&regtested on x86_64.
> 
> Codereview: https://codereview.appspot.com/118040043/
> 
> --
> Alexey
>
Yury Gribov July 22, 2014, 10:30 a.m. UTC | #3 
>> It is required for Kernel AddressSanitizer, as the shadow offset is
>> not known at the compile time,
>
> To get shadow offset this patch uses function __asan_get_shadow_ptr.
> Wouldn't be more effective just to read variable instead of function call?

Depends on how much logic you want to hide there. If it's just "return 
something" than sure
but if you need some synchronization or complex calculations, accessing 
global would not be enough.

-Y
Andrey Ryabinin July 22, 2014, 10:43 a.m. UTC | #4 
On 07/22/14 14:30, Yury Gribov wrote:
>>> It is required for Kernel AddressSanitizer, as the shadow offset is
>>> not known at the compile time,
>>
>> To get shadow offset this patch uses function __asan_get_shadow_ptr.
>> Wouldn't be more effective just to read variable instead of function call?
> 
> Depends on how much logic you want to hide there. If it's just "return something" than sure
> but if you need some synchronization or complex calculations, accessing global would not be enough.
> 

This function just returns some global variable, and I don't think we will need something more complex in future.

> -Y
>
Yury Gribov July 22, 2014, 10:51 a.m. UTC | #5 
> This function just returns some global variable,
>and I don't think we will need something more complex in future.

For kernel probably yes but what about userspace?

-Y
Alexey Preobrazhensky July 25, 2014, 12:26 p.m. UTC | #6 
Our x86_64 implementation it also checks whether frame pointer lies
within direct mapping zone (0xffff880000000000-ffffc80000000000), as
some frames are not in that zone and doesn't have shadow.

On Tue, Jul 22, 2014 at 2:43 PM, Andrey Ryabinin <a.ryabinin@samsung.com> wrote:
> On 07/22/14 14:30, Yury Gribov wrote:
>>>> It is required for Kernel AddressSanitizer, as the shadow offset is
>>>> not known at the compile time,
>>>
>>> To get shadow offset this patch uses function __asan_get_shadow_ptr.
>>> Wouldn't be more effective just to read variable instead of function call?
>>
>> Depends on how much logic you want to hide there. If it's just "return something" than sure
>> but if you need some synchronization or complex calculations, accessing global would not be enough.
>>
>
> This function just returns some global variable, and I don't think we will need something more complex in future.
>
>> -Y
>>
>
diff mbox

Patch

Index: gcc/asan.c
===================================================================
--- gcc/asan.c	(revision 212896)
+++ gcc/asan.c	(working copy)
@@ -979,6 +979,8 @@ 
 			    HOST_WIDE_INT *offsets, tree *decls, int length)
 {
   rtx shadow_base, shadow_mem, ret, mem, orig_base, lab;
+  rtx shadow_start = NULL_RTX;
+  rtx skip_prologue_lab = NULL_RTX, skip_epilogue_lab = NULL_RTX;
   char buf[30];
   unsigned char shadow_bytes[4];
   HOST_WIDE_INT base_offset = offsets[length - 1];
@@ -1112,10 +1114,26 @@ 
   shadow_base = expand_binop (Pmode, lshr_optab, base,
 			      GEN_INT (ASAN_SHADOW_SHIFT),
 			      NULL_RTX, 1, OPTAB_DIRECT);
-  shadow_base
-    = plus_constant (Pmode, shadow_base,
-		     targetm.asan_shadow_offset ()
-		     + (base_align_bias >> ASAN_SHADOW_SHIFT));
+  if (ASAN_FIXED_SHADOW_OFFSET)
+    {
+      shadow_base
+        = plus_constant (Pmode, shadow_base,
+		         targetm.asan_shadow_offset ()
+		         + (base_align_bias >> ASAN_SHADOW_SHIFT));
+    }
+  else
+    {
+      ret = init_one_libfunc ("__asan_get_shadow_ptr");
+      shadow_start = gen_reg_rtx (ptr_mode);
+      emit_library_call_value (ret, shadow_start, LCT_NORMAL, ptr_mode, 0);
+      skip_prologue_lab = gen_label_rtx ();
+      emit_cmp_and_jump_insns (shadow_start, const0_rtx, EQ, NULL_RTX, VOIDmode,
+			       0, skip_prologue_lab, PROB_VERY_UNLIKELY);
+      shadow_base = expand_binop (Pmode, add_optab, shadow_base, shadow_start,
+				  NULL_RTX, 1, OPTAB_DIRECT);
+      shadow_base = plus_constant (Pmode, shadow_base,
+				   base_align_bias >> ASAN_SHADOW_SHIFT);
+    }
   gcc_assert (asan_shadow_set != -1
 	      && (ASAN_RED_ZONE_SIZE >> ASAN_SHADOW_SHIFT) == 4);
   shadow_mem = gen_rtx_MEM (SImode, shadow_base);
@@ -1165,17 +1183,19 @@ 
     }
   do_pending_stack_adjust ();
 
+  if (skip_prologue_lab)
+    emit_label (skip_prologue_lab);
+
   /* Construct epilogue sequence.  */
   start_sequence ();
 
-  lab = NULL_RTX;  
   if (use_after_return_class != -1)
     {
-      rtx lab2 = gen_label_rtx ();
       char c = (char) ASAN_STACK_MAGIC_USE_AFTER_RET;
       int very_likely = REG_BR_PROB_BASE - (REG_BR_PROB_BASE / 2000 - 1);
+      lab = gen_label_rtx ();
       emit_cmp_and_jump_insns (orig_base, base, EQ, NULL_RTX,
-			       VOIDmode, 0, lab2, very_likely);
+			       VOIDmode, 0, lab, very_likely);
       shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
       set_mem_alias_set (shadow_mem, asan_shadow_set);
       mem = gen_rtx_MEM (ptr_mode, base);
@@ -1204,11 +1224,19 @@ 
 			     TYPE_MODE (pointer_sized_int_node),
 			     orig_addr, ptr_mode);
 	}
-      lab = gen_label_rtx ();
-      emit_jump (lab);
-      emit_label (lab2);
+      skip_epilogue_lab = gen_label_rtx ();
+      emit_jump (skip_epilogue_lab);
+      emit_label (lab);
     }
 
+  if (!ASAN_FIXED_SHADOW_OFFSET)
+    {
+      if (!skip_epilogue_lab)
+	skip_epilogue_lab = gen_label_rtx ();
+      emit_cmp_and_jump_insns (shadow_start, const0_rtx, EQ, NULL_RTX, VOIDmode,
+			       0, skip_epilogue_lab, PROB_VERY_UNLIKELY);
+    }
+
   shadow_mem = gen_rtx_MEM (BLKmode, shadow_base);
   set_mem_alias_set (shadow_mem, asan_shadow_set);
 
@@ -1245,9 +1273,10 @@ 
     }
 
   do_pending_stack_adjust ();
-  if (lab)
-    emit_label (lab);
 
+  if (skip_epilogue_lab)
+    emit_label (skip_epilogue_lab);
+
   ret = get_insns ();
   end_sequence ();
   return ret;
Index: gcc/params.def
===================================================================
--- gcc/params.def	(revision 212896)
+++ gcc/params.def	(working copy)
@@ -1081,6 +1081,11 @@ 
          " in function becomes greater or equal than this threshold",
          10000, 0, INT_MAX)
 
+DEFPARAM (PARAM_ASAN_FIXED_SHADOW_OFFSET,
+         "asan-fixed-shadow-offset",
+         "Use fixed offset of shadow memory region",
+         1, 0, 1)
+
 DEFPARAM (PARAM_UNINIT_CONTROL_DEP_ATTEMPTS,
 	  "uninit-control-dep-attempts",
 	  "Maximum number of nested calls to search for control dependencies "
Index: gcc/params.h
===================================================================
--- gcc/params.h	(revision 212896)
+++ gcc/params.h	(working copy)
@@ -234,5 +234,7 @@ 
   PARAM_VALUE (PARAM_ASAN_USE_AFTER_RETURN)
 #define ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD \
   PARAM_VALUE (PARAM_ASAN_INSTRUMENTATION_WITH_CALL_THRESHOLD)
+#define ASAN_FIXED_SHADOW_OFFSET \
+  PARAM_VALUE (PARAM_ASAN_FIXED_SHADOW_OFFSET)
 
 #endif /* ! GCC_PARAMS_H */
Index: gcc/testsuite/c-c++-common/asan/no-fixed-shadow-offset.c
===================================================================
--- gcc/testsuite/c-c++-common/asan/no-fixed-shadow-offset.c	(revision 0)
+++ gcc/testsuite/c-c++-common/asan/no-fixed-shadow-offset.c	(working copy)
@@ -0,0 +1,13 @@ 
+/* { dg-do assemble } */
+/* { dg-options "--param asan-fixed-shadow-offset=0 -save-temps" } */
+
+extern void f(char *);
+
+int main() {
+  char buf[64];
+  f(buf);
+  return 0;
+}
+
+/* { dg-final { scan-assembler "__asan_get_shadow_ptr" } } */
+/* { dg-final { cleanup-saved-temps } } */
Index: libsanitizer/asan/asan_rtl.cc
===================================================================
--- libsanitizer/asan/asan_rtl.cc	(revision 212896)
+++ libsanitizer/asan/asan_rtl.cc	(working copy)
@@ -430,6 +430,11 @@ 
   }
 }
 
+extern "C"
+NOINLINE INTERFACE_ATTRIBUTE uptr __asan_get_shadow_ptr() {
+  return (uptr)SHADOW_OFFSET;
+}
+
 // Force the linker to keep the symbols for various ASan interface functions.
 // We want to keep those in the executable in order to let the instrumented
 // dynamic libraries access the symbol even if it is not used by the executable