Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Masami Hiramatsu
On Thu, 25 May 2017 11:18:02 -0400
Steven Rostedt  wrote:

> On Thu, 25 May 2017 19:34:43 +0900
> Masami Hiramatsu  wrote:
> 
> 
> > OK, I've ensured that following command hits same BUG.
> > 
> > grep t /proc/kallsyms  | cut -f3 -d" " | grep -v .*\\..* | \
> > head -n 256 | while read i; do echo p $i+5 ; done >
> > tracing/kprobe_events
> > 
> > echo 1 > tracing/events/kprobes/enable 
> > echo 0 > tracing/events/kprobes/enable
> > echo > tracing/kprobe_events
> > sleep 5
> 
> Hi Masami,
> 
> Can you add a selftest that checks this too?

Yes, however, it should be included after my fix is
accepted, or it can cause a kernel panic...

Thanks,



-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Masami Hiramatsu
On Thu, 25 May 2017 11:18:02 -0400
Steven Rostedt  wrote:

> On Thu, 25 May 2017 19:34:43 +0900
> Masami Hiramatsu  wrote:
> 
> 
> > OK, I've ensured that following command hits same BUG.
> > 
> > grep t /proc/kallsyms  | cut -f3 -d" " | grep -v .*\\..* | \
> > head -n 256 | while read i; do echo p $i+5 ; done >
> > tracing/kprobe_events
> > 
> > echo 1 > tracing/events/kprobes/enable 
> > echo 0 > tracing/events/kprobes/enable
> > echo > tracing/kprobe_events
> > sleep 5
> 
> Hi Masami,
> 
> Can you add a selftest that checks this too?

Yes, however, it should be included after my fix is
accepted, or it can cause a kernel panic...

Thanks,



-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Steven Rostedt
On Thu, 25 May 2017 19:34:43 +0900
Masami Hiramatsu  wrote:


> OK, I've ensured that following command hits same BUG.
> 
> grep t /proc/kallsyms  | cut -f3 -d" " | grep -v .*\\..* | \
> head -n 256 | while read i; do echo p $i+5 ; done >
> tracing/kprobe_events
> 
> echo 1 > tracing/events/kprobes/enable 
> echo 0 > tracing/events/kprobes/enable
> echo > tracing/kprobe_events
> sleep 5

Hi Masami,

Can you add a selftest that checks this too?

Thanks!

-- Steve

> 
> I'll send a bugfix asap.
> 
> Thank,
> 



Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Steven Rostedt
On Thu, 25 May 2017 19:34:43 +0900
Masami Hiramatsu  wrote:


> OK, I've ensured that following command hits same BUG.
> 
> grep t /proc/kallsyms  | cut -f3 -d" " | grep -v .*\\..* | \
> head -n 256 | while read i; do echo p $i+5 ; done >
> tracing/kprobe_events
> 
> echo 1 > tracing/events/kprobes/enable 
> echo 0 > tracing/events/kprobes/enable
> echo > tracing/kprobe_events
> sleep 5

Hi Masami,

Can you add a selftest that checks this too?

Thanks!

-- Steve

> 
> I'll send a bugfix asap.
> 
> Thank,
> 



Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Masami Hiramatsu
On Thu, 25 May 2017 18:09:10 +0900
Masami Hiramatsu  wrote:

> On Wed, 24 May 2017 18:25:47 -0400
> Steven Rostedt  wrote:
> 
> > On Wed, 24 May 2017 21:13:27 +0200 (CEST)
> > Thomas Gleixner  wrote:
> > 
> > 
> > > > Oops: 0003 [#1] SMP
> > > > Modules linked in:
> > > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > > > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > > > RIP: 0010:new_slab+0x1e8/0x2b4
> > > > RSP: :c9c77b28 EFLAGS: 00010282
> > > > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > > > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > > > RBP: c9c77b70 R08: 002a R09: 
> > > > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > > > R13: 002e R14: 0001 R15: ea000853d700
> > > > FS:  () GS:88021eb8()
> > > > knlGS: CS:  0010 DS:  ES:  CR0:
> > > > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > > > 001406e0 Call Trace:
> > > >  ? interleave_nodes+0x29/0x40
> > > >  ___slab_alloc+0x2e8/0x49e  
> > > 
> > > That does not make any sense, but I'm digging into it.
> > 
> > The trampolines uses the module allocation, and it appears, that needs
> > to become rw before freeing again.
> 
> Oops, I also have to fix kprobes side, because it has same issue.

OK, I've ensured that following command hits same BUG.

grep t /proc/kallsyms  | cut -f3 -d" " | grep -v .*\\..* | \
head -n 256 | while read i; do echo p $i+5 ; done > tracing/kprobe_events

echo 1 > tracing/events/kprobes/enable 
echo 0 > tracing/events/kprobes/enable
echo > tracing/kprobe_events
sleep 5

I'll send a bugfix asap.

Thank,

-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Masami Hiramatsu
On Thu, 25 May 2017 18:09:10 +0900
Masami Hiramatsu  wrote:

> On Wed, 24 May 2017 18:25:47 -0400
> Steven Rostedt  wrote:
> 
> > On Wed, 24 May 2017 21:13:27 +0200 (CEST)
> > Thomas Gleixner  wrote:
> > 
> > 
> > > > Oops: 0003 [#1] SMP
> > > > Modules linked in:
> > > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > > > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > > > RIP: 0010:new_slab+0x1e8/0x2b4
> > > > RSP: :c9c77b28 EFLAGS: 00010282
> > > > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > > > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > > > RBP: c9c77b70 R08: 002a R09: 
> > > > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > > > R13: 002e R14: 0001 R15: ea000853d700
> > > > FS:  () GS:88021eb8()
> > > > knlGS: CS:  0010 DS:  ES:  CR0:
> > > > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > > > 001406e0 Call Trace:
> > > >  ? interleave_nodes+0x29/0x40
> > > >  ___slab_alloc+0x2e8/0x49e  
> > > 
> > > That does not make any sense, but I'm digging into it.
> > 
> > The trampolines uses the module allocation, and it appears, that needs
> > to become rw before freeing again.
> 
> Oops, I also have to fix kprobes side, because it has same issue.

OK, I've ensured that following command hits same BUG.

grep t /proc/kallsyms  | cut -f3 -d" " | grep -v .*\\..* | \
head -n 256 | while read i; do echo p $i+5 ; done > tracing/kprobe_events

echo 1 > tracing/events/kprobes/enable 
echo 0 > tracing/events/kprobes/enable
echo > tracing/kprobe_events
sleep 5

I'll send a bugfix asap.

Thank,

-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Masami Hiramatsu
On Wed, 24 May 2017 18:25:47 -0400
Steven Rostedt  wrote:

> On Wed, 24 May 2017 21:13:27 +0200 (CEST)
> Thomas Gleixner  wrote:
> 
> 
> > > Oops: 0003 [#1] SMP
> > > Modules linked in:
> > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > > RIP: 0010:new_slab+0x1e8/0x2b4
> > > RSP: :c9c77b28 EFLAGS: 00010282
> > > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > > RBP: c9c77b70 R08: 002a R09: 
> > > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > > R13: 002e R14: 0001 R15: ea000853d700
> > > FS:  () GS:88021eb8()
> > > knlGS: CS:  0010 DS:  ES:  CR0:
> > > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > > 001406e0 Call Trace:
> > >  ? interleave_nodes+0x29/0x40
> > >  ___slab_alloc+0x2e8/0x49e  
> > 
> > That does not make any sense, but I'm digging into it.
> 
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.

Oops, I also have to fix kprobes side, because it has same issue.

Thanks!

> 
> I applied this patch, and it appears to fix the bug for me.
> 
> Signed-off-by: Steven Rostedt (VMware) 
> 
> -- Steve
> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 663a35d..5e93a9a 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size)
>  {
>   return module_alloc(size);
>  }
> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
>  {
> + int npages;
> +
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> + set_memory_rw((unsigned long)tramp, npages);
>   module_memfree(tramp);
>  }
>  #else
> @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size)
>  {
>   return NULL;
>  }
> -static inline void tramp_free(void *tramp) { }
> +static inline void tramp_free(void *tramp, int size) { }
>  #endif
>  
>  /* Defined as markers to the end of the ftrace default trampolines */
> @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
> *tramp_size)
>   /* Copy ftrace_caller onto the trampoline memory */
>   ret = probe_kernel_read(trampoline, (void *)start_offset, size);
>   if (WARN_ON(ret < 0)) {
> - tramp_free(trampoline);
> + tramp_free(trampoline, *tramp_size);
>   return 0;
>   }
>  
> @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
> *tramp_size)
>  
>   /* Are we pointing to the reference? */
>   if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) {
> - tramp_free(trampoline);
> + tramp_free(trampoline, *tramp_size);
>   return 0;
>   }
>  
> @@ -943,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
>   if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
>   return;
>  
> - tramp_free((void *)ops->trampoline);
> + tramp_free((void *)ops->trampoline, ops->trampoline_size);
>   ops->trampoline = 0;
>  }
>  


