* [PATCH] kprobes/x86: Return correct length in __copy_instruction()
@ 2015-03-09 11:19 Eugene Shatokhin
2015-03-10 2:59 ` Masami Hiramatsu
0 siblings, 1 reply; 6+ messages in thread
From: Eugene Shatokhin @ 2015-03-09 11:19 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: LKML, Eugene Shatokhin
On x86-64, __copy_instruction() always returns 0 (error) if the
instruction uses %rip-relative addressing. This is because
kernel_insn_init() is called the second time for 'insn' instance
in such cases and sets all its fields to 0.
Because of this, trying to place a Kprobe on such instruction will
fail, register_kprobe() will return -EINVAL.
This patch fixes the problem.
Signed-off-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru>
---
arch/x86/kernel/kprobes/core.c | 9 ++++++---
1 file changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
index 4e3d5a9..03189d8 100644
--- a/arch/x86/kernel/kprobes/core.c
+++ b/arch/x86/kernel/kprobes/core.c
@@ -354,6 +354,7 @@ int __copy_instruction(u8 *dest, u8 *src)
{
struct insn insn;
kprobe_opcode_t buf[MAX_INSN_SIZE];
+ int length;
unsigned long recovered_insn =
recover_probed_instruction(buf, (unsigned long)src);
@@ -361,16 +362,18 @@ int __copy_instruction(u8 *dest, u8 *src)
return 0;
kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
insn_get_length(&insn);
+ length = insn.length;
+
/* Another subsystem puts a breakpoint, failed to recover */
if (insn.opcode.bytes[0] == BREAKPOINT_INSTRUCTION)
return 0;
- memcpy(dest, insn.kaddr, insn.length);
+ memcpy(dest, insn.kaddr, length);
#ifdef CONFIG_X86_64
if (insn_rip_relative(&insn)) {
s64 newdisp;
u8 *disp;
- kernel_insn_init(&insn, dest, insn.length);
+ kernel_insn_init(&insn, dest, length);
insn_get_displacement(&insn);
/*
* The copied instruction uses the %rip-relative addressing
@@ -394,7 +397,7 @@ int __copy_instruction(u8 *dest, u8 *src)
*(s32 *) disp = (s32) newdisp;
}
#endif
- return insn.length;
+ return length;
}
static int arch_copy_kprobe(struct kprobe *p)
--
1.7.11.3
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] kprobes/x86: Return correct length in __copy_instruction()
2015-03-09 11:19 [PATCH] kprobes/x86: Return correct length in __copy_instruction() Eugene Shatokhin
@ 2015-03-10 2:59 ` Masami Hiramatsu
2015-03-16 15:03 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2015-03-10 2:59 UTC (permalink / raw)
To: Eugene Shatokhin, Ingo Molnar; +Cc: LKML, Ingo Molnar
(2015/03/09 20:19), Eugene Shatokhin wrote:
> On x86-64, __copy_instruction() always returns 0 (error) if the
> instruction uses %rip-relative addressing. This is because
> kernel_insn_init() is called the second time for 'insn' instance
> in such cases and sets all its fields to 0.
>
> Because of this, trying to place a Kprobe on such instruction will
> fail, register_kprobe() will return -EINVAL.
Oops, good catch!
Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Thank you!
>
> This patch fixes the problem.
>
> Signed-off-by: Eugene Shatokhin <eugene.shatokhin@rosalab.ru>
> ---
> arch/x86/kernel/kprobes/core.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kernel/kprobes/core.c b/arch/x86/kernel/kprobes/core.c
> index 4e3d5a9..03189d8 100644
> --- a/arch/x86/kernel/kprobes/core.c
> +++ b/arch/x86/kernel/kprobes/core.c
> @@ -354,6 +354,7 @@ int __copy_instruction(u8 *dest, u8 *src)
> {
> struct insn insn;
> kprobe_opcode_t buf[MAX_INSN_SIZE];
> + int length;
> unsigned long recovered_insn =
> recover_probed_instruction(buf, (unsigned long)src);
>
> @@ -361,16 +362,18 @@ int __copy_instruction(u8 *dest, u8 *src)
> return 0;
> kernel_insn_init(&insn, (void *)recovered_insn, MAX_INSN_SIZE);
> insn_get_length(&insn);
> + length = insn.length;
> +
> /* Another subsystem puts a breakpoint, failed to recover */
> if (insn.opcode.bytes[0] == BREAKPOINT_INSTRUCTION)
> return 0;
> - memcpy(dest, insn.kaddr, insn.length);
> + memcpy(dest, insn.kaddr, length);
>
> #ifdef CONFIG_X86_64
> if (insn_rip_relative(&insn)) {
> s64 newdisp;
> u8 *disp;
> - kernel_insn_init(&insn, dest, insn.length);
> + kernel_insn_init(&insn, dest, length);
> insn_get_displacement(&insn);
> /*
> * The copied instruction uses the %rip-relative addressing
> @@ -394,7 +397,7 @@ int __copy_instruction(u8 *dest, u8 *src)
> *(s32 *) disp = (s32) newdisp;
> }
> #endif
> - return insn.length;
> + return length;
> }
>
> static int arch_copy_kprobe(struct kprobe *p)
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kprobes/x86: Return correct length in __copy_instruction()
2015-03-10 2:59 ` Masami Hiramatsu
@ 2015-03-16 15:03 ` Ingo Molnar
2015-03-17 9:45 ` Masami Hiramatsu
0 siblings, 1 reply; 6+ messages in thread
From: Ingo Molnar @ 2015-03-16 15:03 UTC (permalink / raw)
To: Masami Hiramatsu; +Cc: Eugene Shatokhin, Ingo Molnar, LKML, Ingo Molnar
* Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
> (2015/03/09 20:19), Eugene Shatokhin wrote:
> > On x86-64, __copy_instruction() always returns 0 (error) if the
> > instruction uses %rip-relative addressing. This is because
> > kernel_insn_init() is called the second time for 'insn' instance
> > in such cases and sets all its fields to 0.
> >
> > Because of this, trying to place a Kprobe on such instruction will
> > fail, register_kprobe() will return -EINVAL.
>
> Oops, good catch!
>
> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
Mind resending it with your SOB, as I wasn't Cc:-ed on the original
submission?
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kprobes/x86: Return correct length in __copy_instruction()
2015-03-16 15:03 ` Ingo Molnar
@ 2015-03-17 9:45 ` Masami Hiramatsu
2015-04-27 11:03 ` Eugene Shatokhin
0 siblings, 1 reply; 6+ messages in thread
From: Masami Hiramatsu @ 2015-03-17 9:45 UTC (permalink / raw)
To: Ingo Molnar; +Cc: Eugene Shatokhin, Ingo Molnar, LKML, Ingo Molnar
(2015/03/17 0:03), Ingo Molnar wrote:
>
> * Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com> wrote:
>
>> (2015/03/09 20:19), Eugene Shatokhin wrote:
>>> On x86-64, __copy_instruction() always returns 0 (error) if the
>>> instruction uses %rip-relative addressing. This is because
>>> kernel_insn_init() is called the second time for 'insn' instance
>>> in such cases and sets all its fields to 0.
>>>
>>> Because of this, trying to place a Kprobe on such instruction will
>>> fail, register_kprobe() will return -EINVAL.
>>
>> Oops, good catch!
>>
>> Acked-by: Masami Hiramatsu <masami.hiramatsu.pt@hitachi.com>
>
> Mind resending it with your SOB, as I wasn't Cc:-ed on the original
> submission?
Yes, I'll do :)
Thank you,
>
> Thanks,
>
> Ingo
>
--
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Research Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kprobes/x86: Return correct length in __copy_instruction()
2015-03-17 9:45 ` Masami Hiramatsu
@ 2015-04-27 11:03 ` Eugene Shatokhin
2015-05-11 13:47 ` Ingo Molnar
0 siblings, 1 reply; 6+ messages in thread
From: Eugene Shatokhin @ 2015-04-27 11:03 UTC (permalink / raw)
To: Masami Hiramatsu, Ingo Molnar; +Cc: Ingo Molnar, LKML, Ingo Molnar
Hi,
Now that the patch is in mainline (commit
c80e5c0c23ce2282476fdc64c4b5e3d3a40723fd) and kernel 4.1-rc1 is out, do
you mind if I send the backports of that patch to -stable?
Regards,
Eugene
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kprobes/x86: Return correct length in __copy_instruction()
2015-04-27 11:03 ` Eugene Shatokhin
@ 2015-05-11 13:47 ` Ingo Molnar
0 siblings, 0 replies; 6+ messages in thread
From: Ingo Molnar @ 2015-05-11 13:47 UTC (permalink / raw)
To: Eugene Shatokhin; +Cc: Masami Hiramatsu, Ingo Molnar, LKML, Ingo Molnar
* Eugene Shatokhin <eugene.shatokhin@rosalab.ru> wrote:
> Hi,
>
> Now that the patch is in mainline (commit
> c80e5c0c23ce2282476fdc64c4b5e3d3a40723fd) and kernel 4.1-rc1 is out, do you
> mind if I send the backports of that patch to -stable?
No objections from me!
Thanks,
Ingo
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-05-11 13:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-09 11:19 [PATCH] kprobes/x86: Return correct length in __copy_instruction() Eugene Shatokhin
2015-03-10 2:59 ` Masami Hiramatsu
2015-03-16 15:03 ` Ingo Molnar
2015-03-17 9:45 ` Masami Hiramatsu
2015-04-27 11:03 ` Eugene Shatokhin
2015-05-11 13:47 ` Ingo Molnar
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).