diff mbox

asan: support for globals in kernel

Message ID CACT4Y+Z=RCYyAu3tBH2DMW9pgY+XDfcgsYRMa9mhw7eS7=DzLw@mail.gmail.com
State New
Headers show

Commit Message

Dmitry Vyukov Dec. 2, 2014, 5:56 p.m. UTC
Hi,

The following patch adds support for instrumentation of globals for
Linux kernel (-fsanitize=kernel-address). Kernel only supports
constructors with default priority, but the rest works fine.

OK for trunk?


https://codereview.appspot.com/176570043

Comments

Jakub Jelinek Dec. 2, 2014, 6:04 p.m. UTC | #1
On Tue, Dec 02, 2014 at 09:56:36PM +0400, Dmitry Vyukov wrote:
> --- gcc/ChangeLog (revision 218280)
> +++ gcc/ChangeLog (working copy)
> @@ -1,3 +1,8 @@
> +2014-12-02  Dmitry Vyukov  <dvyukov@google.com>
> +
> + * asan.c: (asan_finish_file): Use default priority for constructors
> + in kernel mode.

Seems your MUA is eating tabs, I'll assume they are where I'd expect them to
be.

> @@ -2440,6 +2442,7 @@
>  {
>    varpool_node *vnode;
>    unsigned HOST_WIDE_INT gcount = 0;
> +  int priority;
> 
>    if (shadow_ptr_types[0] == NULL_TREE)
>      asan_init_shadow_ptr_types ();

No need to declare this separately, 

> @@ -2448,6 +2451,13 @@
>       nor after .LASAN* array.  */
>    flag_sanitize &= ~SANITIZE_ADDRESS;
> 
> +  /* For user-space we want asan constructors to run first.
> +     Linux kernel does not support priorities other than default, and the only
> +     other user of constructors is coverage. So we run with the default
> +     priority.  */
> +  priority = flag_sanitize & SANITIZE_USER_ADDRESS ?
> +      MAX_RESERVED_INIT_PRIORITY - 1 : DEFAULT_INIT_PRIORITY;

formatting plus declare it here too.  This should be
  int priority = flag_sanitize & SANITIZE_USER_ADDRESS
		 ? MAX_RESERVED_INIT_PRIORITY - 1 : DEFAULT_INIT_PRIORITY;
(? should be on the second line and aligned below flag_sanitize).

Ok for trunk with that change.

Shall we backport it to 4.9 branch too?

	Jakub
Andrey Ryabinin Dec. 3, 2014, 6:19 a.m. UTC | #2
On 12/02/2014 08:56 PM, Dmitry Vyukov wrote:
> Hi,
> 
> The following patch adds support for instrumentation of globals for
> Linux kernel (-fsanitize=kernel-address). Kernel only supports
> constructors with default priority, but the rest works fine.
> 
> OK for trunk?
> 

I know this is too late already, but why we need this?
IMO it's much better to add support for constructors with priorities in kernel.

We need to do this in kernel anyway, because GCC 4.9.2 don't have this patch and
I assume that we want to make instrumentation of globals work in kernel with 4.9.2


> 
> https://codereview.appspot.com/176570043
> 
> Index: gcc/ChangeLog
> ===================================================================
> --- gcc/ChangeLog (revision 218280)
> +++ gcc/ChangeLog (working copy)
> @@ -1,3 +1,8 @@
> +2014-12-02  Dmitry Vyukov  <dvyukov@google.com>
> +
> + * asan.c: (asan_finish_file): Use default priority for constructors
> + in kernel mode.
> +
>  2014-12-02  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>
> 
>   PR target/64115
> Index: gcc/asan.c
> ===================================================================
> --- gcc/asan.c (revision 218280)
> +++ gcc/asan.c (working copy)
> @@ -1348,7 +1348,9 @@
>   the var that is selected by the linker will have
>   padding or not.  */
>        || DECL_ONE_ONLY (decl)
> -      /* Similarly for common vars.  People can use -fno-common.  */
> +      /* Similarly for common vars.  People can use -fno-common.
> + Note: Linux kernel is built with -fno-common, so we do instrument
> + globals there even if it is C.  */
>        || (DECL_COMMON (decl) && TREE_PUBLIC (decl))
>        /* Don't protect if using user section, often vars placed
>   into user section from multiple TUs are then assumed
> @@ -2440,6 +2442,7 @@
>  {
>    varpool_node *vnode;
>    unsigned HOST_WIDE_INT gcount = 0;
> +  int priority;
> 
>    if (shadow_ptr_types[0] == NULL_TREE)
>      asan_init_shadow_ptr_types ();
> @@ -2448,6 +2451,13 @@
>       nor after .LASAN* array.  */
>    flag_sanitize &= ~SANITIZE_ADDRESS;
> 
> +  /* For user-space we want asan constructors to run first.
> +     Linux kernel does not support priorities other than default, and the only
> +     other user of constructors is coverage. So we run with the default
> +     priority.  */
> +  priority = flag_sanitize & SANITIZE_USER_ADDRESS ?
> +      MAX_RESERVED_INIT_PRIORITY - 1 : DEFAULT_INIT_PRIORITY;
> +
>    if (flag_sanitize & SANITIZE_USER_ADDRESS)
>      {
>        tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT);
> @@ -2503,12 +2513,10 @@
>   build_fold_addr_expr (var),
>   gcount_tree),
>   &dtor_statements);
> -      cgraph_build_static_cdtor ('D', dtor_statements,
> - MAX_RESERVED_INIT_PRIORITY - 1);
> +      cgraph_build_static_cdtor ('D', dtor_statements, priority);
>      }
>    if (asan_ctor_statements)
> -    cgraph_build_static_cdtor ('I', asan_ctor_statements,
> -       MAX_RESERVED_INIT_PRIORITY - 1);
> +    cgraph_build_static_cdtor ('I', asan_ctor_statements, priority);
>    flag_sanitize |= SANITIZE_ADDRESS;
>  }
>
diff mbox

Patch

Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog (revision 218280)
+++ gcc/ChangeLog (working copy)
@@ -1,3 +1,8 @@ 
+2014-12-02  Dmitry Vyukov  <dvyukov@google.com>
+
+ * asan.c: (asan_finish_file): Use default priority for constructors
+ in kernel mode.
+
 2014-12-02  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>

  PR target/64115
Index: gcc/asan.c
===================================================================
--- gcc/asan.c (revision 218280)
+++ gcc/asan.c (working copy)
@@ -1348,7 +1348,9 @@ 
  the var that is selected by the linker will have
  padding or not.  */
       || DECL_ONE_ONLY (decl)
-      /* Similarly for common vars.  People can use -fno-common.  */
+      /* Similarly for common vars.  People can use -fno-common.
+ Note: Linux kernel is built with -fno-common, so we do instrument
+ globals there even if it is C.  */
       || (DECL_COMMON (decl) && TREE_PUBLIC (decl))
       /* Don't protect if using user section, often vars placed
  into user section from multiple TUs are then assumed
@@ -2440,6 +2442,7 @@ 
 {
   varpool_node *vnode;
   unsigned HOST_WIDE_INT gcount = 0;
+  int priority;

   if (shadow_ptr_types[0] == NULL_TREE)
     asan_init_shadow_ptr_types ();
@@ -2448,6 +2451,13 @@ 
      nor after .LASAN* array.  */
   flag_sanitize &= ~SANITIZE_ADDRESS;

+  /* For user-space we want asan constructors to run first.
+     Linux kernel does not support priorities other than default, and the only
+     other user of constructors is coverage. So we run with the default
+     priority.  */
+  priority = flag_sanitize & SANITIZE_USER_ADDRESS ?
+      MAX_RESERVED_INIT_PRIORITY - 1 : DEFAULT_INIT_PRIORITY;
+
   if (flag_sanitize & SANITIZE_USER_ADDRESS)
     {
       tree fn = builtin_decl_implicit (BUILT_IN_ASAN_INIT);
@@ -2503,12 +2513,10 @@ 
  build_fold_addr_expr (var),
  gcount_tree),
  &dtor_statements);
-      cgraph_build_static_cdtor ('D', dtor_statements,
- MAX_RESERVED_INIT_PRIORITY - 1);
+      cgraph_build_static_cdtor ('D', dtor_statements, priority);
     }
   if (asan_ctor_statements)
-    cgraph_build_static_cdtor ('I', asan_ctor_statements,
-       MAX_RESERVED_INIT_PRIORITY - 1);
+    cgraph_build_static_cdtor ('I', asan_ctor_statements, priority);
   flag_sanitize |= SANITIZE_ADDRESS;
 }