| Message ID | 20190716111301.1855-2-leo.yan@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm/arm64: Add support for function error injection | expand |
On Tue, Jul 16, 2019 at 07:13:00PM +0800, Leo Yan wrote: > This patch implement regs_set_return_value() and > override_function_with_return() to support function error injection > for arm64. > > In the exception flow, arm64's general register x30 contains the value > for the link register; so we can just update pt_regs::pc with it rather > than redirecting execution to a dummy function that returns. > > This patch is heavily inspired by the commit 7cd01b08d35f ("powerpc: > Add support for function error injection"). > > Signed-off-by: Leo Yan <leo.yan@linaro.org> Catalin, Will: Gentle ping ... > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/error-injection.h | 13 +++++++++++++ > arch/arm64/include/asm/ptrace.h | 5 +++++ > arch/arm64/lib/Makefile | 2 ++ > arch/arm64/lib/error-inject.c | 19 +++++++++++++++++++ > 5 files changed, 40 insertions(+) > create mode 100644 arch/arm64/include/asm/error-injection.h > create mode 100644 arch/arm64/lib/error-inject.c > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 697ea0510729..a6d9e622977d 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -142,6 +142,7 @@ config ARM64 > select HAVE_EFFICIENT_UNALIGNED_ACCESS > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > + select HAVE_FUNCTION_ERROR_INJECTION > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_GCC_PLUGINS > select HAVE_HW_BREAKPOINT if PERF_EVENTS > diff --git a/arch/arm64/include/asm/error-injection.h b/arch/arm64/include/asm/error-injection.h > new file mode 100644 > index 000000000000..da057e8ed224 > --- /dev/null > +++ b/arch/arm64/include/asm/error-injection.h > @@ -0,0 +1,13 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#ifndef __ASM_ERROR_INJECTION_H_ > +#define __ASM_ERROR_INJECTION_H_ > + > +#include <linux/compiler.h> > +#include <linux/linkage.h> > +#include <asm/ptrace.h> > +#include <asm-generic/error-injection.h> > + > +void override_function_with_return(struct pt_regs *regs); > + > +#endif /* __ASM_ERROR_INJECTION_H_ */ > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > index dad858b6adc6..3aafbbe218a2 100644 > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -294,6 +294,11 @@ static inline unsigned long regs_return_value(struct pt_regs *regs) > return regs->regs[0]; > } > > +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc) > +{ > + regs->regs[0] = rc; > +} > + > /** > * regs_get_kernel_argument() - get Nth function argument in kernel > * @regs: pt_regs of that context > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile > index 33c2a4abda04..f182ccb0438e 100644 > --- a/arch/arm64/lib/Makefile > +++ b/arch/arm64/lib/Makefile > @@ -33,3 +33,5 @@ UBSAN_SANITIZE_atomic_ll_sc.o := n > lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o > > obj-$(CONFIG_CRC32) += crc32.o > + > +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o > diff --git a/arch/arm64/lib/error-inject.c b/arch/arm64/lib/error-inject.c > new file mode 100644 > index 000000000000..35661c2de4b0 > --- /dev/null > +++ b/arch/arm64/lib/error-inject.c > @@ -0,0 +1,19 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/error-injection.h> > +#include <linux/kprobes.h> > + > +void override_function_with_return(struct pt_regs *regs) > +{ > + /* > + * 'regs' represents the state on entry of a predefined function in > + * the kernel/module and which is captured on a kprobe. > + * > + * 'regs->regs[30]' contains the the link register for the probed > + * function and assign it to 'regs->pc', so when kprobe returns > + * back from exception it will override the end of probed function > + * and drirectly return to the predefined function's caller. > + */ > + regs->pc = regs->regs[30]; > +} > +NOKPROBE_SYMBOL(override_function_with_return); > -- > 2.17.1 >
On Tue, Jul 16, 2019 at 07:13:00PM +0800, Leo Yan wrote: > This patch implement regs_set_return_value() and > override_function_with_return() to support function error injection > for arm64. > > In the exception flow, arm64's general register x30 contains the value > for the link register; so we can just update pt_regs::pc with it rather > than redirecting execution to a dummy function that returns. > > This patch is heavily inspired by the commit 7cd01b08d35f ("powerpc: > Add support for function error injection"). > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > --- > arch/arm64/Kconfig | 1 + > arch/arm64/include/asm/error-injection.h | 13 +++++++++++++ > arch/arm64/include/asm/ptrace.h | 5 +++++ > arch/arm64/lib/Makefile | 2 ++ > arch/arm64/lib/error-inject.c | 19 +++++++++++++++++++ > 5 files changed, 40 insertions(+) > create mode 100644 arch/arm64/include/asm/error-injection.h > create mode 100644 arch/arm64/lib/error-inject.c > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 697ea0510729..a6d9e622977d 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -142,6 +142,7 @@ config ARM64 > select HAVE_EFFICIENT_UNALIGNED_ACCESS > select HAVE_FTRACE_MCOUNT_RECORD > select HAVE_FUNCTION_TRACER > + select HAVE_FUNCTION_ERROR_INJECTION > select HAVE_FUNCTION_GRAPH_TRACER > select HAVE_GCC_PLUGINS > select HAVE_HW_BREAKPOINT if PERF_EVENTS > diff --git a/arch/arm64/include/asm/error-injection.h b/arch/arm64/include/asm/error-injection.h > new file mode 100644 > index 000000000000..da057e8ed224 > --- /dev/null > +++ b/arch/arm64/include/asm/error-injection.h > @@ -0,0 +1,13 @@ > +/* SPDX-License-Identifier: GPL-2.0+ */ > + > +#ifndef __ASM_ERROR_INJECTION_H_ > +#define __ASM_ERROR_INJECTION_H_ > + > +#include <linux/compiler.h> > +#include <linux/linkage.h> > +#include <asm/ptrace.h> > +#include <asm-generic/error-injection.h> > + > +void override_function_with_return(struct pt_regs *regs); > + > +#endif /* __ASM_ERROR_INJECTION_H_ */ Why isn't this prototype in the asm-generic header? Seems weird to have to duplicate it for each architecture. > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > index dad858b6adc6..3aafbbe218a2 100644 > --- a/arch/arm64/include/asm/ptrace.h > +++ b/arch/arm64/include/asm/ptrace.h > @@ -294,6 +294,11 @@ static inline unsigned long regs_return_value(struct pt_regs *regs) > return regs->regs[0]; > } > > +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc) > +{ > + regs->regs[0] = rc; > +} > + > /** > * regs_get_kernel_argument() - get Nth function argument in kernel > * @regs: pt_regs of that context > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile > index 33c2a4abda04..f182ccb0438e 100644 > --- a/arch/arm64/lib/Makefile > +++ b/arch/arm64/lib/Makefile > @@ -33,3 +33,5 @@ UBSAN_SANITIZE_atomic_ll_sc.o := n > lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o > > obj-$(CONFIG_CRC32) += crc32.o > + > +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o > diff --git a/arch/arm64/lib/error-inject.c b/arch/arm64/lib/error-inject.c > new file mode 100644 > index 000000000000..35661c2de4b0 > --- /dev/null > +++ b/arch/arm64/lib/error-inject.c > @@ -0,0 +1,19 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/error-injection.h> > +#include <linux/kprobes.h> > + > +void override_function_with_return(struct pt_regs *regs) > +{ > + /* > + * 'regs' represents the state on entry of a predefined function in > + * the kernel/module and which is captured on a kprobe. > + * > + * 'regs->regs[30]' contains the the link register for the probed extra "the" > + * function and assign it to 'regs->pc', so when kprobe returns > + * back from exception it will override the end of probed function > + * and drirectly return to the predefined function's caller. directly > + */ > + regs->pc = regs->regs[30]; I suppose we could be all fancy and do: instruction_pointer_set(regs, procedure_link_pointer(regs)); How about that? Will
Hi Will, Thanks for reviewing. On Wed, Jul 31, 2019 at 05:08:37PM +0100, Will Deacon wrote: > On Tue, Jul 16, 2019 at 07:13:00PM +0800, Leo Yan wrote: > > This patch implement regs_set_return_value() and > > override_function_with_return() to support function error injection > > for arm64. > > > > In the exception flow, arm64's general register x30 contains the value > > for the link register; so we can just update pt_regs::pc with it rather > > than redirecting execution to a dummy function that returns. > > > > This patch is heavily inspired by the commit 7cd01b08d35f ("powerpc: > > Add support for function error injection"). > > > > Signed-off-by: Leo Yan <leo.yan@linaro.org> > > --- > > arch/arm64/Kconfig | 1 + > > arch/arm64/include/asm/error-injection.h | 13 +++++++++++++ > > arch/arm64/include/asm/ptrace.h | 5 +++++ > > arch/arm64/lib/Makefile | 2 ++ > > arch/arm64/lib/error-inject.c | 19 +++++++++++++++++++ > > 5 files changed, 40 insertions(+) > > create mode 100644 arch/arm64/include/asm/error-injection.h > > create mode 100644 arch/arm64/lib/error-inject.c > > > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > > index 697ea0510729..a6d9e622977d 100644 > > --- a/arch/arm64/Kconfig > > +++ b/arch/arm64/Kconfig > > @@ -142,6 +142,7 @@ config ARM64 > > select HAVE_EFFICIENT_UNALIGNED_ACCESS > > select HAVE_FTRACE_MCOUNT_RECORD > > select HAVE_FUNCTION_TRACER > > + select HAVE_FUNCTION_ERROR_INJECTION > > select HAVE_FUNCTION_GRAPH_TRACER > > select HAVE_GCC_PLUGINS > > select HAVE_HW_BREAKPOINT if PERF_EVENTS > > diff --git a/arch/arm64/include/asm/error-injection.h b/arch/arm64/include/asm/error-injection.h > > new file mode 100644 > > index 000000000000..da057e8ed224 > > --- /dev/null > > +++ b/arch/arm64/include/asm/error-injection.h > > @@ -0,0 +1,13 @@ > > +/* SPDX-License-Identifier: GPL-2.0+ */ > > + > > +#ifndef __ASM_ERROR_INJECTION_H_ > > +#define __ASM_ERROR_INJECTION_H_ > > + > > +#include <linux/compiler.h> > > +#include <linux/linkage.h> > > +#include <asm/ptrace.h> > > +#include <asm-generic/error-injection.h> > > + > > +void override_function_with_return(struct pt_regs *regs); > > + > > +#endif /* __ASM_ERROR_INJECTION_H_ */ > > Why isn't this prototype in the asm-generic header? Seems weird to have to > duplicate it for each architecture. Yeah. When I spin for new version patches, will try to refactor in the asm-generic header. > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h > > index dad858b6adc6..3aafbbe218a2 100644 > > --- a/arch/arm64/include/asm/ptrace.h > > +++ b/arch/arm64/include/asm/ptrace.h > > @@ -294,6 +294,11 @@ static inline unsigned long regs_return_value(struct pt_regs *regs) > > return regs->regs[0]; > > } > > > > +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc) > > +{ > > + regs->regs[0] = rc; > > +} > > + > > /** > > * regs_get_kernel_argument() - get Nth function argument in kernel > > * @regs: pt_regs of that context > > diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile > > index 33c2a4abda04..f182ccb0438e 100644 > > --- a/arch/arm64/lib/Makefile > > +++ b/arch/arm64/lib/Makefile > > @@ -33,3 +33,5 @@ UBSAN_SANITIZE_atomic_ll_sc.o := n > > lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o > > > > obj-$(CONFIG_CRC32) += crc32.o > > + > > +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o > > diff --git a/arch/arm64/lib/error-inject.c b/arch/arm64/lib/error-inject.c > > new file mode 100644 > > index 000000000000..35661c2de4b0 > > --- /dev/null > > +++ b/arch/arm64/lib/error-inject.c > > @@ -0,0 +1,19 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > + > > +#include <linux/error-injection.h> > > +#include <linux/kprobes.h> > > + > > +void override_function_with_return(struct pt_regs *regs) > > +{ > > + /* > > + * 'regs' represents the state on entry of a predefined function in > > + * the kernel/module and which is captured on a kprobe. > > + * > > + * 'regs->regs[30]' contains the the link register for the probed > > extra "the" Will fix. > > + * function and assign it to 'regs->pc', so when kprobe returns > > + * back from exception it will override the end of probed function > > + * and drirectly return to the predefined function's caller. > > directly Will fix. > > + */ > > + regs->pc = regs->regs[30]; > > I suppose we could be all fancy and do: > > instruction_pointer_set(regs, procedure_link_pointer(regs)); > > How about that? Ah, good point. Will change to use the common APIs. Thanks, Leo Yan
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 697ea0510729..a6d9e622977d 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -142,6 +142,7 @@ config ARM64 select HAVE_EFFICIENT_UNALIGNED_ACCESS select HAVE_FTRACE_MCOUNT_RECORD select HAVE_FUNCTION_TRACER + select HAVE_FUNCTION_ERROR_INJECTION select HAVE_FUNCTION_GRAPH_TRACER select HAVE_GCC_PLUGINS select HAVE_HW_BREAKPOINT if PERF_EVENTS diff --git a/arch/arm64/include/asm/error-injection.h b/arch/arm64/include/asm/error-injection.h new file mode 100644 index 000000000000..da057e8ed224 --- /dev/null +++ b/arch/arm64/include/asm/error-injection.h @@ -0,0 +1,13 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ + +#ifndef __ASM_ERROR_INJECTION_H_ +#define __ASM_ERROR_INJECTION_H_ + +#include <linux/compiler.h> +#include <linux/linkage.h> +#include <asm/ptrace.h> +#include <asm-generic/error-injection.h> + +void override_function_with_return(struct pt_regs *regs); + +#endif /* __ASM_ERROR_INJECTION_H_ */ diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h index dad858b6adc6..3aafbbe218a2 100644 --- a/arch/arm64/include/asm/ptrace.h +++ b/arch/arm64/include/asm/ptrace.h @@ -294,6 +294,11 @@ static inline unsigned long regs_return_value(struct pt_regs *regs) return regs->regs[0]; } +static inline void regs_set_return_value(struct pt_regs *regs, unsigned long rc) +{ + regs->regs[0] = rc; +} + /** * regs_get_kernel_argument() - get Nth function argument in kernel * @regs: pt_regs of that context diff --git a/arch/arm64/lib/Makefile b/arch/arm64/lib/Makefile index 33c2a4abda04..f182ccb0438e 100644 --- a/arch/arm64/lib/Makefile +++ b/arch/arm64/lib/Makefile @@ -33,3 +33,5 @@ UBSAN_SANITIZE_atomic_ll_sc.o := n lib-$(CONFIG_ARCH_HAS_UACCESS_FLUSHCACHE) += uaccess_flushcache.o obj-$(CONFIG_CRC32) += crc32.o + +obj-$(CONFIG_FUNCTION_ERROR_INJECTION) += error-inject.o diff --git a/arch/arm64/lib/error-inject.c b/arch/arm64/lib/error-inject.c new file mode 100644 index 000000000000..35661c2de4b0 --- /dev/null +++ b/arch/arm64/lib/error-inject.c @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: GPL-2.0 + +#include <linux/error-injection.h> +#include <linux/kprobes.h> + +void override_function_with_return(struct pt_regs *regs) +{ + /* + * 'regs' represents the state on entry of a predefined function in + * the kernel/module and which is captured on a kprobe. + * + * 'regs->regs[30]' contains the the link register for the probed + * function and assign it to 'regs->pc', so when kprobe returns + * back from exception it will override the end of probed function + * and drirectly return to the predefined function's caller. + */ + regs->pc = regs->regs[30]; +} +NOKPROBE_SYMBOL(override_function_with_return);
This patch implement regs_set_return_value() and override_function_with_return() to support function error injection for arm64. In the exception flow, arm64's general register x30 contains the value for the link register; so we can just update pt_regs::pc with it rather than redirecting execution to a dummy function that returns. This patch is heavily inspired by the commit 7cd01b08d35f ("powerpc: Add support for function error injection"). Signed-off-by: Leo Yan <leo.yan@linaro.org> --- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/error-injection.h | 13 +++++++++++++ arch/arm64/include/asm/ptrace.h | 5 +++++ arch/arm64/lib/Makefile | 2 ++ arch/arm64/lib/error-inject.c | 19 +++++++++++++++++++ 5 files changed, 40 insertions(+) create mode 100644 arch/arm64/include/asm/error-injection.h create mode 100644 arch/arm64/lib/error-inject.c