| 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
| ||
|
Message-ID: <55E50FEB.4060603@huawei.com>
Date: Tue, 1 Sep 2015 10:39:39 +0800
From: "Wangnan (F)" <wangnan0@...wei.com>
To: Arnaldo Carvalho de Melo <acme@...hat.com>
CC: <mingo@...nel.org>, <ast@...mgrid.com>,
<linux-kernel@...r.kernel.org>, <lizefan@...wei.com>,
<pi3orama@....com>, He Kuang <hekuang@...wei.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>,
Masami Hiramatsu <masami.hiramatsu.pt@...achi.com>,
Namhyung Kim <namhyung@...nel.org>,
"Paul Mackerras" <paulus@...ba.org>,
Peter Zijlstra <a.p.zijlstra@...llo.nl>
Subject: Re: [PATCH 23/31] perf tools: Introduce arch_get_reg_info() for x86
On 2015/9/1 4:43, Arnaldo Carvalho de Melo wrote:
> Em Sat, Aug 29, 2015 at 04:21:57AM +0000, Wang Nan escreveu:
>> From: He Kuang <hekuang@...wei.com>
>>
>> arch_get_reg_info() 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.
> Is this something like:
>
> /* Query offset/name of register from its name/offset */
> extern int regs_query_register_offset(const char *name);
>
> in ptrace? Can't we reuse that name and even code?
Unfortunately we can't reuse its code, because pt_regs is defined
differently
in user and kernel side.
In arch/x86/kernel/ptrace.c we have:
struct pt_regs_offset {
const char *name;
int offset;
};
#define REG_OFFSET_NAME(r) {.name = #r, .offset = offsetof(struct
pt_regs, r)}
#define REG_OFFSET_END {.name = NULL, .offset = 0}
static const struct pt_regs_offset regoffset_table[] = {
#ifdef CONFIG_X86_64
...
REG_OFFSET_NAME(r15),
REG_OFFSET_NAME(r14),
REG_OFFSET_NAME(r13),
...
The definition of REG_OFFSET_NAME relys on the field name and the string
name of a
register are identical. This is true for kernel, but not true for userspace.
For example, for x86_64, 'struct pt_regs' is defined in
arch/x86/include/asm/ptrace.h
for kernel, and the reigster name is 'ax, cx, dx, si, di ...'. In
contract, which is
defined in arch/x86/include/uapi/asm/ptrace.h for user, and the register
name becomes
'rax, rcx, rdx, rsi, rdi ...'.
Since logical of regs_query_register_offset() is very simple, changing
REG_OFFSET_NAME()
makes it a totally different function.
But yes, let's reuse its name. And it may worth considering to reuse its
code for other
archs.
Thank you.
> Was this that was done and only a rename was made?
>
> - Arnaldo
>
>> This patch replaces original string table by a 'struct reg_info' table,
>> which records offset of registers according to its name.
>>
>> For x86, since there are two sub-archs (x86_32 and x86_64) but we can
>> only get pt_regs for the arch we are currently on, this patch fills
>> offset with '-1' for another sub-arch. This introduces a limitation to
>> perf prologue that, we are unable to generate prologue on a x86_32
>> compiled perf for BPF programs targeted on x86_64 kernel. This
>> limitation is acceptable, because this is a very rare usecase.
>>
>> 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>
>> Link: http://lkml.kernel.org/n/1436445342-1402-34-git-send-email-wangnan0@huawei.com
>> ---
>> tools/perf/arch/x86/Makefile | 1 +
>> tools/perf/arch/x86/util/Build | 2 +
>> tools/perf/arch/x86/util/dwarf-regs.c | 104 ++++++++++++++++++++++++----------
>> 3 files changed, 78 insertions(+), 29 deletions(-)
>>
>> diff --git a/tools/perf/arch/x86/Makefile b/tools/perf/arch/x86/Makefile
>> index 21322e0..a84a6f6f 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_GET_REG_INFO := 1
>> diff --git a/tools/perf/arch/x86/util/Build b/tools/perf/arch/x86/util/Build
>> index 2c55e1b..09429f6 100644
>> --- a/tools/perf/arch/x86/util/Build
>> +++ b/tools/perf/arch/x86/util/Build
>> @@ -3,6 +3,8 @@ libperf-y += tsc.o
>> libperf-y += pmu.o
>> libperf-y += kvm-stat.o
>>
>> +# BPF_PROLOGUE also need dwarf-regs.o. However, if CONFIG_BPF_PROLOGUE
>> +# is true, CONFIG_DWARF must true.
>> libperf-$(CONFIG_DWARF) += dwarf-regs.o
>>
>> libperf-$(CONFIG_LIBUNWIND) += unwind-libunwind.o
>> diff --git a/tools/perf/arch/x86/util/dwarf-regs.c b/tools/perf/arch/x86/util/dwarf-regs.c
>> index be22dd4..9928caf 100644
>> --- a/tools/perf/arch/x86/util/dwarf-regs.c
>> +++ b/tools/perf/arch/x86/util/dwarf-regs.c
>> @@ -22,44 +22,67 @@
>>
>> #include <stddef.h>
>> #include <dwarf-regs.h>
>> +#include <string.h>
>> +#include <linux/ptrace.h>
>> +#include <linux/kernel.h> /* for offsetof */
>> +#include <util/bpf-loader.h>
>> +
>> +struct reg_info {
>> + const char *name; /* Reg string in debuginfo */
>> + int offset; /* Reg offset in struct pt_regs */
>> +};
>>
>> /*
>> * Generic dwarf analysis helpers
>> */
>> -
>> +/*
>> + * x86_64 compiling can't access pt_regs for x86_32, so fill offset
>> + * with -1.
>> + */
>> +#ifdef __x86_64__
>> +# define REG_INFO(n, f) { .name = n, .offset = -1, }
>> +#else
>> +# define REG_INFO(n, f) { .name = n, .offset = offsetof(struct pt_regs, f), }
>> +#endif
>> #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 reg_info x86_32_regs_table[X86_32_MAX_REGS] = {
>> + REG_INFO("%ax", eax),
>> + REG_INFO("%cx", ecx),
>> + REG_INFO("%dx", edx),
>> + REG_INFO("%bx", ebx),
>> + REG_INFO("$stack", esp), /* Stack address instead of %sp */
>> + REG_INFO("%bp", ebp),
>> + REG_INFO("%si", esi),
>> + REG_INFO("%di", edi),
>> };
>>
>> +#undef REG_INFO
>> +#ifdef __x86_64__
>> +# define REG_INFO(n, f) { .name = n, .offset = offsetof(struct pt_regs, f), }
>> +#else
>> +# define REG_INFO(n, f) { .name = n, .offset = -1, }
>> +#endif
>> #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",
>> +struct reg_info x86_64_regs_table[X86_64_MAX_REGS] = {
>> + REG_INFO("%ax", rax),
>> + REG_INFO("%dx", rdx),
>> + REG_INFO("%cx", rcx),
>> + REG_INFO("%bx", rbx),
>> + REG_INFO("%si", rsi),
>> + REG_INFO("%di", rdi),
>> + REG_INFO("%bp", rbp),
>> + REG_INFO("%sp", rsp),
>> + REG_INFO("%r8", r8),
>> + REG_INFO("%r9", r9),
>> + REG_INFO("%r10", r10),
>> + REG_INFO("%r11", r11),
>> + REG_INFO("%r12", r12),
>> + REG_INFO("%r13", r13),
>> + REG_INFO("%r14", r14),
>> + REG_INFO("%r15", r15),
>> };
>>
>> -/* 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
>> @@ -71,5 +94,28 @@ const char *x86_64_regs_table[X86_64_MAX_REGS] = {
>> /* 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) ? arch_regs_table[n].name : NULL;
>> }
>> +
>> +#ifdef HAVE_BPF_PROLOGUE
>> +int arch_get_reg_info(const char *name, int *offset)
>> +{
>> + int i;
>> + struct reg_info *info;
>> +
>> + if (!name || !offset)
>> + return -1;
>> +
>> + for (i = 0; i < ARCH_MAX_REGS; i++) {
>> + info = &arch_regs_table[i];
>> + if (strcmp(info->name, name) == 0) {
>> + if (info->offset < 0)
>> + return -1;
>> + *offset = info->offset;
>> + return 0;
>> + }
>> + }
>> +
>> + return -1;
>> +}
>> +#endif
>> --
>> 2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists