lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Tue, 1 Sep 2015 11:47:41 +0000
From:	平松雅巳 / HIRAMATU,MASAMI 
	<masami.hiramatsu.pt@...achi.com>
To:	"'Wang Nan'" <wangnan0@...wei.com>,
	"acme@...nel.org" <acme@...nel.org>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	He Kuang <hekuang@...wei.com>,
	Alexei Starovoitov <ast@...mgrid.com>,
	Brendan Gregg <brendan.d.gregg@...il.com>,
	Daniel Borkmann <daniel@...earbox.net>,
	"David Ahern" <dsahern@...il.com>, Jiri Olsa <jolsa@...nel.org>,
	Kaixu Xia <xiakaixu@...wei.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Paul Mackerras <paulus@...ba.org>,
	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Zefan Li <lizefan@...wei.com>,
	"pi3orama@....com" <pi3orama@....com>,
	"Arnaldo Carvalho de Melo" <acme@...hat.com>
Subject: RE: [PATCH 23/31] perf tools: Introduce
 regs_query_register_offset() for x86

> From: Wang Nan [mailto:wangnan0@...wei.com]
> 
> regs_query_register_offset() is a helper function which converts
> register name like "%rax" to offset of a register in 'struct pt_regs',
> which is required by BPF prologue generator. Since the function is
> identical, try to reuse the code in arch/x86/kernel/ptrace.c.
> 
> Comment inside dwarf-regs.c list the differences between this
> implementation and kernel code.

Hmm, this also introduce a duplication of the code...
It might be a good time to move them into arch/x86/lib/ and
reuse it directly from perf code.

Thank you,

> 
> get_arch_regstr() switches to regoffset_table and the old string table
> is dropped.
> 
> Signed-off-by: Wang Nan <wangnan0@...wei.com>
> Signed-off-by: He Kuang <hekuang@...wei.com>
> Cc: Alexei Starovoitov <ast@...mgrid.com>
> Cc: Brendan Gregg <brendan.d.gregg@...il.com>
> Cc: Daniel Borkmann <daniel@...earbox.net>
> Cc: David Ahern <dsahern@...il.com>
> Cc: He Kuang <hekuang@...wei.com>
> Cc: Jiri Olsa <jolsa@...nel.org>
> Cc: Kaixu Xia <xiakaixu@...wei.com>
> Cc: Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Paul Mackerras <paulus@...ba.org>
> Cc: Peter Zijlstra <a.p.zijlstra@...llo.nl>
> Cc: Zefan Li <lizefan@...wei.com>
> Cc: pi3orama@....com
> Cc: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
>  tools/perf/arch/x86/Makefile          |   1 +
>  tools/perf/arch/x86/util/Build        |   1 +
>  tools/perf/arch/x86/util/dwarf-regs.c | 122 ++++++++++++++++++++++++----------
>  3 files changed, 90 insertions(+), 34 deletions(-)
> 
> diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
> index 21322e0..09ba923 100644
> --- a/tools/perf/arch/x86/Makefile
> +++ b/tools/perf/arch/x86/Makefile
> @@ -2,3 +2,4 @@ ifndef NO_DWARF
>  PERF_HAVE_DWARF_REGS := 1
>  endif
>  HAVE_KVM_STAT_SUPPORT := 1
> +PERF_HAVE_ARCH_REGS_QUERY_REGISTER_OFFSET := 1
> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
> index 2c55e1b..d4d1f23 100644
> --- a/tools/perf/arch/x86/util/Build
> +++ b/tools/perf/arch/x86/util/Build
> @@ -4,6 +4,7 @@ libperf-y += pmu.o
>  libperf-y += kvm-stat.o
> 
>  libperf-$(CONFIG_DWARF) += dwarf-regs.o
> +libperf-$(CONFIG_BPF_PROLOGUE) += dwarf-regs.o
> 
>  libperf-$(CONFIG_LIBUNWIND)          += unwind-libunwind.o
>  libperf-$(CONFIG_LIBDW_DWARF_UNWIND) += unwind-libdw.o
> diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
> index a08de0a..de5b936 100644
> --- a/tools/perf/arch/x86/util/dwarf-regs.c
> +++ b/tools/perf/arch/x86/util/dwarf-regs.c
> @@ -21,55 +21,109 @@
>   */
> 
>  #include <stddef.h>
> +#include <errno.h> /* for EINVAL */
> +#include <string.h> /* for strcmp */
> +#include <linux/ptrace.h> /* for struct pt_regs */
> +#include <linux/kernel.h> /* for offsetof */
>  #include <dwarf-regs.h>
> 
>  /*
> - * Generic dwarf analysis helpers
> + * See arch/x86/kernel/ptrace.c.
> + * Different from it:
> + *
> + *  - Since struct pt_regs is defined differently for user and kernel,
> + *    but we want to use 'ax, bx' instead of 'rax, rbx' (which is struct
> + *    field name of user's pt_regs), we make REG_OFFSET_NAME to accept
> + *    both string name and reg field name.
> + *
> + *  - Since accessing x86_32's pt_regs from x86_64 building is difficult
> + *    and vise versa, we simply fill offset with -1, so
> + *    get_arch_regstr() still works but regs_query_register_offset()
> + *    returns error.
> + *    The only inconvenience caused by it now is that we are not allowed
> + *    to generate BPF prologue for a x86_64 kernel if perf is built for
> + *    x86_32. This is really a rare usecase.
> + *
> + *  - Order is different from kernel's ptrace.c for get_arch_regstr(), which
> + *    is defined by dwarf.
>   */
> 
> -#define X86_32_MAX_REGS 8
> -const char *x86_32_regs_table[X86_32_MAX_REGS] = {
> -	"%ax",
> -	"%cx",
> -	"%dx",
> -	"%bx",
> -	"$stack",	/* Stack address instead of %sp */
> -	"%bp",
> -	"%si",
> -	"%di",
> +struct pt_regs_offset {
> +	const char *name;
> +	int offset;
> +};
> +
> +#define REG_OFFSET_END {.name = NULL, .offset = 0}
> +
> +#ifdef __x86_64__
> +# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
> +# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = -1}
> +#else
> +# define REG_OFFSET_NAME_64(n, r) {.name = n, .offset = -1}
> +# define REG_OFFSET_NAME_32(n, r) {.name = n, .offset = offsetof(struct pt_regs, r)}
> +#endif
> +
> +static const struct pt_regs_offset x86_32_regoffset_table[] = {
> +	REG_OFFSET_NAME_32("%ax",	eax),
> +	REG_OFFSET_NAME_32("%cx",	ecx),
> +	REG_OFFSET_NAME_32("%dx",	edx),
> +	REG_OFFSET_NAME_32("%bx",	ebx),
> +	REG_OFFSET_NAME_32("$stack",	esp),	/* Stack address instead of %sp */
> +	REG_OFFSET_NAME_32("%bp",	ebp),
> +	REG_OFFSET_NAME_32("%si",	esi),
> +	REG_OFFSET_NAME_32("%di",	edi),
> +	REG_OFFSET_END,
>  };
> 
> -#define X86_64_MAX_REGS 16
> -const char *x86_64_regs_table[X86_64_MAX_REGS] = {
> -	"%ax",
> -	"%dx",
> -	"%cx",
> -	"%bx",
> -	"%si",
> -	"%di",
> -	"%bp",
> -	"%sp",
> -	"%r8",
> -	"%r9",
> -	"%r10",
> -	"%r11",
> -	"%r12",
> -	"%r13",
> -	"%r14",
> -	"%r15",
> +static const struct pt_regs_offset x86_64_regoffset_table[] = {
> +	REG_OFFSET_NAME_64("%ax",	rax),
> +	REG_OFFSET_NAME_64("%dx",	rdx),
> +	REG_OFFSET_NAME_64("%cx",	rcx),
> +	REG_OFFSET_NAME_64("%bx",	rbx),
> +	REG_OFFSET_NAME_64("%si",	rsi),
> +	REG_OFFSET_NAME_64("%di",	rdi),
> +	REG_OFFSET_NAME_64("%bp",	rbp),
> +	REG_OFFSET_NAME_64("%sp",	rsp),
> +	REG_OFFSET_NAME_64("%r8",	r8),
> +	REG_OFFSET_NAME_64("%r9",	r9),
> +	REG_OFFSET_NAME_64("%r10",	r10),
> +	REG_OFFSET_NAME_64("%r11",	r11),
> +	REG_OFFSET_NAME_64("%r12",	r12),
> +	REG_OFFSET_NAME_64("%r13",	r13),
> +	REG_OFFSET_NAME_64("%r14",	r14),
> +	REG_OFFSET_NAME_64("%r15",	r15),
> +	REG_OFFSET_END,
>  };
> 
>  /* TODO: switching by dwarf address size */
>  #ifdef __x86_64__
> -#define ARCH_MAX_REGS X86_64_MAX_REGS
> -#define arch_regs_table x86_64_regs_table
> +#define regoffset_table x86_64_regoffset_table
>  #else
> -#define ARCH_MAX_REGS X86_32_MAX_REGS
> -#define arch_regs_table x86_32_regs_table
> +#define regoffset_table x86_32_regoffset_table
>  #endif
> 
> +/* Minus 1 for the ending REG_OFFSET_END */
> +#define ARCH_MAX_REGS ((sizeof(regoffset_table) / sizeof(regoffset_table[0])) - 1)
> +
>  /* Return architecture dependent register string (for kprobe-tracer) */
>  const char *get_arch_regstr(unsigned int n)
>  {
> -	return (n < ARCH_MAX_REGS) ? arch_regs_table[n] : NULL;
> +	return (n < ARCH_MAX_REGS) ? regoffset_table[n].name : NULL;
> +}
> +
> +/* Reuse code from arch/x86/kernel/ptrace.c */
> +/**
> + * regs_query_register_offset() - query register offset from its name
> + * @name:	the name of a register
> + *
> + * regs_query_register_offset() returns the offset of a register in struct
> + * pt_regs from its name. If the name is invalid, this returns -EINVAL;
> + */
> +int regs_query_register_offset(const char *name)
> +{
> +	const struct pt_regs_offset *roff;
> +	for (roff = regoffset_table; roff->name != NULL; roff++)
> +		if (!strcmp(roff->name, name))
> +			return roff->offset;
> +	return -EINVAL;
>  }
> --
> 1.8.3.4

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