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 <[email protected]>
Reported-by: Timo Juhani Lindfors <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Nadia Yvette Chambers <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
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;
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 <[email protected]>
Reported-by: Timo Juhani Lindfors <[email protected]>
Cc: "David S. Miller" <[email protected]>
Cc: Ananth N Mavinakayanahalli <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Linus Torvalds <[email protected]>
---
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;
}
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 <[email protected]>
> Reported-by: Timo Juhani Lindfors <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Nadia Yvette Chambers <[email protected]>
> Cc: Pavel Emelyanov <[email protected]>
> Cc: Jiri Kosina <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Linus Torvalds <[email protected]>
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
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 <[email protected]>
> Reported-by: Timo Juhani Lindfors <[email protected]>
> Cc: "David S. Miller" <[email protected]>
> Cc: Ananth N Mavinakayanahalli <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Steven Rostedt <[email protected]>
> Cc: Linus Torvalds <[email protected]>
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
Commit-ID: 9a556ab998071457e79b319f2527642dd6e50617
Gitweb: http://git.kernel.org/tip/9a556ab998071457e79b319f2527642dd6e50617
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Thu, 14 Mar 2013 20:52:43 +0900
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
Cc: [email protected]
Cc: Steven Rostedt <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/20130314115242.19690.33573.stgit@mhiramat-M0-7522
Signed-off-by: Ingo Molnar <[email protected]>
---
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;
}
Commit-ID: 65c10553552b487a71bf5e4676743435046fae6f
Gitweb: http://git.kernel.org/tip/65c10553552b487a71bf5e4676743435046fae6f
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Thu, 14 Mar 2013 20:52:30 +0900
Committer: Ingo Molnar <[email protected]>
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 <[email protected]>
Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Ananth N Mavinakayanahalli <[email protected]>
Cc: Pavel Emelyanov <[email protected]>
Cc: Jiri Kosina <[email protected]>
Cc: Nadia Yvette Chambers <[email protected]>
Cc: [email protected]
Cc: David S. Miller <[email protected]>
Cc: Linus Torvalds <[email protected]>
Link: http://lkml.kernel.org/r/20130314115230.19690.39387.stgit@mhiramat-M0-7522
Signed-off-by: Ingo Molnar <[email protected]>
---
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;