Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

[gregkh]

This is a note to let you know that I've just added the patch titled

    kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y

to the 3.18-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch
and it can be found in the queue-3.18 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.


>From foo@baz Tue Dec  5 18:18:39 CET 2017
From: Masami Hiramatsu <mhiramat@kernel.org>
Date: Fri, 20 Oct 2017 08:43:39 +0900
Subject: kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y

From: Masami Hiramatsu <mhiramat@kernel.org>


[ Upstream commit a30b85df7d599f626973e9cd3056fe755bd778e0 ]

We want to wait for all potentially preempted kprobes trampoline
execution to have completed. This guarantees that any freed
trampoline memory is not in use by any task in the system anymore.
synchronize_rcu_tasks() gives such a guarantee, so use it.

Also, this guarantees to wait for all potentially preempted tasks
on the instructions which will be replaced with a jump.

Since this becomes a problem only when CONFIG_PREEMPT=y, enable
CONFIG_TASKS_RCU=y for synchronize_rcu_tasks() in that case.

Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Naveen N . Rao <naveen.n.rao@linux.vnet.ibm.com>
Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Link: http://lkml.kernel.org/r/150845661962.5443.17724352636247312231.stgit@devbox
Signed-off-by: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 arch/Kconfig     |    2 +-
 kernel/kprobes.c |   14 ++++++++------
 2 files changed, 9 insertions(+), 7 deletions(-)

--- a/arch/Kconfig
+++ b/arch/Kconfig
@@ -74,7 +74,7 @@ config JUMP_LABEL
 config OPTPROBES
 	def_bool y
 	depends on KPROBES && HAVE_OPTPROBES
