x86/kprobes: bugfixes and split optprobes
v4 cover x86/kprobes: bugfixes and split optprobes
Hi,
Here are the patches for fixing bugs in kprobes on x86.
- Both bugs are related to the optprobe.
- First bugfix is optprobe to recover instructions when thoes are modified with jump.
- Second bugfix is kprobes to recover it too.
This series also includes a patch which split optprobes parts out from kprobes x86 implementation.
Ingo, I've added the splitting patch according your advice :)
Thank you,
---
Masami Hiramatsu (3):
x86/kprobes: Split out optprobe related code to kprobes-opt.c [BUGFIX] x86/kprobes: Fix a bug which can modify kernel code [BUGFIX] x86/kprobes: Fix to recover instructions on optimized path
arch/x86/kernel/Makefile | 1 arch/x86/kernel/kprobes-common.h | 106 ++++++ arch/x86/kernel/kprobes-opt.c | 516 +++++++++++++++++++++++++++++++ arch/x86/kernel/kprobes.c | 633 ++++---------------------------------- 4 files changed, 684 insertions(+), 572 deletions(-) create mode 100644 arch/x86/kernel/kprobes-common.h create mode 100644 arch/x86/kernel/kprobes-opt.c
Signature · 4 lines
Masami HIRAMATSU Software Platform Research Dept. Linux Technology Center Hitachi, Ltd., Yokohama Research Laboratory E-mail: [email protected]
PATCH v4 1/3 x86/kprobes: Fix to recover instructions on optimized path
Current probed-instruction recovery expects that only breakpoint instruction modifies instruction. However, since kprobes jump optimization can replace original instructions with a jump, that expectation is not enough. And it may cause instruction decoding failure on the function where an optimized probe already exists.
This bug can reproduce easily as below.
1) find a target function address (any kprobe-able function is OK)
$ grep __secure_computing /proc/kallsyms ffffffff810c19d0 T __secure_computing
2) decode the function
$ objdump -d vmlinux --start-address=0xffffffff810c19d0 --stop-address=0xffffffff810c19eb
vmlinux: file format elf64-x86-64
Disassembly of section .text:
ffffffff810c19d0 <__secure_computing>:
ffffffff810c19d0: 55 push %rbp ffffffff810c19d1: 48 89 e5 mov %rsp,%rbp ffffffff810c19d4: e8 67 8f 72 00 callq ffffffff817ea940 <mcount> ffffffff810c19d9: 65 48 8b 04 25 40 b8 mov %gs:0xb840,%rax ffffffff810c19e0: 00 00 ffffffff810c19e2: 83 b8 88 05 00 00 01 cmpl $0x1,0x588(%rax) ffffffff810c19e9: 74 05 je ffffffff810c19f0 <__secure_computing+0x20>
3) put a kprobe-event at an optimize-able place, where no call/jump places within the 5 bytes.
$ su - # cd /sys/kernel/debug/tracing # echo p __secure_computing+0x9 > kprobe_events
4) enable it and check it is optimized.
# echo 1 > events/kprobes/p___secure_computing_9/enable # cat ../kprobes/list ffffffff810c19d9 k __secure_computing+0x9 [OPTIMIZED]
5) put another kprobe on an instruction after previous probe in the same function.
# echo p __secure_computing+0x12 >> kprobe_events bash: echo: write error: Invalid argument # dmesg | tail -n 1 [ 1666.500016] Probing address(0xffffffff810c19e2) is not an instruction boundary.
6) however, if the kprobes optimization is disabled, it works.
# echo 0 > /proc/sys/debug/kprobes-optimization # cat ../kprobes/list ffffffff810c19d9 k __secure_computing+0x9 # echo p __secure_computing+0x12 >> kprobe_events (no error)
This is because kprobes doesn't recover the instruction which is overwritten with a relative jump by another kprobe when finding instruction boundary.
It only recovers the breakpoint instruction.
This patch fixes kprobes to recover such instructions.
With this fix:
# echo p __secure_computing+0x9 > kprobe_events # echo 1 > events/kprobes/p___secure_computing_9/enable # cat ../kprobes/list ffffffff810c1aa9 k __secure_computing+0x9 [OPTIMIZED] # echo p __secure_computing+0x12 >> kprobe_events # cat ../kprobes/list ffffffff810c1aa9 k __secure_computing+0x9 [OPTIMIZED] ffffffff810c1ab2 k __secure_computing+0x12 [DISABLED]
Changes in v4:
- Fix a bug to ensure optimized probe is really optimized by jump.
- Remove kprobe_optready() dependency.
- Cleanup code for preparing optprobe separation.
Changes in v3:
- Fix a build error when CONFIG_OPTPROBE=n. (Thanks, Ingo!) To fix the error, split optprobe instruction recovering path from kprobes path.
- Cleanup comments/styles.
Changes in v2:
- Fix a bug to recover original instruction address in RIP-relative instruction fixup.
- Moved on tip/master.
---
arch/x86/kernel/kprobes.c | 140 +++++++++++++++++++++++++++++++-------------- 1 files changed, 97 insertions(+), 43 deletions(-)
arch/x86/kernel/kprobes.c +97 -43
@@ -207,13 +207,15 @@ retry: } } -/* Recover the probed instruction at addr for further analysis. */ -static int recover_probed_instruction (kprobe_opcode_t *buf,unsigned long addr) +static unsigned long __recover_probed_insn (kprobe_opcode_t *buf, +unsigned long addr) {struct kprobe *kp; + kp =get_kprobe ((void *)addr); +/* There is no probe, return original address */ if (!kp) -return -EINVAL ; +return addr;/* * Basically, kp->ainsn.insn has an original instruction.
@@ -230,14 +232,76 @@ static int recover_probed_instruction(kprobe_opcode_t *buf, unsigned long addr)*/ memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); buf[0 ] = kp->opcode; -return 0 ; +return (unsigned long )buf; +} + +#ifdef CONFIG_OPTPROBES +static unsigned long __recover_optprobed_insn (kprobe_opcode_t *buf, +unsigned long addr) +{ +struct optimized_kprobe *op; +struct kprobe *kp; +long offs; +int i; + +for (i =0 ; i <RELATIVEJUMP_SIZE ; i++) { + kp =get_kprobe ((void *)addr - i); +/* This function only handles jump-optimized kprobe */ +if (kp &&kprobe_optimized (kp)) { + op = container_of(kp, struct optimized_kprobe, kp); +/* If op->list is not empty, op is under optimizing */ +if (list_empty (&op->list)) + goto found; + } + } + +return addr; +found: +/* +* If the kprobe can be optimized, original bytes which can be +* overwritten by jump destination address. In this case, original +* bytes must be recovered from op->optinsn.copied_insn buffer. +*/ +memcpy (buf, (void *)addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); +if (addr == (unsigned long )kp->addr) { + buf[0 ] = kp->opcode; +memcpy (buf +1 , op->optinsn.copied_insn,RELATIVE_ADDR_SIZE ); + }else { + offs = addr - (unsigned long )kp->addr -1 ; +memcpy (buf, op->optinsn.copied_insn + offs,RELATIVE_ADDR_SIZE - offs); + } + +return (unsigned long )buf; +} +#else +static inline unsigned long __recover_optprobed_insn (kprobe_opcode_t *buf, +unsigned long addr) +{ +return addr; +} +#endif + +/* +* Recover the probed instruction at addr for further analysis. +* Caller must lock kprobes by kprobe_mutex, or disable preemption +* for preventing to release referencing kprobes. +*/ +static unsigned long recover_probed_instruction (kprobe_opcode_t *buf, +unsigned long addr) +{ +unsigned long __addr; + + __addr =__recover_optprobed_insn (buf, addr); +if (__addr != addr) +return __addr; + +return __recover_probed_insn (buf, addr); }/* Check if paddr is at an instruction boundary */ static int __kprobes can_probe (unsigned long paddr) { -int ret; -unsigned long addr, offset =0 ; +unsigned long addr, __addr, offset =0 ;struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ];
@@ -247,26 +311,24 @@ static int __kprobes can_probe(unsigned long paddr)/* Decode instructions */ addr = paddr - offset;while (addr < paddr) { -kernel_insn_init (&insn, (void *)addr); -insn_get_opcode (&insn); -/* * Check if the instruction has been modified by another * kprobe, in which case we replace the breakpoint by the * original instruction in our buffer. +* Also, jump optimization will change the breakpoint to +* relative-jump. Since the relative-jump itself is +* normally used, we just go through if there is no kprobe. */ -if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) { - ret =recover_probed_instruction (buf, addr); -if (ret) -/* -* Another debugging subsystem might insert -* this breakpoint. In that case, we can't -* recover it. -*/ -return 0 ; -kernel_insn_init (&insn, buf); - } + __addr =recover_probed_instruction (buf, addr); +kernel_insn_init (&insn, (void *)__addr);insn_get_length (&insn); + +/* +* Another debugging subsystem might insert this breakpoint. +* In that case, we can't recover it. +*/ +if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +return 0 ; addr += insn.length; }
@@ -302,21 +364,17 @@ static int __kprobes is_IF_modifier(kprobe_opcode_t *insn)static int __kprobes __copy_instruction (u8 *dest,u8 *src,int recover) {struct insn insn; -int ret;kprobe_opcode_t buf[MAX_INSN_SIZE ]; +u8 *orig_src = src;/* Back up original src for RIP calculation */ + +if (recover) + src = (u8 *)recover_probed_instruction (buf, (unsigned long )src);kernel_insn_init (&insn, src); -if (recover) { -insn_get_opcode (&insn); -if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) { - ret =recover_probed_instruction (buf, - (unsigned long )src); -if (ret) -return 0 ; -kernel_insn_init (&insn, buf); - } - }insn_get_length (&insn); +/* Another subsystem puts a breakpoint, failed to recover */ +if (recover && insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +return 0 ;memcpy (dest, insn.kaddr, insn.length);#ifdef CONFIG_X86_64
@@ -337,8 +395,7 @@ static int __kprobes __copy_instruction(u8 *dest, u8 *src, int recover)* extension of the original signed 32-bit displacement would * have given. */ - newdisp = (u8 *) src + (s64 ) insn.displacement.value - - (u8 *) dest; + newdisp = (u8 *) orig_src + (s64 ) insn.displacement.value - (u8 *) dest;BUG_ON ((s64 ) (s32 ) newdisp != newdisp);/* Sanity check. */ disp = (u8 *) dest +insn_offset_displacement (&insn); *(s32 *) disp = (s32 ) newdisp;
@@ -1271,8 +1328,7 @@ static int insn_jump_into_range(struct insn *insn, unsigned long start, int len)/* Decode whole function to ensure any instructions don't jump into target */ static int __kprobes can_optimize (unsigned long paddr) { -int ret; -unsigned long addr, size =0 , offset =0 ; +unsigned long addr, __addr, size =0 , offset =0 ;struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ];
@@ -1301,15 +1357,12 @@ static int __kprobes can_optimize(unsigned long paddr)* we can't optimize kprobe in this function. */ return 0 ; -kernel_insn_init (&insn, (void *)addr); -insn_get_opcode (&insn); -if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) { - ret =recover_probed_instruction (buf, addr); -if (ret) -return 0 ; -kernel_insn_init (&insn, buf); - } + __addr =recover_probed_instruction (buf, addr); +kernel_insn_init (&insn, (void *)__addr);insn_get_length (&insn); +/* Another subsystem puts a breakpoint */ +if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +return 0 ;/* Recover address */ insn.kaddr = (void *)addr; insn.next_byte = (void *)(addr + insn.length);
@@ -1366,6 +1419,7 @@ void __kprobes arch_remove_optimized_kprobe(struct optimized_kprobe *op)/* * Copy replacing target instructions * Target instructions MUST be relocatable (checked inside) +* This is called when new aggr(opt)probe is allocated or reused. */ int __kprobes arch_prepare_optimized_kprobe (struct optimized_kprobe *op) {
[tip:perf/core] x86/kprobes: Fix instruction recovery on optimized path
Commit-ID: 86b4ce3156c0dc140907ad03639564000cde694f Gitweb: http://git.kernel.org/tip/86b4ce3156c0dc140907ad03639564000cde694f Author: Masami Hiramatsu <[email protected]> AuthorDate: Mon, 5 Mar 2012 22:32:09 +0900 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 6 Mar 2012 09:49:48 +0100
x86/kprobes: Fix instruction recovery on optimized path
Current probed-instruction recovery expects that only breakpoint instruction modifies instruction. However, since kprobes jump optimization can replace original instructions with a jump, that expectation is not enough. And it may cause instruction decoding failure on the function where an optimized probe already exists.
This bug can reproduce easily as below:
1) find a target function address (any kprobe-able function is OK)
$ grep __secure_computing /proc/kallsyms ffffffff810c19d0 T __secure_computing
2) decode the function
$ objdump -d vmlinux --start-address=0xffffffff810c19d0 --stop-address=0xffffffff810c19eb
vmlinux: file format elf64-x86-64
Disassembly of section .text:
ffffffff810c19d0 <__secure_computing>:
ffffffff810c19d0: 55 push %rbp ffffffff810c19d1: 48 89 e5 mov %rsp,%rbp ffffffff810c19d4: e8 67 8f 72 00 callq ffffffff817ea940 <mcount> ffffffff810c19d9: 65 48 8b 04 25 40 b8 mov %gs:0xb840,%rax ffffffff810c19e0: 00 00 ffffffff810c19e2: 83 b8 88 05 00 00 01 cmpl $0x1,0x588(%rax) ffffffff810c19e9: 74 05 je ffffffff810c19f0 <__secure_computing+0x20>
3) put a kprobe-event at an optimize-able place, where no call/jump places within the 5 bytes.
$ su - # cd /sys/kernel/debug/tracing # echo p __secure_computing+0x9 > kprobe_events
4) enable it and check it is optimized.
# echo 1 > events/kprobes/p___secure_computing_9/enable # cat ../kprobes/list ffffffff810c19d9 k __secure_computing+0x9 [OPTIMIZED]
5) put another kprobe on an instruction after previous probe in the same function.
# echo p __secure_computing+0x12 >> kprobe_events bash: echo: write error: Invalid argument # dmesg | tail -n 1 [ 1666.500016] Probing address(0xffffffff810c19e2) is not an instruction boundary.
6) however, if the kprobes optimization is disabled, it works.
# echo 0 > /proc/sys/debug/kprobes-optimization # cat ../kprobes/list ffffffff810c19d9 k __secure_computing+0x9 # echo p __secure_computing+0x12 >> kprobe_events (no error)
This is because kprobes doesn't recover the instruction which is overwritten with a relative jump by another kprobe when finding instruction boundary.
It only recovers the breakpoint instruction.
This patch fixes kprobes to recover such instructions.
With this fix:
# echo p __secure_computing+0x9 > kprobe_events # echo 1 > events/kprobes/p___secure_computing_9/enable # cat ../kprobes/list ffffffff810c1aa9 k __secure_computing+0x9 [OPTIMIZED] # echo p __secure_computing+0x12 >> kprobe_events # cat ../kprobes/list ffffffff810c1aa9 k __secure_computing+0x9 [OPTIMIZED] ffffffff810c1ab2 k __secure_computing+0x12 [DISABLED]
Changes in v4:
- Fix a bug to ensure optimized probe is really optimized by jump.
- Remove kprobe_optready() dependency.
- Cleanup code for preparing optprobe separation.
Changes in v3:
- Fix a build error when CONFIG_OPTPROBE=n. (Thanks, Ingo!) To fix the error, split optprobe instruction recovering path from kprobes path.
- Cleanup comments/styles.
Changes in v2:
- Fix a bug to recover original instruction address in RIP-relative instruction fixup.
- Moved on tip/master.
--- arch/x86/kernel/kprobes.c | 140 +++++++++++++++++++++++++++++++-------------- 1 files changed, 97 insertions(+), 43 deletions(-)
arch/x86/kernel/kprobes.c +97 -43
@@ -207,13 +207,15 @@ retry: } } -/* Recover the probed instruction at addr for further analysis. */ -static int recover_probed_instruction (kprobe_opcode_t *buf,unsigned long addr) +static unsigned long __recover_probed_insn (kprobe_opcode_t *buf, +unsigned long addr) {struct kprobe *kp; + kp =get_kprobe ((void *)addr); +/* There is no probe, return original address */ if (!kp) -return -EINVAL ; +return addr;/* * Basically, kp->ainsn.insn has an original instruction.
@@ -230,14 +232,76 @@ static int recover_probed_instruction(kprobe_opcode_t *buf, unsigned long addr)*/ memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); buf[0 ] = kp->opcode; -return 0 ; +return (unsigned long )buf; +} + +#ifdef CONFIG_OPTPROBES +static unsigned long __recover_optprobed_insn (kprobe_opcode_t *buf, +unsigned long addr) +{ +struct optimized_kprobe *op; +struct kprobe *kp; +long offs; +int i; + +for (i =0 ; i <RELATIVEJUMP_SIZE ; i++) { + kp =get_kprobe ((void *)addr - i); +/* This function only handles jump-optimized kprobe */ +if (kp &&kprobe_optimized (kp)) { + op = container_of(kp, struct optimized_kprobe, kp); +/* If op->list is not empty, op is under optimizing */ +if (list_empty (&op->list)) + goto found; + } + } + +return addr; +found: +/* +* If the kprobe can be optimized, original bytes which can be +* overwritten by jump destination address. In this case, original +* bytes must be recovered from op->optinsn.copied_insn buffer. +*/ +memcpy (buf, (void *)addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); +if (addr == (unsigned long )kp->addr) { + buf[0 ] = kp->opcode; +memcpy (buf +1 , op->optinsn.copied_insn,RELATIVE_ADDR_SIZE ); + }else { + offs = addr - (unsigned long )kp->addr -1 ; +memcpy (buf, op->optinsn.copied_insn + offs,RELATIVE_ADDR_SIZE - offs); + } + +return (unsigned long )buf; +} +#else +static inline unsigned long __recover_optprobed_insn (kprobe_opcode_t *buf, +unsigned long addr) +{ +return addr; +} +#endif + +/* +* Recover the probed instruction at addr for further analysis. +* Caller must lock kprobes by kprobe_mutex, or disable preemption +* for preventing to release referencing kprobes. +*/ +static unsigned long recover_probed_instruction (kprobe_opcode_t *buf, +unsigned long addr) +{ +unsigned long __addr; + + __addr =__recover_optprobed_insn (buf, addr); +if (__addr != addr) +return __addr; + +return __recover_probed_insn (buf, addr); }/* Check if paddr is at an instruction boundary */ static int __kprobes can_probe (unsigned long paddr) { -int ret; -unsigned long addr, offset =0 ; +unsigned long addr, __addr, offset =0 ;struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ];
@@ -247,26 +311,24 @@ static int __kprobes can_probe(unsigned long paddr)/* Decode instructions */ addr = paddr - offset;while (addr < paddr) { -kernel_insn_init (&insn, (void *)addr); -insn_get_opcode (&insn); -/* * Check if the instruction has been modified by another * kprobe, in which case we replace the breakpoint by the * original instruction in our buffer. +* Also, jump optimization will change the breakpoint to +* relative-jump. Since the relative-jump itself is +* normally used, we just go through if there is no kprobe. */ -if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) { - ret =recover_probed_instruction (buf, addr); -if (ret) -/* -* Another debugging subsystem might insert -* this breakpoint. In that case, we can't -* recover it. -*/ -return 0 ; -kernel_insn_init (&insn, buf); - } + __addr =recover_probed_instruction (buf, addr); +kernel_insn_init (&insn, (void *)__addr);insn_get_length (&insn); + +/* +* Another debugging subsystem might insert this breakpoint. +* In that case, we can't recover it. +*/ +if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +return 0 ; addr += insn.length; }
@@ -302,21 +364,17 @@ static int __kprobes is_IF_modifier(kprobe_opcode_t *insn)static int __kprobes __copy_instruction (u8 *dest,u8 *src,int recover) {struct insn insn; -int ret;kprobe_opcode_t buf[MAX_INSN_SIZE ]; +u8 *orig_src = src;/* Back up original src for RIP calculation */ + +if (recover) + src = (u8 *)recover_probed_instruction (buf, (unsigned long )src);kernel_insn_init (&insn, src); -if (recover) { -insn_get_opcode (&insn); -if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) { - ret =recover_probed_instruction (buf, - (unsigned long )src); -if (ret) -return 0 ; -kernel_insn_init (&insn, buf); - } - }insn_get_length (&insn); +/* Another subsystem puts a breakpoint, failed to recover */ +if (recover && insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +return 0 ;memcpy (dest, insn.kaddr, insn.length);#ifdef CONFIG_X86_64
@@ -337,8 +395,7 @@ static int __kprobes __copy_instruction(u8 *dest, u8 *src, int recover)* extension of the original signed 32-bit displacement would * have given. */ - newdisp = (u8 *) src + (s64 ) insn.displacement.value - - (u8 *) dest; + newdisp = (u8 *) orig_src + (s64 ) insn.displacement.value - (u8 *) dest;BUG_ON ((s64 ) (s32 ) newdisp != newdisp);/* Sanity check. */ disp = (u8 *) dest +insn_offset_displacement (&insn); *(s32 *) disp = (s32 ) newdisp;
@@ -1271,8 +1328,7 @@ static int insn_jump_into_range(struct insn *insn, unsigned long start, int len)/* Decode whole function to ensure any instructions don't jump into target */ static int __kprobes can_optimize (unsigned long paddr) { -int ret; -unsigned long addr, size =0 , offset =0 ; +unsigned long addr, __addr, size =0 , offset =0 ;struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ];
@@ -1301,15 +1357,12 @@ static int __kprobes can_optimize(unsigned long paddr)* we can't optimize kprobe in this function. */ return 0 ; -kernel_insn_init (&insn, (void *)addr); -insn_get_opcode (&insn); -if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) { - ret =recover_probed_instruction (buf, addr); -if (ret) -return 0 ; -kernel_insn_init (&insn, buf); - } + __addr =recover_probed_instruction (buf, addr); +kernel_insn_init (&insn, (void *)__addr);insn_get_length (&insn); +/* Another subsystem puts a breakpoint */ +if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +return 0 ;/* Recover address */ insn.kaddr = (void *)addr; insn.next_byte = (void *)(addr + insn.length);
@@ -1366,6 +1419,7 @@ void __kprobes arch_remove_optimized_kprobe(struct optimized_kprobe *op)/* * Copy replacing target instructions * Target instructions MUST be relocatable (checked inside) +* This is called when new aggr(opt)probe is allocated or reused. */ int __kprobes arch_prepare_optimized_kprobe (struct optimized_kprobe *op) {
PATCH v4 2/3 x86/kprobes: Fix a bug which can modify kernel code
In reply to Masami HiramatsuFix a bug in kprobes which can modify kernel code permanently at run-time. In the result, kernel can crash when it executes the modified code.
This bug can happen when we put two probes enough near and the first probe is optimized. When the second probe is set up, it copies a byte which is already modified by the first probe, and executes it when the probe is hit. Even worse, the first probe and the second probe are removed respectively, the second probe writes back the copied (modified) instruction.
To fix this bug, kprobes always recovers the original code and copies the first byte from recovered instruction.
---
arch/x86/kernel/kprobes.c | 33 +++++++++++++++------------------ 1 files changed, 15 insertions(+), 18 deletions(-)
arch/x86/kernel/kprobes.c +15 -18
@@ -361,19 +361,15 @@ static int __kprobes is_IF_modifier(kprobe_opcode_t *insn)* If not, return null. * Only applicable to 64-bit x86. */ -static int __kprobes __copy_instruction (u8 *dest,u8 *src,int recover) +static int __kprobes __copy_instruction (u8 *dest,u8 *src) {struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ]; -u8 *orig_src = src;/* Back up original src for RIP calculation */ -if (recover) - src = (u8 *)recover_probed_instruction (buf, (unsigned long )src); - -kernel_insn_init (&insn, src); +kernel_insn_init (&insn, (void *)recover_probed_instruction (buf, (unsigned long )src));insn_get_length (&insn);/* Another subsystem puts a breakpoint, failed to recover */ -if (recover && insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION )return 0 ;memcpy (dest, insn.kaddr, insn.length);
@@ -395,7 +391,7 @@ static int __kprobes __copy_instruction(u8 *dest, u8 *src, int recover)* extension of the original signed 32-bit displacement would * have given. */ - newdisp = (u8 *) orig_src + (s64 ) insn.displacement.value - (u8 *) dest; + newdisp = (u8 *) src + (s64 ) insn.displacement.value - (u8 *) dest;BUG_ON ((s64 ) (s32 ) newdisp != newdisp);/* Sanity check. */ disp = (u8 *) dest +insn_offset_displacement (&insn); *(s32 *) disp = (s32 ) newdisp;
@@ -406,18 +402,20 @@ static int __kprobes __copy_instruction(u8 *dest, u8 *src, int recover)static void __kprobes arch_copy_kprobe (struct kprobe *p) { +/* Copy an instruction with recovering if other optprobe modifies it.*/ +__copy_instruction (p->ainsn.insn, p->addr); +/* -* Copy an instruction without recovering int3, because it will be -* put by another subsystem. +* __copy_instruction can modify the displacement of the instruction, +* but it doesn't affect boostable check. */ -__copy_instruction (p->ainsn.insn, p->addr,0 ); - -if (can_boost (p->addr)) +if (can_boost (p->ainsn.insn)) p->ainsn.boostable =0 ;else p->ainsn.boostable =-1 ; - p->opcode = *p->addr; +/* Also, displacement change doesn't affect the first byte */ + p->opcode = p->ainsn.insn[0 ]; }int __kprobes arch_prepare_kprobe (struct kprobe *p)
@@ -1276,7 +1274,7 @@ static int __kprobes copy_optimized_instructions(u8 *dest, u8 *src)int len =0 , ret;while (len <RELATIVEJUMP_SIZE ) { - ret =__copy_instruction (dest + len, src + len,1 ); + ret =__copy_instruction (dest + len, src + len);if (!ret || !can_boost (dest + len))return -EINVAL ; len += ret;
@@ -1328,7 +1326,7 @@ static int insn_jump_into_range(struct insn *insn, unsigned long start, int len)/* Decode whole function to ensure any instructions don't jump into target */ static int __kprobes can_optimize (unsigned long paddr) { -unsigned long addr, __addr, size =0 , offset =0 ; +unsigned long addr, size =0 , offset =0 ;struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ];
@@ -1357,8 +1355,7 @@ static int __kprobes can_optimize(unsigned long paddr)* we can't optimize kprobe in this function. */ return 0 ; - __addr =recover_probed_instruction (buf, addr); -kernel_insn_init (&insn, (void *)__addr); +kernel_insn_init (&insn, (void *)recover_probed_instruction (buf, addr));insn_get_length (&insn);/* Another subsystem puts a breakpoint */ if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION )
[tip:perf/core] x86/kprobes: Fix a bug which can modify kernel code permanently
Commit-ID: 464846888d9aad186cab3acdae6b654f9eb19772 Gitweb: http://git.kernel.org/tip/464846888d9aad186cab3acdae6b654f9eb19772 Author: Masami Hiramatsu <[email protected]> AuthorDate: Mon, 5 Mar 2012 22:32:16 +0900 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 6 Mar 2012 09:49:49 +0100
x86/kprobes: Fix a bug which can modify kernel code permanently
Fix a bug in kprobes which can modify kernel code permanently at run-time. In the result, kernel can crash when it executes the modified code.
This bug can happen when we put two probes enough near and the first probe is optimized. When the second probe is set up, it copies a byte which is already modified by the first probe, and executes it when the probe is hit. Even worse, the first probe and the second probe are removed respectively, the second probe writes back the copied (modified) instruction.
To fix this bug, kprobes always recovers the original code and copies the first byte from recovered instruction.
--- arch/x86/kernel/kprobes.c | 33 +++++++++++++++------------------ 1 files changed, 15 insertions(+), 18 deletions(-)
arch/x86/kernel/kprobes.c +15 -18
@@ -361,19 +361,15 @@ static int __kprobes is_IF_modifier(kprobe_opcode_t *insn)* If not, return null. * Only applicable to 64-bit x86. */ -static int __kprobes __copy_instruction (u8 *dest,u8 *src,int recover) +static int __kprobes __copy_instruction (u8 *dest,u8 *src) {struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ]; -u8 *orig_src = src;/* Back up original src for RIP calculation */ -if (recover) - src = (u8 *)recover_probed_instruction (buf, (unsigned long )src); - -kernel_insn_init (&insn, src); +kernel_insn_init (&insn, (void *)recover_probed_instruction (buf, (unsigned long )src));insn_get_length (&insn);/* Another subsystem puts a breakpoint, failed to recover */ -if (recover && insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION ) +if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION )return 0 ;memcpy (dest, insn.kaddr, insn.length);
@@ -395,7 +391,7 @@ static int __kprobes __copy_instruction(u8 *dest, u8 *src, int recover)* extension of the original signed 32-bit displacement would * have given. */ - newdisp = (u8 *) orig_src + (s64 ) insn.displacement.value - (u8 *) dest; + newdisp = (u8 *) src + (s64 ) insn.displacement.value - (u8 *) dest;BUG_ON ((s64 ) (s32 ) newdisp != newdisp);/* Sanity check. */ disp = (u8 *) dest +insn_offset_displacement (&insn); *(s32 *) disp = (s32 ) newdisp;
@@ -406,18 +402,20 @@ static int __kprobes __copy_instruction(u8 *dest, u8 *src, int recover)static void __kprobes arch_copy_kprobe (struct kprobe *p) { +/* Copy an instruction with recovering if other optprobe modifies it.*/ +__copy_instruction (p->ainsn.insn, p->addr); +/* -* Copy an instruction without recovering int3, because it will be -* put by another subsystem. +* __copy_instruction can modify the displacement of the instruction, +* but it doesn't affect boostable check. */ -__copy_instruction (p->ainsn.insn, p->addr,0 ); - -if (can_boost (p->addr)) +if (can_boost (p->ainsn.insn)) p->ainsn.boostable =0 ;else p->ainsn.boostable =-1 ; - p->opcode = *p->addr; +/* Also, displacement change doesn't affect the first byte */ + p->opcode = p->ainsn.insn[0 ]; }int __kprobes arch_prepare_kprobe (struct kprobe *p)
@@ -1276,7 +1274,7 @@ static int __kprobes copy_optimized_instructions(u8 *dest, u8 *src)int len =0 , ret;while (len <RELATIVEJUMP_SIZE ) { - ret =__copy_instruction (dest + len, src + len,1 ); + ret =__copy_instruction (dest + len, src + len);if (!ret || !can_boost (dest + len))return -EINVAL ; len += ret;
@@ -1328,7 +1326,7 @@ static int insn_jump_into_range(struct insn *insn, unsigned long start, int len)/* Decode whole function to ensure any instructions don't jump into target */ static int __kprobes can_optimize (unsigned long paddr) { -unsigned long addr, __addr, size =0 , offset =0 ; +unsigned long addr, size =0 , offset =0 ;struct insn insn;kprobe_opcode_t buf[MAX_INSN_SIZE ];
@@ -1357,8 +1355,7 @@ static int __kprobes can_optimize(unsigned long paddr)* we can't optimize kprobe in this function. */ return 0 ; - __addr =recover_probed_instruction (buf, addr); -kernel_insn_init (&insn, (void *)__addr); +kernel_insn_init (&insn, (void *)recover_probed_instruction (buf, addr));insn_get_length (&insn);/* Another subsystem puts a breakpoint */ if (insn.opcode.bytes[0 ] ==BREAKPOINT_INSTRUCTION )
PATCH v4 3/3 x86/kprobes: Split out optprobe related code to kprobes-opt.c
In reply to Masami HiramatsuSplit out optprobe related code to arch/x86/kernel/kprobes-opt.c for maintenanceability.
---
arch/x86/kernel/Makefile | 1 arch/x86/kernel/kprobes-common.h | 106 +++++++ arch/x86/kernel/kprobes-opt.c | 516 +++++++++++++++++++++++++++++++++ arch/x86/kernel/kprobes.c | 590 +------------------------------------- 4 files changed, 637 insertions(+), 576 deletions(-) create mode 100644 arch/x86/kernel/kprobes-common.h create mode 100644 arch/x86/kernel/kprobes-opt.c
arch/x86/kernel/Makefile +1 -0
@@ -69,6 +69,7 @@ obj-$(CONFIG_KEXEC) += machine_kexec_$(BITS).o obj-$(CONFIG_KEXEC) += relocate_kernel_$(BITS).o crash.o obj-$(CONFIG_CRASH_DUMP) += crash_dump_$(BITS).o obj-$(CONFIG_KPROBES) += kprobes.o +obj-$(CONFIG_OPTPROBES) += kprobes-opt.o obj-$(CONFIG_MODULES) += module.o obj-$(CONFIG_DOUBLEFAULT) += doublefault_32.o obj-$(CONFIG_KGDB) += kgdb.o
arch/x86/kernel/kprobes-common.h new file +106 -0
106 lines not on this page · raw message
arch/x86/kernel/kprobes-opt.c new file +516 -0
516 lines not on this page · raw message
arch/x86/kernel/kprobes.c +14 -576
649 lines not on this page · raw message
[tip:perf/core] x86/kprobes: Split out optprobe related code to kprobes-opt.c
Commit-ID: 3f33ab1c0c741bfab2138c14ba1918a7905a1e8b Gitweb: http://git.kernel.org/tip/3f33ab1c0c741bfab2138c14ba1918a7905a1e8b Author: Masami Hiramatsu <[email protected]> AuthorDate: Mon, 5 Mar 2012 22:32:22 +0900 Committer: Ingo Molnar <[email protected]> CommitDate: Tue, 6 Mar 2012 09:49:49 +0100
x86/kprobes: Split out optprobe related code to kprobes-opt.c
Split out optprobe related code to arch/x86/kernel/kprobes-opt.c for maintenanceability.
--- arch/x86/kernel/Makefile | 1 + arch/x86/kernel/kprobes-common.h | 102 ++++++ arch/x86/kernel/kprobes-opt.c | 512 +++++++++++++++++++++++++++++++ arch/x86/kernel/kprobes.c | 625 ++------------------------------------ 4 files changed, 646 insertions(+), 594 deletions(-)
arch/x86/kernel/Makefile +1 -0
@@ -69,6 +69,7 @@ obj-$(CONFIG_KEXEC) += machine_kexec_$(BITS).o obj-$(CONFIG_KEXEC) += relocate_kernel_$(BITS).o crash.o obj-$(CONFIG_CRASH_DUMP) += crash_dump_$(BITS).o obj-$(CONFIG_KPROBES) += kprobes.o +obj-$(CONFIG_OPTPROBES) += kprobes-opt.o obj-$(CONFIG_MODULES) += module.o obj-$(CONFIG_DOUBLEFAULT) += doublefault_32.o obj-$(CONFIG_KGDB) += kgdb.o
arch/x86/kernel/kprobes-common.h new file +102 -0
102 lines not on this page · raw message
arch/x86/kernel/kprobes-opt.c new file +512 -0
512 lines not on this page · raw message
arch/x86/kernel/kprobes.c +31 -594
730 lines not on this page · raw message
Quoted 25 lines · Masami Hiramatsu
Hi,
Here are the patches for fixing bugs in kprobes on x86.
- Both bugs are related to the optprobe.
- First bugfix is optprobe to recover instructions when thoes are modified with jump.
- Second bugfix is kprobes to recover it too.This series also includes a patch which split optprobes parts out from kprobes x86 implementation.
Ingo, I've added the splitting patch according your advice :)
Thank you,
---
Masami Hiramatsu (3):
x86/kprobes: Split out optprobe related code tokprobes-opt.c[BUGFIX] x86/kprobes: Fix a bug which can modify kernel code [BUGFIX] x86/kprobes: Fix to recover instructions on optimized path
Quoted 7 lines
arch/x86/kernel/Makefile | 1 arch/x86/kernel/kprobes-common.h | 106 ++++++ arch/x86/kernel/kprobes-opt.c | 516 +++++++++++++++++++++++++++++++ arch/x86/kernel/kprobes.c | 633 ++++---------------------------------- 4 files changed, 684 insertions(+), 572 deletions(-) create mode 100644 arch/x86/kernel/kprobes-common.h create mode 100644 arch/x86/kernel/kprobes-opt.c
Thanks, this looks very nice. I've applied them to tip:perf/core and will push them out later today, assuming there are no test failures.
Thanks,
Ingo