Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1650211 > unrolled thread
| Started by | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| First post | 2017-05-25 08:30 +0200 |
| Last post | 2017-05-26 16:00 +0200 |
| Articles | 13 — 7 participants |
Back to article view | Back to linux.kernel
This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by
below is the oldest one visible, not the original post.
Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner <tglx@linutronix.de> - 2017-05-25 08:30 +0200
[PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner <tglx@linutronix.de> - 2017-05-25 11:00 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Steven Rostedt <rostedt@goodmis.org> - 2017-05-25 17:20 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX "Luis R. Rodriguez" <mcgrof@kernel.org> - 2017-05-25 19:50 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Kees Cook <keescook@chromium.org> - 2017-05-25 22:00 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner <tglx@linutronix.de> - 2017-05-26 09:10 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Heiko Carstens <heiko.carstens@de.ibm.com> - 2017-05-26 11:40 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner <tglx@linutronix.de> - 2017-05-26 12:00 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Michael Ellerman <mpe@ellerman.id.au> - 2017-05-26 13:50 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Masami Hiramatsu <mhiramat@kernel.org> - 2017-05-26 11:50 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Steven Rostedt <rostedt@goodmis.org> - 2017-05-26 15:40 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Steven Rostedt <rostedt@goodmis.org> - 2017-05-26 16:00 +0200
Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX Thomas Gleixner <tglx@linutronix.de> - 2017-05-26 16:00 +0200
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-25 08:30 +0200 |
| Subject | Re: [PATCH] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tKUrw-4D7-3@gated-at.bofh.it> |
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
[toc] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-25 11:00 +0200 |
| Subject | [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tKWMG-5YF-15@gated-at.bofh.it> |
| In reply to | #1650211 |
ftrace use 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.
Add proper set_memory_rw/ro() calls to protect the trampolines after
modification.
Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
---
arch/x86/kernel/ftrace.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
--- a/arch/x86/kernel/ftrace.c
+++ b/arch/x86/kernel/ftrace.c
@@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned
{
return module_alloc(size);
}
-static inline void tramp_free(void *tramp)
+static inline void tramp_free(void *tramp, int size)
{
+ int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+
+ set_memory_nx((unsigned long)tramp, npages);
+ set_memory_rw((unsigned long)tramp, npages);
module_memfree(tramp);
}
#else
@@ -699,7 +703,7 @@ static inline void *alloc_tramp(unsigned
{
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
/* 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
/* 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;
}
@@ -839,7 +843,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 +852,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, &size);
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 +870,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);
@@ -939,7 +947,7 @@ void arch_ftrace_trampoline_free(struct
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;
}
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-25 17:20 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tL2Iq-1qK-23@gated-at.bofh.it> |
| In reply to | #1650336 |
On Thu, 25 May 2017 10:57:51 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > ftrace use 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. > > Add proper set_memory_rw/ro() calls to protect the trampolines after > modification. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> OK, I pulled this in and I'm currently running it through my test suite. I'm about to board a flight, hopefully it runs smoothly and will finish by the time I get home. -- Steve
[toc] | [prev] | [next] | [standalone]
| From | "Luis R. Rodriguez" <mcgrof@kernel.org> |
|---|---|
| Date | 2017-05-25 19:50 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tL53z-2KJ-7@gated-at.bofh.it> |
| In reply to | #1650336 |
On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote:
> ftrace use 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.
>
> Add proper set_memory_rw/ro() calls to protect the trampolines after
> modification.
>
> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> ---
> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------
> 1 file changed, 14 insertions(+), 6 deletions(-)
>
> --- a/arch/x86/kernel/ftrace.c
> +++ b/arch/x86/kernel/ftrace.c
> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned
> {
> return module_alloc(size);
> }
> -static inline void tramp_free(void *tramp)
> +static inline void tramp_free(void *tramp, int size)
> {
> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> +
> + set_memory_nx((unsigned long)tramp, npages);
> + set_memory_rw((unsigned long)tramp, npages);
> module_memfree(tramp);
> }
Can/should module_memfree() just do this for users? With Masami's fix that'd
be 2 users already.
Luis
[toc] | [prev] | [next] | [standalone]
| From | Kees Cook <keescook@chromium.org> |
|---|---|
| Date | 2017-05-25 22:00 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tL75o-40J-7@gated-at.bofh.it> |
| In reply to | #1650701 |
On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote:
>> ftrace use 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.
>>
>> Add proper set_memory_rw/ro() calls to protect the trampolines after
>> modification.
>>
>> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
>> ---
>> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------
>> 1 file changed, 14 insertions(+), 6 deletions(-)
>>
>> --- a/arch/x86/kernel/ftrace.c
>> +++ b/arch/x86/kernel/ftrace.c
>> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned
>> {
>> return module_alloc(size);
>> }
>> -static inline void tramp_free(void *tramp)
>> +static inline void tramp_free(void *tramp, int size)
>> {
>> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> +
>> + set_memory_nx((unsigned long)tramp, npages);
>> + set_memory_rw((unsigned long)tramp, npages);
>> module_memfree(tramp);
>> }
>
> Can/should module_memfree() just do this for users? With Masami's fix that'd
> be 2 users already.
It seems like it really should. That would put it in a single place
and avoid this mistake again in the future. Does module_memfree() have
access to the allocation size, or does that need to get plumbed?
-Kees
--
Kees Cook
Pixel Security
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-26 09:10 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLhxL-2Oy-3@gated-at.bofh.it> |
| In reply to | #1650781 |
On Thu, 25 May 2017, Kees Cook wrote:
> On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote:
> >> ftrace use 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.
> >>
> >> Add proper set_memory_rw/ro() calls to protect the trampolines after
> >> modification.
> >>
> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> >> ---
> >> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------
> >> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>
> >> --- a/arch/x86/kernel/ftrace.c
> >> +++ b/arch/x86/kernel/ftrace.c
> >> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned
> >> {
> >> return module_alloc(size);
> >> }
> >> -static inline void tramp_free(void *tramp)
> >> +static inline void tramp_free(void *tramp, int size)
> >> {
> >> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >> +
> >> + set_memory_nx((unsigned long)tramp, npages);
> >> + set_memory_rw((unsigned long)tramp, npages);
> >> module_memfree(tramp);
> >> }
> >
> > Can/should module_memfree() just do this for users? With Masami's fix that'd
> > be 2 users already.
>
> It seems like it really should. That would put it in a single place
> and avoid this mistake again in the future. Does module_memfree() have
> access to the allocation size, or does that need to get plumbed?
No, it doesn't. But the number of instances is pretty limited.
Btw, looking at BPF. It allocates memory via module_alloc() which means
it's RWX. There is nothing in that BPF code which changes the permissions
afterwards ....
Thanks,
tglx
[toc] | [prev] | [next] | [standalone]
| From | Heiko Carstens <heiko.carstens@de.ibm.com> |
|---|---|
| Date | 2017-05-26 11:40 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLjSW-490-15@gated-at.bofh.it> |
| In reply to | #1651118 |
On Fri, May 26, 2017 at 09:03:13AM +0200, Thomas Gleixner wrote: > > It seems like it really should. That would put it in a single place > > and avoid this mistake again in the future. Does module_memfree() have > > access to the allocation size, or does that need to get plumbed? > > No, it doesn't. But the number of instances is pretty limited. > > Btw, looking at BPF. It allocates memory via module_alloc() which means > it's RWX. There is nothing in that BPF code which changes the permissions > afterwards .... For BPF you're probably referring to bpf_jit_binary_alloc()? Permissions are changed with bpf_jit_binary_lock_ro() within each architecure backend. Well, except for powerpc (cc'ed Michael).
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-26 12:00 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLkch-4g5-3@gated-at.bofh.it> |
| In reply to | #1651264 |
On Fri, 26 May 2017, Heiko Carstens wrote: > On Fri, May 26, 2017 at 09:03:13AM +0200, Thomas Gleixner wrote: > > > It seems like it really should. That would put it in a single place > > > and avoid this mistake again in the future. Does module_memfree() have > > > access to the allocation size, or does that need to get plumbed? > > > > No, it doesn't. But the number of instances is pretty limited. > > > > Btw, looking at BPF. It allocates memory via module_alloc() which means > > it's RWX. There is nothing in that BPF code which changes the permissions > > afterwards .... > > For BPF you're probably referring to bpf_jit_binary_alloc()? Permissions > are changed with bpf_jit_binary_lock_ro() within each architecure backend. > > Well, except for powerpc (cc'ed Michael). The problem starts elsewhere. module_alloc() should not allocate RWX memory in the first place. It should allocated RW and then the usage sites should set it to RX when the code is ready to go. Thanks, tglx
[toc] | [prev] | [next] | [standalone]
| From | Michael Ellerman <mpe@ellerman.id.au> |
|---|---|
| Date | 2017-05-26 13:50 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLlUJ-5nB-13@gated-at.bofh.it> |
| In reply to | #1651264 |
Heiko Carstens <heiko.carstens@de.ibm.com> writes: > On Fri, May 26, 2017 at 09:03:13AM +0200, Thomas Gleixner wrote: >> > It seems like it really should. That would put it in a single place >> > and avoid this mistake again in the future. Does module_memfree() have >> > access to the allocation size, or does that need to get plumbed? >> >> No, it doesn't. But the number of instances is pretty limited. >> >> Btw, looking at BPF. It allocates memory via module_alloc() which means >> it's RWX. There is nothing in that BPF code which changes the permissions >> afterwards .... > > For BPF you're probably referring to bpf_jit_binary_alloc()? Permissions > are changed with bpf_jit_binary_lock_ro() within each architecure backend. > > Well, except for powerpc (cc'ed Michael). [hangs head in shame] Thanks, we are working on this stuff (stricter RWX perms) at the moment, so will add this to the list. It's complicated somewhat by the variety of MMUs we support, but still. cheers
[toc] | [prev] | [next] | [standalone]
| From | Masami Hiramatsu <mhiramat@kernel.org> |
|---|---|
| Date | 2017-05-26 11:50 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLk2B-4cR-1@gated-at.bofh.it> |
| In reply to | #1650781 |
On Thu, 25 May 2017 12:51:21 -0700
Kees Cook <keescook@chromium.org> wrote:
> On Thu, May 25, 2017 at 10:46 AM, Luis R. Rodriguez <mcgrof@kernel.org> wrote:
> > On Thu, May 25, 2017 at 10:57:51AM +0200, Thomas Gleixner wrote:
> >> ftrace use 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.
> >>
> >> Add proper set_memory_rw/ro() calls to protect the trampolines after
> >> modification.
> >>
> >> Signed-off-by: Thomas Gleixner <tglx@linutronix.de>
> >> ---
> >> arch/x86/kernel/ftrace.c | 20 ++++++++++++++------
> >> 1 file changed, 14 insertions(+), 6 deletions(-)
> >>
> >> --- a/arch/x86/kernel/ftrace.c
> >> +++ b/arch/x86/kernel/ftrace.c
> >> @@ -689,8 +689,12 @@ static inline void *alloc_tramp(unsigned
> >> {
> >> return module_alloc(size);
> >> }
> >> -static inline void tramp_free(void *tramp)
> >> +static inline void tramp_free(void *tramp, int size)
> >> {
> >> + int npages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> >> +
> >> + set_memory_nx((unsigned long)tramp, npages);
> >> + set_memory_rw((unsigned long)tramp, npages);
> >> module_memfree(tramp);
> >> }
> >
> > Can/should module_memfree() just do this for users? With Masami's fix that'd
> > be 2 users already.
>
> It seems like it really should. That would put it in a single place
> and avoid this mistake again in the future. Does module_memfree() have
> access to the allocation size, or does that need to get plumbed?
module_memfree() is just a wrapper of vfree, so find_vm_area()
will help us to get the size.
Thank you,
>
> -Kees
>
> --
> Kees Cook
> Pixel Security
--
Masami Hiramatsu <mhiramat@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-26 15:40 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLnDb-6sK-5@gated-at.bofh.it> |
| In reply to | #1650336 |
On Thu, 25 May 2017 10:57:51 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > ftrace use 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. > > Add proper set_memory_rw/ro() calls to protect the trampolines after > modification. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- Unfortunately, this was triggered in my tests: ftrace: allocating 54840 entries in 215 pages Starting tracer 'function' ------------[ cut here ]------------ kernel BUG at /work/autotest/nobackup/linux-test.git/arch/x86/mm/pageattr.c:189! invalid opcode: 0000 [#1] SMP Modules linked in: CPU: 0 PID: 0 Comm: swapper Not tainted 4.12.0-rc2-test+ #3 Hardware name: MSI MS-7823/CSM-H87M-G43 (MS-7823), BIOS V1.6 02/22/2014 task: ffffffffb4222500 task.stack: ffffffffb4200000 RIP: 0010:change_page_attr_set_clr+0x269/0x302 RSP: 0000:ffffffffb4203c88 EFLAGS: 00010046 RAX: 0000000000000046 RBX: 0000000000000000 RCX: 00000001b6000000 RDX: ffffffffb4203d40 RSI: 0000000000000000 RDI: ffffffffb4240d60 RBP: ffffffffb4203d18 R08: 00000001b6000000 R09: 0000000000000001 R10: ffffffffb4203aa8 R11: 0000000000000003 R12: ffffffffc029b000 R13: ffffffffb4203d40 R14: 0000000000000001 R15: 0000000000000000 FS: 0000000000000000(0000) GS:ffff9a639ea00000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: ffff9a636b384000 CR3: 00000001ea21d000 CR4: 00000000000406b0 Call Trace: change_page_attr_clear+0x1f/0x21 set_memory_ro+0x1e/0x20 arch_ftrace_update_trampoline+0x207/0x21c ? ftrace_caller+0x64/0x64 ? 0xffffffffc029b000 ftrace_startup+0xf4/0x198 register_ftrace_function+0x26/0x3c function_trace_init+0x5e/0x73 tracer_init+0x1e/0x23 tracing_set_tracer+0x127/0x15a register_tracer+0x19b/0x1bc init_function_trace+0x90/0x92 early_trace_init+0x236/0x2b3 start_kernel+0x200/0x3f5 x86_64_start_reservations+0x29/0x2b x86_64_start_kernel+0x17c/0x18f secondary_startup_64+0x9f/0x9f ? secondary_startup_64+0x9f/0x9f Code: 89 df e8 79 f4 ff ff 48 85 c0 74 12 f6 00 01 74 0d be 00 10 00 00 48 89 df e8 84 e8 ff ff 49 ff c4 eb a4 9c 58 0f ba e0 09 72 02 <0f> 0b 49 8d 84 24 ff 0f 00 00 48 25 00 f0 ff ff 49 39 c4 74 02 RIP: change_page_attr_set_clr+0x269/0x302 RSP: ffffffffb4203c88 ---[ end trace 418d67f4f812a298 ]--- Kernel panic - not syncing: Attempted to kill the idle task! ---[ end Kernel panic - not syncing: Attempted to kill the idle task! It appears that if you enable function tracing at boot up, calling set_memory_ro() with interrupts disabled can cause this. As pageattr.c at line 189 has: BUG_ON(irqs_disabled()); in cpa_flush_range() -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Steven Rostedt <rostedt@goodmis.org> |
|---|---|
| Date | 2017-05-26 16:00 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLnWx-6BD-1@gated-at.bofh.it> |
| In reply to | #1651395 |
On Fri, 26 May 2017 15:50:38 +0200 (CEST) Thomas Gleixner <tglx@linutronix.de> wrote: > That's very early boot, right? So interrupts have to be disabled. > > So this wants to be: > > BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); > I was thinking the same thing. I'll add a patch and retest. Thanks! -- Steve
[toc] | [prev] | [next] | [standalone]
| From | Thomas Gleixner <tglx@linutronix.de> |
|---|---|
| Date | 2017-05-26 16:00 +0200 |
| Subject | Re: [PATCH V2] x86/ftrace: Make sure that ftrace trampolines are not RWX |
| Message-ID | <tLnWx-6BD-3@gated-at.bofh.it> |
| In reply to | #1651395 |
On Fri, 26 May 2017, Steven Rostedt wrote: > On Thu, 25 May 2017 10:57:51 +0200 (CEST) > Thomas Gleixner <tglx@linutronix.de> wrote: > > > ftrace use 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. > > > > Add proper set_memory_rw/ro() calls to protect the trampolines after > > modification. > > > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > > --- > > Unfortunately, this was triggered in my tests: > > ftrace: allocating 54840 entries in 215 pages > Starting tracer 'function' > ------------[ cut here ]------------ > kernel BUG at /work/autotest/nobackup/linux-test.git/arch/x86/mm/pageattr.c:189! > > It appears that if you enable function tracing at boot up, calling > set_memory_ro() with interrupts disabled can cause this. As pageattr.c > at line 189 has: > > BUG_ON(irqs_disabled()); That's very early boot, right? So interrupts have to be disabled. So this wants to be: BUG_ON(irqs_disabled() && !early_boot_irqs_disabled); Thanks, tglx
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web