linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined
@ 2013-03-14 11:52 Masami Hiramatsu
  2013-03-14 11:52 ` [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe Masami Hiramatsu
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2013-03-14 11:52 UTC (permalink / raw)
  To: Ingo Molnar, Linus Torvalds, linux-kernel
  Cc: Timo Juhani Lindfors, Ananth N Mavinakayanahalli,
	Pavel Emelyanov, Jiri Kosina, Ingo Molnar, Nadia Yvette Chambers,
	yrl.pp-manager.tt, David S. Miller

Because hash_64() is called from the get_kprobe() inside
int3 handler, kernel causes int3 recursion and crashes if
kprobes user puts a probe on it.

Usually hash_64() is inlined into caller function, but in
some cases, it has instances by gcc's interprocedural
constant propagation.

This patch uses __always_inline instead of inline to
prevent gcc from doing such things.

Changes in v2:
 - Use __always_inline instead of using __kprobes

Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Reported-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Nadia Yvette Chambers <nyc@holomorphy.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
---
 include/linux/hash.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/hash.h b/include/linux/hash.h
index 61c97ae..f09a0ae 100644
--- a/include/linux/hash.h
+++ b/include/linux/hash.h
@@ -15,6 +15,7 @@
  */
 
 #include <asm/types.h>
+#include <linux/compiler.h>
 
 /* 2^31 + 2^29 - 2^25 + 2^22 - 2^19 - 2^16 + 1 */
 #define GOLDEN_RATIO_PRIME_32 0x9e370001UL
@@ -31,7 +32,7 @@
 #error Wordsize not 32 or 64
 #endif
 
-static inline u64 hash_64(u64 val, unsigned int bits)
+static __always_inline u64 hash_64(u64 val, unsigned int bits)
 {
 	u64 hash = val;
 


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe
  2013-03-14 11:52 [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Masami Hiramatsu
@ 2013-03-14 11:52 ` Masami Hiramatsu
  2013-03-15  4:40   ` Ananth N Mavinakayanahalli
  2013-03-18 11:11   ` [tip:perf/urgent] " tip-bot for Masami Hiramatsu
  2013-03-15  4:40 ` [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Ananth N Mavinakayanahalli
  2013-03-18 11:10 ` [tip:perf/urgent] kprobes: Make " tip-bot for Masami Hiramatsu
  2 siblings, 2 replies; 6+ messages in thread