-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Masami Hiramatsu
On Wed, 24 May 2017 18:25:47 -0400
Steven Rostedt  wrote:

> On Wed, 24 May 2017 21:13:27 +0200 (CEST)
> Thomas Gleixner  wrote:
> 
> 
> > > Oops: 0003 [#1] SMP
> > > Modules linked in:
> > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > > RIP: 0010:new_slab+0x1e8/0x2b4
> > > RSP: :c9c77b28 EFLAGS: 00010282
> > > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > > RBP: c9c77b70 R08: 002a R09: 
> > > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > > R13: 002e R14: 0001 R15: ea000853d700
> > > FS:  () GS:88021eb8()
> > > knlGS: CS:  0010 DS:  ES:  CR0:
> > > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > > 001406e0 Call Trace:
> > >  ? interleave_nodes+0x29/0x40
> > >  ___slab_alloc+0x2e8/0x49e  
> > 
> > That does not make any sense, but I'm digging into it.
> 
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.

Oops, I also have to fix kprobes side, because it has same issue.

Thanks!

> 
> I applied this patch, and it appears to fix the bug for me.
> 
> Signed-off-by: Steven Rostedt (VMware) 
> 
> -- Steve
> 
> diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
> index 663a35d..5e93a9a 100644
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size)
>  {
>   return module_alloc(size);
>  }
> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
>  {
> + int npages;
> +
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> + set_memory_rw((unsigned long)tramp, npages);
>   module_memfree(tramp);
>  }
>  #else
> @@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size)
>  {
>   return NULL;
>  }
> -static inline void tramp_free(void *tramp) { }
> +static inline void tramp_free(void *tramp, int size) { }
>  #endif
>  
>  /* Defined as markers to the end of the ftrace default trampolines */
> @@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
> *tramp_size)
>   /* Copy ftrace_caller onto the trampoline memory */
>   ret = probe_kernel_read(trampoline, (void *)start_offset, size);
>   if (WARN_ON(ret < 0)) {
> - tramp_free(trampoline);
> + tramp_free(trampoline, *tramp_size);
>   return 0;
>   }
>  
> @@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
> *tramp_size)
>  
>   /* Are we pointing to the reference? */
>   if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) {
> - tramp_free(trampoline);
> + tramp_free(trampoline, *tramp_size);
>   return 0;
>   }
>  
> @@ -943,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
>   if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
>   return;
>  
> - tramp_free((void *)ops->trampoline);
> + tramp_free((void *)ops->trampoline, ops->trampoline_size);
>   ops->trampoline = 0;
>  }
>  


-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Thomas Gleixner
On Wed, 24 May 2017, Steven Rostedt wrote:
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.

Indeed. I realized that when enabling more debug options, which led to a
reliable triple fault.

How intuitive.

> I applied this patch, and it appears to fix the bug for me.

It fixes the bug, but ...

> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
>  {
> + int npages;
> +
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;

For correctness sake this wants

set_memory_nx(...);

as well.

> + set_memory_rw((unsigned long)tramp, npages);
>   module_memfree(tramp);
>  }

I'll clean that up and post a V2.

Thanks,

tglx


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-25 Thread Thomas Gleixner
On Wed, 24 May 2017, Steven Rostedt wrote:
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.

Indeed. I realized that when enabling more debug options, which led to a
reliable triple fault.

How intuitive.

> I applied this patch, and it appears to fix the bug for me.

It fixes the bug, but ...

> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
>  {
> + int npages;
> +
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;

For correctness sake this wants

set_memory_nx(...);

as well.

> + set_memory_rw((unsigned long)tramp, npages);
>   module_memfree(tramp);
>  }

I'll clean that up and post a V2.

Thanks,

