Re: [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe

2013-03-14 Thread Ananth N Mavinakayanahalli
On Thu, Mar 14, 2013 at 08:52:43PM +0900, Masami Hiramatsu wrote:
> Currently kprobes check whether the copied instruction modifies
> IF (interrupt flag) on each probe hit. This means not only
> introducing overhead but also involving inat_get_opcode_attribute
> into kprobes hot path, and it can cause an infinit recursive
> call (and kernel panic in the end).
> 
> Actually, since the copied instruction itself never be modified
> on the buffer, it is needless to analyze the instruction every
> probe hit.
> 
> To fix this issue, we checks it only once when registering probe
> and store the result on ainsn->if_modifier.
> 
> Signed-off-by: Masami Hiramatsu 
> Reported-by: Timo Juhani Lindfors 
> Cc: "David S. Miller" 
> Cc: Ananth N Mavinakayanahalli 
> Cc: Thomas Gleixner 
> Cc: Ingo Molnar 
> Cc: "H. Peter Anvin" 
> Cc: Steven Rostedt 
> Cc: Linus Torvalds 

Acked-by: Ananth N Mavinakayanahalli 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe

2013-03-14 Thread Masami Hiramatsu
Currently kprobes check whether the copied instruction modifies
IF (interrupt flag) on each probe hit. This means not only
introducing overhead but also involving inat_get_opcode_attribute
into kprobes hot path, and it can cause an infinit recursive
call (and kernel panic in the end).

Actually, since the copied instruction itself never be modified
on the buffer, it is needless to analyze the instruction every
probe hit.

To fix this issue, we checks it only once when registering probe
and store the result on ainsn->if_modifier.

Signed-off-by: Masami Hiramatsu 
Reported-by: Timo Juhani Lindfors 
Cc: "David S. Miller" 
Cc: Ananth N Mavinakayanahalli 
Cc: Thomas Gleixner 
Cc: Ingo Molnar 
Cc: "H. Peter Anvin" 
Cc: Steven Rostedt 
Cc: Linus Torvalds 
---
 arch/x86/include/asm/kprobes.h |1 +
 arch/x86/kernel/kprobes/core.c |5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index d3ddd17..5a6d287 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -77,6 +77,7 @@ struct arch_specific_insn {
 * a post_handler or break_handler).
 */
int boostable;
+   bool if_modifier;
 };
 
 struct arch_optimized_insn {
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 3f06e61..7bfe318 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -375,6 +375,9 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)
else
p->ainsn.boostable = -1;
 
+   /* Check whether the instruction modifies Interrupt Flag or not */
+   p->ainsn.if_modifier = is_IF_modifier(p->ainsn.insn);
+
/* Also, displacement change doesn't affect the first byte */
p->opcode = p->ainsn.insn[0];
 }
@@ -434,7 +437,7 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
__this_cpu_write(current_kprobe, p);
kcb->kprobe_saved_flags = kcb->kprobe_old_flags
= (regs->flags & (X86_EFLAGS_TF | X86_EFLAGS_IF));
-   if (is_IF_modifier(p->ainsn.insn))
+   if (p->ainsn.if_modifier)
kcb->kprobe_saved_flags &= ~X86_EFLAGS_IF;
 }
 

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


[PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe

2013-03-14 Thread Masami Hiramatsu
Currently kprobes check whether the copied instruction modifies
IF (interrupt flag) on each probe hit. This means not only
introducing overhead but also involving inat_get_opcode_attribute
into kprobes hot path, and it can cause an infinit recursive
call (and kernel panic in the end).

Actually, since the copied instruction itself never be modified
on the buffer, it is needless to analyze the instruction every
probe hit.

To fix this issue, we checks it only once when registering probe
and store the result on ainsn-if_modifier.

Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
Reported-by: Timo Juhani Lindfors timo.lindf...@iki.fi
Cc: David S. Miller da...@davemloft.net
Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com
Cc: Thomas Gleixner t...@linutronix.de
Cc: Ingo Molnar mi...@redhat.com
Cc: H. Peter Anvin h...@zytor.com
Cc: Steven Rostedt rost...@goodmis.org
Cc: Linus Torvalds torva...@linux-foundation.org
---
 arch/x86/include/asm/kprobes.h |1 +
 arch/x86/kernel/kprobes/core.c |5 -
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/x86/include/asm/kprobes.h b/arch/x86/include/asm/kprobes.h
index d3ddd17..5a6d287 100644
--- a/arch/x86/include/asm/kprobes.h
+++ b/arch/x86/include/asm/kprobes.h
@@ -77,6 +77,7 @@ struct arch_specific_insn {
 * a post_handler or break_handler).
 */
int boostable;
+   bool if_modifier;
 };
 
 struct arch_optimized_insn {
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 3f06e61..7bfe318 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -375,6 +375,9 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)
else
p-ainsn.boostable = -1;
 
+   /* Check whether the instruction modifies Interrupt Flag or not */
+   p-ainsn.if_modifier = is_IF_modifier(p-ainsn.insn);
+
/* Also, displacement change doesn't affect the first byte */
p-opcode = p-ainsn.insn[0];
 }
@@ -434,7 +437,7 @@ static void __kprobes set_current_kprobe(struct kprobe *p, 
struct pt_regs *regs,
__this_cpu_write(current_kprobe, p);
kcb-kprobe_saved_flags = kcb-kprobe_old_flags
= (regs-flags  (X86_EFLAGS_TF | X86_EFLAGS_IF));
-   if (is_IF_modifier(p-ainsn.insn))
+   if (p-ainsn.if_modifier)
kcb-kprobe_saved_flags = ~X86_EFLAGS_IF;
 }
 

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe

2013-03-14 Thread Ananth N Mavinakayanahalli
On Thu, Mar 14, 2013 at 08:52:43PM +0900, Masami Hiramatsu wrote:
 Currently kprobes check whether the copied instruction modifies
 IF (interrupt flag) on each probe hit. This means not only
 introducing overhead but also involving inat_get_opcode_attribute
 into kprobes hot path, and it can cause an infinit recursive
 call (and kernel panic in the end).
 
 Actually, since the copied instruction itself never be modified
 on the buffer, it is needless to analyze the instruction every
 probe hit.
 
 To fix this issue, we checks it only once when registering probe
 and store the result on ainsn-if_modifier.
 
 Signed-off-by: Masami Hiramatsu masami.hiramatsu...@hitachi.com
 Reported-by: Timo Juhani Lindfors timo.lindf...@iki.fi
 Cc: David S. Miller da...@davemloft.net
 Cc: Ananth N Mavinakayanahalli ana...@in.ibm.com
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Ingo Molnar mi...@redhat.com
 Cc: H. Peter Anvin h...@zytor.com
 Cc: Steven Rostedt rost...@goodmis.org
 Cc: Linus Torvalds torva...@linux-foundation.org

Acked-by: Ananth N Mavinakayanahalli ana...@in.ibm.com

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/