Subject: [PATCH -tip ] [BUGFIX] kprobes: Fix arch_prepare_kprobe to handle copy insn failures

Fix arch_prepare_kprobe() to handle failures in copy instruction
correctly. This fix is related to the previous fix: 8101376
which made __copy_instruction return an error result if failed,
but caller site was not updated to handle it. Thus, this is the
other half of the bugfix.

This fix is also related to the following bug-report:

https://bugzilla.redhat.com/show_bug.cgi?id=910649

Signed-off-by: Masami Hiramatsu <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: Steven Rostedt <[email protected]>
Cc: Ingo Molnar <[email protected]>
---
arch/x86/kernel/kprobes/core.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 9895a9a..211bce4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -365,10 +365,14 @@ int __kprobes __copy_instruction(u8 *dest, u8 *src)
return insn.length;
}

-static void __kprobes arch_copy_kprobe(struct kprobe *p)
+static int __kprobes arch_copy_kprobe(struct kprobe *p)
{
+ int ret;
+
/* Copy an instruction with recovering if other optprobe modifies it.*/
- __copy_instruction(p->ainsn.insn, p->addr);
+ ret = __copy_instruction(p->ainsn.insn, p->addr);
+ if (!ret)
+ return -EINVAL;

/*
* __copy_instruction can modify the displacement of the instruction,
@@ -384,6 +388,8 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)

/* Also, displacement change doesn't affect the first byte */
p->opcode = p->ainsn.insn[0];
+
+ return 0;
}

int __kprobes arch_prepare_kprobe(struct kprobe *p)
@@ -397,8 +403,8 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
p->ainsn.insn = get_insn_slot();
if (!p->ainsn.insn)
return -ENOMEM;
- arch_copy_kprobe(p);
- return 0;
+
+ return arch_copy_kprobe(p);
}

void __kprobes arch_arm_kprobe(struct kprobe *p)


2013-06-06 21:06:49

by Steven Rostedt

[permalink] [raw]
Subject: Re: [PATCH -tip ] [BUGFIX] kprobes: Fix arch_prepare_kprobe to handle copy insn failures

On Wed, 2013-06-05 at 12:12 +0900, Masami Hiramatsu wrote:
> Fix arch_prepare_kprobe() to handle failures in copy instruction
> correctly. This fix is related to the previous fix: 8101376
> which made __copy_instruction return an error result if failed,
> but caller site was not updated to handle it. Thus, this is the
> other half of the bugfix.
>
> This fix is also related to the following bug-report:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=910649
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Frank Ch. Eigler <[email protected]>
> Cc: Steven Rostedt <[email protected]>

Acked-by: Steven Rostedt <[email protected]>

-- Steve

> Cc: Ingo Molnar <[email protected]>
> ---
> arch/x86/kernel/kprobes/core.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>

2013-06-07 21:18:35

by Jonathan Lebon

[permalink] [raw]
Subject: Re: [PATCH -tip ] [BUGFIX] kprobes: Fix arch_prepare_kprobe to handle copy insn failures

Tested-by: Jonathan Lebon <[email protected]>

Stress-tested overnight in a VM with no problems.

Jonathan

----- Original Message -----
From: "Steven Rostedt" <[email protected]>
To: "Masami Hiramatsu" <[email protected]>
Cc: "Ingo Molnar" <[email protected]>, [email protected], "Ingo Molnar" <[email protected]>, "Frank Ch. Eigler" <[email protected]>, [email protected], "yrl pp-manager tt" <[email protected]>
Sent: Thursday, June 6, 2013 5:06:46 PM
Subject: Re: [PATCH -tip ] [BUGFIX] kprobes: Fix arch_prepare_kprobe to handle copy insn failures

On Wed, 2013-06-05 at 12:12 +0900, Masami Hiramatsu wrote:
> Fix arch_prepare_kprobe() to handle failures in copy instruction
> correctly. This fix is related to the previous fix: 8101376
> which made __copy_instruction return an error result if failed,
> but caller site was not updated to handle it. Thus, this is the
> other half of the bugfix.
>
> This fix is also related to the following bug-report:
>
> https://bugzilla.redhat.com/show_bug.cgi?id=910649
>
> Signed-off-by: Masami Hiramatsu <[email protected]>
> Cc: Frank Ch. Eigler <[email protected]>
> Cc: Steven Rostedt <[email protected]>

Acked-by: Steven Rostedt <[email protected]>

-- Steve

> Cc: Ingo Molnar <[email protected]>
> ---
> arch/x86/kernel/kprobes/core.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>

Subject: [tip:perf/urgent] kprobes: Fix arch_prepare_kprobe to handle copy insn failures

Commit-ID: 003002e04ed38618fc37b92ba128f5ca79d39f4f
Gitweb: http://git.kernel.org/tip/003002e04ed38618fc37b92ba128f5ca79d39f4f
Author: Masami Hiramatsu <[email protected]>
AuthorDate: Wed, 5 Jun 2013 12:12:16 +0900
Committer: Ingo Molnar <[email protected]>
CommitDate: Thu, 20 Jun 2013 14:25:48 +0200

kprobes: Fix arch_prepare_kprobe to handle copy insn failures

Fix arch_prepare_kprobe() to handle failures in copy instruction
correctly. This fix is related to the previous fix: 8101376
which made __copy_instruction return an error result if failed,
but caller site was not updated to handle it. Thus, this is the
other half of the bugfix.

This fix is also related to the following bug-report:

https://bugzilla.redhat.com/show_bug.cgi?id=910649

Signed-off-by: Masami Hiramatsu <[email protected]>
Acked-by: Steven Rostedt <[email protected]>
Tested-by: Jonathan Lebon <[email protected]>
Cc: Frank Ch. Eigler <[email protected]>
Cc: [email protected]
Cc: [email protected]
Link: http://lkml.kernel.org/r/20130605031216.15285.2001.stgit@mhiramat-M0-7522
Signed-off-by: Ingo Molnar <[email protected]>
---
arch/x86/kernel/kprobes/core.c | 14 ++++++++++----
1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 9895a9a..211bce4 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -365,10 +365,14 @@ int __kprobes __copy_instruction(u8 *dest, u8 *src)
return insn.length;
}

-static void __kprobes arch_copy_kprobe(struct kprobe *p)
+static int __kprobes arch_copy_kprobe(struct kprobe *p)
{
+ int ret;
+
/* Copy an instruction with recovering if other optprobe modifies it.*/
- __copy_instruction(p->ainsn.insn, p->addr);
+ ret = __copy_instruction(p->ainsn.insn, p->addr);
+ if (!ret)
+ return -EINVAL;

/*
* __copy_instruction can modify the displacement of the instruction,
@@ -384,6 +388,8 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)

/* Also, displacement change doesn't affect the first byte */
p->opcode = p->ainsn.insn[0];
+
+ return 0;
}

int __kprobes arch_prepare_kprobe(struct kprobe *p)
@@ -397,8 +403,8 @@ int __kprobes arch_prepare_kprobe(struct kprobe *p)
p->ainsn.insn = get_insn_slot();
if (!p->ainsn.insn)
return -ENOMEM;
- arch_copy_kprobe(p);
- return 0;
+
+ return arch_copy_kprobe(p);
}

void __kprobes arch_arm_kprobe(struct kprobe *p)