[PATCH -tip 1/2] [RESEND] kprobes: Move enable/disable_kprobe() out from debugfs code

[PATCH -tip 1/2] [RESEND] kprobes: Move enable/disable_kprobe() out from debugfs code

[Masami Hiramatsu]

Move enable/disable_kprobe() API out from debugfs related code,
because these interfaces are not related to debugfs interface.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Acked-by:  Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Acked-by: Tony Luck <tony.luck@intel.com>
Cc: Ingo Molnar <mingo@elte.hu>
---

 kernel/kprobes.c |  132 +++++++++++++++++++++++++++---------------------------
 1 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0ed46f3..282035f 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1588,6 +1588,72 @@ static void __kprobes kill_kprobe(struct kprobe *p)
 	arch_remove_kprobe(p);
 }
 
+/* Disable one kprobe */
+int __kprobes disable_kprobe(struct kprobe *kp)
+{
+	int ret = 0;
+	struct kprobe *p;
+
+	mutex_lock(&kprobe_mutex);
+
+	/* Check whether specified probe is valid. */
+	p = __get_valid_kprobe(kp);
+	if (unlikely(p == NULL)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* If the probe is already disabled (or gone), just return */
+	if (kprobe_disabled(kp))
+		goto out;
+
+	kp->flags |= KPROBE_FLAG_DISABLED;
+	if (p != kp)
+		/* When kp != p, p is always enabled. */
+		try_to_disable_aggr_kprobe(p);
+
+	if (!kprobes_all_disarmed && kprobe_disabled(p))
+		disarm_kprobe(p);
+out:
+	mutex_unlock(&kprobe_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(disable_kprobe);
+
+/* Enable one kprobe */
+int __kprobes enable_kprobe(struct kprobe *kp)
+{
+	int ret = 0;
+	struct kprobe *p;
+
+	mutex_lock(&kprobe_mutex);
+
+	/* Check whether specified probe is valid. */
+	p = __get_valid_kprobe(kp);
+	if (unlikely(p == NULL)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (kprobe_gone(kp)) {
+		/* This kprobe has gone, we couldn't enable it. */
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (p != kp)
+		kp->flags &= ~KPROBE_FLAG_DISABLED;
+
+	if (!kprobes_all_disarmed && kprobe_disabled(p)) {
+		p->flags &= ~KPROBE_FLAG_DISABLED;
+		arm_kprobe(p);
+	}
+out:
+	mutex_unlock(&kprobe_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(enable_kprobe);
+
 void __kprobes dump_kprobe(struct kprobe *kp)
 {
 	printk(KERN_WARNING "Dumping kprobe:\n");
@@ -1805,72 +1871,6 @@ static const struct file_operations debugfs_kprobes_operations = {
 	.release        = seq_release,
 };
 
-/* Disable one kprobe */
-int __kprobes disable_kprobe(struct kprobe *kp)
-{
-	int ret = 0;
-	struct kprobe *p;
-
-	mutex_lock(&kprobe_mutex);
-
-	/* Check whether specified probe is valid. */
-	p = __get_valid_kprobe(kp);
-	if (unlikely(p == NULL)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* If the probe is already disabled (or gone), just return */
-	if (kprobe_disabled(kp))
-		goto out;
-
-	kp->flags |= KPROBE_FLAG_DISABLED;
-	if (p != kp)
-		/* When kp != p, p is always enabled. */
-		try_to_disable_aggr_kprobe(p);
-
-	if (!kprobes_all_disarmed && kprobe_disabled(p))
-		disarm_kprobe(p);
-out:
-	mutex_unlock(&kprobe_mutex);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(disable_kprobe);
-
-/* Enable one kprobe */
-int __kprobes enable_kprobe(struct kprobe *kp)
-{
-	int ret = 0;
-	struct kprobe *p;
-
-	mutex_lock(&kprobe_mutex);
-
-	/* Check whether specified probe is valid. */
-	p = __get_valid_kprobe(kp);
-	if (unlikely(p == NULL)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (kprobe_gone(kp)) {
-		/* This kprobe has gone, we couldn't enable it. */
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (p != kp)
-		kp->flags &= ~KPROBE_FLAG_DISABLED;
-
-	if (!kprobes_all_disarmed && kprobe_disabled(p)) {
-		p->flags &= ~KPROBE_FLAG_DISABLED;
-		arm_kprobe(p);
-	}
-out:
-	mutex_unlock(&kprobe_mutex);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(enable_kprobe);
-
 static void __kprobes arm_all_kprobes(void)
 {
 	struct hlist_head *head;


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

[PATCH -tip 2/2] [BUGFIX] kprobes/x86: Fix removed int3 checking order

[Masami Hiramatsu]

Fix kprobe/x86 to check removed int3 when failing to get kprobe
from hlist. Since we have a time window between checking int3
exists on probed address and getting kprobe on that address,
we can have following senario.
-------
CPU1                     CPU2
hit int3
check int3 exists
                         remove int3
                         remove kprobe from hlist
get kprobe from hlist
no kprobe->OOPS!
-------
This patch moves int3 checking if there is no kprobe on that
address for fixing this problem as follows;
------
CPU1                     CPU2
hit int3
                         remove int3
                         remove kprobe from hlist
get kprobe from hlist
no kprobe->check int3 exists
         ->rollback&retry
------

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Ingo Molnar <mingo@elte.hu>
---

 arch/x86/kernel/kprobes.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index f2f56c0..345a4b1 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -542,20 +542,6 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 	struct kprobe_ctlblk *kcb;
 
 	addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
-	if (*addr != BREAKPOINT_INSTRUCTION) {
-		/*
-		 * The breakpoint instruction was removed right
-		 * after we hit it.  Another cpu has removed
-		 * either a probepoint or a debugger breakpoint
-		 * at this address.  In either case, no further
-		 * handling of this interrupt is appropriate.
-		 * Back up over the (now missing) int3 and run
-		 * the original instruction.
-		 */
-		regs->ip = (unsigned long)addr;
-		return 1;
-	}
-
 	/*
 	 * We don't want to be preempted for the entire
 	 * duration of kprobe processing. We conditionally
@@ -587,6 +573,19 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 				setup_singlestep(p, regs, kcb, 0);
 			return 1;
 		}
+	} else if (*addr != BREAKPOINT_INSTRUCTION) {
+		/*
+		 * The breakpoint instruction was removed right
+		 * after we hit it.  Another cpu has removed
+		 * either a probepoint or a debugger breakpoint
+		 * at this address.  In either case, no further
+		 * handling of this interrupt is appropriate.
+		 * Back up over the (now missing) int3 and run
+		 * the original instruction.
+		 */
+		regs->ip = (unsigned long)addr;
+		preempt_enable_no_resched();
+		return 1;
 	} else if (kprobe_running()) {
 		p = __get_cpu_var(current_kprobe);
 		if (p->break_handler && p->break_handler(p, regs)) {


-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

Re: [PATCH -tip 2/2] [BUGFIX] kprobes/x86: Fix removed int3 checking order

[Ananth N Mavinakayanahalli]

On Tue, Apr 27, 2010 at 06:33:49PM -0400, Masami Hiramatsu wrote:
> Fix kprobe/x86 to check removed int3 when failing to get kprobe
> from hlist. Since we have a time window between checking int3
> exists on probed address and getting kprobe on that address,
> we can have following senario.
> -------
> CPU1                     CPU2
> hit int3
> check int3 exists
>                          remove int3
>                          remove kprobe from hlist
> get kprobe from hlist
> no kprobe->OOPS!
> -------

Do you have a testcase for this issue?

> This patch moves int3 checking if there is no kprobe on that
> address for fixing this problem as follows;
> ------
> CPU1                     CPU2
> hit int3
>                          remove int3
>                          remove kprobe from hlist
> get kprobe from hlist
> no kprobe->check int3 exists
>          ->rollback&retry
> ------

You may also want to fix up the comment on top of kprobe_handler() about
the interrupt gate as its only true for x86_32 and not x86_64, right?

> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Dave Anderson <anderson@redhat.com>
> Cc: Ingo Molnar <mingo@elte.hu>

Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Re: [PATCH -tip 2/2] [BUGFIX] kprobes/x86: Fix removed int3 checking order

[Masami Hiramatsu]

Ananth N Mavinakayanahalli wrote:
> On Tue, Apr 27, 2010 at 06:33:49PM -0400, Masami Hiramatsu wrote:
>> Fix kprobe/x86 to check removed int3 when failing to get kprobe
>> from hlist. Since we have a time window between checking int3
>> exists on probed address and getting kprobe on that address,
>> we can have following senario.
>> -------
>> CPU1                     CPU2
>> hit int3
>> check int3 exists
>>                          remove int3
>>                          remove kprobe from hlist
>> get kprobe from hlist
>> no kprobe->OOPS!
>> -------
> 
> Do you have a testcase for this issue?

I heard this issue was found by systemtap team on stable kernel(means
no jump optimization). Their testsuites caused an oops (but not 100%
reproducible) with "pr10854" testcase, which registers over 5000
probes at once and removes it soon.

>> This patch moves int3 checking if there is no kprobe on that
>> address for fixing this problem as follows;
>> ------
>> CPU1                     CPU2
>> hit int3
>>                          remove int3
>>                          remove kprobe from hlist
>> get kprobe from hlist
>> no kprobe->check int3 exists
>>          ->rollback&retry
>> ------
> 
> You may also want to fix up the comment on top of kprobe_handler() about
> the interrupt gate as its only true for x86_32 and not x86_64, right?

Hmm, I couldn't find it, could you tell me more details?
(and maybe, it's another issue)

what I could find is int3 handler is registered as interrupt gate
on both of x86-32/64.

void __init trap_init(void)
{
...
        /* int3 can be called from all */
        set_system_intr_gate_ist(3, &int3, DEBUG_STACK);


> 
>> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
>> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
>> Cc: Dave Anderson <anderson@redhat.com>
>> Cc: Ingo Molnar <mingo@elte.hu>
> 
> Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>

Thank you,

-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

Re: [PATCH -tip 2/2] [BUGFIX] kprobes/x86: Fix removed int3 checking order

[Ananth N Mavinakayanahalli]

On Wed, Apr 28, 2010 at 11:39:40AM -0400, Masami Hiramatsu wrote:
> Ananth N Mavinakayanahalli wrote:
> > On Tue, Apr 27, 2010 at 06:33:49PM -0400, Masami Hiramatsu wrote:

...

> > You may also want to fix up the comment on top of kprobe_handler() about
> > the interrupt gate as its only true for x86_32 and not x86_64, right?
> 
> Hmm, I couldn't find it, could you tell me more details?
> (and maybe, it's another issue)
> 
> what I could find is int3 handler is registered as interrupt gate
> on both of x86-32/64.
> 
> void __init trap_init(void)
> {
> ...
>         /* int3 can be called from all */
>         set_system_intr_gate_ist(3, &int3, DEBUG_STACK);

Ah, never mind. I was wrong. IIRC, before the 32-64 bit merger, 64bit
still used a trap gate.

Ananth

[tip:perf/urgent] kprobes/x86: Fix removed int3 checking order

[tip-bot for Masami Hiramatsu]

Commit-ID:  51a66680aa450c81a1f31570700b6ade4886c841
Gitweb:     http://git.kernel.org/tip/51a66680aa450c81a1f31570700b6ade4886c841
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Tue, 27 Apr 2010 18:33:49 -0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 8 May 2010 13:19:29 +0200

kprobes/x86: Fix removed int3 checking order

Fix kprobe/x86 to check removed int3 when failing to get kprobe
from hlist. Since we have a time window between checking int3
exists on probed address and getting kprobe on that address,
we can have following scenario:

 -------
 CPU1                     CPU2
 hit int3
 check int3 exists
                          remove int3
                          remove kprobe from hlist
 get kprobe from hlist
 no kprobe->OOPS!
 -------

This patch moves int3 checking if there is no kprobe on that
address for fixing this problem as follows:

 ------
 CPU1                     CPU2
 hit int3
                          remove int3
                          remove kprobe from hlist
 get kprobe from hlist
 no kprobe->check int3 exists
          ->rollback&retry
 ------

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: systemtap <systemtap@sources.redhat.com>
Cc: DLE <dle-develop@lists.sourceforge.net>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <20100427223348.2322.9112.stgit@localhost6.localdomain6>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/kprobes.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index b43bbae..1658efd 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -534,20 +534,6 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 	struct kprobe_ctlblk *kcb;
 
 	addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
-	if (*addr != BREAKPOINT_INSTRUCTION) {
-		/*
-		 * The breakpoint instruction was removed right
-		 * after we hit it.  Another cpu has removed
-		 * either a probepoint or a debugger breakpoint
-		 * at this address.  In either case, no further
-		 * handling of this interrupt is appropriate.
-		 * Back up over the (now missing) int3 and run
-		 * the original instruction.
-		 */
-		regs->ip = (unsigned long)addr;
-		return 1;
-	}
-
 	/*
 	 * We don't want to be preempted for the entire
 	 * duration of kprobe processing. We conditionally
@@ -579,6 +565,19 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 				setup_singlestep(p, regs, kcb, 0);
 			return 1;
 		}
+	} else if (*addr != BREAKPOINT_INSTRUCTION) {
+		/*
+		 * The breakpoint instruction was removed right
+		 * after we hit it.  Another cpu has removed
+		 * either a probepoint or a debugger breakpoint
+		 * at this address.  In either case, no further
+		 * handling of this interrupt is appropriate.
+		 * Back up over the (now missing) int3 and run
+		 * the original instruction.
+		 */
+		regs->ip = (unsigned long)addr;
+		preempt_enable_no_resched();
+		return 1;
 	} else if (kprobe_running()) {
 		p = __get_cpu_var(current_kprobe);
 		if (p->break_handler && p->break_handler(p, regs)) {

Re: [PATCH -tip 1/2] [RESEND] kprobes: Move enable/disable_kprobe() out from debugfs code

[Masami Hiramatsu]

Masami Hiramatsu wrote:
> Move enable/disable_kprobe() API out from debugfs related code,
> because these interfaces are not related to debugfs interface.

Hi Ingo,

Could you also merge this fix too?
This fixes a compiler warning when CONFIG_KPROBES && !CONFIG_DEBUG_FS.

Thank you,

> 
> Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
> Acked-by:  Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Acked-by: Tony Luck <tony.luck@intel.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> ---
> 
>  kernel/kprobes.c |  132 +++++++++++++++++++++++++++---------------------------
>  1 files changed, 66 insertions(+), 66 deletions(-)
> 
> diff --git a/kernel/kprobes.c b/kernel/kprobes.c
> index 0ed46f3..282035f 100644
> --- a/kernel/kprobes.c
> +++ b/kernel/kprobes.c
> @@ -1588,6 +1588,72 @@ static void __kprobes kill_kprobe(struct kprobe *p)
>  	arch_remove_kprobe(p);
>  }
>  
> +/* Disable one kprobe */
> +int __kprobes disable_kprobe(struct kprobe *kp)
> +{
> +	int ret = 0;
> +	struct kprobe *p;
> +
> +	mutex_lock(&kprobe_mutex);
> +
> +	/* Check whether specified probe is valid. */
> +	p = __get_valid_kprobe(kp);
> +	if (unlikely(p == NULL)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* If the probe is already disabled (or gone), just return */
> +	if (kprobe_disabled(kp))
> +		goto out;
> +
> +	kp->flags |= KPROBE_FLAG_DISABLED;
> +	if (p != kp)
> +		/* When kp != p, p is always enabled. */
> +		try_to_disable_aggr_kprobe(p);
> +
> +	if (!kprobes_all_disarmed && kprobe_disabled(p))
> +		disarm_kprobe(p);
> +out:
> +	mutex_unlock(&kprobe_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(disable_kprobe);
> +
> +/* Enable one kprobe */
> +int __kprobes enable_kprobe(struct kprobe *kp)
> +{
> +	int ret = 0;
> +	struct kprobe *p;
> +
> +	mutex_lock(&kprobe_mutex);
> +
> +	/* Check whether specified probe is valid. */
> +	p = __get_valid_kprobe(kp);
> +	if (unlikely(p == NULL)) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (kprobe_gone(kp)) {
> +		/* This kprobe has gone, we couldn't enable it. */
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (p != kp)
> +		kp->flags &= ~KPROBE_FLAG_DISABLED;
> +
> +	if (!kprobes_all_disarmed && kprobe_disabled(p)) {
> +		p->flags &= ~KPROBE_FLAG_DISABLED;
> +		arm_kprobe(p);
> +	}
> +out:
> +	mutex_unlock(&kprobe_mutex);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(enable_kprobe);
> +
>  void __kprobes dump_kprobe(struct kprobe *kp)
>  {
>  	printk(KERN_WARNING "Dumping kprobe:\n");
> @@ -1805,72 +1871,6 @@ static const struct file_operations debugfs_kprobes_operations = {
>  	.release        = seq_release,
>  };
>  
> -/* Disable one kprobe */
> -int __kprobes disable_kprobe(struct kprobe *kp)
> -{
> -	int ret = 0;
> -	struct kprobe *p;
> -
> -	mutex_lock(&kprobe_mutex);
> -
> -	/* Check whether specified probe is valid. */
> -	p = __get_valid_kprobe(kp);
> -	if (unlikely(p == NULL)) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	/* If the probe is already disabled (or gone), just return */
> -	if (kprobe_disabled(kp))
> -		goto out;
> -
> -	kp->flags |= KPROBE_FLAG_DISABLED;
> -	if (p != kp)
> -		/* When kp != p, p is always enabled. */
> -		try_to_disable_aggr_kprobe(p);
> -
> -	if (!kprobes_all_disarmed && kprobe_disabled(p))
> -		disarm_kprobe(p);
> -out:
> -	mutex_unlock(&kprobe_mutex);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(disable_kprobe);
> -
> -/* Enable one kprobe */
> -int __kprobes enable_kprobe(struct kprobe *kp)
> -{
> -	int ret = 0;
> -	struct kprobe *p;
> -
> -	mutex_lock(&kprobe_mutex);
> -
> -	/* Check whether specified probe is valid. */
> -	p = __get_valid_kprobe(kp);
> -	if (unlikely(p == NULL)) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	if (kprobe_gone(kp)) {
> -		/* This kprobe has gone, we couldn't enable it. */
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	if (p != kp)
> -		kp->flags &= ~KPROBE_FLAG_DISABLED;
> -
> -	if (!kprobes_all_disarmed && kprobe_disabled(p)) {
> -		p->flags &= ~KPROBE_FLAG_DISABLED;
> -		arm_kprobe(p);
> -	}
> -out:
> -	mutex_unlock(&kprobe_mutex);
> -	return ret;
> -}
> -EXPORT_SYMBOL_GPL(enable_kprobe);
> -
>  static void __kprobes arm_all_kprobes(void)
>  {
>  	struct hlist_head *head;
> 
> 

-- 
Masami Hiramatsu
e-mail: mhiramat@redhat.com

Re: [PATCH -tip 1/2] [RESEND] kprobes: Move enable/disable_kprobe() out from debugfs code

[Ingo Molnar]

* Masami Hiramatsu <mhiramat@redhat.com> wrote:

> Masami Hiramatsu wrote:
> > Move enable/disable_kprobe() API out from debugfs related code,
> > because these interfaces are not related to debugfs interface.
> 
> Hi Ingo,
> 
> Could you also merge this fix too?
> This fixes a compiler warning when CONFIG_KPROBES && !CONFIG_DEBUG_FS.

Sure, i've applied it to the v2.6.35 queue.

	Ingo

[tip:perf/core] kprobes: Move enable/disable_kprobe() out from debugfs code

[tip-bot for Masami Hiramatsu]

Commit-ID:  c0614829c16ab9d31f1b7d40516decfbf3d32102
Gitweb:     http://git.kernel.org/tip/c0614829c16ab9d31f1b7d40516decfbf3d32102
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Tue, 27 Apr 2010 18:33:12 -0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Sat, 8 May 2010 18:08:30 +0200

kprobes: Move enable/disable_kprobe() out from debugfs code

Move enable/disable_kprobe() API out from debugfs related code,
because these interfaces are not related to debugfs interface.

This fixes a compiler warning.

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Acked-by:  Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Acked-by: Tony Luck <tony.luck@intel.com>
Cc: systemtap <systemtap@sources.redhat.com>
Cc: DLE <dle-develop@lists.sourceforge.net>
LKML-Reference: <20100427223312.2322.60512.stgit@localhost6.localdomain6>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/kprobes.c |  132 +++++++++++++++++++++++++++---------------------------
 1 files changed, 66 insertions(+), 66 deletions(-)

diff --git a/kernel/kprobes.c b/kernel/kprobes.c
index 0ed46f3..282035f 100644
--- a/kernel/kprobes.c
+++ b/kernel/kprobes.c
@@ -1588,6 +1588,72 @@ static void __kprobes kill_kprobe(struct kprobe *p)
 	arch_remove_kprobe(p);
 }
 
+/* Disable one kprobe */
+int __kprobes disable_kprobe(struct kprobe *kp)
+{
+	int ret = 0;
+	struct kprobe *p;
+
+	mutex_lock(&kprobe_mutex);
+
+	/* Check whether specified probe is valid. */
+	p = __get_valid_kprobe(kp);
+	if (unlikely(p == NULL)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* If the probe is already disabled (or gone), just return */
+	if (kprobe_disabled(kp))
+		goto out;
+
+	kp->flags |= KPROBE_FLAG_DISABLED;
+	if (p != kp)
+		/* When kp != p, p is always enabled. */
+		try_to_disable_aggr_kprobe(p);
+
+	if (!kprobes_all_disarmed && kprobe_disabled(p))
+		disarm_kprobe(p);
+out:
+	mutex_unlock(&kprobe_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(disable_kprobe);
+
+/* Enable one kprobe */
+int __kprobes enable_kprobe(struct kprobe *kp)
+{
+	int ret = 0;
+	struct kprobe *p;
+
+	mutex_lock(&kprobe_mutex);
+
+	/* Check whether specified probe is valid. */
+	p = __get_valid_kprobe(kp);
+	if (unlikely(p == NULL)) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (kprobe_gone(kp)) {
+		/* This kprobe has gone, we couldn't enable it. */
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (p != kp)
+		kp->flags &= ~KPROBE_FLAG_DISABLED;
+
+	if (!kprobes_all_disarmed && kprobe_disabled(p)) {
+		p->flags &= ~KPROBE_FLAG_DISABLED;
+		arm_kprobe(p);
+	}
+out:
+	mutex_unlock(&kprobe_mutex);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(enable_kprobe);
+
 void __kprobes dump_kprobe(struct kprobe *kp)
 {
 	printk(KERN_WARNING "Dumping kprobe:\n");
@@ -1805,72 +1871,6 @@ static const struct file_operations debugfs_kprobes_operations = {
 	.release        = seq_release,
 };
 
-/* Disable one kprobe */
-int __kprobes disable_kprobe(struct kprobe *kp)
-{
-	int ret = 0;
-	struct kprobe *p;
-
-	mutex_lock(&kprobe_mutex);
-
-	/* Check whether specified probe is valid. */
-	p = __get_valid_kprobe(kp);
-	if (unlikely(p == NULL)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* If the probe is already disabled (or gone), just return */
-	if (kprobe_disabled(kp))
-		goto out;
-
-	kp->flags |= KPROBE_FLAG_DISABLED;
-	if (p != kp)
-		/* When kp != p, p is always enabled. */
-		try_to_disable_aggr_kprobe(p);
-
-	if (!kprobes_all_disarmed && kprobe_disabled(p))
-		disarm_kprobe(p);
-out:
-	mutex_unlock(&kprobe_mutex);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(disable_kprobe);
-
-/* Enable one kprobe */
-int __kprobes enable_kprobe(struct kprobe *kp)
-{
-	int ret = 0;
-	struct kprobe *p;
-
-	mutex_lock(&kprobe_mutex);
-
-	/* Check whether specified probe is valid. */
-	p = __get_valid_kprobe(kp);
-	if (unlikely(p == NULL)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (kprobe_gone(kp)) {
-		/* This kprobe has gone, we couldn't enable it. */
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (p != kp)
-		kp->flags &= ~KPROBE_FLAG_DISABLED;
-
-	if (!kprobes_all_disarmed && kprobe_disabled(p)) {
-		p->flags &= ~KPROBE_FLAG_DISABLED;
-		arm_kprobe(p);
-	}
-out:
-	mutex_unlock(&kprobe_mutex);
-	return ret;
-}
-EXPORT_SYMBOL_GPL(enable_kprobe);
-
 static void __kprobes arm_all_kprobes(void)
 {
 	struct hlist_head *head;

[tip:perf/urgent] kprobes/x86: Fix removed int3 checking order

[tip-bot for Masami Hiramatsu]

Commit-ID:  829e92458532b1dbfeb972435d45bb060cdbf5a3
Gitweb:     http://git.kernel.org/tip/829e92458532b1dbfeb972435d45bb060cdbf5a3
Author:     Masami Hiramatsu <mhiramat@redhat.com>
AuthorDate: Tue, 27 Apr 2010 18:33:49 -0400
Committer:  Ingo Molnar <mingo@elte.hu>
CommitDate: Tue, 11 May 2010 09:14:25 +0200

kprobes/x86: Fix removed int3 checking order

Fix kprobe/x86 to check removed int3 when failing to get kprobe
from hlist. Since we have a time window between checking int3
exists on probed address and getting kprobe on that address,
we can have following scenario:

 -------
 CPU1                     CPU2
 hit int3
 check int3 exists
                          remove int3
                          remove kprobe from hlist
 get kprobe from hlist
 no kprobe->OOPS!
 -------

This patch moves int3 checking if there is no kprobe on that
address for fixing this problem as follows:

 ------
 CPU1                     CPU2
 hit int3
                          remove int3
                          remove kprobe from hlist
 get kprobe from hlist
 no kprobe->check int3 exists
          ->rollback&retry
 ------

Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com>
Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: systemtap <systemtap@sources.redhat.com>
Cc: DLE <dle-develop@lists.sourceforge.net>
Cc: Dave Anderson <anderson@redhat.com>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
LKML-Reference: <20100427223348.2322.9112.stgit@localhost6.localdomain6>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 arch/x86/kernel/kprobes.c |   27 +++++++++++++--------------
 1 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/arch/x86/kernel/kprobes.c b/arch/x86/kernel/kprobes.c
index b43bbae..1658efd 100644
--- a/arch/x86/kernel/kprobes.c
+++ b/arch/x86/kernel/kprobes.c
@@ -534,20 +534,6 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 	struct kprobe_ctlblk *kcb;
 
 	addr = (kprobe_opcode_t *)(regs->ip - sizeof(kprobe_opcode_t));
-	if (*addr != BREAKPOINT_INSTRUCTION) {
-		/*
-		 * The breakpoint instruction was removed right
-		 * after we hit it.  Another cpu has removed
-		 * either a probepoint or a debugger breakpoint
-		 * at this address.  In either case, no further
-		 * handling of this interrupt is appropriate.
-		 * Back up over the (now missing) int3 and run
-		 * the original instruction.
-		 */
-		regs->ip = (unsigned long)addr;
-		return 1;
-	}
-
 	/*
 	 * We don't want to be preempted for the entire
 	 * duration of kprobe processing. We conditionally
@@ -579,6 +565,19 @@ static int __kprobes kprobe_handler(struct pt_regs *regs)
 				setup_singlestep(p, regs, kcb, 0);
 			return 1;
 		}
+	} else if (*addr != BREAKPOINT_INSTRUCTION) {
+		/*
+		 * The breakpoint instruction was removed right
+		 * after we hit it.  Another cpu has removed
+		 * either a probepoint or a debugger breakpoint
+		 * at this address.  In either case, no further
+		 * handling of this interrupt is appropriate.
+		 * Back up over the (now missing) int3 and run
+		 * the original instruction.
+		 */
+		regs->ip = (unsigned long)addr;
+		preempt_enable_no_resched();
+		return 1;
 	} else if (kprobe_running()) {
 		p = __get_cpu_var(current_kprobe);
 		if (p->break_handler && p->break_handler(p, regs)) {