Move enable/disable_kprobe() API out from debugfs related code,
because these interfaces are not related to debugfs interface.
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
Acked-by: Tony Luck <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
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: [email protected]
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 <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Dave Anderson <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
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: [email protected]
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 <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Dave Anderson <[email protected]>
> Cc: Ingo Molnar <[email protected]>
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
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 <[email protected]>
>> Cc: Ananth N Mavinakayanahalli <[email protected]>
>> Cc: Dave Anderson <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>
> Acked-by: Ananth N Mavinakayanahalli <[email protected]>
Thank you,
--
Masami Hiramatsu
e-mail: [email protected]
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