| Message ID | 1467995754-32508-2-git-send-email-dave.long@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote: > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -74,6 +74,7 @@ > #define COMPAT_PT_DATA_ADDR 0x10004 > #define COMPAT_PT_TEXT_END_ADDR 0x10008 > #ifndef __ASSEMBLY__ > +#include <linux/bug.h> > > /* sizeof(struct user) for AArch32 */ > #define COMPAT_USER_SZ 296 > @@ -119,6 +120,8 @@ struct pt_regs { > u64 syscallno; > }; > > +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate) > + > #define arch_has_single_step() (1) > > #ifdef CONFIG_COMPAT > @@ -147,6 +150,55 @@ struct pt_regs { > #define user_stack_pointer(regs) \ > (!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp) > > +extern int regs_query_register_offset(const char *name); > +extern const char *regs_query_register_name(unsigned int offset); Is regs_query_register_offset() used anywhere? I grep'ed the kernel with these patches applied but couldn't find any use. > +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr); This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we make it static and remove the declaration?
On 07/15/2016 06:57 AM, Catalin Marinas wrote: > On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote: >> --- a/arch/arm64/include/asm/ptrace.h >> +++ b/arch/arm64/include/asm/ptrace.h >> @@ -74,6 +74,7 @@ >> #define COMPAT_PT_DATA_ADDR 0x10004 >> #define COMPAT_PT_TEXT_END_ADDR 0x10008 >> #ifndef __ASSEMBLY__ >> +#include <linux/bug.h> >> >> /* sizeof(struct user) for AArch32 */ >> #define COMPAT_USER_SZ 296 >> @@ -119,6 +120,8 @@ struct pt_regs { >> u64 syscallno; >> }; >> >> +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate) >> + >> #define arch_has_single_step() (1) >> >> #ifdef CONFIG_COMPAT >> @@ -147,6 +150,55 @@ struct pt_regs { >> #define user_stack_pointer(regs) \ >> (!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp) >> >> +extern int regs_query_register_offset(const char *name); >> +extern const char *regs_query_register_name(unsigned int offset); > > Is regs_query_register_offset() used anywhere? I grep'ed the kernel with > these patches applied but couldn't find any use. > It's referenced in kernel/trace/trace_probe.c. >> +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr); > > This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we > make it static and remove the declaration? > OK. Thanks, -dl
On Fri, Jul 15, 2016 at 10:51:23AM -0400, David Long wrote: > On 07/15/2016 06:57 AM, Catalin Marinas wrote: > > On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote: > > > --- a/arch/arm64/include/asm/ptrace.h > > > +++ b/arch/arm64/include/asm/ptrace.h > > > @@ -74,6 +74,7 @@ > > > #define COMPAT_PT_DATA_ADDR 0x10004 > > > #define COMPAT_PT_TEXT_END_ADDR 0x10008 > > > #ifndef __ASSEMBLY__ > > > +#include <linux/bug.h> > > > > > > /* sizeof(struct user) for AArch32 */ > > > #define COMPAT_USER_SZ 296 > > > @@ -119,6 +120,8 @@ struct pt_regs { > > > u64 syscallno; > > > }; > > > > > > +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate) > > > + > > > #define arch_has_single_step() (1) > > > > > > #ifdef CONFIG_COMPAT > > > @@ -147,6 +150,55 @@ struct pt_regs { > > > #define user_stack_pointer(regs) \ > > > (!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp) > > > > > > +extern int regs_query_register_offset(const char *name); > > > +extern const char *regs_query_register_name(unsigned int offset); > > > > Is regs_query_register_offset() used anywhere? I grep'ed the kernel with > > these patches applied but couldn't find any use. > > It's referenced in kernel/trace/trace_probe.c. I meant regs_query_register_name() (vim completion wrote the first one). > > > +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr); > > > > This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we > > make it static and remove the declaration? > > OK. I can change it locally. Are these going to be used in the future by uprobes?
On 07/15/2016 11:13 AM, Catalin Marinas wrote: > On Fri, Jul 15, 2016 at 10:51:23AM -0400, David Long wrote: >> On 07/15/2016 06:57 AM, Catalin Marinas wrote: >>> On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote: >>>> --- a/arch/arm64/include/asm/ptrace.h >>>> +++ b/arch/arm64/include/asm/ptrace.h >>>> @@ -74,6 +74,7 @@ >>>> #define COMPAT_PT_DATA_ADDR 0x10004 >>>> #define COMPAT_PT_TEXT_END_ADDR 0x10008 >>>> #ifndef __ASSEMBLY__ >>>> +#include <linux/bug.h> >>>> >>>> /* sizeof(struct user) for AArch32 */ >>>> #define COMPAT_USER_SZ 296 >>>> @@ -119,6 +120,8 @@ struct pt_regs { >>>> u64 syscallno; >>>> }; >>>> >>>> +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate) >>>> + >>>> #define arch_has_single_step() (1) >>>> >>>> #ifdef CONFIG_COMPAT >>>> @@ -147,6 +150,55 @@ struct pt_regs { >>>> #define user_stack_pointer(regs) \ >>>> (!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp) >>>> >>>> +extern int regs_query_register_offset(const char *name); >>>> +extern const char *regs_query_register_name(unsigned int offset); >>> >>> Is regs_query_register_offset() used anywhere? I grep'ed the kernel with >>> these patches applied but couldnperf_regs.c't find any use. >> >> It's referenced in kernel/trace/trace_probe.c. > > I meant regs_query_register_name() (vim completion wrote the first one). > I had assumed it was used by kgdb to provide human-readable register names for debugger output, but apparently that is handled inside the kgdb.c stub for each architecture. I can only assume this is currently provided in all (or at least most) architectures because some code outside the kernel tree needs (or used to need?) to be able to do the reverse of regs_query_register_offset(). It's easy enough to remove this, but that will make arm64 unlike the other architectures in its CONFIG_HAVE_REGS_AND_STACK_ACCESS_API support. >>>> +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr); >>> >>> This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we >>> make it static and remove the declaration? >> >> OK. > > I can change it locally. It's looking like there will need to be another iteration of the patch for a few small things anyway, although those changes could also be done as subsequent improvements. > > Are these going to be used in the future by uprobes? > It would appear not. Thanks, -dl
On Fri, Jul 15, 2016 at 01:51:02PM -0400, David Long wrote: > On 07/15/2016 11:13 AM, Catalin Marinas wrote: > >On Fri, Jul 15, 2016 at 10:51:23AM -0400, David Long wrote: > >>On 07/15/2016 06:57 AM, Catalin Marinas wrote: > >>>On Fri, Jul 08, 2016 at 12:35:45PM -0400, David Long wrote: > >>>>+extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr); > >>> > >>>This one only seems to be used in arch/arm64/kernel/ptrace.c. Can we > >>>make it static and remove the declaration? > >> > >>OK. > > > >I can change it locally. > > It's looking like there will need to be another iteration of the patch for a > few small things anyway, although those changes could also be done as > subsequent improvements. I've pushed the branch below with my fixups on this series. Please post any minor changes you have as additional patches on top. Thanks. git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-next/kprobes
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 5a0a691..fab133c 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -85,6 +85,7 @@ config ARM64 select HAVE_PERF_EVENTS select HAVE_PERF_REGS select HAVE_PERF_USER_STACK_DUMP + select HAVE_REGS_AND_STACK_ACCESS_API select HAVE_RCU_TABLE_FREE select HAVE_SYSCALL_TRACEPOINTS select IOMMU_DMA if IOMMU_SUPPORT diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index a307eb6..6c0c7d3 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -74,6 +74,7 @@ #define COMPAT_PT_DATA_ADDR 0x10004 #define COMPAT_PT_TEXT_END_ADDR 0x10008 #ifndef __ASSEMBLY__ +#include <linux/bug.h> /* sizeof(struct user) for AArch32 */ #define COMPAT_USER_SZ 296 @@ -119,6 +120,8 @@ struct pt_regs { u64 syscallno; }; +#define MAX_REG_OFFSET offsetof(struct pt_regs, pstate) + #define arch_has_single_step() (1) #ifdef CONFIG_COMPAT @@ -147,6 +150,55 @@ struct pt_regs { #define user_stack_pointer(regs) \ (!compat_user_mode(regs) ? (regs)->sp : (regs)->compat_sp) +extern int regs_query_register_offset(const char *name); +extern const char *regs_query_register_name(unsigned int offset); +extern bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr); +extern unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, + unsigned int n); + +/** + * regs_get_register() - get register value from its offset + * @regs: pt_regs from which register value is gotten + * @offset: offset of the register. + * + * regs_get_register returns the value of a register whose offset from @regs. + * The @offset is the offset of the register in struct pt_regs. + * If @offset is bigger than MAX_REG_OFFSET, this returns 0. + */ +static inline u64 regs_get_register(struct pt_regs *regs, + unsigned int offset) +{ + u64 val = 0; + + WARN_ON(offset & 7); + + offset >>= 3; + switch (offset) { + case 0 ... 30: + val = regs->regs[offset]; + break; + case offsetof(struct pt_regs, sp) >> 3: + val = regs->sp; + break; + case offsetof(struct pt_regs, pc) >> 3: + val = regs->pc; + break; + case offsetof(struct pt_regs, pstate) >> 3: + val = regs->pstate; + break; + default: + val = 0; + } + + return val; +} + +/* Valid only for Kernel mode traps. */ +static inline unsigned long kernel_stack_pointer(struct pt_regs *regs) +{ + return regs->sp; +} + static inline unsigned long regs_return_value(struct pt_regs *regs) { return regs->regs[0]; diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 3f6cd5c..2c88c33 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -48,6 +48,124 @@ #define CREATE_TRACE_POINTS #include <trace/events/syscalls.h> +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} +#define GPR_OFFSET_NAME(r) \ + {.name = "x" #r, .offset = offsetof(struct pt_regs, regs[r])} + +static const struct pt_regs_offset regoffset_table[] = { + GPR_OFFSET_NAME(0), + GPR_OFFSET_NAME(1), + GPR_OFFSET_NAME(2), + GPR_OFFSET_NAME(3), + GPR_OFFSET_NAME(4), + GPR_OFFSET_NAME(5), + GPR_OFFSET_NAME(6), + GPR_OFFSET_NAME(7), + GPR_OFFSET_NAME(8), + GPR_OFFSET_NAME(9), + GPR_OFFSET_NAME(10), + GPR_OFFSET_NAME(11), + GPR_OFFSET_NAME(12), + GPR_OFFSET_NAME(13), + GPR_OFFSET_NAME(14), + GPR_OFFSET_NAME(15), + GPR_OFFSET_NAME(16), + GPR_OFFSET_NAME(17), + GPR_OFFSET_NAME(18), + GPR_OFFSET_NAME(19), + GPR_OFFSET_NAME(20), + GPR_OFFSET_NAME(21), + GPR_OFFSET_NAME(22), + GPR_OFFSET_NAME(23), + GPR_OFFSET_NAME(24), + GPR_OFFSET_NAME(25), + GPR_OFFSET_NAME(26), + GPR_OFFSET_NAME(27), + GPR_OFFSET_NAME(28), + GPR_OFFSET_NAME(29), + GPR_OFFSET_NAME(30), + {.name = "lr", .offset = offsetof(struct pt_regs, regs[30])}, + REG_OFFSET_NAME(sp), + REG_OFFSET_NAME(pc), + REG_OFFSET_NAME(pstate), + REG_OFFSET_END, +}; + +/** + * 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; +} + +/** + * regs_query_register_name() - query register name from its offset + * @offset: the offset of a register in struct pt_regs. + * + * regs_query_register_name() returns the name of a register from its + * offset in struct pt_regs. If the @offset is invalid, this returns NULL; + */ +const char *regs_query_register_name(unsigned int offset) +{ + const struct pt_regs_offset *roff; + + for (roff = regoffset_table; roff->name != NULL; roff++) + if (roff->offset == offset) + return roff->name; + return NULL; +} + +/** + * regs_within_kernel_stack() - check the address in the stack + * @regs: pt_regs which contains kernel stack pointer. + * @addr: address which is checked. + * + * regs_within_kernel_stack() checks @addr is within the kernel stack page(s). + * If @addr is within the kernel stack, it returns true. If not, returns false. + */ +bool regs_within_kernel_stack(struct pt_regs *regs, unsigned long addr) +{ + return ((addr & ~(THREAD_SIZE - 1)) == + (kernel_stack_pointer(regs) & ~(THREAD_SIZE - 1))) || + on_irq_stack(addr, raw_smp_processor_id()); +} + +/** + * regs_get_kernel_stack_nth() - get Nth entry of the stack + * @regs: pt_regs which contains kernel stack pointer. + * @n: stack entry number. + * + * regs_get_kernel_stack_nth() returns @n th entry of the kernel stack which + * is specified by @regs. If the @n th entry is NOT in the kernel stack, + * this returns 0. + */ +unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, unsigned int n) +{ + unsigned long *addr = (unsigned long *)kernel_stack_pointer(regs); + + addr += n; + if (regs_within_kernel_stack(regs, (unsigned long)addr)) + return *addr; + else + return 0; +} + /* * TODO: does not yet catch signals sent when the child dies. * in exit.c or in signal.c.