tglx


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Luis R. Rodriguez
On Wed, May 24, 2017 at 06:25:47PM -0400, Steven Rostedt wrote:
> On Wed, 24 May 2017 21:13:27 +0200 (CEST)
> Thomas Gleixner  wrote:
> 
> 
> > > Oops: 0003 [#1] SMP
> > > Modules linked in:
> > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > > RIP: 0010:new_slab+0x1e8/0x2b4
> > > RSP: :c9c77b28 EFLAGS: 00010282
> > > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > > RBP: c9c77b70 R08: 002a R09: 
> > > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > > R13: 002e R14: 0001 R15: ea000853d700
> > > FS:  () GS:88021eb8()
> > > knlGS: CS:  0010 DS:  ES:  CR0:
> > > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > > 001406e0 Call Trace:
> > >  ? interleave_nodes+0x29/0x40
> > >  ___slab_alloc+0x2e8/0x49e  
> > 
> > That does not make any sense, but I'm digging into it.
> 
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.
> 
> I applied this patch, and it appears to fix the bug for me.
> 
> Signed-off-by: Steven Rostedt (VMware) 

Awesome, this also cures my panic:

mcgrof@piggy ~/linux-next (git::20170524-fixwarn)$ sudo 
tools/testing/selftests/ftrace/ftracetest 
...
# of passed:  45
# of failed:  0
# of unresolved:  0
# of untested:  0
# of unsupported:  0
# of xfailed:  0
# of undefined(test bug):  0

So the combination of both:

Tested-by: Luis R. Rodriguez 

  Luis


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Luis R. Rodriguez
On Wed, May 24, 2017 at 06:25:47PM -0400, Steven Rostedt wrote:
> On Wed, 24 May 2017 21:13:27 +0200 (CEST)
> Thomas Gleixner  wrote:
> 
> 
> > > Oops: 0003 [#1] SMP
> > > Modules linked in:
> > > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > > RIP: 0010:new_slab+0x1e8/0x2b4
> > > RSP: :c9c77b28 EFLAGS: 00010282
> > > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > > RBP: c9c77b70 R08: 002a R09: 
> > > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > > R13: 002e R14: 0001 R15: ea000853d700
> > > FS:  () GS:88021eb8()
> > > knlGS: CS:  0010 DS:  ES:  CR0:
> > > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > > 001406e0 Call Trace:
> > >  ? interleave_nodes+0x29/0x40
> > >  ___slab_alloc+0x2e8/0x49e  
> > 
> > That does not make any sense, but I'm digging into it.
> 
> The trampolines uses the module allocation, and it appears, that needs
> to become rw before freeing again.
> 
> I applied this patch, and it appears to fix the bug for me.
> 
> Signed-off-by: Steven Rostedt (VMware) 

Awesome, this also cures my panic:

mcgrof@piggy ~/linux-next (git::20170524-fixwarn)$ sudo 
tools/testing/selftests/ftrace/ftracetest 
...
# of passed:  45
# of failed:  0
# of unresolved:  0
# of untested:  0
# of unsupported:  0
# of xfailed:  0
# of undefined(test bug):  0

So the combination of both:

Tested-by: Luis R. Rodriguez 

  Luis


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Steven Rostedt
On Wed, 24 May 2017 21:13:27 +0200 (CEST)
Thomas Gleixner  wrote:


> > Oops: 0003 [#1] SMP
> > Modules linked in:
> > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > RIP: 0010:new_slab+0x1e8/0x2b4
> > RSP: :c9c77b28 EFLAGS: 00010282
> > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > RBP: c9c77b70 R08: 002a R09: 
> > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > R13: 002e R14: 0001 R15: ea000853d700
> > FS:  () GS:88021eb8()
> > knlGS: CS:  0010 DS:  ES:  CR0:
> > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > 001406e0 Call Trace:
> >  ? interleave_nodes+0x29/0x40
> >  ___slab_alloc+0x2e8/0x49e  
> 
> That does not make any sense, but I'm digging into it.

The trampolines uses the module allocation, and it appears, that needs
to become rw before freeing again.

I applied this patch, and it appears to fix the bug for me.

Signed-off-by: Steven Rostedt (VMware) 

-- Steve

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 663a35d..5e93a9a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size)
 {
return module_alloc(size);
 }
-static inline void tramp_free(void *tramp)
+static inline void tramp_free(void *tramp, int size)
 {
+   int npages;
+
+   npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+   set_memory_rw((unsigned long)tramp, npages);
module_memfree(tramp);
 }
 #else
@@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size)
 {
return NULL;
 }
-static inline void tramp_free(void *tramp) { }
+static inline void tramp_free(void *tramp, int size) { }
 #endif
 
 /* Defined as markers to the end of the ftrace default trampolines */
@@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
*tramp_size)
/* Copy ftrace_caller onto the trampoline memory */
ret = probe_kernel_read(trampoline, (void *)start_offset, size);
if (WARN_ON(ret < 0)) {
-   tramp_free(trampoline);
+   tramp_free(trampoline, *tramp_size);
return 0;
}
 
@@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
*tramp_size)
 
/* Are we pointing to the reference? */
if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) {
-   tramp_free(trampoline);
+   tramp_free(trampoline, *tramp_size);
return 0;
}
 
@@ -943,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
return;
 
-   tramp_free((void *)ops->trampoline);
+   tramp_free((void *)ops->trampoline, ops->trampoline_size);
ops->trampoline = 0;
 }
 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Steven Rostedt
On Wed, 24 May 2017 21:13:27 +0200 (CEST)
Thomas Gleixner  wrote:


> > Oops: 0003 [#1] SMP
> > Modules linked in:
> > CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> > Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6
> > 02/22/2014 task: 8802153a8000 task.stack: c9c74000
> > RIP: 0010:new_slab+0x1e8/0x2b4
> > RSP: :c9c77b28 EFLAGS: 00010282
> > RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> > RDX:  RSI: 880214f5c000 RDI: 880216003f00
> > RBP: c9c77b70 R08: 002a R09: 
> > R10: 000201e2 R11: 00020190 R12: 880214f5c000
> > R13: 002e R14: 0001 R15: ea000853d700
> > FS:  () GS:88021eb8()
> > knlGS: CS:  0010 DS:  ES:  CR0:
> > 80050033 CR2: 880214f5c000 CR3: 0221d000 CR4:
> > 001406e0 Call Trace:
> >  ? interleave_nodes+0x29/0x40
> >  ___slab_alloc+0x2e8/0x49e  
> 
> That does not make any sense, but I'm digging into it.

The trampolines uses the module allocation, and it appears, that needs
to become rw before freeing again.

I applied this patch, and it appears to fix the bug for me.

Signed-off-by: Steven Rostedt (VMware) 

-- Steve

diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c
index 663a35d..5e93a9a 100644
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned long size)
 {
return module_alloc(size);
 }
-static inline void tramp_free(void *tramp)
+static inline void tramp_free(void *tramp, int size)
 {
+   int npages;
+
+   npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+   set_memory_rw((unsigned long)tramp, npages);
module_memfree(tramp);
 }
 #else
@@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned long size)
 {
return NULL;
 }
-static inline void tramp_free(void *tramp) { }
+static inline void tramp_free(void *tramp, int size) { }
 #endif
 
 /* Defined as markers to the end of the ftrace default trampolines */
@@ -771,7 +775,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
*tramp_size)
/* Copy ftrace_caller onto the trampoline memory */
ret = probe_kernel_read(trampoline, (void *)start_offset, size);
if (WARN_ON(ret < 0)) {
-   tramp_free(trampoline);
+   tramp_free(trampoline, *tramp_size);
return 0;
}
 
@@ -797,7 +801,7 @@ create_trampoline(struct ftrace_ops *ops, unsigned int 
*tramp_size)
 
/* Are we pointing to the reference? */
if (WARN_ON(memcmp(op_ptr.op, op_ref, 3) != 0)) {
-   tramp_free(trampoline);
+   tramp_free(trampoline, *tramp_size);
return 0;
}
 