From: Masami Hiramatsu @ 2013-03-14 11:52 UTC (permalink / raw)
  To: Ingo Molnar, Linus Torvalds, linux-kernel
  Cc: Timo Juhani Lindfors, Ananth N Mavinakayanahalli,
	yrl.pp-manager.tt, Steven Rostedt, H. Peter Anvin,
	Thomas Gleixner, David S. Miller

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.pt@hitachi.com>
Reported-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@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;
 }
 


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined
  2013-03-14 11:52 [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Masami Hiramatsu
  2013-03-14 11:52 ` [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe Masami Hiramatsu
@ 2013-03-15  4:40 ` Ananth N Mavinakayanahalli
  2013-03-18 11:10 ` [tip:perf/urgent] kprobes: Make " tip-bot for Masami Hiramatsu
  2 siblings, 0 replies; 6+ messages in thread
From: Ananth N Mavinakayanahalli @ 2013-03-15  4:40 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Linus Torvalds, linux-kernel, Timo Juhani Lindfors,
	Pavel Emelyanov, Jiri Kosina, Ingo Molnar, Nadia Yvette Chambers,
	yrl.pp-manager.tt, David S. Miller

On Thu, Mar 14, 2013 at 08:52:30PM +0900, Masami Hiramatsu wrote:
> Because hash_64() is called from the get_kprobe() inside
> int3 handler, kernel causes int3 recursion and crashes if
> kprobes user puts a probe on it.
> 
> Usually hash_64() is inlined into caller function, but in
> some cases, it has instances by gcc's interprocedural
> constant propagation.
> 
> This patch uses __always_inline instead of inline to
> prevent gcc from doing such things.
> 
> Changes in v2:
>  - Use __always_inline instead of using __kprobes
> 
> Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
> Reported-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Nadia Yvette Chambers <nyc@holomorphy.com>
> Cc: Pavel Emelyanov <xemul@parallels.com>
> Cc: Jiri Kosina <jkosina@suse.cz>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Ingo Molnar <mingo@kernel.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe
  2013-03-14 11:52 ` [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe Masami Hiramatsu
@ 2013-03-15  4:40   ` Ananth N Mavinakayanahalli
  2013-03-18 11:11   ` [tip:perf/urgent] " tip-bot for Masami Hiramatsu
  1 sibling, 0 replies; 6+ messages in thread
From: Ananth N Mavinakayanahalli @ 2013-03-15  4:40 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: Ingo Molnar, Linus Torvalds, linux-kernel, Timo Juhani Lindfors,
	yrl.pp-manager.tt, Steven Rostedt, H. Peter Anvin,
	Thomas Gleixner, David S. Miller

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.pt@hitachi.com>
> Reported-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
> Cc: "David S. Miller" <davem@davemloft.net>
> Cc: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Steven Rostedt <rostedt@goodmis.org>
> Cc: Linus Torvalds <torvalds@linux-foundation.org>

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


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [tip:perf/urgent] kprobes: Make hash_64() as always inlined
  2013-03-14 11:52 [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Masami Hiramatsu
  2013-03-14 11:52 ` [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe Masami Hiramatsu
  2013-03-15  4:40 ` [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Ananth N Mavinakayanahalli
@ 2013-03-18 11:10 ` tip-bot for Masami Hiramatsu
  2 siblings, 0 replies; 6+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2013-03-18 11:10 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, torvalds, xemul, jkosina, davem, nyc,
	ananth, masami.hiramatsu.pt, tglx, timo.lindfors

Commit-ID:  65c10553552b487a71bf5e4676743435046fae6f
Gitweb:     http://git.kernel.org/tip/65c10553552b487a71bf5e4676743435046fae6f
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Thu, 14 Mar 2013 20:52:30 +0900
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 18 Mar 2013 10:21:23 +0100

kprobes: Make hash_64() as always inlined

Because hash_64() is called from the get_kprobe() inside
int3 handler, kernel causes int3 recursion and crashes if
kprobes user puts a probe on it.

Usually hash_64() is inlined into caller function, but in
some cases, it has instances by gcc's interprocedural
constant propagation.

This patch uses __always_inline instead of inline to
prevent gcc from doing such things.

Reported-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: Pavel Emelyanov <xemul@parallels.com>
Cc: Jiri Kosina <jkosina@suse.cz>
Cc: Nadia Yvette Chambers <nyc@holomorphy.com>
Cc: yrl.pp-manager.tt@hitachi.com
Cc: David S. Miller <davem@davemloft.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20130314115230.19690.39387.stgit@mhiramat-M0-7522
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 include/linux/hash.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/linux/hash.h b/include/linux/hash.h
index 61c97ae..f09a0ae 100644
--- a/include/linux/hash.h
+++ b/include/linux/hash.h
@@ -15,6 +15,7 @@
  */
 
 #include <asm/types.h>
+#include <linux/compiler.h>
 
 /* 2^31 + 2^29 - 2^25 + 2^22 - 2^19 - 2^16 + 1 */
 #define GOLDEN_RATIO_PRIME_32 0x9e370001UL
@@ -31,7 +32,7 @@
 #error Wordsize not 32 or 64
 #endif
 
-static inline u64 hash_64(u64 val, unsigned int bits)
+static __always_inline u64 hash_64(u64 val, unsigned int bits)
 {
 	u64 hash = val;
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [tip:perf/urgent] kprobes/x86: Check Interrupt Flag modifier when registering probe
  2013-03-14 11:52 ` [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe Masami Hiramatsu
  2013-03-15  4:40   ` Ananth N Mavinakayanahalli
@ 2013-03-18 11:11   ` tip-bot for Masami Hiramatsu
  1 sibling, 0 replies; 6+ messages in thread
From: tip-bot for Masami Hiramatsu @ 2013-03-18 11:11 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, mingo, torvalds, davem, ananth,
	masami.hiramatsu.pt, rostedt, tglx, timo.lindfors

Commit-ID:  9a556ab998071457e79b319f2527642dd6e50617
Gitweb:     http://git.kernel.org/tip/9a556ab998071457e79b319f2527642dd6e50617
Author:     Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
AuthorDate: Thu, 14 Mar 2013 20:52:43 +0900
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Mon, 18 Mar 2013 10:21:23 +0100

kprobes/x86: Check Interrupt Flag modifier when registering probe

Currently kprobes check whether the copied instruction modifies
IF (interrupt flag) on each probe hit. This results not only in
introducing overhead but also involving
inat_get_opcode_attribute into the kprobes hot path, and it can
cause an infinite recursive call (and kernel panic in the end).

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

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

Reported-by: Timo Juhani Lindfors <timo.lindfors@iki.fi>
Signed-off-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Acked-by: Ananth N Mavinakayanahalli <ananth@in.ibm.com>
Cc: yrl.pp-manager.tt@hitachi.com
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: David S. Miller <davem@davemloft.net>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Link: http://lkml.kernel.org/r/20130314115242.19690.33573.stgit@mhiramat-M0-7522
Signed-off-by: Ingo Molnar <mingo@kernel.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;
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2013-03-18 11:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-03-14 11:52 [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Masami Hiramatsu
2013-03-14 11:52 ` [PATCH -tip v2 2/2] [BUGFIX] kprobes/x86: Check Interrupt Flag modifier when registering probe Masami Hiramatsu
2013-03-15  4:40   ` Ananth N Mavinakayanahalli
2013-03-18 11:11   ` [tip:perf/urgent] " tip-bot for Masami Hiramatsu
2013-03-15  4:40 ` [PATCH -tip v2 1/2] [BUGFIX] kprobes: make hash_64() as always inlined Ananth N Mavinakayanahalli
2013-03-18 11:10 ` [tip:perf/urgent] kprobes: Make " tip-bot for Masami Hiramatsu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).