tracing: kprobe-based event tracer and x86 instruction decoder
Subsystems (6)
v5 cover tracing: kprobe-based event tracer and x86 instruction decoder
Hi,
Here are the patches of kprobe-based event tracer for x86, version 5, which allows you to probe various kernel events through ftrace interface.
This version supports only x86(-32/-64) (but porting it on other arch just needs kprobes/kretprobes and register and stack access APIs).
This patchset also includes x86(-64) instruction decoder which supports non-SSE/FP opcodes and includes x86 opcode map. I think it will be possible to share this opcode map with KVM's decoder.
This series can be applied on the latest linux-2.6-tip tree.
This patchset includes following changes:
- Add x86 instruction decoder [1/7]
- Check insertion point safety in kprobe [2/7]
- Cleanup fix_riprel() with insn decoder [3/7]
- Add kprobe-tracer plugin [4/7]
- Fix kernel_trap_sp() on x86 according to systemtap runtime. [5/7]
- Add arch-dep register and stack fetching functions [6/7]
- Support fetching various status (register/stack/memory/etc.) [7/7]
Future items:
- .init function tracing support.
- Support primitive types(long, ulong, int, uint, etc) for args.
kprobe-based event tracer
---------------------------
This tracer is similar to the events tracer which is based on Tracepoint infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe and kretprobe). It probes anywhere where kprobes can probe(this means, all functions body except for __kprobes functions).
Unlike the function tracer, this tracer can probe instructions inside of kernel functions. It allows you to check which instruction has been executed.
Unlike the Tracepoint based events tracer, this tracer can add new probe points on the fly.
Similar to the events tracer, this tracer doesn't need to be activated via current_tracer, instead of that, just set probe points via /debug/tracing/kprobe_events.
Synopsis of kprobe_events:
p SYMBOL[+offs|-offs]|MEMADDR [FETCHARGS] : set a probe
r SYMBOL[+0] [FETCHARGS] : set a return probe
FETCHARGS:
%REG : Fetch register REG
sN : Fetch Nth entry of stack (N >= 0)
@ADDR : Fetch memory at ADDR (ADDR should be in kernel)
@SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol) aN : Fetch function argument. (N >= 0)(*) rv : Fetch return value.(**) ra : Fetch return address.(**)
+|-offs(FETCHARG) : fetch memory at FETCHARG +|- offs address.(***)
(*) aN may not correct on asmlinkaged functions and at the middle of
function body.
(**) only for return probe.
(***) this is useful for fetching a field of data structures.
E.g.
echo p do_sys_open a0 a1 a2 a3 > /debug/tracing/kprobe_events
This sets a kprobe on the top of do_sys_open() function with recording
1st to 4th arguments.
echo r do_sys_open rv rp >> /debug/tracing/kprobe_events
This sets a kretprobe on the return point of do_sys_open() function with
recording return value and return address.
echo > /debug/tracing/kprobe_events
This clears all probe points. and you can see the traced information via
/debug/tracing/trace.
cat /debug/tracing/trace
# tracer: nop
#
# TASK-PID CPU# TIMESTAMP FUNCTION
# | | | | |
<...>-2376 [001] 262.389131: do_sys_open: @do_sys_open+0 0xffffff9c 0x98db83e 0x8880 0x0
<...>-2376 [001] 262.391166: sys_open: <-do_sys_open+0 0x5 0xc06e8ebb
<...>-2376 [001] 264.384876: do_sys_open: @do_sys_open+0 0xffffff9c 0x98db83e 0x8880 0x0
<...>-2376 [001] 264.386880: sys_open: <-do_sys_open+0 0x5 0xc06e8ebb
<...>-2084 [001] 265.380330: do_sys_open: @do_sys_open+0 0xffffff9c 0x804be3e 0x0 0x1b6
<...>-2084 [001] 265.380399: sys_open: <-do_sys_open+0 0x3 0xc06e8ebb @SYMBOL means that kernel hits a probe, and <-SYMBOL means kernel returns
from SYMBOL(e.g. "sys_open: <-do_sys_open+0" means kernel returns from do_sys_open to sys_open).
Thank you,
---
Masami Hiramatsu (7):
tracing: add arguments support on kprobe-based event tracer
x86: add pt_regs register and stack access APIs
x86: fix kernel_trap_sp()
tracing: add kprobe-based event tracer
kprobes: cleanup fix_riprel() using insn decoder on x86
kprobes: checks probe address is instruction boudary on x86
x86: instruction decorder API
Documentation/trace/ftrace.txt | 70 +++ arch/x86/include/asm/inat.h | 125 +++++ arch/x86/include/asm/insn.h | 134 +++++ arch/x86/include/asm/ptrace.h | 70 +++ arch/x86/kernel/kprobes.c | 182 +++---- arch/x86/kernel/ptrace.c | 60 ++ arch/x86/lib/Makefile | 9 arch/x86/lib/inat.c | 80 +++ arch/x86/lib/insn.c | 471 +++++++++++++++++++ arch/x86/lib/x86-opcode-map.txt | 711 +++++++++++++++++++++++++++++ arch/x86/scripts/gen-insn-attr-x86.awk | 314 +++++++++++++ kernel/trace/Kconfig | 9 kernel/trace/Makefile | 1 kernel/trace/trace_kprobe.c | 793 ++++++++++++++++++++++++++++++++ 14 files changed, 2922 insertions(+), 107 deletions(-) create mode 100644 arch/x86/include/asm/inat.h create mode 100644 arch/x86/include/asm/insn.h create mode 100644 arch/x86/lib/inat.c create mode 100644 arch/x86/lib/insn.c create mode 100644 arch/x86/lib/x86-opcode-map.txt create mode 100644 arch/x86/scripts/gen-insn-attr-x86.awk create mode 100644 kernel/trace/trace_kprobe.c
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
PATCH v5 1/7 x86: instruction decorder API
Add x86 instruction decoder to arch-specific libraries. This decoder can decode x86 instructions used in kernel into prefix, opcode, modrm, sib, displacement and immediates. This can also show the length of instructions.
This version introduces instruction attributes for decoding instructions. The instruction attribute tables are generated from the opcode map file (x86-opcode-map.txt) by the generator script(gen-insn-attr-x86.awk).
Currently, the opcode maps are based on opcode maps in Intel(R) 64 and IA-32 Architectures Software Developers Manual Vol.2: Appendix.A, and consist of below two types of opcode tables.
1-byte/2-bytes/3-bytes opcodes, which has 256 elements, are written as below;
Table: table-name
Referrer: escaped-name
opcode: mnemonic|GrpXXX [operand1[,operand2...]] [(extra1)[,(extra2)...] [| 2nd-mnemonic ...]
(or)
opcode: escape # escaped-name
EndTable
Group opcodes, which has 8 elements, are written as below;
GrpTable: GrpXXX
reg: mnemonic [operand1[,operand2...]] [(extra1)[,(extra2)...] [| 2nd-mnemonic ...] EndTable
These opcode maps do NOT include most of SSE and FP opcodes, because those opcodes are not used in the kernel.
Changes from prototype:
- Support movaps which is used by EFI support code
- Support nopw with many prefixes.
TODO:
- Integrate user-space test harness as a build-time test.
---
arch/x86/include/asm/inat.h | 125 ++++++ arch/x86/include/asm/insn.h | 134 ++++++ arch/x86/lib/Makefile | 9 arch/x86/lib/inat.c | 80 ++++ arch/x86/lib/insn.c | 471 +++++++++++++++++++++ arch/x86/lib/x86-opcode-map.txt | 711 ++++++++++++++++++++++++++++++++ arch/x86/scripts/gen-insn-attr-x86.awk | 314 ++++++++++++++ 7 files changed, 1844 insertions(+), 0 deletions(-) create mode 100644 arch/x86/include/asm/inat.h create mode 100644 arch/x86/include/asm/insn.h create mode 100644 arch/x86/lib/inat.c create mode 100644 arch/x86/lib/insn.c create mode 100644 arch/x86/lib/x86-opcode-map.txt create mode 100644 arch/x86/scripts/gen-insn-attr-x86.awk
arch/x86/include/asm/inat.h new file +125 -0
@@ -0,0 +1,125 @@ +#ifndef _ASM_INAT_INAT_H +#define _ASM_INAT_INAT_H +/* +* x86 instruction attributes +* +* Written by Masami Hiramatsu <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License as published by +* the Free Software Foundation; either version 2 of the License, or +* (at your option) any later version. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. +* +*/ +#include <linux/types.h> + +/* Instruction attributes */ +typedef u32 insn_attr_t ; + +/* +* Internal bits. Don't use bitmasks directly, because these bits are +* unstable. You should add checking macros and use that macro in +* your code. +*/ + +#define INAT_OPCODE_TABLE_SIZE 256 +#define INAT_GROUP_TABLE_SIZE 8 + +/* Legacy instruction prefixes */ +#define INAT_PFX_OPNDSZ 1/* 0x66 */ /* LPFX1 */ +#define INAT_PFX_REPNE 2/* 0xF2 */ /* LPFX2 */ +#define INAT_PFX_REPE 3/* 0xF3 */ /* LPFX3 */ +#define INAT_PFX_LOCK 4/* 0xF0 */ +#define INAT_PFX_CS 5/* 0x2E */ +#define INAT_PFX_DS 6/* 0x3E */ +#define INAT_PFX_ES 7/* 0x26 */ +#define INAT_PFX_FS 8/* 0x64 */ +#define INAT_PFX_GS 9/* 0x65 */ +#define INAT_PFX_SS 10/* 0x36 */ +#define INAT_PFX_ADDRSZ 11/* 0x67 */ + +#define INAT_LPREFIX_MAX 3 + +/* Immediate size */ +#define INAT_IMM_BYTE 1 +#define INAT_IMM_WORD 2 +#define INAT_IMM_DWORD 3 +#define INAT_IMM_QWORD 4 +#define INAT_IMM_PTR 5 +#define INAT_IMM_VWORD32 6 +#define INAT_IMM_VWORD 7 + +/* Legacy prefix */ +#define INAT_PFX_OFFS 0 +#define INAT_PFX_BITS 4 +#define INAT_PFX_MAX ((1 << INAT_PFX_BITS) - 1) +#define INAT_PFX_MASK (INAT_PFX_MAX << INAT_PFX_OFFS) +/* Escape opcodes */ +#define INAT_ESC_OFFS (INAT_PFX_OFFS + INAT_PFX_BITS) +#define INAT_ESC_BITS 2 +#define INAT_ESC_MAX ((1 << INAT_ESC_BITS) - 1) +#define INAT_ESC_MASK (INAT_ESC_MAX << INAT_ESC_OFFS) +/* Group opcodes (1-16) */ +#define INAT_GRP_OFFS (INAT_ESC_OFFS + INAT_ESC_BITS) +#define INAT_GRP_BITS 5 +#define INAT_GRP_MAX ((1 << INAT_GRP_BITS) - 1) +#define INAT_GRP_MASK (INAT_GRP_MAX << INAT_GRP_OFFS) +/* Immediates */ +#define INAT_IMM_OFFS (INAT_GRP_OFFS + INAT_GRP_BITS) +#define INAT_IMM_BITS 3 +#define INAT_IMM_MASK (((1 << INAT_IMM_BITS) - 1) << INAT_IMM_OFFS) +/* Flags */ +#define INAT_FLAG_OFFS (INAT_IMM_OFFS + INAT_IMM_BITS) +#define INAT_REXPFX (1 << INAT_FLAG_OFFS) +#define INAT_MODRM (1 << (INAT_FLAG_OFFS + 1)) +#define INAT_FORCE64 (1 << (INAT_FLAG_OFFS + 2)) +#define INAT_ADDIMM (1 << (INAT_FLAG_OFFS + 3)) +#define INAT_MOFFSET (1 << (INAT_FLAG_OFFS + 4)) +#define INAT_VARIANT (1 << (INAT_FLAG_OFFS + 5)) + +/* Attribute search APIs */ +extern insn_attr_t inat_get_opcode_attribute (u8 opcode); +extern insn_attr_t inat_get_escape_attribute (u8 opcode,u8 last_pfx, +insn_attr_t esc_attr); +extern insn_attr_t inat_get_group_attribute (u8 modrm,u8 last_pfx, +insn_attr_t esc_attr); + +/* Attribute checking macros. Use these macros in your code */ +#define INAT_IS_PREFIX (attr) (attr & INAT_PFX_MASK) +#define INAT_IS_ADDRSZ (attr) ((attr & INAT_PFX_MASK) == INAT_PFX_ADDRSZ) +#define INAT_IS_OPNDSZ (attr) ((attr & INAT_PFX_MASK) == INAT_PFX_OPNDSZ) +#define INAT_LPREFIX_NUM (attr) \ + (((attr & INAT_PFX_MASK) > INAT_LPREFIX_MAX) ? 0 :\ + (attr & INAT_PFX_MASK)) +#define INAT_MAKE_PREFIX (pfx) (pfx << INAT_PFX_OFFS) + +#define INAT_IS_ESCAPE (attr) (attr & INAT_ESC_MASK) +#define INAT_ESCAPE_NUM (attr) ((attr & INAT_ESC_MASK) >> INAT_ESC_OFFS) +#define INAT_MAKE_ESCAPE (esc) (esc << INAT_ESC_OFFS) + +#define INAT_IS_GROUP (attr) (attr & INAT_GRP_MASK) +#define INAT_GROUP_NUM (attr) ((attr & INAT_GRP_MASK) >> INAT_GRP_OFFS) +#define INAT_GROUP_COMMON (attr) (attr & ~INAT_GRP_MASK) +#define INAT_MAKE_GROUP (grp) ((grp << INAT_GRP_OFFS) | INAT_MODRM) + +#define INAT_HAS_IMM (attr) (attr & INAT_IMM_MASK) +#define INAT_IMM_SIZE (attr) ((attr & INAT_IMM_MASK) >> INAT_IMM_OFFS) +#define INAT_MAKE_IMM (imm) (imm << INAT_IMM_OFFS) + +#define INAT_IS_REX_PREFIX (attr) (attr & INAT_REXPFX) +#define INAT_HAS_MODRM (attr) (attr & INAT_MODRM) +#define INAT_IS_FORCE64 (attr) (attr & INAT_FORCE64) +#define INAT_HAS_ADDIMM (attr) (attr & INAT_ADDIMM) +#define INAT_HAS_MOFFSET (attr) (attr & INAT_MOFFSET) +#define INAT_HAS_VARIANT (attr) (attr & INAT_VARIANT) + +#endif
arch/x86/include/asm/insn.h new file +134 -0
134 lines not on this page · raw message
arch/x86/lib/Makefile +9 -0
@@ -2,12 +2,21 @@ # Makefile for x86 specific library files. # +quiet_cmd_inat_tables = GEN $@ + cmd_inat_tables = awk -f $(srctree)/arch/x86/scripts/gen-insn-attr-x86.awk $(srctree)/arch/x86/lib/x86-opcode-map.txt > $@ + +$(obj)/inat.o: $(obj)/inat-tables.c + +$(obj)/inat-tables.c: FORCE + $(call cmd,inat_tables) + obj-$(CONFIG_SMP) := msr-on-cpu.o lib-y := delay.o lib-y += thunk_$(BITS).o lib-y += usercopy_$(BITS).o getuser.o putuser.o lib-y += memcpy_$(BITS).o +lib-y += insn.o inat.o ifeq ($(CONFIG_X86_32),y) lib-y += checksum_32.o
arch/x86/lib/inat.c new file +80 -0
80 lines not on this page · raw message
arch/x86/lib/insn.c new file +471 -0
471 lines not on this page · raw message
arch/x86/lib/x86-opcode-map.txt new file +711 -0
711 lines not on this page · raw message
arch/x86/scripts/gen-insn-attr-x86.awk new file +314 -0
314 lines not on this page · raw message
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
Quoted 4 lines · Masami Hiramatsu
Add x86 instruction decoder to arch-specific libraries. This decoder can decode x86 instructions used in kernel into prefix, opcode, modrm, sib, displacement and immediates. This can also show the length of instructions.
Could also be used to implement a simple disasembler for OOPS output ala arch/s390/kernel/dis.c?
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
Quoted 8 lines · Christoph Hellwig
Quoted 4 lines · Masami Hiramatsu
Add x86 instruction decoder to arch-specific libraries. This decoder can decode x86 instructions used in kernel into prefix, opcode, modrm, sib, displacement and immediates. This can also show the length of instructions.
Could also be used to implement a simple disasembler for OOPS output ala
arch/s390/kernel/dis.c?
Yes, but as you may know, it's not so "simple"...especially show it as "gas-like(a.k.a. at&t)" expression :-)
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
In reply to Masami Hiramatsuarch/x86/scripts/gen-insn-attr-x86.awk line 1
Quoted 3 lines · Masami Hiramatsu
+++
b/arch/x86/scripts/gen-insn-attr-x86.awkarch/x86/scripts/gen-insn-attr-x86.awk new file +1
-0@@ -0,0 +1,314 @@ +#!/bin/awk -f
On some distributions (debian) it is /usr/bin/awk.
--
Gleb.
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
Quoted 5 lines · Gleb Natapov
arch/x86/scripts/gen-insn-attr-x86.awk line 1
Quoted 3 lines · Masami Hiramatsu
+++
b/arch/x86/scripts/gen-insn-attr-x86.awkarch/x86/scripts/gen-insn-attr-x86.awk new file +1
-0@@ -0,0 +1,314 @@ +#!/bin/awk -fOn some distributions (debian) it is
/usr/bin/awk.
True, but on most of them (all?) there is also an appropriate link in /bin. If shebang could have more that one argument, then '/usr/bin/env awk -f' would be the best solution I think.
Signature · 1 line
Przemysław Pawełczyk
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
Quoted 8 lines · Przemysssaw Paweeeczyk
Quoted 5 lines · Gleb Natapov
arch/x86/scripts/gen-insn-attr-x86.awk line 1
Quoted 3 lines · Masami Hiramatsu
+++
b/arch/x86/scripts/gen-insn-attr-x86.awkarch/x86/scripts/gen-insn-attr-x86.awk new file +1
-0@@ -0,0 +1,314 @@ +#!/bin/awk -fOn some distributions (debian) it is
/usr/bin/awk.True, but on most of them (all?) there is also an appropriate link in /bin.
Nope, not on debian testing. Although I assume if kernel compilation will start to fail it will appear :)
Quoted 3 lines
If shebang could have more that one argument, then '
/usr/bin/envawk -f' would be the best solution I think.
--
Gleb.
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
Quoted 13 lines · Gleb Natapov
Quoted 7 lines · Przemysssaw Paweeeczyk
Quoted 5 lines · Gleb Natapov
arch/x86/scripts/gen-insn-attr-x86.awk line 1
Quoted 3 lines · Masami Hiramatsu
+++
b/arch/x86/scripts/gen-insn-attr-x86.awkarch/x86/scripts/gen-insn-attr-x86.awk new file +1
-0@@ -0,0 +1,314 @@ +#!/bin/awk -fOn some distributions (debian) it is
/usr/bin/awk.True, but on most of them (all?) there is also an appropriate link in /bin.
Nope, not on debian testing. Although I assume if kernel compilation will start to fail it will appear :)
Quoted 2 lines
If shebang could have more that one argument, then '
/usr/bin/envawk -f' would be the best solution I think.
Ah, I see.
Actually, it will be executed from Makefile with 'awk -f'.
arch/x86/lib/Makefile line 7
Quoted 9 lines
arch/x86/lib/Makefile +3
-0@@ -2,12 +2,21 @@ # Makefile for x86 specific library files. # +quiet_cmd_inat_tables = GEN $@ + cmd_inat_tables = awk -f $(srctree)/arch/x86/scripts/gen-insn-attr-x86.awk $(srctree)/arch/x86/lib/x86-opcode-map.txt > $@ +
So, if awk is on the PATH, it will pass. Maybe, I need to add 'HOSTAWK = awk' line in Makefile.
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 1/7] x86: instruction decorder API
Quoted 24 lines · Masami Hiramatsu
Quoted 13 lines · Gleb Natapov
Quoted 7 lines · Przemysssaw Paweeeczyk
Quoted 5 lines · Gleb Natapov
arch/x86/scripts/gen-insn-attr-x86.awk line 1
Quoted 3 lines · Masami Hiramatsu
+++
b/arch/x86/scripts/gen-insn-attr-x86.awkarch/x86/scripts/gen-insn-attr-x86.awk new file +1
-0@@ -0,0 +1,314 @@ +#!/bin/awk -fOn some distributions (debian) it is
/usr/bin/awk.True, but on most of them (all?) there is also an appropriate link in /bin.
Nope, not on debian testing. Although I assume if kernel compilation will start to fail it will appear :)
Quoted 2 lines
If shebang could have more that one argument, then '
/usr/bin/envawk -f' would be the best solution I think.Ah, I see.
Actually, it will be executed fromMakefilewith 'awk -f'.Quoted 6 lines
arch/x86/lib/Makefile +0
-0@@ -2,12 +2,21 @@ # Makefile for x86 specific library files. #
Quoted 5 lines
arch/x86/lib/Makefile line 7
Quoted 3 lines
arch/x86/lib/Makefile +3
-0+quiet_cmd_inat_tables = GEN $@ + cmd_inat_tables = awk -f $(srctree)/arch/x86/scripts/gen-insn-attr-x86.awk $(srctree)/arch/x86/lib/x86-opcode-map.txt > $@ +So, if awk is on the PATH, it will pass.
Ah, that is good enough I thing. I tried to run scrip manually.
Quoted 2 lines
Maybe, I need to add 'HOSTAWK = awk' line in
Makefile.
--
Gleb.
PATCH v5 2/7 kprobes: checks probe address is instruction boudary on x86
In reply to Masami HiramatsuEnsure safeness of inserting kprobes by checking whether the specified address is at the first byte of a instruction on x86.
This is done by decoding probed function from its head to the probe point.
---
arch/x86/kernel/kprobes.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-)
arch/x86/kernel/kprobes.c +54 -0
@@ -48,12 +48,14 @@#include <linux/preempt.h> #include <linux/module.h> #include <linux/kdebug.h> +#include <linux/kallsyms.h> #include <asm/cacheflush.h> #include <asm/desc.h> #include <asm/pgtable.h> #include <asm/uaccess.h> #include <asm/alternative.h> +#include <asm/insn.h> void jprobe_return_end (void );
@@ -244,6 +246,56 @@ retry: } } +/* Recover the probed instruction at addr for further analysis. */ +static int recover_probed_instruction (kprobe_opcode_t *buf,unsigned long addr) +{ +struct kprobe *kp; + kp =get_kprobe ((void *)addr); +if (!kp) +return -EINVAL ; + +/* +* Don't use p->ainsn.insn, which could be modified -- e.g., +* by fix_riprel(). +*/ +memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); + buf[0 ] = kp->opcode; +return 0 ; +} + +/* Dummy buffers for kallsyms_lookup */ +static char __dummy_buf[KSYM_NAME_LEN ]; + +/* Check if paddr is at an instruction boundary */ +static int __kprobes can_probe (unsigned long paddr) +{ +int ret; +unsigned long addr, offset =0 ; +struct insn insn; +kprobe_opcode_t buf[MAX_INSN_SIZE ]; + +/* Lookup symbol including addr */ +if (!kallsyms_lookup (paddr,NULL , &offset,NULL , __dummy_buf)) +return 0 ; + +/* Decode instructions */ + addr = paddr - offset; +while (addr < paddr) { +insn_init_kernel (&insn, (void *)addr); +insn_get_opcode (&insn); +if (OPCODE1 (&insn) ==BREAKPOINT_INSTRUCTION ) { + ret =recover_probed_instruction (buf, addr); +if (ret) +return 0 ; +insn_init_kernel (&insn, buf); + } +insn_get_length (&insn); + addr += insn.length; + } + +return (addr == paddr); +} +/* * Returns non-zero if opcode modifies the interrupt flag. */
@@ -359,6 +411,8 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)int __kprobes arch_prepare_kprobe (struct kprobe *p) { +if (!can_probe ((unsigned long )p->addr)) +return -EILSEQ ;/* insn: must be on special executable page on x86. */ p->ainsn.insn =get_insn_slot ();if (!p->ainsn.insn)
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Steven Rostedt
maintainerRe: [PATCH -tip v5 2/7] kprobes: checks probe address is instruction boudary on x86
Quoted 34 lines · Masami Hiramatsu
Ensure safeness of inserting kprobes by checking whether the specified address is at the first byte of a instruction on x86.
This is done by decoding probed function from its head to the probe point.Signed-off-by Masami Hiramatsu <[email protected]>Cc Ananth N Mavinakayanahalli <[email protected]>Cc Jim Keniston <[email protected]>Cc Ingo Molnar <[email protected]>---
arch/x86/kernel/kprobes.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-)arch/x86/kernel/kprobes.c +2
-0@@ -48,12 +48,14 @@#include <linux/preempt.h> #include <linux/module.h> #include <linux/kdebug.h> +#include <linux/kallsyms.h> #include <asm/cacheflush.h> #include <asm/desc.h> #include <asm/pgtable.h> #include <asm/uaccess.h> #include <asm/alternative.h> +#include <asm/insn.h> void jprobe_return_end (void );@@ -244,6 +246,56 @@ retry: }
arch/x86/kernel/kprobes.c retry: · line 256
Quoted 10 lines
arch/x86/kernel/kprobes.c +8
-0} +/* Recover the probed instruction at addr for further analysis. */ +static int recover_probed_instruction (kprobe_opcode_t *buf,unsigned long addr) +{ +struct kprobe *kp; + kp =get_kprobe ((void *)addr); +if (!kp) +return -EINVAL ; +
I'm just doing a casual scan of the patch set.
arch/x86/kernel/kprobes.c retry: · line 258
Quoted 2 lines
arch/x86/kernel/kprobes.c +2
-0+/* +* Don't use p->ainsn.insn, which could be modified -- e.g.,
This comment talks about "p", what's that? It's not used in this function.
arch/x86/kernel/kprobes.c retry: · line 262
Quoted 4 lines
arch/x86/kernel/kprobes.c +4
-0+* by fix_riprel(). +*/ +memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); + buf[0 ] = kp->opcode;
Why is it OK to copy addr to "buf" and then rewrite the first character of buf? Does it have something to do with the above "p"?
I don't mean to be critical here, but I've been doing "Mother Day" activities all weekend and for some reason that was also the best time for everyone to Cc me on patches. I'm way behind in my email, and it would be nice if the comments described why things that "look" wrong are not.
Quoted 5 lines
arch/x86/kernel/kprobes.c +5
-0+return 0 ; +} + +/* Dummy buffers for kallsyms_lookup */ +static char __dummy_buf[KSYM_NAME_LEN ];
arch/x86/kernel/kprobes.c retry: · line 277
Quoted 10 lines
arch/x86/kernel/kprobes.c +10
-0+ +/* Check if paddr is at an instruction boundary */ +static int __kprobes can_probe (unsigned long paddr) +{ +int ret; +unsigned long addr, offset =0 ; +struct insn insn; +kprobe_opcode_t buf[MAX_INSN_SIZE ]; + +/* Lookup symbol including addr */
The above comment is very close to a "add one to i" for i++ type of comment.
arch/x86/kernel/kprobes.c retry: · line 287
Quoted 10 lines
arch/x86/kernel/kprobes.c +10
-0+if (!kallsyms_lookup (paddr,NULL , &offset,NULL , __dummy_buf)) +return 0 ; + +/* Decode instructions */ + addr = paddr - offset; +while (addr < paddr) { +insn_init_kernel (&insn, (void *)addr); +insn_get_opcode (&insn); +if (OPCODE1 (&insn) ==BREAKPOINT_INSTRUCTION ) { + ret =recover_probed_instruction (buf, addr);
Oh, the above puts back the original op code. That is why it is OK?
I'd comment that a little bit more. Just so that reviewers have an easier idea of what is happening.
arch/x86/kernel/kprobes.c retry: · line 290
Quoted 3 lines
arch/x86/kernel/kprobes.c +3
-0+if (ret) +return 0 ; +insn_init_kernel (&insn, buf);
insn_init_kernel? Is that like a text poke or something?
Quoted 12 lines
arch/x86/kernel/kprobes.c +8
-0+ } +insn_get_length (&insn); + addr += insn.length; + } + +return (addr == paddr); +} +/* * Returns non-zero if opcode modifies the interrupt flag. */
arch/x86/kernel/kprobes.c static void __kprobes arch_copy_kprobe(struct kprobe *p) · line 417
Quoted 7 lines
arch/x86/kernel/kprobes.c +2
-0@@ -359,6 +411,8 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)int __kprobes arch_prepare_kprobe (struct kprobe *p) { +if (!can_probe ((unsigned long )p->addr)) +return -EILSEQ ;/* insn: must be on special executable page on x86. */ p->ainsn.insn =get_insn_slot ();
Oh look, I found the phantom "p"!
-- Steve
Quoted 12 lines
arch/x86/kernel/kprobes.c +0
-0if (!p->ainsn.insn)Signature · 8 lines
-- Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 2/7] kprobes: checks probe address is instruction boudary on x86
Quoted 38 lines · Steven Rostedt
Quoted 36 lines · Masami Hiramatsu
Ensure safeness of inserting kprobes by checking whether the specified address is at the first byte of a instruction on x86.
This is done by decoding probed function from its head to the probe point.Signed-off-by Masami Hiramatsu <[email protected]>Cc Ananth N Mavinakayanahalli <[email protected]>Cc Jim Keniston <[email protected]>Cc Ingo Molnar <[email protected]>---
arch/x86/kernel/kprobes.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 54 insertions(+), 0 deletions(-)arch/x86/kernel/kprobes.c +2
-0@@ -48,12 +48,14 @@#include <linux/preempt.h> #include <linux/module.h> #include <linux/kdebug.h> +#include <linux/kallsyms.h> #include <asm/cacheflush.h> #include <asm/desc.h> #include <asm/pgtable.h> #include <asm/uaccess.h> #include <asm/alternative.h> +#include <asm/insn.h> void jprobe_return_end (void );@@ -244,6 +246,56 @@ retry: } }
Quoted 10 lines
arch/x86/kernel/kprobes.c retry: · line 256
Quoted 8 lines
arch/x86/kernel/kprobes.c +8
-0+/* Recover the probed instruction at addr for further analysis. */ +static int recover_probed_instruction (kprobe_opcode_t *buf,unsigned long addr) +{ +struct kprobe *kp; + kp =get_kprobe ((void *)addr); +if (!kp) +return -EINVAL ; +I'm just doing a casual scan of the patch set.
Thank you!
Quoted 5 lines
arch/x86/kernel/kprobes.c retry: · line 258
Quoted 3 lines
arch/x86/kernel/kprobes.c +2
-0+/* +* Don't use p->ainsn.insn, which could be modified -- e.g., This comment talks about "p", what's that? It's not used in this function.
oops, this should be kp.
Quoted 8 lines
arch/x86/kernel/kprobes.c retry: · line 262
Quoted 5 lines
arch/x86/kernel/kprobes.c +4
-0+* by fix_riprel(). +*/ +memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); + buf[0 ] = kp->opcode;Why is it OK to copy addr to "buf" and then rewrite the first character of buf? Does it have something to do with the above "p"?
Yes, each kprobe copied probed instruction to kp->ainsn.insn, which is an executable buffer for single stepping.
So, basically, kp->ainsn.insn has an original instruction. However, RIP-relative instruction can not do single-stepping at different place, fix_riprel() tweaks the displacement of that instruction. In that case, we can't recover the instruction from the kp->ainsn.insn.
On the other hand, kp->opcode has a copy of the first byte of the probed instruction, which is overwritten by int3. And the instruction at kp->addr is not modified by kprobes except for the first byte, we can recover the original instruction from it and kp->opcode.
Quoted 16 lines
I don't mean to be critical here, but I've been doing "Mother Day" activities all weekend and for some reason that was also the best time for everyone to Cc me on patches. I'm way behind in my email, and it would be nice if the comments described why things that "look" wrong are not.
Quoted 10 lines
arch/x86/kernel/kprobes.c +10
-0+return 0 ; +} + +/* Dummy buffers for kallsyms_lookup */ +static char __dummy_buf[KSYM_NAME_LEN ]; + +/* Check if paddr is at an instruction boundary */ +static int __kprobes can_probe (unsigned long paddr) +{ +int ret;
Quoted 8 lines
arch/x86/kernel/kprobes.c retry: · line 277
Quoted 5 lines
arch/x86/kernel/kprobes.c +5
-0+unsigned long addr, offset =0 ; +struct insn insn; +kprobe_opcode_t buf[MAX_INSN_SIZE ]; + +/* Lookup symbol including addr */ The above comment is very close to a "add one to i" for i++ type of comment.
Agreed.
Quoted 13 lines
arch/x86/kernel/kprobes.c retry: · line 287
Quoted 11 lines
arch/x86/kernel/kprobes.c +10
-0+if (!kallsyms_lookup (paddr,NULL , &offset,NULL , __dummy_buf)) +return 0 ; + +/* Decode instructions */ + addr = paddr - offset; +while (addr < paddr) { +insn_init_kernel (&insn, (void *)addr); +insn_get_opcode (&insn); +if (OPCODE1 (&insn) ==BREAKPOINT_INSTRUCTION ) { + ret =recover_probed_instruction (buf, addr);Oh, the above puts back the original op code. That is why it is OK?
Oops, no. I have to use get_kprobe() instead. Thanks!
Quoted 9 lines
I'd comment that a little bit more. Just so that reviewers have an easier idea of what is happening.
arch/x86/kernel/kprobes.c retry: · line 290
Quoted 3 lines
arch/x86/kernel/kprobes.c +3
-0+if (ret) +return 0 ; +insn_init_kernel (&insn, buf);
insn_init_kernel? Is that like a text poke or something?
it's a wrapper of insn_init() which initialize struct insn.
Thank you,
Quoted 36 lines
Quoted 12 lines
arch/x86/kernel/kprobes.c +8
-0+ } +insn_get_length (&insn); + addr += insn.length; + } + +return (addr == paddr); +} +/* * Returns non-zero if opcode modifies the interrupt flag. */ arch/x86/kernel/kprobes.c static void __kprobes arch_copy_kprobe(struct kprobe *p) · line 417
Quoted 7 lines
arch/x86/kernel/kprobes.c +2
-0@@ -359,6 +411,8 @@ static void __kprobes arch_copy_kprobe(struct kprobe *p)int __kprobes arch_prepare_kprobe (struct kprobe *p) { +if (!can_probe ((unsigned long )p->addr)) +return -EILSEQ ;/* insn: must be on special executable page on x86. */ p->ainsn.insn =get_insn_slot ();Oh look, I found the phantom "p"!
-- Steve
Quoted 12 lines
arch/x86/kernel/kprobes.c +0
-0if (!p->ainsn.insn)Signature · 8 lines
-- Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 2/7] kprobes: checks probe address is instruction boudary on x86
Quoted 12 lines · Masami Hiramatsu
Quoted 11 lines
arch/x86/kernel/kprobes.c retry: · line 287
Quoted 10 lines
arch/x86/kernel/kprobes.c +10
-0+if (!kallsyms_lookup (paddr,NULL , &offset,NULL , __dummy_buf)) +return 0 ; + +/* Decode instructions */ + addr = paddr - offset; +while (addr < paddr) { +insn_init_kernel (&insn, (void *)addr); +insn_get_opcode (&insn); +if (OPCODE1 (&insn) ==BREAKPOINT_INSTRUCTION ) { + ret =recover_probed_instruction (buf, addr);Oh, the above puts back the original op code. That is why it is OK?
Quoted 1 line
Oops, no. I have to use
get_kprobe()instead. Thanks!
Ah, I forgot another possibility. There might be another subsystem, like kgdb, will put their break point on the kernel.
In that case, decoder will decode the instruction is a break point instruction and the first opcode is int3. So, this part is correct. In the future, we need to add a generic recover_instruction() code for those text modification subsystems.
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Steven Rostedt
maintainerRe: [PATCH -tip v5 2/7] kprobes: checks probe address is instruction boudary on x86
In reply to Masami HiramatsuQuoted 18 lines · Masami Hiramatsu
Quoted 9 lines
arch/x86/kernel/kprobes.c retry: · line 262
Quoted 6 lines
arch/x86/kernel/kprobes.c +4
-0+* by fix_riprel(). +*/ +memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); + buf[0 ] = kp->opcode;Why is it OK to copy addr to "buf" and then rewrite the first character of buf? Does it have something to do with the above "p"?
Yes, each kprobe copied probed instruction to
kp->ainsn.insn, which is an executable buffer for single stepping.
So, basically,kp->ainsn.insnhas an original instruction. However, RIP-relative instruction can not do single-stepping at different place,fix_riprel()tweaks the displacement of that instruction. In that case, we can't recover the instruction from thekp->ainsn.insn.
Quoted 5 lines
On the other hand,
kp->opcodehas a copy of the first byte of the probed instruction, which is overwritten by int3. And the instruction atkp->addris not modified by kprobes except for the first byte, we can recover the original instruction from it andkp->opcode.
For code that is awkward, complex or non-trivial, don't be afraid to put in a paragraph explaining the code. The above explanation should be a comment in the code. Otherwise people like me would just look at it and say "huh?".
Note, I'm a bit cranky this morning, so I hope I don't offend anyone.
-- Steve
Re: [PATCH -tip v5 2/7] kprobes: checks probe address is instruction boudary on x86
Quoted 21 lines · Steven Rostedt
Quoted 14 lines · Masami Hiramatsu
Quoted 6 lines
arch/x86/kernel/kprobes.c retry: · line 262
Quoted 4 lines
arch/x86/kernel/kprobes.c +4
-0+* by fix_riprel(). +*/ +memcpy (buf, kp->addr,MAX_INSN_SIZE *sizeof (kprobe_opcode_t)); + buf[0 ] = kp->opcode;Why is it OK to copy addr to "buf" and then rewrite the first character of buf? Does it have something to do with the above "p"?
Yes, each kprobe copied probed instruction to
kp->ainsn.insn, which is an executable buffer for single stepping.
So, basically,kp->ainsn.insnhas an original instruction. However, RIP-relative instruction can not do single-stepping at different place,fix_riprel()tweaks the displacement of that instruction. In that case, we can't recover the instruction from thekp->ainsn.insn.Quoted 5 lines
On the other hand,
kp->opcodehas a copy of the first byte of the probed instruction, which is overwritten by int3. And the instruction atkp->addris not modified by kprobes except for the first byte, we can recover the original instruction from it andkp->opcode.
Quoted 6 lines
For code that is awkward, complex or non-trivial, don't be afraid to put in a paragraph explaining the code. The above explanation should be a comment in the code. Otherwise people like me would just look at it and say "huh?".
Note, I'm a bit cranky this morning, so I hope I don't offend anyone.
No, that's very helpful review for me. Thanks :-)
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
PATCH v5 3/7 kprobes: cleanup fix_riprel() using insn decoder on x86
In reply to Masami HiramatsuCleanup fix_riprel() in arch/x86/kernel/kprobes.c by using x86 instruction decoder.
---
arch/x86/kernel/kprobes.c | 128 ++++++++------------------------------------- 1 files changed, 23 insertions(+), 105 deletions(-)
arch/x86/kernel/kprobes.c +23 -105
@@ -108,50 +108,6 @@ static const u32 twobyte_is_boostable[256 / 32] = {/* ----------------------------------------------- */ /* 0 1 2 3 4 5 6 7 8 9 a b c d e f */ }; -static const u32 onebyte_has_modrm[256 /32 ] = { -/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */ -/* ----------------------------------------------- */ -W (0x00 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ) |/* 00 */ -W (0x10 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ) ,/* 10 */ -W (0x20 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ) |/* 20 */ -W (0x30 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ) ,/* 30 */ -W (0x40 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) |/* 40 */ -W (0x50 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) ,/* 50 */ -W (0x60 ,0 ,0 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,1 ,0 ,1 ,0 ,0 ,0 ,0 ) |/* 60 */ -W (0x70 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) ,/* 70 */ -W (0x80 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) |/* 80 */ -W (0x90 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) ,/* 90 */ -W (0xa0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) |/* a0 */ -W (0xb0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) ,/* b0 */ -W (0xc0 ,1 ,1 ,0 ,0 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) |/* c0 */ -W (0xd0 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) ,/* d0 */ -W (0xe0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) |/* e0 */ -W (0xf0 ,0 ,0 ,0 ,0 ,0 ,0 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,0 ,1 ,1 )/* f0 */ -/* ----------------------------------------------- */ -/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */ -}; -static const u32 twobyte_has_modrm[256 /32 ] = { -/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */ -/* ----------------------------------------------- */ -W (0x00 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,1 ,0 ,1 ) |/* 0f */ -W (0x10 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) ,/* 1f */ -W (0x20 ,1 ,1 ,1 ,1 ,1 ,0 ,1 ,0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) |/* 2f */ -W (0x30 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) ,/* 3f */ -W (0x40 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) |/* 4f */ -W (0x50 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) ,/* 5f */ -W (0x60 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) |/* 6f */ -W (0x70 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ) ,/* 7f */ -W (0x80 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) |/* 8f */ -W (0x90 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) ,/* 9f */ -W (0xa0 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,1 ,1 ,1 ,1 ,1 ) |/* af */ -W (0xb0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,0 ,0 ,1 ,1 ,1 ,1 ,1 ,1 ) ,/* bf */ -W (0xc0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ) |/* cf */ -W (0xd0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) ,/* df */ -W (0xe0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ) |/* ef */ -W (0xf0 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,1 ,0 )/* ff */ -/* ----------------------------------------------- */ -/* 0 1 2 3 4 5 6 7 8 9 a b c d e f */ -};#undef Wstruct kretprobe_blackpoint kretprobe_blacklist[] = {
@@ -329,68 +285,30 @@ static int __kprobes is_IF_modifier(kprobe_opcode_t *insn)static void __kprobes fix_riprel (struct kprobe *p) {#ifdef CONFIG_X86_64 -u8 *insn = p->ainsn.insn; -s64 disp; -int need_modrm; - -/* Skip legacy instruction prefixes. */ -while (1 ) { -switch (*insn) { -case 0x66 : -case 0x67 : -case 0x2e : -case 0x3e : -case 0x26 : -case 0x64 : -case 0x65 : -case 0x36 : -case 0xf0 : -case 0xf3 : -case 0xf2 : - ++insn; -continue ; - } -break ; - } +struct insn insn; +insn_init_kernel (&insn, p->ainsn.insn); -/* Skip REX instruction prefix. */ -if (is_REX_prefix (insn)) - ++insn; - -if (*insn ==0x0f ) { -/* Two-byte opcode. */ - ++insn; - need_modrm =test_bit (*insn, - (unsigned long *)twobyte_has_modrm); - }else -/* One-byte opcode. */ - need_modrm =test_bit (*insn, - (unsigned long *)onebyte_has_modrm); - -if (need_modrm) { -u8 modrm = *++insn; -if ((modrm &0xc7 ) ==0x05 ) { -/* %rip+disp32 addressing mode */ -/* Displacement follows ModRM byte. */ - ++insn; -/* -* The copied instruction uses the %rip-relative -* addressing mode. Adjust the displacement for the -* difference between the original location of this -* instruction and the location of the copy that will -* actually be run. The tricky bit here is making sure -* that the sign extension happens correctly in this -* calculation, since we need a signed 32-bit result to -* be sign-extended to 64 bits when it's added to the -* %rip value and yield the same 64-bit result that the -* sign-extension of the original signed 32-bit -* displacement would have given. -*/ - disp = (u8 *) p->addr + *((s32 *) insn) - - (u8 *) p->ainsn.insn; -BUG_ON ((s64 ) (s32 ) disp != disp);/* Sanity check. */ - *(s32 *)insn = (s32 ) disp; - } +if (insn_rip_relative (&insn)) { +s64 newdisp; +u8 *disp; +insn_get_displacement (&insn); +/* +* The copied instruction uses the %rip-relative addressing +* mode. Adjust the displacement for the difference between +* the original location of this instruction and the location +* of the copy that will actually be run. The tricky bit here +* is making sure that the sign extension happens correctly in +* this calculation, since we need a signed 32-bit result to +* be sign-extended to 64 bits when it's added to the %rip +* value and yield the same 64-bit result that the sign- +* extension of the original signed 32-bit displacement would +* have given. +*/ + newdisp = (u8 *) p->addr + (s64 ) insn.displacement.value - + (u8 *) p->ainsn.insn; +BUG_ON ((s64 ) (s32 ) newdisp != newdisp);/* Sanity check. */ + disp = (u8 *) p->ainsn.insn +INSN_DISPLACEMENT_OFFS (&insn); + *(s32 *) disp = (s32 ) newdisp; }#endif }
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
PATCH v5 4/7 tracing: add kprobe-based event tracer
In reply to Masami HiramatsuAdd kprobes based event tracer on ftrace.
This tracer is similar to the events tracer which is based on Tracepoint infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe and kretprobe). It probes anywhere where kprobes can probe(this means, all functions body except for __kprobes functions).
Changes from v4:
- Change interface name from 'kprobe_probes' to 'kprobe_events'
- Skip comments (words after '#') from inputs of 'kprobe_events'.
---
Documentation/trace/ftrace.txt | 55 +++++ kernel/trace/Kconfig | 9 + kernel/trace/Makefile | 1 kernel/trace/trace_kprobe.c | 404 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 469 insertions(+), 0 deletions(-) create mode 100644 kernel/trace/trace_kprobe.c
Documentation/trace/ftrace.txt +55 -0
@@ -1310,6 +1310,61 @@ dereference in a kernel module: [...] +kprobe-based event tracer +--------------------------- + +This tracer is similar to the events tracer which is based on Tracepoint +infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe +and kretprobe). It probes anywhere where kprobes can probe(this means, all +functions body except for __kprobes functions). + +Unlike the function tracer, this tracer can probe instructions inside of +kernel functions. It allows you to check which instruction has been executed. + +Unlike the Tracepoint based events tracer, this tracer can add new probe points +on the fly. + +Similar to the events tracer, this tracer doesn't need to be activated via +current_tracer, instead of that, just set probe points via +/debug/tracing/kprobe_events. + +Synopsis of kprobe_events: + p SYMBOL[+offs|-offs]|MEMADDR : set a probe + r SYMBOL[+0] : set a return probe + +E.g. + echo p sys_open > /debug/tracing/kprobe_events + + This sets a kprobe on the top of sys_open() function. + + echo r sys_open >> /debug/tracing/kprobe_events + + This sets a kretprobe on the return point of sys_open() function. + + echo > /debug/tracing/kprobe_events + + This clears all probe points. and you can see the traced information via +/debug/tracing/trace. + + cat /debug/tracing/trace +# tracer: nop +# +# TASK-PID CPU# TIMESTAMP FUNCTION +# | | | | | + <...>-5117 [003] 416.481638: sys_open: @sys_open+0 + <...>-5117 [003] 416.481662: syscall_call: <-sys_open+0 + <...>-5117 [003] 416.481739: sys_open: @sys_open+0 + <...>-5117 [003] 416.481762: sysenter_do_call: <-sys_open+0 + <...>-5117 [003] 416.481818: sys_open: @sys_open+0 + <...>-5117 [003] 416.481842: sysenter_do_call: <-sys_open+0 + <...>-5117 [003] 416.481882: sys_open: @sys_open+0 + <...>-5117 [003] 416.481905: sysenter_do_call: <-sys_open+0 + + @SYMBOL means that kernel hits a probe, and <-SYMBOL means kernel returns +from SYMBOL(e.g. "sysenter_do_call: <-sys_open+0" means kernel returns from +sys_open to sysenter_do_call). + + function graph tracer ---------------------------
kernel/trace/Kconfig +9 -0
@@ -398,6 +398,15 @@ config BLK_DEV_IO_TRACE If unsure, say N. +config KPROBE_TRACER + depends on KPROBES + depends on X86 + bool "Trace kprobes" + select TRACING + help + This tracer probes everywhere where kprobes can probe it, and + records various registers and memories specified by user. + config DYNAMIC_FTRACE bool "enable/disable ftrace tracepoints dynamically" depends on FUNCTION_TRACER
kernel/trace/Makefile +1 -0
@@ -51,5 +51,6 @@ obj-$(CONFIG_EVENT_TRACING) += trace_export.o obj-$(CONFIG_FTRACE_SYSCALLS) += trace_syscalls.o obj-$(CONFIG_EVENT_PROFILE) += trace_event_profile.o obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o +obj-$(CONFIG_KPROBE_TRACER) += trace_kprobe.o libftrace-y := ftrace.o
kernel/trace/trace_kprobe.c new file +404 -0
404 lines not on this page · raw message
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
Hi,
2009/5/9 Masami Hiramatsu <[email protected]>:
Quoted 501 lines
Add kprobes based event tracer on ftrace.
This tracer is similar to the events tracer which is based on Tracepoint infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe and kretprobe). It probes anywhere where kprobes can
probe(this means, all functions body except for __kprobes functions).Changes from v4:
- Change interface name from 'kprobe_probes' to 'kprobe_events'
- Skip comments (words after '#') from inputs of 'kprobe_events'.Signed-off-by Masami Hiramatsu <[email protected]>Cc Steven Rostedt <[email protected]>Cc Ananth N Mavinakayanahalli <[email protected]>Cc Ingo Molnar <[email protected]>Cc Frederic Weisbecker <[email protected]>---
Documentation/trace/ftrace.txt | 55 +++++
kernel/trace/Kconfig | 9 +
kernel/trace/Makefile | 1
kernel/trace/trace_kprobe.c | 404 ++++++++++++++++++++++++++++++++++++++++4 files changed, 469 insertions(+), 0 deletions(-)create mode 100644
kernel/trace/trace_kprobe.cDocumentation/trace/ftrace.txt +55
-0@@ -1310,6 +1310,61 @@ dereference in a kernel module: +kprobe-based event tracer +--------------------------- + +This tracer is similar to the events tracer which is based on Tracepoint +infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe +and kretprobe). It probes anywhere where kprobes can probe(this means, all +functions body except for __kprobes functions). + +Unlike the function tracer, this tracer can probe instructions inside of +kernel functions. It allows you to check which instruction has been executed. + +Unlike the Tracepoint based events tracer, this tracer can add new probe points +on the fly. + +Similar to the events tracer, this tracer doesn't need to be activated via +current_tracer, instead of that, just set probe points via +/debug/tracing/kprobe_events. + +Synopsis of kprobe_events: + p SYMBOL[+offs|-offs]|MEMADDR : set a probe + r SYMBOL[+0] : set a return probe + +E.g. + echo p sys_open > /debug/tracing/kprobe_events + + This sets a kprobe on the top of sys_open() function. + + echo r sys_open >> /debug/tracing/kprobe_events + + This sets a kretprobe on the return point of sys_open() function. + + echo > /debug/tracing/kprobe_events + + This clears all probe points. and you can see the traced information via +/debug/tracing/trace. + + cat /debug/tracing/trace +# tracer: nop +# +# TASK-PID CPU# TIMESTAMP FUNCTION +# | | | | | + <...>-5117 [003] 416.481638: sys_open: @sys_open+0 + <...>-5117 [003] 416.481662: syscall_call: <-sys_open+0 + <...>-5117 [003] 416.481739: sys_open: @sys_open+0 + <...>-5117 [003] 416.481762: sysenter_do_call: <-sys_open+0 + <...>-5117 [003] 416.481818: sys_open: @sys_open+0 + <...>-5117 [003] 416.481842: sysenter_do_call: <-sys_open+0 + <...>-5117 [003] 416.481882: sys_open: @sys_open+0 + <...>-5117 [003] 416.481905: sysenter_do_call: <-sys_open+0 + + @SYMBOL means that kernel hits a probe, and <-SYMBOL means kernel returns +from SYMBOL(e.g. "sysenter_do_call: <-sys_open+0" means kernel returns from +sys_open to sysenter_do_call). + +function graph tracer
---------------------------kernel/trace/Kconfig +0
-0@@ -398,6 +398,15 @@ config BLK_DEV_IO_TRACEIf unsure, say N.
kernel/trace/Kconfig +9
-0+config KPROBE_TRACER + depends on KPROBES + depends on X86 + bool "Trace kprobes" + select TRACING + help + This tracer probes everywhere where kprobes can probe it, and + records various registers and memories specified by user. +config
DYNAMIC_FTRACE
bool "enable/disable ftrace tracepoints dynamically" depends onFUNCTION_TRACERkernel/trace/Makefile +0
-0@@ -51,5 +51,6 @@ obj-$(CONFIG_EVENT_TRACING) += trace_export.oobj-$(CONFIG_FTRACE_SYSCALLS) += trace_syscalls.o obj-$(CONFIG_EVENT_PROFILE) += trace_event_profile.o obj-$(CONFIG_EVENT_TRACING) += trace_events_filter.o +obj-$(CONFIG_KPROBE_TRACER) += trace_kprobe.o
libftrace-y := ftrace.o
kernel/trace/trace_kprobe.c new file +373
-0@@ -0,0 +1,404 @@ +/* +* kprobe based kernel tracer +* +* Created by Masami Hiramatsu <[email protected]> +* +* This program is free software; you can redistribute it and/or modify +* it under the terms of the GNU General Public License version 2 as +* published by the Free Software Foundation. +* +* This program is distributed in the hope that it will be useful, +* but WITHOUT ANY WARRANTY; without even the implied warranty of +* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +* GNU General Public License for more details. +* +* You should have received a copy of the GNU General Public License +* along with this program; if not, write to the Free Software +* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA +*/ + +#include <linux/module.h> +#include <linux/uaccess.h> +#include <linux/kprobes.h> +#include <linux/seq_file.h> +#include <linux/slab.h> +#include <linux/smp.h> +#include <linux/debugfs.h> +#include <linux/types.h> +#include <linux/string.h> +#include <linux/ctype.h> + +#include <linux/ftrace.h> +#include "trace.h" + +/** +* kprobe_trace_core +*/ +#define TRACE_MAXARGS 6 + +struct trace_probe { + struct list_head list; + union { + struct kprobe kp; + struct kretprobe rp; + }; + const char *symbol; /* symbol name */ +}; + +static void kprobe_trace_record (unsigned long ip,struct trace_probe *tp, + struct pt_regs *regs); + +static int kprobe_trace_func (struct kprobe *kp,struct pt_regs *regs) +{ + struct trace_probe *tp = container_of(kp, struct trace_probe, kp); + + kprobe_trace_record(instruction_pointer(regs), tp, regs); + return 0; +} + +static int kretprobe_trace_func (struct kretprobe_instance *ri, + struct pt_regs *regs) +{ + struct trace_probe *tp = container_of(ri->rp, struct trace_probe, rp); + + kprobe_trace_record((unsigned long)ri->ret_addr, tp, regs); + return 0; +} + +static int probe_is_return (struct trace_probe *tp) +{ + return (tp->rp.handler == kretprobe_trace_func); +} + +static const char *probe_symbol (struct trace_probe *tp) +{ + return tp->symbol ? tp->symbol : "unknown"; +} + +static long probe_offset (struct trace_probe *tp) +{ + return (probe_is_return(tp)) ? tp->rp.kp.offset : tp->kp.offset; +} + +static void *probe_address (struct trace_probe *tp) +{ + return (probe_is_return(tp)) ? tp->rp.kp.addr : tp->kp.addr; +} + + +static DEFINE_MUTEX (probe_lock); +static LIST_HEAD (probe_list); + +static struct trace_probe *alloc_trace_probe (const char *symbol) +{ + struct trace_probe *tp; + + tp = kzalloc(sizeof(struct trace_probe), GFP_KERNEL); + if (!tp) + return ERR_PTR(-ENOMEM); + + if (symbol) { + tp->symbol = kstrdup(symbol, GFP_KERNEL); + if (!tp->symbol) { + kfree(tp); + return ERR_PTR(-ENOMEM); + } + } + + INIT_LIST_HEAD(&tp->list); + return tp; +} + +static void free_trace_probe (struct trace_probe *tp) +{ + kfree(tp->symbol); + kfree(tp); +} + +static int register_trace_probe (struct trace_probe *tp) +{ + int ret; + + mutex_lock(&probe_lock); + list_add_tail(&tp->list, &probe_list); + + if (probe_is_return(tp)) + ret = register_kretprobe(&tp->rp); + else + ret = register_kprobe(&tp->kp); + + if (ret) { + pr_warning("Could not insert probe(%d)\n", ret); + if (ret == -EILSEQ) { + pr_warning("Probing address(0x%p) is not an " + "instruction boundary.\n", + probe_address(tp)); + ret = -EINVAL; + } + list_del(&tp->list); + } + mutex_unlock(&probe_lock); + return ret; +} + +static void unregister_trace_probe (struct trace_probe *tp) +{ + if (probe_is_return(tp)) + unregister_kretprobe(&tp->rp); + else + unregister_kprobe(&tp->kp); + list_del(&tp->list); +} + +static int create_trace_probe (int argc,char **argv) +{ + /* + * Argument syntax: + * - Add kprobe: p SYMBOL[+OFFS|-OFFS]|ADDRESS + * - Add kretprobe: r SYMBOL[+0] + */ + struct trace_probe *tp; + struct kprobe *kp; + char *tmp; + int ret = 0; + int is_return = 0; + char *symbol = NULL; + long offset = 0; + void *addr = NULL; + + if (argc < 2) + return -EINVAL; + + if (argv[0][0] == 'p') + is_return = 0; + else if (argv[0][0] == 'r') + is_return = 1; + else + return -EINVAL; + + if (isdigit(argv[1][0])) { + if (is_return) + return -EINVAL; + /* an address specified */ + ret = strict_strtoul(&argv[0][2], 0, (unsigned long *)&addr); + if (ret) + return ret; + } else { + /* a symbol specified */ + symbol = argv[1]; + /* TODO: support .init module functions */ + tmp = strchr(symbol, '+'); + if (!tmp) + tmp = strchr(symbol, '-'); + + if (tmp) { + /* skip sign because strict_strtol doesn't accept '+' */ + ret = strict_strtol(tmp + 1, 0, &offset); + if (ret) + return ret; + if (*tmp == '-') + offset = -offset; + *tmp = '\0'; + } + if (offset && is_return) + return -EINVAL; + } + + /* setup a probe */ + tp = alloc_trace_probe(symbol); + if (IS_ERR(tp)) + return PTR_ERR(tp); + + if (is_return) { + kp = &tp->rp.kp; + tp->rp.handler = kretprobe_trace_func; + } else { + kp = &tp->kp; + tp->kp.pre_handler = kprobe_trace_func; + } + + if (tp->symbol) { + /* TODO: check offset is collect by using insn_decoder */ + kp->symbol_name = tp->symbol; + kp->offset = offset; + } else + kp->addr = addr; + + ret = register_trace_probe(tp); + if (ret) + goto error; + return 0; + +error: + free_trace_probe(tp); + return ret; +} + +static void cleanup_all_probes (void ) +{ + struct trace_probe *tp; + mutex_lock(&probe_lock); + /* TODO: Use batch unregistration */ + while (!list_empty(&probe_list)) { + tp = list_entry(probe_list.next, struct trace_probe, list); + unregister_trace_probe(tp); + free_trace_probe(tp); + } + mutex_unlock(&probe_lock); +} + + +/* Probes listing interfaces */ +static void *probes_seq_start (struct seq_file *m,loff_t *pos) +{ + mutex_lock(&probe_lock); + return seq_list_start(&probe_list, *pos); +} + +static void *probes_seq_next (struct seq_file *m,void *v,loff_t *pos) +{ + return seq_list_next(v, &probe_list, pos); +} + +static void probes_seq_stop (struct seq_file *m,void *v) +{ + mutex_unlock(&probe_lock); +} + +static int probes_seq_show (struct seq_file *m,void *v) +{ + struct trace_probe *tp = v; + + if (tp == NULL) + return 0; + + if (tp->symbol) + seq_printf(m, "%c %s%+ld\n", + probe_is_return(tp) ? 'r' : 'p', + probe_symbol(tp), probe_offset(tp)); + else + seq_printf(m, "%c 0x%p\n", + probe_is_return(tp) ? 'r' : 'p', + probe_address(tp)); + return 0; +} + +static const struct seq_operations probes_seq_op = { + .start = probes_seq_start, + .next = probes_seq_next, + .stop = probes_seq_stop, + .show = probes_seq_show +}; + +static int probes_open (struct inode *inode,struct file *file) +{ + if ((file->f_mode & FMODE_WRITE) && + !(file->f_flags & O_APPEND)) + cleanup_all_probes(); + + return seq_open(file, &probes_seq_op); +} + + +#define WRITE_BUFSIZE 128 + +static ssize_t probes_write (struct file *file,const char __user *buffer, + size_t count, loff_t *ppos) +{ + char *kbuf, *tmp; + char **argv = NULL; + int argc = 0; + int ret; + size_t done; + size_t size; + + if (!count || count < 0) + return 0; + + kbuf = kmalloc(WRITE_BUFSIZE, GFP_KERNEL); + if (!kbuf) + return -ENOMEM; + + ret = done = 0; + do { + size = count - done; + if (size > WRITE_BUFSIZE) + size = WRITE_BUFSIZE; + if (copy_from_user(kbuf, buffer + done, size)) { + ret = -EFAULT; + goto out; + } + kbuf[size] = '\0'; + tmp = strchr(kbuf, '\n'); + if (!tmp) { + pr_warning("Line length is too long: " + "Should be less than %d.", WRITE_BUFSIZE); + ret = -EINVAL; + goto out; + } + *tmp = '\0'; + size = tmp - kbuf + 1; + done += size; + /* Remove comments */ + tmp = strchr(kbuf, '#'); + if (tmp) + *tmp = '\0'; + + argv = argv_split(GFP_KERNEL, kbuf, &argc); + if (!argv) { + ret = -ENOMEM; + goto out; + } + + if (argc) + ret = create_trace_probe(argc, argv); + + argv_free(argv); + if (ret < 0) + goto out; + + } while (done < count); + ret = done; +out: + kfree(kbuf); + return ret; +} + +static const struct file_operations kprobe_events_ops = { + .owner = THIS_MODULE, + .open = probes_open, + .read = seq_read, + .llseek = seq_lseek, + .release = seq_release, + .write = probes_write,
kernel/trace/trace_kprobe.c line 383
Quoted 10 lines
kernel/trace/trace_kprobe.c new file +10
-0+}; + +/* event recording functions */ +static void kprobe_trace_record (unsigned long ip,struct trace_probe *tp, + struct pt_regs *regs) +{ + __trace_bprintk(ip, "%s%s%+ld\n", + probe_is_return(tp) ? "<-" : "@", + probe_symbol(tp), probe_offset(tp)); +}
What happens here if you have:
kprobe_trace_record() {
probe_symbol() {
.... probes_open() {
cleanup_all_probes() {
free_trace_probe();
return tp->symbol ? ....; //crack!I wonder if you shouldn't use a per_cpu list of probes, spinlocked/irqsaved accessed
and also a kind of prevention against nmi.
Or better, you can use rcu, but we won't be able to probe rcu functions....
Frederic.
Quoted 32 lines
kernel/trace/trace_kprobe.c new file +21
-1+ +/* Make a debugfs interface for controling probe points */ +static __init int init_kprobe_trace (void ) +{ + struct dentry *d_tracer; + struct dentry *entry; + + d_tracer = tracing_init_dentry(); + if (!d_tracer) + return 0; + + entry = debugfs_create_file("kprobe_events", 0644, d_tracer, + NULL, &kprobe_events_ops); + + if (!entry) + pr_warning("Could not create debugfs " + "'kprobe_events' entry\n"); + return 0; +} +fs_initcall (init_kprobe_trace); + --Masami Hiramatsu
Software Engineer
Hitachi Computer Products (America) Inc.
Software Solutions Divisione-mail: [email protected]
Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
Quoted 3 lines · Frédéric Weisbecker
Hi,
2009/5/9 Masami Hiramatsu <[email protected]>:
[...]
Quoted 21 lines
kernel/trace/trace_kprobe.c line 383
Quoted 9 lines
kernel/trace/trace_kprobe.c new file +9
-0+ +/* event recording functions */ +static void kprobe_trace_record (unsigned long ip,struct trace_probe *tp, +struct pt_regs *regs) +{ +__trace_bprintk (ip,"%s%s%+ld\n" , +probe_is_return (tp) ?"<-" :"@" , +probe_symbol (tp),probe_offset (tp)); +}What happens here if you have:
kprobe_trace_record() { probe_symbol() { .... probes_open() { cleanup_all_probes() { free_trace_probe(); return tp->symbol ? ....; //crack!
Quoted 3 lines
I wonder if you shouldn't use a
per_cpulist of probes, spinlocked/irqsaved accessed
and also a kind of prevention against nmi.
Sure, cleanup_all_probes() invokes unregister_kprobe() via unregister_trace_probe(), which waits running probe-handlers by using synchronize_sched()(because kprobes disables preemption around its handlers), before free_trace_probe().
So you don't need any locks there :-)
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
Quoted 30 lines · Masami Hiramatsu
Quoted 3 lines · Frédéric Weisbecker
Hi,
2009/5/9 Masami Hiramatsu <[email protected]>:
[...]
Quoted 21 lines
kernel/trace/trace_kprobe.c line 383
Quoted 9 lines
kernel/trace/trace_kprobe.c new file +9
-0+ +/* event recording functions */ +static void kprobe_trace_record (unsigned long ip,struct trace_probe *tp, +struct pt_regs *regs) +{ +__trace_bprintk (ip,"%s%s%+ld\n" , +probe_is_return (tp) ?"<-" :"@" , +probe_symbol (tp),probe_offset (tp)); +}What happens here if you have:
kprobe_trace_record() { probe_symbol() { .... probes_open() { cleanup_all_probes() { free_trace_probe(); return tp->symbol ? ....; //crack!Quoted 3 lines
I wonder if you shouldn't use a
per_cpulist of probes, spinlocked/irqsaved accessed
and also a kind of prevention against nmi.
Quoted 10 lines
Sure,
cleanup_all_probes()invokesunregister_kprobe()viaunregister_trace_probe(), which waits running probe-handlers by using synchronize_sched()(because kprobes disables preemption around its handlers), beforefree_trace_probe().So you don't need any locks there :-)
Thank you,
Aah, ok :)
So this patch looks sane.
Thanks.
Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
In reply to Masami HiramatsuQuoted 6 lines · Masami Hiramatsu
Add kprobes based event tracer on ftrace.
This tracer is similar to the events tracer which is based on Tracepoint infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe and kretprobe). It probes anywhere where kprobes can
probe(this means, all functions body except for __kprobes functions).
That's some pretty cool functionality, especially together with patch 7.
But as with so many tracing bits in the kernel it's just lowlevel bits without a good user interface. We'd really need some high-level way for sysadmins/developers to use it. E.g. a version of the systemtap compiler that doesn't build a kernel module but instead uses the event tracer + the kprobes tracer.
Or a model like Tom's zedtrace where a perl script would do the dwarf lookups and generates these probes in addition to the filtered event traces.
Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
Quoted 11 lines · Christoph Hellwig
Quoted 7 lines · Masami Hiramatsu
Add kprobes based event tracer on ftrace.
This tracer is similar to the events tracer which is based on Tracepoint infrastructure. Instead of Tracepoint, this tracer is based on kprobes(kprobe and kretprobe). It probes anywhere where kprobes can
probe(this means, all functions body except for __kprobes functions).That's some pretty cool functionality, especially together with patch 7.
Yes. I insisted on this model, because this is essentially kprobes-done-right. Exposing unsafe kernel instrumentation APIs was a big mistake to merge upstream, it delayed the proper design of this stuff by almost a decade.
There's two more details to be solved before this can go into the tracing tree.
Quoted 5 lines
But as with so many tracing bits in the kernel it's just lowlevel bits without a good user interface. We'd really need some high-level way for sysadmins/developers to use it. E.g. a version of the systemtap compiler that doesn't build a kernel module but instead uses the event tracer + the kprobes tracer.
Yes, exactly.
Quoted 3 lines
Or a model like Tom's zedtrace where a perl script would do the dwarf lookups and generates these probes in addition to the filtered event traces.
Correct, that's the other, IMHO superior direction that is being pursued. If you look at the evolution of the filter code it gives the seeds for safe scripting done in the kernel.
Such filters/scripts can then be reused for a whole lot more stuff, such as security rules. (netfilters could use it too, etc.)
Ingo
Re: [PATCH -tip v5 4/7] tracing: add kprobe-based event tracer
In reply to Christoph HellwigQuoted 5 lines · Christoph Hellwig
[...] But as with so many tracing bits in the kernel it's just lowlevel bits without a good user interface. We'd really need some high-level way for sysadmins/developers to use it. E.g. a version of the systemtap compiler that doesn't build a kernel module but instead uses the event tracer + the kprobes tracer. [...]
This (the translator detecting that a particular script is simple enough to be executed in this manner) is theoretically possible.
- FChE
PATCH v5 5/7 x86: fix kernel_trap_sp()
In reply to Masami HiramatsuUse ®s->sp instead of regs for getting the top of stack in kernel mode. (on x86-64, regs->sp always points the top of stack)
[ impact: Oprofile decodes only stack for backtracing on i386 ]
---
arch/x86/include/asm/ptrace.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
arch/x86/include/asm/ptrace.h +2 -2
@@ -187,14 +187,14 @@ static inline int v8086_mode(struct pt_regs *regs)/* * X86_32 CPUs don't save ss and esp if the CPU is already in kernel mode -* when it traps. So regs will be the current sp. +* when it traps. So ®s->sp will be the current sp. * * This is valid only for kernel mode traps. */ static inline unsigned long kernel_trap_sp (struct pt_regs *regs) {#ifdef CONFIG_X86_32 -return (unsigned long )regs; +return (unsigned long )®s->sp;#else return regs->sp;#endif
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Re: [PATCH -tip v5 5/7] x86: fix kernel_trap_sp()
Quoted 2 lines · Masami Hiramatsu
Use
®s->spinstead of regs for getting the top of stack in kernel mode. (on x86-64,regs->spalways points the top of stack)
Shouldn't this patch be sent for inclusion ASAP instead of sitting in this series?
Re: [PATCH -tip v5 5/7] x86: fix kernel_trap_sp()
Quoted 6 lines · Christoph Hellwig
Quoted 2 lines · Masami Hiramatsu
Use
®s->spinstead of regs for getting the top of stack in kernel mode. (on x86-64,regs->spalways points the top of stack)Shouldn't this patch be sent for inclusion ASAP instead of sitting in this series?
Yes, and it's just for asking a comment from oprofile developers, because this change will change its behavior a bit.
Anyway, I'll post it separately against upstream.
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
PATCH v5 6/7 x86: add pt_regs register and stack access APIs
In reply to Masami HiramatsuAdd following APIs for accessing registers and stack entries from pt_regs.
- query_register_offset(const char *name) Query the offset of "name" register.
- query_register_name(unsigned offset)
Query the name of register by its offset.
- get_register(struct pt_regs *regs, unsigned offset) Get the value of a register by its offset.
- valid_stack_address(struct pt_regs *regs, unsigned long addr) Check the address is in the stack.
- get_stack_nth(struct pt_regs *reg, unsigned nth) Get Nth entry of the stack. (N >= 0)
- get_argument_nth(struct pt_regs *reg, unsigned nth) Get Nth argument at function call. (N >= 0)
---
arch/x86/include/asm/ptrace.h | 66 +++++++++++++++++++++++++++++++++++++++++ arch/x86/kernel/ptrace.c | 60 +++++++++++++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 0 deletions(-)
arch/x86/include/asm/ptrace.h +66 -0
@@ -7,6 +7,7 @@#ifdef __KERNEL__#include <asm/segment.h> +#include <asm/page_types.h> #endif #ifndef __ASSEMBLY__
@@ -215,6 +216,71 @@ static inline unsigned long user_stack_pointer(struct pt_regs *regs)return regs->sp; } +/* Query offset/name of register from its name/offset */ +extern int query_register_offset (const char *name); +extern const char *query_register_name (unsigned offset); +#define MAX_REG_OFFSET (offsetof(struct pt_regs, ss)) + +/* Get register value from its offset */ +static inline unsigned long get_register (struct pt_regs *regs,unsigned offset) +{ +if (unlikely (offset >MAX_REG_OFFSET )) +return 0 ; +return *(unsigned long *)((unsigned long )regs + offset); +} + +/* Check the address in the stack */ +static inline int valid_stack_address (struct pt_regs *regs,unsigned long addr) +{ +return ((addr & ~(THREAD_SIZE -1 )) == + (kernel_trap_sp (regs) & ~(THREAD_SIZE -1 ))); +} + +/* Get Nth entry of the stack */ +static inline unsigned long get_stack_nth (struct pt_regs *regs,unsigned n) +{ +unsigned long *addr = (unsigned long *)kernel_trap_sp (regs); + addr += n; +if (valid_stack_address (regs, (unsigned long )addr)) +return *addr; +else +return 0 ; +} + +/* Get Nth argument at function call */ +static inline unsigned long get_argument_nth (struct pt_regs *regs,unsigned n) +{ +#ifdef CONFIG_X86_32 +#define NR_REGPARMS 3 +if (n <NR_REGPARMS ) { +switch (n) { +case 0 :return regs->ax; +case 1 :return regs->dx; +case 2 :return regs->cx; + } +return 0 ; +#else /* CONFIG_X86_64 */ +#define NR_REGPARMS 6 +if (n <NR_REGPARMS ) { +switch (n) { +case 0 :return regs->di; +case 1 :return regs->si; +case 2 :return regs->dx; +case 3 :return regs->cx; +case 4 :return regs->r8; +case 5 :return regs->r9; + } +return 0 ; +#endif + }else { +/* +* The typical case: arg n is on the stack. +* (Note: stack[0] = return address, so skip it) +*/ +return get_stack_nth (regs,1 + n -NR_REGPARMS ); + } +} +/* * These are defined as per linux/ptrace.h, which see. */
arch/x86/kernel/ptrace.c +60 -0
66 lines not on this page · raw message
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
PATCH v5 7/7 tracing: add arguments support on kprobe-based event tracer
In reply to Masami HiramatsuSupport following probe arguments and add fetch functions on kprobe-based event tracer.
%REG : Fetch register REG
sN : Fetch Nth entry of stack (N >= 0)
@ADDR : Fetch memory at ADDR (ADDR should be in kernel)
@SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol) aN : Fetch function argument. (N >= 0) rv : Fetch return value. ra : Fetch return address.
+|-offs(FETCHARG) : fetch memory at FETCHARG +|- offs address.
---
Documentation/trace/ftrace.txt | 47 +++- kernel/trace/trace_kprobe.c | 431 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 441 insertions(+), 37 deletions(-)
Documentation/trace/ftrace.txt +31 -16
@@ -1329,17 +1329,34 @@ current_tracer, instead of that, just set probe points via /debug/tracing/kprobe_events. Synopsis of kprobe_events: - p SYMBOL[+offs|-offs]|MEMADDR : set a probe - r SYMBOL[+0] : set a return probe + p SYMBOL[+offs|-offs]|MEMADDR [FETCHARGS] : set a probe + r SYMBOL[+0] [FETCHARGS] : set a return probe + + FETCHARGS: + %REG : Fetch register REG + sN : Fetch Nth entry of stack (N >= 0) + @ADDR : Fetch memory at ADDR (ADDR should be in kernel) + @SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol) + aN : Fetch function argument. (N >= 0)(*) + rv : Fetch return value.(**) + ra : Fetch return address.(**) + +|-offs(FETCHARG) : fetch memory at FETCHARG +|- offs address.(***) + + (*) aN may not correct on asmlinkaged functions and at the middle of + function body. + (**) only for return probe. + (***) this is useful for fetching a field of data structures. E.g. - echo p sys_open > /debug/tracing/kprobe_events + echo p do_sys_open a0 a1 a2 a3 > /debug/tracing/kprobe_events - This sets a kprobe on the top of sys_open() function. + This sets a kprobe on the top of do_sys_open() function with recording +1st to 4th arguments. - echo r sys_open >> /debug/tracing/kprobe_events + echo r do_sys_open rv ra >> /debug/tracing/kprobe_events - This sets a kretprobe on the return point of sys_open() function. + This sets a kretprobe on the return point of do_sys_open() function with +recording return value and return address. echo > /debug/tracing/kprobe_events
@@ -1351,18 +1368,16 @@ E.g. # # TASK-PID CPU# TIMESTAMP FUNCTION # | | | | | - <...>-5117 [003] 416.481638: sys_open: @sys_open+0 - <...>-5117 [003] 416.481662: syscall_call: <-sys_open+0 - <...>-5117 [003] 416.481739: sys_open: @sys_open+0 - <...>-5117 [003] 416.481762: sysenter_do_call: <-sys_open+0 - <...>-5117 [003] 416.481818: sys_open: @sys_open+0 - <...>-5117 [003] 416.481842: sysenter_do_call: <-sys_open+0 - <...>-5117 [003] 416.481882: sys_open: @sys_open+0 - <...>-5117 [003] 416.481905: sysenter_do_call: <-sys_open+0 + <...>-2376 [001] 262.389131: do_sys_open: @do_sys_open+0 0xffffff9c 0x98db83e 0x8880 0x0 + <...>-2376 [001] 262.391166: sys_open: <-do_sys_open+0 0x5 0xc06e8ebb + <...>-2376 [001] 264.384876: do_sys_open: @do_sys_open+0 0xffffff9c 0x98db83e 0x8880 0x0 + <...>-2376 [001] 264.386880: sys_open: <-do_sys_open+0 0x5 0xc06e8ebb + <...>-2084 [001] 265.380330: do_sys_open: @do_sys_open+0 0xffffff9c 0x804be3e 0x0 0x1b6 + <...>-2084 [001] 265.380399: sys_open: <-do_sys_open+0 0x3 0xc06e8ebb @SYMBOL means that kernel hits a probe, and <-SYMBOL means kernel returns -from SYMBOL(e.g. "sysenter_do_call: <-sys_open+0" means kernel returns from -sys_open to sysenter_do_call). +from SYMBOL(e.g. "sys_open: <-do_sys_open+0" means kernel returns from +do_sys_open to sys_open). function graph tracer
kernel/trace/trace_kprobe.c +410 -21
508 lines not on this page · raw message
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Steven Rostedt
maintainerRe: [PATCH -tip v5 7/7] tracing: add arguments support on kprobe-based event tracer
Quoted 25 lines · Masami Hiramatsu
Support following probe arguments and add fetch functions on kprobe-based event tracer.
%REG : Fetch register REG
sN : Fetch Nth entry of stack (N >= 0)
@ADDR : Fetch memory at ADDR (ADDR should be in kernel)@SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol) aN : Fetch function argument. (N >= 0) rv : Fetch return value. ra : Fetch return address.+|-offs(FETCHARG) : fetch memory at FETCHARG +|- offs address.
Signed-off-by Masami Hiramatsu <[email protected]>Cc Steven Rostedt <[email protected]>Cc Ananth N Mavinakayanahalli <[email protected]>Cc Ingo Molnar <[email protected]>Cc Frederic Weisbecker <[email protected]>---
Documentation/trace/ftrace.txt | 47 +++- kernel/trace/trace_kprobe.c | 431 ++++++++++++++++++++++++++++++++++++++-- 2 files changed, 441 insertions(+), 37 deletions(-)Documentation/trace/ftrace.txt +0
-0
Documentation/trace/ftrace.txt
Quoted 1 line
Documentation/trace/ftrace.txt +0
-1--- a/Documentation/trace/ftrace.txt
The Documentation/trace/ftrace.txt file is getting too big. Could you make a separate "Documentation/trace/kprobes.txt file, and split out the kprobe bits.
Thanks,
-- Steve
Quoted 42 lines
+++
b/Documentation/trace/ftrace.txtDocumentation/trace/ftrace.txt +23
-6@@ -1329,17 +1329,34 @@ current_tracer, instead of that, just set probe points via /debug/tracing/kprobe_events. Synopsis of kprobe_events: - p SYMBOL[+offs|-offs]|MEMADDR : set a probe - r SYMBOL[+0] : set a return probe + p SYMBOL[+offs|-offs]|MEMADDR [FETCHARGS] : set a probe + r SYMBOL[+0] [FETCHARGS] : set a return probe + + FETCHARGS: + %REG : Fetch register REG + sN : Fetch Nth entry of stack (N >= 0) + @ADDR : Fetch memory at ADDR (ADDR should be in kernel) + @SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol) + aN : Fetch function argument. (N >= 0)(*) + rv : Fetch return value.(**) + ra : Fetch return address.(**) + +|-offs(FETCHARG) : fetch memory at FETCHARG +|- offs address.(***) + + (*) aN may not correct on asmlinkaged functions and at the middle of + function body. + (**) only for return probe. + (***) this is useful for fetching a field of data structures. E.g. - echo p sys_open > /debug/tracing/kprobe_events + echo p do_sys_open a0 a1 a2 a3 > /debug/tracing/kprobe_events - This sets a kprobe on the top of sys_open() function. + This sets a kprobe on the top of do_sys_open() function with recording +1st to 4th arguments. - echo r sys_open >> /debug/tracing/kprobe_events + echo r do_sys_open rv ra >> /debug/tracing/kprobe_events - This sets a kretprobe on the return point of sys_open() function. + This sets a kretprobe on the return point of do_sys_open() function with +recording return value and return address. echo > /debug/tracing/kprobe_events
Quoted 25 lines · Masami Hiramatsu
Hi,
Here are the patches of kprobe-based event tracer for x86, version 5, which allows you to probe various kernel events through ftrace interface.
This version supports only x86(-32/-64) (but porting it on other arch just needs kprobes/kretprobes and register and stack access APIs).
This patchset also includes x86(-64) instruction decoder which supports non-SSE/FP opcodes and includes x86 opcode map. I think it will be possible to share this opcode map with KVM's decoder.
This series can be applied on the latest linux-2.6-tip tree.
This patchset includes following changes:
- Add x86 instruction decoder [1/7]
- Check insertion point safety in kprobe [2/7]
- Cleanupfix_riprel()with insn decoder [3/7]
- Add kprobe-tracer plugin [4/7]
- Fixkernel_trap_sp()on x86 according to systemtap runtime. [5/7]
- Add arch-dep register and stack fetching functions [6/7]
- Support fetching various status (register/stack/memory/etc.) [7/7]
Quoted 3 lines
Future items:
- .init function tracing support.
- Support primitivetypes(long, ulong, int, uint, etc)for args.
Ok, this looks pretty complete already.
Two high-level comments:
- There's no self-test - would it be possible to add one? See trace_selftest* in kernel/trace/
- No generic integration.
It would be nice if these ops:
Quoted 7 lines
E.g.
echo pdo_sys_opena0 a1 a2 a3 > /debug/tracing/kprobe_eventsThis sets a kprobe on the top of
do_sys_open()function with recording
1st to 4th arguments.echo r
do_sys_openrv rp >> /debug/tracing/kprobe_events
were just generally available in just about any other tracer - a bit like the event tracer.
It would also be nice to use the 'function attributes' facilities of the function tracer, combined with a new special syntax of the function-filter regex parser, to enable the recovery of return values (or the call arguments), for selected set of functions.
For example, today we can already do things like:
echo 'sys_read:traceon:4' > /debug/tracing/set_ftrace_filter
for 'trace triggers': the above will trigger tracing to be enabled on the entry of sys_read(), 4 times.
Likewise, something like:
echo 'sys_read:args' > /debug/tracing/set_ftrace_filter echo 'sys_read:return' > /debug/tracing/set_ftrace_filter
Could activate kprobes based argument and return-value tracing.
Ingo
Quoted 31 lines · Ingo Molnar
Quoted 25 lines · Masami Hiramatsu
Hi,
Here are the patches of kprobe-based event tracer for x86, version 5, which allows you to probe various kernel events through ftrace interface.
This version supports only x86(-32/-64) (but porting it on other arch just needs kprobes/kretprobes and register and stack access APIs).
This patchset also includes x86(-64) instruction decoder which supports non-SSE/FP opcodes and includes x86 opcode map. I think it will be possible to share this opcode map with KVM's decoder.
This series can be applied on the latest linux-2.6-tip tree.
This patchset includes following changes:
- Add x86 instruction decoder [1/7]
- Check insertion point safety in kprobe [2/7]
- Cleanupfix_riprel()with insn decoder [3/7]
- Add kprobe-tracer plugin [4/7]
- Fixkernel_trap_sp()on x86 according to systemtap runtime. [5/7]
- Add arch-dep register and stack fetching functions [6/7]
- Support fetching various status (register/stack/memory/etc.) [7/7]Quoted 3 lines
Future items:
- .init function tracing support.
- Support primitivetypes(long, ulong, int, uint, etc)for args.
Quoted 6 lines
Ok, this looks pretty complete already.
Two high-level comments:
- There's no self-test - would it be possible to add one? See trace_selftest* in
kernel/trace/
I'm not so sure. Currently, it seems that those self-tests are only for tracers which define new event-entry on ring-buffer. Since this tracer just use ftrace_bprintk, it might need another kind of selftest. e.g. comparing outputs with expected patterns.
In that case, would it be better to make a user-space self test including filters and tracepoints?
Quoted 1 line
- No generic integration.
Right, this just rides on the ftrace's printk :-)
Quoted 26 lines
It would be nice if these ops:
Quoted 7 lines
E.g.
echo pdo_sys_opena0 a1 a2 a3 > /debug/tracing/kprobe_eventsThis sets a kprobe on the top of
do_sys_open()function with recording
1st to 4th arguments.echo r
do_sys_openrv rp >> /debug/tracing/kprobe_eventswere just generally available in just about any other tracer - a bit like the event tracer.
It would also be nice to use the 'function attributes' facilities of the function tracer, combined with a new special syntax of the function-filter regex parser, to enable the recovery of return values (or the call arguments), for selected set of functions.
For example, today we can already do things like:
echo 'sys_read:traceon:4' > /debug/tracing/set_ftrace_filter
for 'trace triggers': the above will trigger tracing to be enabled on the entry of
sys_read(), 4 times.
Quoted 6 lines
Likewise, something like:
echo 'sys_read:args' > /debug/tracing/set_ftrace_filter echo 'sys_read:return' > /debug/tracing/set_ftrace_filter
Could activate kprobes based argument and return-value tracing.
Ah, that's a good idea. And also, I have another idea for using filters with kprobes, which is adding new kprobe-entry as a new trace-event when user defines a new probe via kprobe_events. Thus, user can use it as a dynamic-defined tracepoint.
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Steven Rostedt
maintainerQuoted 6 lines · Masami Hiramatsu
Quoted 5 lines
Two high-level comments:
- There's no self-test - would it be possible to add one? See trace_selftest* in
kernel/trace/
Quoted 7 lines
I'm not so sure. Currently, it seems that those self-tests are only for tracers which define new event-entry on ring-buffer. Since this tracer just use
ftrace_bprintk, it might need another kind of selftest. e.g. comparing outputs with expected patterns.
In that case, would it be better to make a user-space self test including filters and tracepoints?
Or have the workings in the selftest in kernel. As if a user started it. It does not need to write to the ring buffer, that is just what I did. The event selftests don't check if anything was written to the ring buffer, they just make sure that the tests don't crash the system.
-- Steve
Quoted 13 lines · Steven Rostedt
Quoted 11 lines · Masami Hiramatsu
Quoted 4 lines
Two high-level comments:
- There's no self-test - would it be possible to add one? See trace_selftest* in
kernel/trace/I'm not so sure. Currently, it seems that those self-tests are only for tracers which define new event-entry on ring-buffer. Since this tracer just use
ftrace_bprintk, it might need another kind of selftest. e.g. comparing outputs with expected patterns.
In that case, would it be better to make a user-space self test including filters and tracepoints?
Quoted 4 lines
Or have the workings in the selftest in kernel. As if a user started it. It does not need to write to the ring buffer, that is just what I did. The event selftests don't check if anything was written to the ring buffer, they just make sure that the tests don't crash the system.
Would you mean that it is enough to enable some probes and just see what happened at boot time?
That's so easy to add.
Thank you :-),
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Quoted 14 lines · Masami Hiramatsu
Quoted 13 lines · Steven Rostedt
Quoted 11 lines · Masami Hiramatsu
Quoted 4 lines
Two high-level comments:
- There's no self-test - would it be possible to add one? See trace_selftest* in
kernel/trace/I'm not so sure. Currently, it seems that those self-tests are only for tracers which define new event-entry on ring-buffer. Since this tracer just use
ftrace_bprintk, it might need another kind of selftest. e.g. comparing outputs with expected patterns.
In that case, would it be better to make a user-space self test including filters and tracepoints?
Quoted 8 lines
Quoted 4 lines
Or have the workings in the selftest in kernel. As if a user started it. It does not need to write to the ring buffer, that is just what I did. The event selftests don't check if anything was written to the ring buffer, they just make sure that the tests don't crash the system.
Would you mean that it is enough to enable some probes and just see what happened at boot time?
That's so easy to add.
Yes, that's the idea!
Try to think of regressions/crashes/misbehavior you generally trigger while you developed kprobes, and try to add a reasonable set of probes that test the code from those angles.
It doesnt have to be a full, complex test-suite, but even just 80% of coverage of functionality keeps 4/5th of all regressions out of the kernel at a very early stage ...
Ingo
Quoted 31 lines · Ingo Molnar
Quoted 25 lines · Masami Hiramatsu
Hi,
Here are the patches of kprobe-based event tracer for x86, version 5, which allows you to probe various kernel events through ftrace interface.
This version supports only x86(-32/-64) (but porting it on other arch just needs kprobes/kretprobes and register and stack access APIs).
This patchset also includes x86(-64) instruction decoder which supports non-SSE/FP opcodes and includes x86 opcode map. I think it will be possible to share this opcode map with KVM's decoder.
This series can be applied on the latest linux-2.6-tip tree.
This patchset includes following changes:
- Add x86 instruction decoder [1/7]
- Check insertion point safety in kprobe [2/7]
- Cleanupfix_riprel()with insn decoder [3/7]
- Add kprobe-tracer plugin [4/7]
- Fixkernel_trap_sp()on x86 according to systemtap runtime. [5/7]
- Add arch-dep register and stack fetching functions [6/7]
- Support fetching various status (register/stack/memory/etc.) [7/7]Quoted 3 lines
Future items:
- .init function tracing support.
- Support primitivetypes(long, ulong, int, uint, etc)for args.
Quoted 8 lines
Ok, this looks pretty complete already.
Two high-level comments:
- There's no self-test - would it be possible to add one? See trace_selftest* in
kernel/trace/- No generic integration.
Hmm, Ingo, could you tell me what I can do for the integration? Would you means that I should use filters?
Thank you,
Signature · 7 lines
Masami Hiramatsu Software Engineer Hitachi Computer Products (America) Inc. Software Solutions Division e-mail: [email protected]
Quoted 36 lines · Masami Hiramatsu
Quoted 35 lines · Ingo Molnar
Quoted 25 lines · Masami Hiramatsu
Hi,
Here are the patches of kprobe-based event tracer for x86, version 5, which allows you to probe various kernel events through ftrace interface.
This version supports only x86(-32/-64) (but porting it on other arch just needs kprobes/kretprobes and register and stack access APIs).
This patchset also includes x86(-64) instruction decoder which supports non-SSE/FP opcodes and includes x86 opcode map. I think it will be possible to share this opcode map with KVM's decoder.
This series can be applied on the latest linux-2.6-tip tree.
This patchset includes following changes:
- Add x86 instruction decoder [1/7]
- Check insertion point safety in kprobe [2/7]
- Cleanupfix_riprel()with insn decoder [3/7]
- Add kprobe-tracer plugin [4/7]
- Fixkernel_trap_sp()on x86 according to systemtap runtime. [5/7]
- Add arch-dep register and stack fetching functions [6/7]
- Support fetching various status (register/stack/memory/etc.) [7/7]Quoted 3 lines
Future items:
- .init function tracing support.
- Support primitivetypes(long, ulong, int, uint, etc)for args.Ok, this looks pretty complete already.
Two high-level comments:
Quoted 7 lines
Quoted 4 lines
- There's no self-test - would it be possible to add one? See trace_selftest* in
kernel/trace/- No generic integration.
Hmm, Ingo, could you tell me what I can do for the integration? Would you means that I should use filters?
yeah, that - and for the tracepoints to show up under /debug/tracing/events/. They'd in essence be 'flexible', dynamic event tracepoints that extend upon existing, built-in tracepoints. To user-space tools the two would show up in a very similar way and with a similar usage (once they are injected).
Ingo