@@ -943,7 +947,7 @@ void arch_ftrace_trampoline_free(struct ftrace_ops *ops)
if (!ops || !(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
return;
 
-   tramp_free((void *)ops->trampoline);
+   tramp_free((void *)ops->trampoline, ops->trampoline_size);
ops->trampoline = 0;
 }
 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Luis R. Rodriguez
On Wed, May 24, 2017 at 08:53:00PM +0200, Thomas Gleixner wrote:
> On Wed, 24 May 2017, Luis R. Rodriguez wrote:
> > [  440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: GE
> 
> 4.12.0-rc1-next-20170519+ #155
> 
> Can you please try that patch against plain 4.12-rc2?

Yup, just tried it. Same trace. Could we need to do something
special at __unregister_ftrace_function() and friends ?

  Luis


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Luis R. Rodriguez
On Wed, May 24, 2017 at 08:53:00PM +0200, Thomas Gleixner wrote:
> On Wed, 24 May 2017, Luis R. Rodriguez wrote:
> > [  440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: GE
> 
> 4.12.0-rc1-next-20170519+ #155
> 
> Can you please try that patch against plain 4.12-rc2?

Yup, just tried it. Same trace. Could we need to do something
special at __unregister_ftrace_function() and friends ?

  Luis


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Thomas Gleixner
On Wed, 24 May 2017, Steven Rostedt wrote:

> OK, it crashed on one of my tests. I removed the patch and it boots
> fine, and crashes when I add it back.

It reproduces here with your config.

> BUG: unable to handle kernel paging request at 880214f5c000
> IP: new_slab+0x1e8/0x2b4
> PGD 338c067 
> P4D 338c067 
> PUD 338f067 
> PMD 212022063 
> PTE 800214f5c161
> 
> Oops: 0003 [#1] SMP
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
> task: 8802153a8000 task.stack: c9c74000
> RIP: 0010:new_slab+0x1e8/0x2b4
> RSP: :c9c77b28 EFLAGS: 00010282
> RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> RDX:  RSI: 880214f5c000 RDI: 880216003f00
> RBP: c9c77b70 R08: 002a R09: 
> R10: 000201e2 R11: 00020190 R12: 880214f5c000
> R13: 002e R14: 0001 R15: ea000853d700
> FS:  () GS:88021eb8() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 880214f5c000 CR3: 0221d000 CR4: 001406e0
> Call Trace:
>  ? interleave_nodes+0x29/0x40
>  ___slab_alloc+0x2e8/0x49e

That does not make any sense, but I'm digging into it.

Thanks,

tglx




Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Thomas Gleixner
On Wed, 24 May 2017, Steven Rostedt wrote:

> OK, it crashed on one of my tests. I removed the patch and it boots
> fine, and crashes when I add it back.

It reproduces here with your config.

> BUG: unable to handle kernel paging request at 880214f5c000
> IP: new_slab+0x1e8/0x2b4
> PGD 338c067 
> P4D 338c067 
> PUD 338f067 
> PMD 212022063 
> PTE 800214f5c161
> 
> Oops: 0003 [#1] SMP
> Modules linked in:
> CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
> Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
> task: 8802153a8000 task.stack: c9c74000
> RIP: 0010:new_slab+0x1e8/0x2b4
> RSP: :c9c77b28 EFLAGS: 00010282
> RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
> RDX:  RSI: 880214f5c000 RDI: 880216003f00
> RBP: c9c77b70 R08: 002a R09: 
> R10: 000201e2 R11: 00020190 R12: 880214f5c000
> R13: 002e R14: 0001 R15: ea000853d700
> FS:  () GS:88021eb8() knlGS:
> CS:  0010 DS:  ES:  CR0: 80050033
> CR2: 880214f5c000 CR3: 0221d000 CR4: 001406e0
> Call Trace:
>  ? interleave_nodes+0x29/0x40
>  ___slab_alloc+0x2e8/0x49e

That does not make any sense, but I'm digging into it.

Thanks,

tglx




Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Thomas Gleixner
On Wed, 24 May 2017, Luis R. Rodriguez wrote:
> [  440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: GE

4.12.0-rc1-next-20170519+ #155

Can you please try that patch against plain 4.12-rc2?

Thanks,

tglx


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Thomas Gleixner
On Wed, 24 May 2017, Luis R. Rodriguez wrote:
> [  440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: GE

4.12.0-rc1-next-20170519+ #155

Can you please try that patch against plain 4.12-rc2?

Thanks,

tglx


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Luis R. Rodriguez
On Wed, May 24, 2017 at 01:47:28PM -0400, Steven Rostedt wrote:
> OK, it crashed on one of my tests. I removed the patch and it boots
> fine, and crashes when I add it back.
> 
> Here's the crash:
> 
> Running tests on all trace events:
> Testing all events: OK
> Testing ftrace filter: OK
> trace_kprobe: Testing kprobe tracing: 
> BUG: unable to handle kernel paging request at 880214f5c000
> IP: new_slab+0x1e8/0x2b4
> PGD 338c067 
> P4D 338c067 
> PUD 338f067 
> PMD 212022063 
> PTE 800214f5c161

The good news is that this also fixes the other issue I reported with
CONFIG_KPROBES_SANITY_TEST=y and the same WARN complaint without a crash [0].
Please note that the issue I reported actually is present since the
infrastructure to report on these warnings was added through commit
e1a58320a38dfa72be48a0f1a3a92273663ba6db ("x86/mm: Warn on W^X mappings")
which was added through kernel 4.3.0-rc3.

Thomas's patch says: "Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated
trampoline for ftrace_ops"), and this was added through kernel v3.19-rc1 so it
may well be the same issue given the traces on my issue also reflect a
module_alloc() trace after arch_ftrace_update_trampoline() which matches what
Thomas also saw when inspecting his buggy page with kmemleak echo dump [1].

[0] https://lkml.kernel.org/r/20170524175508.gz8...@wotan.suse.de
[1] https://lkml.kernel.org/r/alpine.DEB.2.20.1705232243520.2409@nanos

BUT I'm also able to reprorduce a crash too though easily, as follows where
as before this commit it was passing all 45 tests just fine:

mcgrof@piggy ~/linux-next (git::next-20170519)$ sudo 
tools/testing/selftests/ftrace/ftracetest
=== Ftrace unit tests ===   
[1] Basic trace file check  [PASS]  
[2] Basic test for tracers  [PASS]  
[3] Basic trace clock test  [PASS]  
[4] Basic event tracing check   [PASS]  
[5] event tracing - enable/disable with event level files   [PASS]  
[6] event tracing - restricts events based on pid   [PASS]  
[7] event tracing - enable/disable with subsystem level files   [PASS]  
[8] event tracing - enable/disable with top level files [PASS]  
[9] ftrace - function graph filters with stack tracer   [PASS]  
[10] ftrace - function graph filters[PASS]  
[11] ftrace - test for function event triggers 



I managed to get the kernel log:

[  429.416279] in mmio_trace_init
[  429.504315] mmiotrace: Disabling non-boot CPUs...
[  429.545572] Unregister pv shared memory for cpu 1
[  429.551725] smpboot: CPU 1 is now offline
[  429.553044] mmiotrace: CPU1 is down.
[  429.572874] Unregister pv shared memory for cpu 2
[  429.576361] smpboot: CPU 2 is now offline
[  429.578355] mmiotrace: CPU2 is down.
[  429.596755] Unregister pv shared memory for cpu 3
[  429.601946] smpboot: CPU 3 is now offline
[  429.602768] mmiotrace: CPU3 is down.
[  429.603418] mmiotrace: enabled.
[  429.605001] in mmio_trace_reset
[  429.605003] mmiotrace: Re-enabling CPUs...
[  429.605933] x86: Booting SMP configuration:
[  429.606505] smpboot: Booting Node 0 Processor 1 APIC 0x1
[  429.617989] kvm-clock: cpu 1, msr 1:3fff0041, secondary cpu clock
[  429.640600] KVM setup async PF for cpu 1
[  429.640603] kvm-stealtime: cpu 1, msr 13fc8da40
[  429.642384] mmiotrace: enabled CPU1.
[  429.643391] smpboot: Booting Node 0 Processor 2 APIC 0x2
[  429.654788] kvm-clock: cpu 2, msr 1:3fff0081, secondary cpu clock
[  429.675512] KVM setup async PF for cpu 2
[  429.675921] kvm-stealtime: cpu 2, msr 13fd0da40
[  429.676895] mmiotrace: enabled CPU2.
[  429.678531] smpboot: Booting Node 0 Processor 3 APIC 0x3
[  429.68] kvm-clock: cpu 3, msr 1:3fff00c1, secondary cpu clock
[  429.710808] KVM setup async PF for cpu 3
[  429.711183] kvm-stealtime: cpu 3, msr 13fd8da40
[  429.712123] mmiotrace: enabled CPU3.
[  429.728330] mmiotrace: disabled.
[  440.537183] BUG: unable to handle kernel paging request at 964cf9376000
[  440.538899] IP: tlb_next_batch.isra.54+0x41/0x70
[  440.540022] PGD b736f067 
[  440.540024] P4D b736f067 
[  440.540707] PUD b7373067 
[  440.541318] PMD 12ff82063 
[  440.541953] PTE 800139376161

[  440.543555] Oops: 0003 [#1] SMP
[  440.543966] Modules linked in: ppdev(E) joydev(E) evdev(E) serio_raw(E) 
pcspkr(E) bochs_drm(E) drm_kms_helper(E) ttm(E) drm(E) i2c_piix4(E) sg(E) 
parport_pc(E) parport(E) acpi_cpufreq(E) button(E) sch_fq_codel(E) autofs4(E) 
crc32c_generic(E) ext4(E) crc16(E) jbd2(E) mbcache(E) sr_mod(E) cdrom(E) 
sd_mod(E) ata_generic(E) psmouse(E) floppy(E) ata_piix(E) e1000(E) libata(E) 
scsi_mod(E)
[  440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: GE   
4.12.0-rc1-next-20170519+ #155
[  440.549159] Hardware name: QEMU Standard 

Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Luis R. Rodriguez
On Wed, May 24, 2017 at 01:47:28PM -0400, Steven Rostedt wrote:
> OK, it crashed on one of my tests. I removed the patch and it boots
> fine, and crashes when I add it back.
> 
> Here's the crash:
> 
> Running tests on all trace events:
> Testing all events: OK
> Testing ftrace filter: OK
> trace_kprobe: Testing kprobe tracing: 
> BUG: unable to handle kernel paging request at 880214f5c000
> IP: new_slab+0x1e8/0x2b4
> PGD 338c067 
> P4D 338c067 
> PUD 338f067 
> PMD 212022063 
> PTE 800214f5c161

The good news is that this also fixes the other issue I reported with
CONFIG_KPROBES_SANITY_TEST=y and the same WARN complaint without a crash [0].
Please note that the issue I reported actually is present since the
infrastructure to report on these warnings was added through commit
e1a58320a38dfa72be48a0f1a3a92273663ba6db ("x86/mm: Warn on W^X mappings")
which was added through kernel 4.3.0-rc3.

Thomas's patch says: "Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated
trampoline for ftrace_ops"), and this was added through kernel v3.19-rc1 so it
may well be the same issue given the traces on my issue also reflect a
module_alloc() trace after arch_ftrace_update_trampoline() which matches what
Thomas also saw when inspecting his buggy page with kmemleak echo dump [1].

[0] https://lkml.kernel.org/r/20170524175508.gz8...@wotan.suse.de
[1] https://lkml.kernel.org/r/alpine.DEB.2.20.1705232243520.2409@nanos

BUT I'm also able to reprorduce a crash too though easily, as follows where
as before this commit it was passing all 45 tests just fine:

mcgrof@piggy ~/linux-next (git::next-20170519)$ sudo 
tools/testing/selftests/ftrace/ftracetest
=== Ftrace unit tests ===   
[1] Basic trace file check  [PASS]  
[2] Basic test for tracers  [PASS]  
[3] Basic trace clock test  [PASS]  
[4] Basic event tracing check   [PASS]  
[5] event tracing - enable/disable with event level files   [PASS]  
[6] event tracing - restricts events based on pid   [PASS]  
[7] event tracing - enable/disable with subsystem level files   [PASS]  
[8] event tracing - enable/disable with top level files [PASS]  
[9] ftrace - function graph filters with stack tracer   [PASS]  
[10] ftrace - function graph filters[PASS]  
[11] ftrace - test for function event triggers 



I managed to get the kernel log:

[  429.416279] in mmio_trace_init
[  429.504315] mmiotrace: Disabling non-boot CPUs...
[  429.545572] Unregister pv shared memory for cpu 1
[  429.551725] smpboot: CPU 1 is now offline
[  429.553044] mmiotrace: CPU1 is down.
[  429.572874] Unregister pv shared memory for cpu 2
[  429.576361] smpboot: CPU 2 is now offline
[  429.578355] mmiotrace: CPU2 is down.
[  429.596755] Unregister pv shared memory for cpu 3
[  429.601946] smpboot: CPU 3 is now offline
[  429.602768] mmiotrace: CPU3 is down.
[  429.603418] mmiotrace: enabled.
[  429.605001] in mmio_trace_reset
[  429.605003] mmiotrace: Re-enabling CPUs...
[  429.605933] x86: Booting SMP configuration:
[  429.606505] smpboot: Booting Node 0 Processor 1 APIC 0x1
[  429.617989] kvm-clock: cpu 1, msr 1:3fff0041, secondary cpu clock
[  429.640600] KVM setup async PF for cpu 1
[  429.640603] kvm-stealtime: cpu 1, msr 13fc8da40
[  429.642384] mmiotrace: enabled CPU1.
[  429.643391] smpboot: Booting Node 0 Processor 2 APIC 0x2
[  429.654788] kvm-clock: cpu 2, msr 1:3fff0081, secondary cpu clock
[  429.675512] KVM setup async PF for cpu 2
[  429.675921] kvm-stealtime: cpu 2, msr 13fd0da40
[  429.676895] mmiotrace: enabled CPU2.
[  429.678531] smpboot: Booting Node 0 Processor 3 APIC 0x3
[  429.68] kvm-clock: cpu 3, msr 1:3fff00c1, secondary cpu clock
[  429.710808] KVM setup async PF for cpu 3
[  429.711183] kvm-stealtime: cpu 3, msr 13fd8da40
[  429.712123] mmiotrace: enabled CPU3.
[  429.728330] mmiotrace: disabled.
[  440.537183] BUG: unable to handle kernel paging request at 964cf9376000
[  440.538899] IP: tlb_next_batch.isra.54+0x41/0x70
[  440.540022] PGD b736f067 
[  440.540024] P4D b736f067 
[  440.540707] PUD b7373067 
[  440.541318] PMD 12ff82063 
[  440.541953] PTE 800139376161

[  440.543555] Oops: 0003 [#1] SMP
[  440.543966] Modules linked in: ppdev(E) joydev(E) evdev(E) serio_raw(E) 
pcspkr(E) bochs_drm(E) drm_kms_helper(E) ttm(E) drm(E) i2c_piix4(E) sg(E) 
parport_pc(E) parport(E) acpi_cpufreq(E) button(E) sch_fq_codel(E) autofs4(E) 
crc32c_generic(E) ext4(E) crc16(E) jbd2(E) mbcache(E) sr_mod(E) cdrom(E) 
sd_mod(E) ata_generic(E) psmouse(E) floppy(E) ata_piix(E) e1000(E) libata(E) 
scsi_mod(E)
[  440.548057] CPU: 2 PID: 1765 Comm: ftracetest Tainted: GE   
4.12.0-rc1-next-20170519+ #155
[  440.549159] Hardware name: QEMU Standard 

Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Steven Rostedt
OK, it crashed on one of my tests. I removed the patch and it boots
fine, and crashes when I add it back.

Here's the crash:

Running tests on all trace events:
Testing all events: OK
Testing ftrace filter: OK
trace_kprobe: Testing kprobe tracing: 
BUG: unable to handle kernel paging request at 880214f5c000
IP: new_slab+0x1e8/0x2b4
PGD 338c067 
P4D 338c067 
PUD 338f067 
PMD 212022063 
PTE 800214f5c161

Oops: 0003 [#1] SMP
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
task: 8802153a8000 task.stack: c9c74000
RIP: 0010:new_slab+0x1e8/0x2b4
RSP: :c9c77b28 EFLAGS: 00010282
RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
RDX:  RSI: 880214f5c000 RDI: 880216003f00
RBP: c9c77b70 R08: 002a R09: 
R10: 000201e2 R11: 00020190 R12: 880214f5c000
R13: 002e R14: 0001 R15: ea000853d700
FS:  () GS:88021eb8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 880214f5c000 CR3: 0221d000 CR4: 001406e0
Call Trace:
 ? interleave_nodes+0x29/0x40
 ___slab_alloc+0x2e8/0x49e
 ? trace_create_new_event+0x1f/0x63
 ? trace_add_event_call+0x27/0x91
 ? mark_lock+0x24/0x1ec
 ? trace_create_new_event+0x1f/0x63
 __slab_alloc+0x46/0x6b
 ? __slab_alloc+0x46/0x6b
 ? trace_create_new_event+0x1f/0x63
 kmem_cache_alloc+0x59/0x1b2
 trace_create_new_event+0x1f/0x63
 __trace_add_new_event+0xd/0x29
 trace_add_event_call+0x66/0x91
 create_trace_kprobe+0x722/0x7f5
 ? argv_split+0x6c/0xce
 ? probes_open+0x3b/0x3b
 ? set_debug_rodata+0x17/0x17
 traceprobe_command+0x42/0x57
 kprobe_trace_self_tests_init+0x3c/0x338
 ? kprobe_trace_selftest_target+0x13/0x13
 ? set_debug_rodata+0x17/0x17
 do_one_initcall+0x90/0x131
 ? set_debug_rodata+0x17/0x17
 kernel_init_freeable+0x1f4/0x27c
 ? rest_init+0x12d/0x12d
 kernel_init+0xe/0xfa
 ret_from_fork+0x2e/0x40
Code: ff ff 48 8b 53 48 48 85 d2 74 05 4c 89 e7 ff d2 66 41 8b 57 1a 81 e2 ff 
7f 00 00 44 39 f2 48 63 53 20 7e 0d 48 63 4b 18 4c 01 e1 <49> 89 0c 14 eb 08 49 
c7 04 14 00 00 00 00 48 63 53 18 41 ff c6 
RIP: new_slab+0x1e8/0x2b4 RSP: c9c77b28
CR2: 880214f5c000
---[ end trace e8a2200699d40525 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009

Kernel Offset: disabled
---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009

sched: Unexpected reschedule of offline CPU#1!
[ cut here ]
WARNING: CPU: 3 PID: 1 at /work/git/linux-trace.git/arch/x86/kernel/smp.c:128 
native_smp_send_reschedule+0x23/0x3b
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Tainted: G  D 4.12.0-rc2-test+ #42
Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
task: 8802153a8000 task.stack: c9c74000
RIP: 0010:native_smp_send_reschedule+0x23/0x3b
RSP: :88021eb83e60 EFLAGS: 00010086
RAX: 002e RBX: 0001 RCX: 8802153a8000
RDX: 8226ba60 RSI: 8109ff35 RDI: 8108e779
RBP: 88021eb83e60 R08: 0001 R09: 0001
R10: 88021eb83de8 R11: 832b238c R12: 0003
R13: 0003 R14: 880216002270 R15: 0001
FS:  () GS:88021eb8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 880214f5c000 CR3: 0221d000 CR4: 001406e0
Call Trace:
 
 trigger_load_balance+0x2dc/0x2e5
 scheduler_tick+0x92/0x9b
 update_process_times+0x47/0x54
 tick_sched_handle+0x44/0x53
 tick_sched_timer+0x39/0x5f
 __hrtimer_run_queues+0x141/0x2c8
 ? tick_sched_do_timer+0x2e/0x2e
 hrtimer_interrupt+0x6c/0x12e
 local_apic_timer_interrupt+0x4b/0x4e
 smp_apic_timer_interrupt+0x29/0x39
 apic_timer_interrupt+0x90/0xa0
RIP: 0010:panic+0x1e1/0x21f
RSP: :c9c77e70 EFLAGS: 0246 ORIG_RAX: ff10
RAX: 0003810a049e RBX:  RCX: 8802153a8000
RDX: c9c77e00 RSI: 81133f1a RDI: 81091058
RBP: c9c77ee0 R08: 0001 R09: 0001
R10: c9c77d78 R11: 8265a60a R12: 
R13:  R14: 0001 R15: 0009
 
 ? panic+0x1de/0x21f
 ? trace_hardirqs_on+0xd/0xf
 do_exit+0x568/0xa40
 rewind_stack_do_exit+0x17/0x20
Code: ff 90 a0 00 00 00 5d c3 0f 1f 44 00 00 55 89 f8 48 89 e5 48 0f a3 05 ec 
05 41 01 72 12 89 fe 48 c7 c7 5d 0b fe 81 e8 7a 2a 10 00 <0f> ff eb 12 48 8b 05 
31 62 0e 01 be fd 00 00 00 ff 90 a0 00 00 
---[ end trace e8a2200699d40526 ]---


I attached the config. I don't currently have time to look deeper into
it. But I should have some time later today.

-- Steve


config.gz
Description: application/gzip


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Steven Rostedt
OK, it crashed on one of my tests. I removed the patch and it boots
fine, and crashes when I add it back.

Here's the crash:

Running tests on all trace events:
Testing all events: OK
Testing ftrace filter: OK
trace_kprobe: Testing kprobe tracing: 
BUG: unable to handle kernel paging request at 880214f5c000
IP: new_slab+0x1e8/0x2b4
PGD 338c067 
P4D 338c067 
PUD 338f067 
PMD 212022063 
PTE 800214f5c161

Oops: 0003 [#1] SMP
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.12.0-rc2-test+ #42
Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
task: 8802153a8000 task.stack: c9c74000
RIP: 0010:new_slab+0x1e8/0x2b4
RSP: :c9c77b28 EFLAGS: 00010282
RAX: 4004 RBX: 880216003f00 RCX: 880214f5c058
RDX:  RSI: 880214f5c000 RDI: 880216003f00
RBP: c9c77b70 R08: 002a R09: 
R10: 000201e2 R11: 00020190 R12: 880214f5c000
R13: 002e R14: 0001 R15: ea000853d700
FS:  () GS:88021eb8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 880214f5c000 CR3: 0221d000 CR4: 001406e0
Call Trace:
 ? interleave_nodes+0x29/0x40
 ___slab_alloc+0x2e8/0x49e
 ? trace_create_new_event+0x1f/0x63
 ? trace_add_event_call+0x27/0x91
 ? mark_lock+0x24/0x1ec
 ? trace_create_new_event+0x1f/0x63
 __slab_alloc+0x46/0x6b
 ? __slab_alloc+0x46/0x6b
 ? trace_create_new_event+0x1f/0x63
 kmem_cache_alloc+0x59/0x1b2
 trace_create_new_event+0x1f/0x63
 __trace_add_new_event+0xd/0x29
 trace_add_event_call+0x66/0x91
 create_trace_kprobe+0x722/0x7f5
 ? argv_split+0x6c/0xce
 ? probes_open+0x3b/0x3b
 ? set_debug_rodata+0x17/0x17
 traceprobe_command+0x42/0x57
 kprobe_trace_self_tests_init+0x3c/0x338
 ? kprobe_trace_selftest_target+0x13/0x13
 ? set_debug_rodata+0x17/0x17
 do_one_initcall+0x90/0x131
 ? set_debug_rodata+0x17/0x17
 kernel_init_freeable+0x1f4/0x27c
 ? rest_init+0x12d/0x12d
 kernel_init+0xe/0xfa
 ret_from_fork+0x2e/0x40
Code: ff ff 48 8b 53 48 48 85 d2 74 05 4c 89 e7 ff d2 66 41 8b 57 1a 81 e2 ff 
7f 00 00 44 39 f2 48 63 53 20 7e 0d 48 63 4b 18 4c 01 e1 <49> 89 0c 14 eb 08 49 
c7 04 14 00 00 00 00 48 63 53 18 41 ff c6 
RIP: new_slab+0x1e8/0x2b4 RSP: c9c77b28
CR2: 880214f5c000
---[ end trace e8a2200699d40525 ]---
Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009

Kernel Offset: disabled
---[ end Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009

sched: Unexpected reschedule of offline CPU#1!
[ cut here ]
WARNING: CPU: 3 PID: 1 at /work/git/linux-trace.git/arch/x86/kernel/smp.c:128 
native_smp_send_reschedule+0x23/0x3b
Modules linked in:
CPU: 3 PID: 1 Comm: swapper/0 Tainted: G  D 4.12.0-rc2-test+ #42
Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014
task: 8802153a8000 task.stack: c9c74000
RIP: 0010:native_smp_send_reschedule+0x23/0x3b
RSP: :88021eb83e60 EFLAGS: 00010086
RAX: 002e RBX: 0001 RCX: 8802153a8000
RDX: 8226ba60 RSI: 8109ff35 RDI: 8108e779
RBP: 88021eb83e60 R08: 0001 R09: 0001
R10: 88021eb83de8 R11: 832b238c R12: 0003
R13: 0003 R14: 880216002270 R15: 0001
FS:  () GS:88021eb8() knlGS:
CS:  0010 DS:  ES:  CR0: 80050033
CR2: 880214f5c000 CR3: 0221d000 CR4: 001406e0
Call Trace:
 
 trigger_load_balance+0x2dc/0x2e5
 scheduler_tick+0x92/0x9b
 update_process_times+0x47/0x54
 tick_sched_handle+0x44/0x53
 tick_sched_timer+0x39/0x5f
 __hrtimer_run_queues+0x141/0x2c8
 ? tick_sched_do_timer+0x2e/0x2e
 hrtimer_interrupt+0x6c/0x12e
 local_apic_timer_interrupt+0x4b/0x4e
 smp_apic_timer_interrupt+0x29/0x39
 apic_timer_interrupt+0x90/0xa0
RIP: 0010:panic+0x1e1/0x21f
RSP: :c9c77e70 EFLAGS: 0246 ORIG_RAX: ff10
RAX: 0003810a049e RBX:  RCX: 8802153a8000
RDX: c9c77e00 RSI: 81133f1a RDI: 81091058
RBP: c9c77ee0 R08: 0001 R09: 0001
R10: c9c77d78 R11: 8265a60a R12: 
R13:  R14: 0001 R15: 0009
 
 ? panic+0x1de/0x21f
 ? trace_hardirqs_on+0xd/0xf
 do_exit+0x568/0xa40
 rewind_stack_do_exit+0x17/0x20
Code: ff 90 a0 00 00 00 5d c3 0f 1f 44 00 00 55 89 f8 48 89 e5 48 0f a3 05 ec 
05 41 01 72 12 89 fe 48 c7 c7 5d 0b fe 81 e8 7a 2a 10 00 <0f> ff eb 12 48 8b 05 
31 62 0e 01 be fd 00 00 00 ff 90 a0 00 00 
---[ end trace e8a2200699d40526 ]---


I attached the config. I don't currently have time to look deeper into
it. But I should have some time later today.

-- Steve


config.gz
Description: application/gzip


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Steven Rostedt
On Wed, 24 May 2017 15:47:17 +0200 (CEST)
Thomas Gleixner  wrote:

> ftrace uses module_alloc() to allocate trampoline pages. The mapping
> of module_alloc() is RWX, which makes sense as the memory is written
> to right after allocation. But nothing makes these pages RO after
> writing to them.
> 
> This problem exists since ftrace uses trampolines on x86, but it went
> unnoticed because the W=X sanity check only triggers when the tracer
> builtin selftests are enabled. Though the mappings are also created
> W+X w/o the self tests when the tracer is used after booting.
> 
> Add proper set_memory_rw/ro() calls to [un]protect the trampolines
> before and after modification.
> 
> Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline
> for ftrace_ops") Signed-off-by: Thomas Gleixner 

Thanks! I was thinking that this was the issue after your last reply.
I'll send this to my box at home and run my tests against it. I'll let
you know tomorrow the results.

-- Steve


> ---
>  arch/x86/kernel/ftrace.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc
>   unsigned long offset;
>   unsigned long ip;
>   unsigned int size;
> - int ret;
> + int ret, npages;
>  
>   if (ops->trampoline) {
>   /*
> @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc
>*/
>   if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
>   return;
> + npages = PAGE_ALIGN(ops->trampoline_size) >>
> PAGE_SHIFT;
> + set_memory_rw(ops->trampoline, npages);
>   } else {
>   ops->trampoline = create_trampoline(ops, );
>   if (!ops->trampoline)
>   return;
>   ops->trampoline_size = size;
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>   }
>  
>   offset = calc_trampoline_call_offset(ops->flags &
> FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +866,7 @@ void
> arch_ftrace_update_trampoline(struc /* Do a safe modify in case the
> trampoline is executing */ new = ftrace_call_replace(ip, (unsigned
> long)func); ret = update_ftrace_func(ip, new);
> + set_memory_ro(ops->trampoline, npages);
>  
>   /* The update should never fail */
>   WARN_ON(ret);



Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Steven Rostedt
On Wed, 24 May 2017 15:47:17 +0200 (CEST)
Thomas Gleixner  wrote:

> ftrace uses module_alloc() to allocate trampoline pages. The mapping
> of module_alloc() is RWX, which makes sense as the memory is written
> to right after allocation. But nothing makes these pages RO after
> writing to them.
> 
> This problem exists since ftrace uses trampolines on x86, but it went
> unnoticed because the W=X sanity check only triggers when the tracer
> builtin selftests are enabled. Though the mappings are also created
> W+X w/o the self tests when the tracer is used after booting.
> 
> Add proper set_memory_rw/ro() calls to [un]protect the trampolines
> before and after modification.
> 
> Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline
> for ftrace_ops") Signed-off-by: Thomas Gleixner 

Thanks! I was thinking that this was the issue after your last reply.
I'll send this to my box at home and run my tests against it. I'll let
you know tomorrow the results.

-- Steve


> ---
>  arch/x86/kernel/ftrace.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc
>   unsigned long offset;
>   unsigned long ip;
>   unsigned int size;
> - int ret;
> + int ret, npages;
>  
>   if (ops->trampoline) {
>   /*
> @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc
>*/
>   if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
>   return;
> + npages = PAGE_ALIGN(ops->trampoline_size) >>
> PAGE_SHIFT;
> + set_memory_rw(ops->trampoline, npages);
>   } else {
>   ops->trampoline = create_trampoline(ops, );
>   if (!ops->trampoline)
>   return;
>   ops->trampoline_size = size;
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>   }
>  
>   offset = calc_trampoline_call_offset(ops->flags &
> FTRACE_OPS_FL_SAVE_REGS); @@ -863,6 +866,7 @@ void
> arch_ftrace_update_trampoline(struc /* Do a safe modify in case the
> trampoline is executing */ new = ftrace_call_replace(ip, (unsigned
> long)func); ret = update_ftrace_func(ip, new);
> + set_memory_ro(ops->trampoline, npages);
>  
>   /* The update should never fail */
>   WARN_ON(ret);



Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Masami Hiramatsu
On Wed, 24 May 2017 15:47:17 +0200 (CEST)
Thomas Gleixner  wrote:

> ftrace uses module_alloc() to allocate trampoline pages. The mapping of
> module_alloc() is RWX, which makes sense as the memory is written to right
> after allocation. But nothing makes these pages RO after writing to them.
> 
> This problem exists since ftrace uses trampolines on x86, but it went
> unnoticed because the W=X sanity check only triggers when the tracer
> builtin selftests are enabled. Though the mappings are also created W+X w/o
> the self tests when the tracer is used after booting.
> 
> Add proper set_memory_rw/ro() calls to [un]protect the trampolines before
> and after modification.
> 

This looks good to me. (same as I did for kprobes trampoline pages)

Reviewed-by: Masami Hiramatsu 

Thank you,

> Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for 
> ftrace_ops")
> Signed-off-by: Thomas Gleixner 
> ---
>  arch/x86/kernel/ftrace.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc
>   unsigned long offset;
>   unsigned long ip;
>   unsigned int size;
> - int ret;
> + int ret, npages;
>  
>   if (ops->trampoline) {
>   /*
> @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc
>*/
>   if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
>   return;
> + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT;
> + set_memory_rw(ops->trampoline, npages);
>   } else {
>   ops->trampoline = create_trampoline(ops, );
>   if (!ops->trampoline)
>   return;
>   ops->trampoline_size = size;
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>   }
>  
>   offset = calc_trampoline_call_offset(ops->flags & 
> FTRACE_OPS_FL_SAVE_REGS);
> @@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc
>   /* Do a safe modify in case the trampoline is executing */
>   new = ftrace_call_replace(ip, (unsigned long)func);
>   ret = update_ftrace_func(ip, new);
> + set_memory_ro(ops->trampoline, npages);
>  
>   /* The update should never fail */
>   WARN_ON(ret);


-- 
Masami Hiramatsu 


Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Masami Hiramatsu
On Wed, 24 May 2017 15:47:17 +0200 (CEST)
Thomas Gleixner  wrote:

> ftrace uses module_alloc() to allocate trampoline pages. The mapping of
> module_alloc() is RWX, which makes sense as the memory is written to right
> after allocation. But nothing makes these pages RO after writing to them.
> 
> This problem exists since ftrace uses trampolines on x86, but it went
> unnoticed because the W=X sanity check only triggers when the tracer
> builtin selftests are enabled. Though the mappings are also created W+X w/o
> the self tests when the tracer is used after booting.
> 
> Add proper set_memory_rw/ro() calls to [un]protect the trampolines before
> and after modification.
> 

This looks good to me. (same as I did for kprobes trampoline pages)

Reviewed-by: Masami Hiramatsu 

Thank you,

> Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for 
> ftrace_ops")
> Signed-off-by: Thomas Gleixner 
> ---
>  arch/x86/kernel/ftrace.c |6 +-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc
>   unsigned long offset;
>   unsigned long ip;
>   unsigned int size;
> - int ret;
> + int ret, npages;
>  
>   if (ops->trampoline) {
>   /*
> @@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc
>*/
>   if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
>   return;
> + npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT;
> + set_memory_rw(ops->trampoline, npages);
>   } else {
>   ops->trampoline = create_trampoline(ops, );
>   if (!ops->trampoline)
>   return;
>   ops->trampoline_size = size;
> + npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>   }
>  
>   offset = calc_trampoline_call_offset(ops->flags & 
> FTRACE_OPS_FL_SAVE_REGS);
> @@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc
>   /* Do a safe modify in case the trampoline is executing */
>   new = ftrace_call_replace(ip, (unsigned long)func);
>   ret = update_ftrace_func(ip, new);
> + set_memory_ro(ops->trampoline, npages);
>  
>   /* The update should never fail */
>   WARN_ON(ret);


-- 
Masami Hiramatsu 


[PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Thomas Gleixner
ftrace uses module_alloc() to allocate trampoline pages. The mapping of
module_alloc() is RWX, which makes sense as the memory is written to right
after allocation. But nothing makes these pages RO after writing to them.

This problem exists since ftrace uses trampolines on x86, but it went
unnoticed because the W=X sanity check only triggers when the tracer
builtin selftests are enabled. Though the mappings are also created W+X w/o
the self tests when the tracer is used after booting.

Add proper set_memory_rw/ro() calls to [un]protect the trampolines before
and after modification.

Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for 
ftrace_ops")
Signed-off-by: Thomas Gleixner 
---
 arch/x86/kernel/ftrace.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc
unsigned long offset;
unsigned long ip;
unsigned int size;
-   int ret;
+   int ret, npages;
 
if (ops->trampoline) {
/*
@@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc
 */
if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
return;
+   npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT;
+   set_memory_rw(ops->trampoline, npages);
} else {
ops->trampoline = create_trampoline(ops, );
if (!ops->trampoline)
return;
ops->trampoline_size = size;
+   npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
}
 
offset = calc_trampoline_call_offset(ops->flags & 
FTRACE_OPS_FL_SAVE_REGS);
@@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc
/* Do a safe modify in case the trampoline is executing */
new = ftrace_call_replace(ip, (unsigned long)func);
ret = update_ftrace_func(ip, new);
+   set_memory_ro(ops->trampoline, npages);
 
/* The update should never fail */
WARN_ON(ret);


[PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX

2017-05-24 Thread Thomas Gleixner
ftrace uses module_alloc() to allocate trampoline pages. The mapping of
module_alloc() is RWX, which makes sense as the memory is written to right
after allocation. But nothing makes these pages RO after writing to them.

This problem exists since ftrace uses trampolines on x86, but it went
unnoticed because the W=X sanity check only triggers when the tracer
builtin selftests are enabled. Though the mappings are also created W+X w/o
the self tests when the tracer is used after booting.

Add proper set_memory_rw/ro() calls to [un]protect the trampolines before
and after modification.

Fixes: f3bea49115b2 ("ftrace/x86: Add dynamic allocated trampoline for 
ftrace_ops")
Signed-off-by: Thomas Gleixner 
---
 arch/x86/kernel/ftrace.c |6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -839,7 +839,7 @@ void arch_ftrace_update_trampoline(struc
unsigned long offset;
unsigned long ip;
unsigned int size;
-   int ret;
+   int ret, npages;
 
if (ops->trampoline) {
/*
@@ -848,11 +848,14 @@ void arch_ftrace_update_trampoline(struc
 */
if (!(ops->flags & FTRACE_OPS_FL_ALLOC_TRAMP))
return;
+   npages = PAGE_ALIGN(ops->trampoline_size) >> PAGE_SHIFT;
+   set_memory_rw(ops->trampoline, npages);
} else {
ops->trampoline = create_trampoline(ops, );
if (!ops->trampoline)
return;
ops->trampoline_size = size;
+   npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
}
 
offset = calc_trampoline_call_offset(ops->flags & 
FTRACE_OPS_FL_SAVE_REGS);
@@ -863,6 +866,7 @@ void arch_ftrace_update_trampoline(struc
/* Do a safe modify in case the trampoline is executing */
new = ftrace_call_replace(ip, (unsigned long)func);
ret = update_ftrace_func(ip, new);
+   set_memory_ro(ops->trampoline, npages);
 
/* The update should never fail */
WARN_ON(ret);