perf/probe: verify instruction/offset in perf before adding a uprobe
Subsystems (2)
A message that is not in the archive (sent off-list or not yet received): [email protected]
PATCH v2 1/1 perf/probe: verify instruction/offset in perf before adding a uprobe
This patch is to augment Srikar's perf support for uprobes patch
(https://lkml.org/lkml/2012/4/11/191) with the following features:
a. Instruction verification for user space tracing
b. Function boundary validation support to uprobes as its kernel counterpart (Commit-ID: 1c1bc922).
This will help in ensuring uprobe is placed at right location inside the intended function.
To verify instructions in perf before adding a uprobe, we need to use arch/x86/lib/insn.c. Since perf Makefile enables -Wswitch-default flag it causes build warnings/failures. Masami's patch
https://lkml.org/lkml/2012/4/12/598 addresses those warnings and it is a pre-req for this patch.
v2 addresses Masami's review comments: Rebuild insn.c and inat.c while building perf and a few other minor ones.
---
tools/perf/Makefile | 1 tools/perf/arch/x86/Makefile | 20 ++++++++ tools/perf/arch/x86/util/probe-event.c | 84 ++++++++++++++++++++++++++++++++ tools/perf/util/include/linux/string.h | 1 tools/perf/util/probe-event.c | 22 ++++++++ tools/perf/util/probe-event.h | 2 + tools/perf/util/symbol.c | 2 + tools/perf/util/symbol.h | 1 8 files changed, 132 insertions(+), 1 deletions(-) create mode 100644 tools/perf/arch/x86/util/probe-event.c
tools/perf/Makefile +1 -0
@@ -61,6 +61,7 @@ ARCH ?= $(shell echo $(uname_M) | sed -e s/i.86/i386/ -e s/sun4u/sparc64/ \ CC = $(CROSS_COMPILE)gcc AR = $(CROSS_COMPILE)ar +AWK = awk # Additional ARCH settings for x86 ifeq ($(ARCH),i386)
tools/perf/arch/x86/Makefile +20 -0
@@ -1,5 +1,25 @@ +inat_tables_script = ../../arch/$(ARCH)/tools/gen-insn-attr-x86.awk +inat_tables_maps = ../../arch/$(ARCH)/lib/x86-opcode-map.txt +cmd_inat_tables = $(AWK) -f $(inat_tables_script) $(inat_tables_maps) > $@ || rm -f $@ + +BASIC_CFLAGS += -I. -I../../arch/$(ARCH)/include + ifndef NO_DWARF PERF_HAVE_DWARF_REGS := 1 LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/dwarf-regs.o endif LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/header.o +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/util/probe-event.o +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/inat.o +LIB_OBJS += $(OUTPUT)arch/$(ARCH)/insn.o + +$(OUTPUT)arch/$(ARCH)/inat.o:../../arch/$(ARCH)/lib/inat-tables.c + +../../arch/$(ARCH)/lib/inat-tables.c: $(inat_tables_script) $(inat_tables_maps) + $(cmd_inat_tables) + +$(OUTPUT)arch/$(ARCH)/inat.o:../../arch/$(ARCH)/lib/inat.c $(OUTPUT)PERF-CFLAGS + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $< + +$(OUTPUT)arch/$(ARCH)/insn.o:../../arch/$(ARCH)/lib/insn.c $(OUTPUT)PERF-CFLAGS + $(QUIET_CC)$(CC) -o $@ -c $(ALL_CFLAGS) $<
tools/perf/arch/x86/util/probe-event.c new file +84 -0
84 lines not on this page · raw message
tools/perf/util/include/linux/string.h +1 -0
@@ -1 +1,2 @@#include <string.h> +#include <perf.h>
tools/perf/util/probe-event.c +21 -1
@@ -2254,6 +2254,17 @@ int show_available_funcs(const char *target, struct strfilter *_filter, }/* +* Check if a given offset from start of a function is valid or not +*/ +bool __attribute__((weak))can_probe (char *name__used , +unsigned long long vaddr__used , +unsigned long offset__used , +u8 class__used ) +{ +return true; +} + +/* * uprobe_events only accepts address: * Convert function and any offset to address */
@@ -2307,7 +2318,16 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)if (map->start > sym->start) vaddr = map->start; - vaddr += sym->start + pp->offset + map->pgoff; + + vaddr += sym->start + map->pgoff; +if (pp->offset) +if ((vaddr + pp->offset > sym->end) || + !can_probe (name, vaddr, pp->offset, + map->dso->class)) { +pr_err ("Failed to insert probe, ensure offset is within function and on insn boundary.\n" ); +return -EINVAL ; + } + vaddr += pp->offset; pp->offset =0 ;if (!pev->event) {
tools/perf/util/probe-event.h +2 -0
@@ -137,4 +137,6 @@ extern int show_available_funcs(const char *module, struct strfilter *filter,/* Maximum index number of event-name postfix */ #define MAX_EVENT_INDEX 1024 +extern bool can_probe (char *name,unsigned long long vaddr, +unsigned long offset,u8 class);#endif /*_PROBE_EVENT_H */
tools/perf/util/symbol.c +2 -0
@@ -1184,6 +1184,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name, goto out_close; } + dso->class =gelf_getclass (elf);if (gelf_getehdr (elf, &ehdr) ==NULL ) {pr_debug ("%s: cannot get elf header.\n" , __func__); goto out_elf_end;
@@ -1326,6 +1327,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name, curr_dso->kernel = dso->kernel; curr_dso->long_name = dso->long_name; curr_dso->long_name_len = dso->long_name_len; + curr_dso->class = dso->class; curr_map =map__new2 (start, curr_dso, map->type);if (curr_map ==NULL ) {
tools/perf/util/symbol.h +1 -0
@@ -174,6 +174,7 @@ struct dso {char *long_name;u16 long_name_len;u16 short_name_len; +u8 class;char name[0 ]; };
Em Tue, Apr 24, 2012 at 12:06:46PM +0530, Prashanth Nageshappa escreveu:
Quoted 10 lines
This patch is to augment Srikar's perf support for uprobes patch
(https://lkml.org/lkml/2012/4/11/191) with the following features:a. Instruction verification for user space tracing
b. Function boundary validation support to uprobes as its kernel counterpart (Commit-ID:1c1bc922).This will help in ensuring uprobe is placed at right location inside the intended function.
Quoted 8 lines
To verify instructions in perf before adding a uprobe, we need to use
arch/x86/lib/insn.c. Since perfMakefileenables -Wswitch-default flag it causes build warnings/failures. Masami's patch
https://lkml.org/lkml/2012/4/12/598 addresses those warnings and it is a pre-req for this patch.v2 addresses Masami's review comments: Rebuild
insn.candinat.cwhile building perf and a few other minor ones.
Some minor nits below
tools/perf/arch/x86/util/probe-event.c line 4
Quoted 7 lines
tools/perf/arch/x86/util/probe-event.c new file +4
-0@@ -0,0 +1,84 @@ +bool can_probe (char *name,unsigned long long vaddr,unsigned long offset, +u8 class) +{ +unsigned long long eaddr, saddr;
Why not the much shorter 'u64'?
tools/perf/arch/x86/util/probe-event.c line 52
Quoted 12 lines
tools/perf/arch/x86/util/probe-event.c new file +12
-0+unsigned long fileoffset, readbytes; +int fd =0 ; +bool ret = false; +char *buf =NULL ; +struct insn insn; + + fd =open (name,O_RDONLY ); +if (fd ==-1 ) { +pr_warning ("Failed to open %s: %s\n" , name,strerror (errno)); +return ret; + } + buf = (char *)malloc (offset +MAX_INSN_SIZE );
No need to cast malloc's return
tools/perf/arch/x86/util/probe-event.c line 60
Quoted 8 lines
tools/perf/arch/x86/util/probe-event.c new file +8
-0+if (buf ==NULL ) { +pr_warning ("Failed to allocate memory" ); + goto out; + } + fileoffset =lseek (fd, vaddr,SEEK_SET ); +if (fileoffset != vaddr) { +pr_warning ("Failed to lseek %s: %s\n" , name,strerror (errno)); + goto out;
Goto out_close and...
tools/perf/arch/x86/util/probe-event.c line 67
Quoted 7 lines
tools/perf/arch/x86/util/probe-event.c new file +7
-0+ } + saddr = (unsigned long long )buf; + eaddr = (unsigned long long )buf + offset; + readbytes =read (fd, buf, offset +MAX_INSN_SIZE ); +if (readbytes != offset +MAX_INSN_SIZE ) { +pr_warning ("Failed to read %s: %s\n" , name,strerror (errno)); + goto out;
out_close
tools/perf/arch/x86/util/probe-event.c line 78
Quoted 11 lines
tools/perf/arch/x86/util/probe-event.c new file +11
-0+ } +while (saddr < eaddr) { +insn_init (&insn, (void *)saddr, class ==ELFCLASS64 ); +insn_get_length (&insn); + saddr += insn.length; + } + ret = (saddr == eaddr); + +out: +if (buf) +free (buf);
free can handle NULL pointers just fine
tools/perf/arch/x86/util/probe-event.c line 81
Quoted 3 lines
tools/perf/arch/x86/util/probe-event.c new file +3
-0+ +if (fd) +close (fd);
You can move this to before 'free' and label it out_close, avoiding the test.
Quoted 36 lines
tools/perf/arch/x86/util/probe-event.c new file +3
-0+ +return ret; +}tools/perf/util/include/linux/string.h +1
-0@@ -1 +1,2 @@#include <string.h> +#include <perf.h> tools/perf/util/probe-event.c +11
-0@@ -2254,6 +2254,17 @@ int show_available_funcs(const char *target, struct strfilter *_filter, }/* +* Check if a given offset from start of a function is valid or not +*/ +bool __attribute__((weak))can_probe (char *name__used , +unsigned long long vaddr__used , +unsigned long offset__used , +u8 class__used ) +{ +return true; +} + +/* * uprobe_events only accepts address: * Convert function and any offset to address */ @@ -2307,7 +2318,16 @@ static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec)if (map->start > sym->start) vaddr = map->start;
tools/perf/util/probe-event.c static int convert_name_to_addr(struct perf_probe_event *pev, const char *exec) · line 2329
Quoted 10 lines
tools/perf/util/probe-event.c +9
-1- vaddr += sym->start + pp->offset + map->pgoff; + + vaddr += sym->start + map->pgoff; +if (pp->offset) +if ((vaddr + pp->offset > sym->end) || + !can_probe (name, vaddr, pp->offset, + map->dso->class)) { +pr_err ("Failed to insert probe, ensure offset is within function and on insn boundary.\n" ); +return -EINVAL ; + }
Multi line if bodies gets visually nicer when enclosed with {}
Quoted 40 lines
tools/perf/util/probe-event.c +1
-0+ vaddr += pp->offset; pp->offset =0 ;if (!pev->event) {tools/perf/util/probe-event.h +2
-0@@ -137,4 +137,6 @@ extern int show_available_funcs(const char *module, struct strfilter *filter,/* Maximum index number of event-name postfix */ #define MAX_EVENT_INDEX 1024 +extern bool can_probe (char *name,unsigned long long vaddr, +unsigned long offset,u8 class);#endif /*_PROBE_EVENT_H */ tools/perf/util/symbol.c +2
-0@@ -1184,6 +1184,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name, goto out_close; } + dso->class =gelf_getclass (elf);if (gelf_getehdr (elf, &ehdr) ==NULL ) {pr_debug ("%s: cannot get elf header.\n" , __func__); goto out_elf_end;@@ -1326,6 +1327,7 @@ static int dso__load_sym(struct dso *dso, struct map *map, const char *name, curr_dso->kernel = dso->kernel; curr_dso->long_name = dso->long_name; curr_dso->long_name_len = dso->long_name_len; + curr_dso->class = dso->class; curr_map =map__new2 (start, curr_dso, map->type);if (curr_map ==NULL ) {tools/perf/util/symbol.h +0
-0
tools/perf/util/symbol.h struct dso { · line 180
Quoted 7 lines
tools/perf/util/symbol.h +1
-0@@ -174,6 +174,7 @@ struct dso {char *long_name;u16 long_name_len;u16 short_name_len; +u8 class;char name[0 ]; };
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to [email protected]. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>