-	depends on !PREEMPT
+	select TASKS_RCU if PREEMPT
 
 config KPROBES_ON_FTRACE
 	def_bool y
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -540,13 +540,15 @@ static void kprobe_optimizer(struct work
 	do_unoptimize_kprobes();
 
 	/*
-	 * Step 2: Wait for quiesence period to ensure all running interrupts
-	 * are done. Because optprobe may modify multiple instructions
-	 * there is a chance that Nth instruction is interrupted. In that
-	 * case, running interrupt can return to 2nd-Nth byte of jump
-	 * instruction. This wait is for avoiding it.
+	 * Step 2: Wait for quiesence period to ensure all potentially
+	 * preempted tasks to have normally scheduled. Because optprobe
+	 * may modify multiple instructions, there is a chance that Nth
+	 * instruction is preempted. In that case, such tasks can return
+	 * to 2nd-Nth byte of jump instruction. This wait is for avoiding it.
+	 * Note that on non-preemptive kernel, this is transparently converted
+	 * to synchronoze_sched() to wait for all interrupts to have completed.
 	 */
-	synchronize_sched();
+	synchronize_rcu_tasks();
 
 	/* Step 3: Optimize kprobes after quiesence period */
 	do_optimize_kprobes();


Patches currently in stable-queue which might be from mhiramat@kernel.org are

queue-3.18/kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch

Re: Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

[Masami Hiramatsu]

Hi Greg,

On Tue, 05 Dec 2017 18:30:54 +0100
<gregkh@linuxfoundation.org> wrote:

> 
> This is a note to let you know that I've just added the patch titled
> 
>     kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y
> 
> to the 3.18-stable tree which can be found at:
>     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> 
> The filename of the patch is:
>      kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch
> and it can be found in the queue-3.18 subdirectory.
> 
> If you, or anyone else, feels it should not be added to the stable tree,
> please let <stable@vger.kernel.org> know about it.
> 

This is not for bugfix, but improvement.
And also, it depends on TASKS_RCU & commit 7e42776d5ed1fe9a941ed8876c5d15cd7cf5d89f.

So, please don't pull it on stable tree.

Thank you,

> 
> From foo@baz Tue Dec  5 18:18:39 CET 2017
> From: Masami Hiramatsu <mhiramat@kernel.org>
> Date: Fri, 20 Oct 2017 08:43:39 +0900
> Subject: kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y
> 
> From: Masami Hiramatsu <mhiramat@kernel.org>
> 
> 
> [ Upstream commit a30b85df7d599f626973e9cd3056fe755bd778e0 ]
> 
> We want to wait for all potentially preempted kprobes trampoline
> execution to have completed. This guarantees that any freed
> trampoline memory is not in use by any task in the system anymore.
> synchronize_rcu_tasks() gives such a guarantee, so use it.
> 
> Also, this guarantees to wait for all potentially preempted tasks
> on the instructions which will be replaced with a jump.
> 
> Since this becomes a problem only when CONFIG_PREEMPT=y, enable
> CONFIG_TASKS_RCU=y for synchronize_rcu_tasks() in that case.
> 
> Signed-off-by: Masami Hiramatsu <mhiramat@kernel.org>
> Acked-by: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Ananth N Mavinakayanahalli <ananth@linux.vnet.ibm.com>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>
> Cc: Naveen N . Rao <naveen.n.rao@linux.vnet.ibm.com>
> Cc: Paul E . McKenney <paulmck@linux.vnet.ibm.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Link: http://lkml.kernel.org/r/150845661962.5443.17724352636247312231.stgit@devbox
> Signed-off-by: Ingo Molnar <mingo@kernel.org>
> Signed-off-by: Sasha Levin <alexander.levin@verizon.com>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  arch/Kconfig     |    2 +-
>  kernel/kprobes.c |   14 ++++++++------
>  2 files changed, 9 insertions(+), 7 deletions(-)
> 
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -74,7 +74,7 @@ config JUMP_LABEL
>  config OPTPROBES
>  	def_bool y
>  	depends on KPROBES && HAVE_OPTPROBES
> -	depends on !PREEMPT
> +	select TASKS_RCU if PREEMPT
>  
>  config KPROBES_ON_FTRACE
>  	def_bool y
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -540,13 +540,15 @@ static void kprobe_optimizer(struct work
>  	do_unoptimize_kprobes();
>  
>  	/*
> -	 * Step 2: Wait for quiesence period to ensure all running interrupts
> -	 * are done. Because optprobe may modify multiple instructions
> -	 * there is a chance that Nth instruction is interrupted. In that
> -	 * case, running interrupt can return to 2nd-Nth byte of jump
> -	 * instruction. This wait is for avoiding it.
> +	 * Step 2: Wait for quiesence period to ensure all potentially
> +	 * preempted tasks to have normally scheduled. Because optprobe
> +	 * may modify multiple instructions, there is a chance that Nth
> +	 * instruction is preempted. In that case, such tasks can return
> +	 * to 2nd-Nth byte of jump instruction. This wait is for avoiding it.
> +	 * Note that on non-preemptive kernel, this is transparently converted
> +	 * to synchronoze_sched() to wait for all interrupts to have completed.
>  	 */
> -	synchronize_sched();
> +	synchronize_rcu_tasks();
>  
>  	/* Step 3: Optimize kprobes after quiesence period */
>  	do_optimize_kprobes();
> 
> 
> Patches currently in stable-queue which might be from mhiramat@kernel.org are
> 
> queue-3.18/kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

[alexander.levin]

On Thu, Dec 07, 2017 at 08:27:37AM +0900, Masami Hiramatsu wrote:
>Hi Greg,
>
>On Tue, 05 Dec 2017 18:30:54 +0100
><gregkh@linuxfoundation.org> wrote:
>
>>
>> This is a note to let you know that I've just added the patch titled
>>
>>     kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y
>>
>> to the 3.18-stable tree which can be found at:
>>     https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_git_-3Fp-3Dlinux_kernel_git_stable_stable-2Dqueue.git-3Ba-3Dsummary&d=DwICAg&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=9-cmq2QbY49vsZXgMeNFamBo1WYKlpsVWfgnFtvHoG8&s=-feM_ZVFHDeIovYdEj2VhiuNt29qe-U1JXSuc-31dAA&e=
>>
>> The filename of the patch is:
>>      kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch
>> and it can be found in the queue-3.18 subdirectory.
>>
>> If you, or anyone else, feels it should not be added to the stable tree,
>> please let <stable@vger.kernel.org> know about it.
>>
>
>This is not for bugfix, but improvement.
>And also, it depends on TASKS_RCU & commit 7e42776d5ed1fe9a941ed8876c5d15cd7cf5d89f.
>
>So, please don't pull it on stable tree.

Thanks for the clarification.

Just to help me understand this better, what I understood from the
patch is that we might execute freed trampoline memory, is that not
the case here?

-- 

Thanks,
Sasha

Re: Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

[Masami Hiramatsu]

On Wed, 6 Dec 2017 23:37:50 +0000
alexander.levin@verizon.com wrote:

> On Thu, Dec 07, 2017 at 08:27:37AM +0900, Masami Hiramatsu wrote:
> >Hi Greg,
> >
> >On Tue, 05 Dec 2017 18:30:54 +0100
> ><gregkh@linuxfoundation.org> wrote:
> >
> >>
> >> This is a note to let you know that I've just added the patch titled
> >>
> >>     kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y
> >>
> >> to the 3.18-stable tree which can be found at:
> >>     https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_git_-3Fp-3Dlinux_kernel_git_stable_stable-2Dqueue.git-3Ba-3Dsummary&d=DwICAg&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=9-cmq2QbY49vsZXgMeNFamBo1WYKlpsVWfgnFtvHoG8&s=-feM_ZVFHDeIovYdEj2VhiuNt29qe-U1JXSuc-31dAA&e=
> >>
> >> The filename of the patch is:
> >>      kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch
> >> and it can be found in the queue-3.18 subdirectory.
> >>
> >> If you, or anyone else, feels it should not be added to the stable tree,
> >> please let <stable@vger.kernel.org> know about it.
> >>
> >
> >This is not for bugfix, but improvement.
> >And also, it depends on TASKS_RCU & commit 7e42776d5ed1fe9a941ed8876c5d15cd7cf5d89f.
> >
> >So, please don't pull it on stable tree.
> 
> Thanks for the clarification.
> 
> Just to help me understand this better, what I understood from the
> patch is that we might execute freed trampoline memory, is that not
> the case here?

No, the patch is only for PREEMPT kernel. Without the patch, PREEMPT
kernel disables optprobe itself. On the other hand, non preempt kernel
enables optprobe but it uses synchronize_sched() for waiting quiesense
period, so that all interrupt routines are done and optprobe can free the
trampoline memory safely.

Note that non preempt kernel doesn't preempt tasks, it only interrupts
tasks and return soon (without task switching). But PREEMPT kernel can
preempt a task, this means a task can be interrupted and switched to 
other task on the way of execution on trampoline memory. This requires
synchronize_rcu_tasks() to wait for switching back to the preempted
tasks.

The commit 7e42776d5ed exposes synchronize_rcu_tasks() as a alias of
synchronize_sched() when PREEMPT=n.

Thank you,


-- 
Masami Hiramatsu <mhiramat@kernel.org>

Re: Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

[Greg KH]

On Thu, Dec 07, 2017 at 08:27:37AM +0900, Masami Hiramatsu wrote:
> Hi Greg,
> 
> On Tue, 05 Dec 2017 18:30:54 +0100
> <gregkh@linuxfoundation.org> wrote:
> 
> > 
> > This is a note to let you know that I've just added the patch titled
> > 
> >     kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y
> > 
> > to the 3.18-stable tree which can be found at:
> >     http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
> > 
> > The filename of the patch is:
> >      kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch
> > and it can be found in the queue-3.18 subdirectory.
> > 
> > If you, or anyone else, feels it should not be added to the stable tree,
> > please let <stable@vger.kernel.org> know about it.
> > 
> 
> This is not for bugfix, but improvement.
> And also, it depends on TASKS_RCU & commit 7e42776d5ed1fe9a941ed8876c5d15cd7cf5d89f.
> 
> So, please don't pull it on stable tree.

Ok, now dropped from the 3.18 stable tree, thanks.

greg k-h

Re: Patch "kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y" has been added to the 3.18-stable tree

[alexander.levin]

On Thu, Dec 07, 2017 at 02:54:58PM +0900, Masami Hiramatsu wrote:
>On Wed, 6 Dec 2017 23:37:50 +0000
>alexander.levin@verizon.com wrote:
>
>> On Thu, Dec 07, 2017 at 08:27:37AM +0900, Masami Hiramatsu wrote:
>> >Hi Greg,
>> >
>> >On Tue, 05 Dec 2017 18:30:54 +0100
>> ><gregkh@linuxfoundation.org> wrote:
>> >
>> >>
>> >> This is a note to let you know that I've just added the patch titled
>> >>
>> >>     kprobes: Use synchronize_rcu_tasks() for optprobe with CONFIG_PREEMPT=y
>> >>
>> >> to the 3.18-stable tree which can be found at:
>> >>     https://urldefense.proofpoint.com/v2/url?u=http-3A__www.kernel.org_git_-3Fp-3Dlinux_kernel_git_stable_stable-2Dqueue.git-3Ba-3Dsummary&d=DwICAg&c=udBTRvFvXC5Dhqg7UHpJlPps3mZ3LRxpb6__0PomBTQ&r=bUtaaC9mlBij4OjEG_D-KPul_335azYzfC4Rjgomobo&m=9-cmq2QbY49vsZXgMeNFamBo1WYKlpsVWfgnFtvHoG8&s=-feM_ZVFHDeIovYdEj2VhiuNt29qe-U1JXSuc-31dAA&e=
>> >>
>> >> The filename of the patch is:
>> >>      kprobes-use-synchronize_rcu_tasks-for-optprobe-with-config_preempt-y.patch
>> >> and it can be found in the queue-3.18 subdirectory.
>> >>
>> >> If you, or anyone else, feels it should not be added to the stable tree,
>> >> please let <stable@vger.kernel.org> know about it.
>> >>
>> >
>> >This is not for bugfix, but improvement.
>> >And also, it depends on TASKS_RCU & commit 7e42776d5ed1fe9a941ed8876c5d15cd7cf5d89f.
>> >
>> >So, please don't pull it on stable tree.
>>
>> Thanks for the clarification.
>>
>> Just to help me understand this better, what I understood from the
>> patch is that we might execute freed trampoline memory, is that not
>> the case here?
>
>No, the patch is only for PREEMPT kernel. Without the patch, PREEMPT
>kernel disables optprobe itself. On the other hand, non preempt kernel
>enables optprobe but it uses synchronize_sched() for waiting quiesense
>period, so that all interrupt routines are done and optprobe can free the
>trampoline memory safely.
>
>Note that non preempt kernel doesn't preempt tasks, it only interrupts
>tasks and return soon (without task switching). But PREEMPT kernel can
>preempt a task, this means a task can be interrupted and switched to
>other task on the way of execution on trampoline memory. This requires
>synchronize_rcu_tasks() to wait for switching back to the preempted
>tasks.
>
>The commit 7e42776d5ed exposes synchronize_rcu_tasks() as a alias of
>synchronize_sched() when PREEMPT=n.
>
>Thank you,

Thank you for the explanation Masami!

>-- 
>Masami Hiramatsu <mhiramat@kernel.org>

-- 

Thanks,
Sasha