Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-29 Thread Steven Rostedt
On Sat, 29 Jun 2019 22:56:47 +0200 (CEST) Jiri Kosina wrote: > > Care to send a patch? :-) > > From: Jiri Kosina > Subject: [PATCH] ftrace/x86: anotate text_mutex split between > ftrace_arch_code_modify_post_process() and ftrace_arch_code_modify_prepare() Care to send a proper patch ;-) As

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-29 Thread Jiri Kosina
On Fri, 28 Jun 2019, Steven Rostedt wrote: > > > > How is that supposed to work? > > > > > > > > ftrace > > > > prepare() > > > > setrw() > > > > setro() > > > > patch <- FAIL > > > > > > /me dodges frozen shark > > > > > > Yo

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Steven Rostedt
On Fri, 28 Jun 2019 19:33:30 +0200 (CEST) Jiri Kosina wrote: > On Thu, 27 Jun 2019, Josh Poimboeuf wrote: > > > > How is that supposed to work? > > > > > > ftrace > > > prepare() > > >setrw() > > > setro() > > > patch <- FAIL > > > > /me dodges f

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Jiri Kosina
On Thu, 27 Jun 2019, Josh Poimboeuf wrote: > > How is that supposed to work? > > > > ftrace > > prepare() > > setrw() > > setro() > > patch <- FAIL > > /me dodges frozen shark > > You are right of course. My brain has apparently already sh

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Josh Poimboeuf
On Fri, Jun 28, 2019 at 11:46:27AM -0400, Steven Rostedt wrote: > On Fri, 28 Jun 2019 09:54:24 -0400 > Steven Rostedt wrote: > > > On Fri, 28 Jun 2019 12:52:32 +0200 > > Petr Mladek wrote: > > > > > On Fri 2019-06-28 09:32:03, Miroslav Benes wrote: > > > > On Thu, 27 Jun 2019, Petr Mladek wro

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Steven Rostedt
On Fri, 28 Jun 2019 09:54:24 -0400 Steven Rostedt wrote: > On Fri, 28 Jun 2019 12:52:32 +0200 > Petr Mladek wrote: > > > On Fri 2019-06-28 09:32:03, Miroslav Benes wrote: > > > On Thu, 27 Jun 2019, Petr Mladek wrote: > > > > @@ -2611,12 +2610,10 @@ static void ftrace_run_update_code(int c

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Steven Rostedt
On Fri, 28 Jun 2019 12:52:32 +0200 Petr Mladek wrote: > On Fri 2019-06-28 09:32:03, Miroslav Benes wrote: > > On Thu, 27 Jun 2019, Petr Mladek wrote: > > > @@ -2611,12 +2610,10 @@ static void ftrace_run_update_code(int command) > > > { > > > int ret; > > > > > > - mutex_lock(&text_mutex);

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Petr Mladek
On Fri 2019-06-28 09:32:03, Miroslav Benes wrote: > On Thu, 27 Jun 2019, Petr Mladek wrote: > > @@ -2611,12 +2610,10 @@ static void ftrace_run_update_code(int command) > > { > > int ret; > > > > - mutex_lock(&text_mutex); > > - > > ret = ftrace_arch_code_modify_prepare(); > > FTRAC

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-28 Thread Miroslav Benes
On Thu, 27 Jun 2019, Petr Mladek wrote: > The commit 9f255b632bf12c4dd7 ("module: Fix livepatch/ftrace module text > permissions race") causes a possible deadlock between register_kprobe() > and ftrace_run_update_code() when ftrace is using stop_machine(). > > The existing dependency chain (in re

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Josh Poimboeuf
On Thu, Jun 27, 2019 at 09:13:04PM -0400, Steven Rostedt wrote: > On Thu, 27 Jun 2019 18:19:52 -0500 > Josh Poimboeuf wrote: > > > > Maybe a comment or two would help though. > > > > I'm adding the following change. Care to add a "reviewed-by" for this > one? > > -- Steve > > diff --git a/a

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Steven Rostedt
On Thu, 27 Jun 2019 18:19:52 -0500 Josh Poimboeuf wrote: > Maybe a comment or two would help though. > I'm adding the following change. Care to add a "reviewed-by" for this one? -- Steve diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c index 33786044d5ac..d7e93b2783fd 100644

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Thomas Gleixner
On Thu, 27 Jun 2019, Josh Poimboeuf wrote: > On Fri, Jun 28, 2019 at 01:09:08AM +0200, Thomas Gleixner wrote: > > On Thu, 27 Jun 2019, Steven Rostedt wrote: > > > I agree with Josh on this. As the original bug was the race between > > > ftrace and live patching / modules changing the text from ro t

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Steven Rostedt
On Thu, 27 Jun 2019 18:19:52 -0500 Josh Poimboeuf wrote: > /me dodges frozen shark > > You are right of course. My brain has apparently already shut off for > the day. And I agreed with your miscalculation. I guess I should have looked deeper into it. Or have less faith in you ;-) > > Maybe

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Josh Poimboeuf
On Fri, Jun 28, 2019 at 01:09:08AM +0200, Thomas Gleixner wrote: > On Thu, 27 Jun 2019, Steven Rostedt wrote: > > On Thu, 27 Jun 2019 17:47:29 -0500 > > > Releasing the lock in a separate function seems a bit surprising and > > > fragile, would it be possible to do something like this instead? > >

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Steven Rostedt
On Fri, 28 Jun 2019 01:09:08 +0200 (CEST) Thomas Gleixner wrote: > On Thu, 27 Jun 2019, Steven Rostedt wrote: > > On Thu, 27 Jun 2019 17:47:29 -0500 > > > Releasing the lock in a separate function seems a bit surprising and > > > fragile, would it be possible to do something like this instead?

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Thomas Gleixner
On Thu, 27 Jun 2019, Steven Rostedt wrote: > On Thu, 27 Jun 2019 17:47:29 -0500 > > Releasing the lock in a separate function seems a bit surprising and > > fragile, would it be possible to do something like this instead? > > > > diff --git a/arch/x86/kernel/ftrace.c b/arch/x86/kernel/ftrace.c > >

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Steven Rostedt
On Thu, 27 Jun 2019 17:47:29 -0500 Josh Poimboeuf wrote: > Thanks a lot for fixing this Petr. > > On Thu, Jun 27, 2019 at 10:13:34AM +0200, Petr Mladek wrote: > > @@ -35,6 +36,7 @@ > > > > int ftrace_arch_code_modify_prepare(void) > > { > > + mutex_lock(&text_mutex); > > set_kernel_tex

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Josh Poimboeuf
Thanks a lot for fixing this Petr. On Thu, Jun 27, 2019 at 10:13:34AM +0200, Petr Mladek wrote: > @@ -35,6 +36,7 @@ > > int ftrace_arch_code_modify_prepare(void) > { > + mutex_lock(&text_mutex); > set_kernel_text_rw(); > set_all_modules_text_rw(); > return 0; > @@ -44,6 +

Re: [PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Thomas Gleixner
On Thu, 27 Jun 2019, Petr Mladek wrote: > Fortunately, the problematic fix is needed only on x86_64. It is > the only architecture that calls set_all_modules_text_rw() > in ftrace path and supports livepatching at the same time. > > Therefore it is enough to move text_mutex handling from the gener

[PATCH] ftrace: Remove possible deadlock between register_kprobe() and ftrace_run_update_code()

2019-06-27 Thread Petr Mladek
The commit 9f255b632bf12c4dd7 ("module: Fix livepatch/ftrace module text permissions race") causes a possible deadlock between register_kprobe() and ftrace_run_update_code() when ftrace is using stop_machine(). The existing dependency chain (in reverse order) is: -> #1 (text_mutex){+.+.}: