arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives
Following build warnings / errors noticed on arm with clang-13 / 14 on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Regressions found on arm:
- build-clang-13-bcm2835_defconfig - build-clang-nightly-imx_v6_v7_defconfig - build-clang-nightly-orion5x_defconfig - build-clang-13-keystone_defconfig - build-clang-13-omap2plus_defconfig - build-clang-14-imx_v6_v7_defconfig - build-clang-nightly-omap2plus_defconfig - build-clang-nightly-multi_v5_defconfig - build-clang-nightly-bcm2835_defconfig - build-clang-13-imx_v6_v7_defconfig - build-clang-13-imx_v4_v5_defconfig - build-clang-14-imx_v4_v5_defconfig - build-clang-13-orion5x_defconfig - build-clang-14-multi_v5_defconfig-65236a87 - build-clang-14-lkftconfig - build-clang-nightly-imx_v4_v5_defconfig - build-clang-13-multi_v5_defconfig - build-clang-13-lkftconfig - build-clang-nightly-keystone_defconfig - build-clang-14-multi_v5_defconfig - build-clang-14-orion5x_defconfig - build-clang-14-omap2plus_defconfig - build-clang-nightly-multi_v5_defconfig-65236a87 - build-clang-14-bcm2835_defconfig - build-clang-14-keystone_defconfig - build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives "stmdb sp, {sp, lr, pc} \n\t" ^ <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive "stmdb sp!, {r0 - r11} \n\t" ^ <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^ 2 errors generated. make[5]: *** [/builds/linux/scripts/Makefile.build:250: arch/arm/probes/kprobes/core.o] Error 1
build log: https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
-- Linaro LKFT https://lkft.linaro.org
Hi Naresh,
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
Following build warnings / errors noticed on arm with clang-13 / 14 on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Regressions found on arm:
- build-clang-13-bcm2835_defconfig
- build-clang-nightly-imx_v6_v7_defconfig
- build-clang-nightly-orion5x_defconfig
- build-clang-13-keystone_defconfig
- build-clang-13-omap2plus_defconfig
- build-clang-14-imx_v6_v7_defconfig
- build-clang-nightly-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig
- build-clang-nightly-bcm2835_defconfig
- build-clang-13-imx_v6_v7_defconfig
- build-clang-13-imx_v4_v5_defconfig
- build-clang-14-imx_v4_v5_defconfig
- build-clang-13-orion5x_defconfig
- build-clang-14-multi_v5_defconfig-65236a87
- build-clang-14-lkftconfig
- build-clang-nightly-imx_v4_v5_defconfig
- build-clang-13-multi_v5_defconfig
- build-clang-13-lkftconfig
- build-clang-nightly-keystone_defconfig
- build-clang-14-multi_v5_defconfig
- build-clang-14-orion5x_defconfig
- build-clang-14-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig-65236a87
- build-clang-14-bcm2835_defconfig
- build-clang-14-keystone_defconfig
- build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives "stmdb sp, {sp, lr, pc} \n\t" ^ <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive "stmdb sp!, {r0 - r11} \n\t" ^ <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^ 2 errors generated. make[5]: *** [/builds/linux/scripts/Makefile.build:250: arch/arm/probes/kprobes/core.o] Error 1
build log: https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
Thank you for the testing and report! I brought this up on GitHub on Friday as I noticed this as well:
https://github.com/ClangBuiltLinux/linux/issues/1718
It sounds like we can avoid this by rewriting __kretprobe_trampoline() in out of line assembly but I have not had a chance to sit down and try it.
Cheers, Nathan
+ our mailing list, I should have added it with that message.
On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
Hi Naresh,
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
Following build warnings / errors noticed on arm with clang-13 / 14 on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Regressions found on arm:
- build-clang-13-bcm2835_defconfig
- build-clang-nightly-imx_v6_v7_defconfig
- build-clang-nightly-orion5x_defconfig
- build-clang-13-keystone_defconfig
- build-clang-13-omap2plus_defconfig
- build-clang-14-imx_v6_v7_defconfig
- build-clang-nightly-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig
- build-clang-nightly-bcm2835_defconfig
- build-clang-13-imx_v6_v7_defconfig
- build-clang-13-imx_v4_v5_defconfig
- build-clang-14-imx_v4_v5_defconfig
- build-clang-13-orion5x_defconfig
- build-clang-14-multi_v5_defconfig-65236a87
- build-clang-14-lkftconfig
- build-clang-nightly-imx_v4_v5_defconfig
- build-clang-13-multi_v5_defconfig
- build-clang-13-lkftconfig
- build-clang-nightly-keystone_defconfig
- build-clang-14-multi_v5_defconfig
- build-clang-14-orion5x_defconfig
- build-clang-14-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig-65236a87
- build-clang-14-bcm2835_defconfig
- build-clang-14-keystone_defconfig
- build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives "stmdb sp, {sp, lr, pc} \n\t" ^ <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive "stmdb sp!, {r0 - r11} \n\t" ^ <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^ 2 errors generated. make[5]: *** [/builds/linux/scripts/Makefile.build:250: arch/arm/probes/kprobes/core.o] Error 1
build log: https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
Thank you for the testing and report! I brought this up on GitHub on Friday as I noticed this as well:
https://github.com/ClangBuiltLinux/linux/issues/1718
It sounds like we can avoid this by rewriting __kretprobe_trampoline() in out of line assembly but I have not had a chance to sit down and try it.
Cheers, Nathan
On Mon, Sep 26, 2022 at 8:46 AM Nathan Chancellor nathan@kernel.org wrote:
- our mailing list, I should have added it with that message.
On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
Hi Naresh,
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
Following build warnings / errors noticed on arm with clang-13 / 14 on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Regressions found on arm:
- build-clang-13-bcm2835_defconfig
- build-clang-nightly-imx_v6_v7_defconfig
- build-clang-nightly-orion5x_defconfig
- build-clang-13-keystone_defconfig
- build-clang-13-omap2plus_defconfig
- build-clang-14-imx_v6_v7_defconfig
- build-clang-nightly-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig
- build-clang-nightly-bcm2835_defconfig
- build-clang-13-imx_v6_v7_defconfig
- build-clang-13-imx_v4_v5_defconfig
- build-clang-14-imx_v4_v5_defconfig
- build-clang-13-orion5x_defconfig
- build-clang-14-multi_v5_defconfig-65236a87
- build-clang-14-lkftconfig
- build-clang-nightly-imx_v4_v5_defconfig
- build-clang-13-multi_v5_defconfig
- build-clang-13-lkftconfig
- build-clang-nightly-keystone_defconfig
- build-clang-14-multi_v5_defconfig
- build-clang-14-orion5x_defconfig
- build-clang-14-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig-65236a87
- build-clang-14-bcm2835_defconfig
- build-clang-14-keystone_defconfig
- build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives "stmdb sp, {sp, lr, pc} \n\t" ^ <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive "stmdb sp!, {r0 - r11} \n\t" ^ <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^ 2 errors generated. make[5]: *** [/builds/linux/scripts/Makefile.build:250: arch/arm/probes/kprobes/core.o] Error 1
build log: https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
Thank you for the testing and report! I brought this up on GitHub on Friday as I noticed this as well:
Thanks for the reports. I'll take a look at filing additional bug reports against clang, then moving the definition of __kretprobe_trampoline to out of line assembler.
It sounds like we can avoid this by rewriting __kretprobe_trampoline() in out of line assembly but I have not had a chance to sit down and try it.
Cheers, Nathan
On Mon, Sep 26, 2022 at 10:42:45AM -0700, Nick Desaulniers wrote:
On Mon, Sep 26, 2022 at 8:46 AM Nathan Chancellor nathan@kernel.org wrote:
- our mailing list, I should have added it with that message.
On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
Hi Naresh,
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
Following build warnings / errors noticed on arm with clang-13 / 14 on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Regressions found on arm:
- build-clang-13-bcm2835_defconfig
- build-clang-nightly-imx_v6_v7_defconfig
- build-clang-nightly-orion5x_defconfig
- build-clang-13-keystone_defconfig
- build-clang-13-omap2plus_defconfig
- build-clang-14-imx_v6_v7_defconfig
- build-clang-nightly-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig
- build-clang-nightly-bcm2835_defconfig
- build-clang-13-imx_v6_v7_defconfig
- build-clang-13-imx_v4_v5_defconfig
- build-clang-14-imx_v4_v5_defconfig
- build-clang-13-orion5x_defconfig
- build-clang-14-multi_v5_defconfig-65236a87
- build-clang-14-lkftconfig
- build-clang-nightly-imx_v4_v5_defconfig
- build-clang-13-multi_v5_defconfig
- build-clang-13-lkftconfig
- build-clang-nightly-keystone_defconfig
- build-clang-14-multi_v5_defconfig
- build-clang-14-orion5x_defconfig
- build-clang-14-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig-65236a87
- build-clang-14-bcm2835_defconfig
- build-clang-14-keystone_defconfig
- build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives "stmdb sp, {sp, lr, pc} \n\t" ^ <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive "stmdb sp!, {r0 - r11} \n\t" ^ <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^ 2 errors generated. make[5]: *** [/builds/linux/scripts/Makefile.build:250: arch/arm/probes/kprobes/core.o] Error 1
build log: https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
Thank you for the testing and report! I brought this up on GitHub on Friday as I noticed this as well:
Thanks for the reports. I'll take a look at filing additional bug reports against clang, then moving the definition of __kretprobe_trampoline to out of line assembler.
Are you saying that .save should be accepted without a .fnstart?
On Mon, Sep 26, 2022 at 11:02 AM Russell King (Oracle) linux@armlinux.org.uk wrote:
On Mon, Sep 26, 2022 at 10:42:45AM -0700, Nick Desaulniers wrote:
On Mon, Sep 26, 2022 at 08:44:05AM -0700, Nathan Chancellor wrote:
Thank you for the testing and report! I brought this up on GitHub on Friday as I noticed this as well:
Thanks for the reports. I'll take a look at filing additional bug reports against clang, then moving the definition of __kretprobe_trampoline to out of line assembler.
Are you saying that .save should be accepted without a .fnstart?
No. It's just a bug in clang's inline assembler. But it does make sense to just move it to out of line assembler anyways; having it be inline provides little to no benefit.
Should I be using UNWIND from arch/arm/include/asm/unwind.h on these .fnstart/.save/.pad/.fnend directives?
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^
__kretprobe_trampoline's definition is already entirely inline asm. Move it to out-of-line asm to avoid breaking the build.
Link: https://github.com/llvm/llvm-project/issues/57993 Link: https://github.com/ClangBuiltLinux/linux/issues/1718 Reported-by: Nathan Chancellor nathan@kernel.org Reported-by: Linux Kernel Functional Testing lkft@linaro.org Suggested-by: Logan Chien loganchien@google.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com --- Note: I wasn't quite sure if a Fixes tag against 1069c1dd20a3 was appropriate here? Either way, if 1069c1dd20a3 gets picked up for stable without this, it will break clang builds.
arch/arm/probes/kprobes/Makefile | 1 + arch/arm/probes/kprobes/core.c | 54 ++---------------- .../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++ include/asm-generic/kprobes.h | 13 +++-- 4 files changed, 69 insertions(+), 54 deletions(-) create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile index 6159010dac4a..cdbe9dd99e28 100644 --- a/arch/arm/probes/kprobes/Makefile +++ b/arch/arm/probes/kprobes/Makefile @@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n KASAN_SANITIZE_actions-arm.o := n KASAN_SANITIZE_actions-thumb.o := n obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o +obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o test-kprobes-objs := test-core.o
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c index 1435b508aa36..17d7e0259e63 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -375,58 +375,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, return NOTIFY_DONE; }
-/* - * When a retprobed function returns, trampoline_handler() is called, - * calling the kretprobe's handler. We construct a struct pt_regs to - * give a view of registers r0-r11, sp, lr, and pc to the user - * return-handler. This is not a complete pt_regs structure, but that - * should be enough for stacktrace from the return handler with or - * without pt_regs. - */ -void __naked __kprobes __kretprobe_trampoline(void) -{ - __asm__ __volatile__ ( - "ldr lr, =__kretprobe_trampoline \n\t" -#ifdef CONFIG_FRAME_POINTER - /* __kretprobe_trampoline makes a framepointer on pt_regs. */ -#ifdef CONFIG_CC_IS_CLANG - "stmdb sp, {sp, lr, pc} \n\t" - "sub sp, sp, #12 \n\t" - /* In clang case, pt_regs->ip = lr. */ - "stmdb sp!, {r0 - r11, lr} \n\t" - /* fp points regs->r11 (fp) */ - "add fp, sp, #44 \n\t" -#else /* !CONFIG_CC_IS_CLANG */ - /* In gcc case, pt_regs->ip = fp. */ - "stmdb sp, {fp, sp, lr, pc} \n\t" - "sub sp, sp, #16 \n\t" - "stmdb sp!, {r0 - r11} \n\t" - /* fp points regs->r15 (pc) */ - "add fp, sp, #60 \n\t" -#endif /* CONFIG_CC_IS_CLANG */ -#else /* !CONFIG_FRAME_POINTER */ - /* store SP, LR on stack and add EABI unwind hint */ - "stmdb sp, {sp, lr, pc} \n\t" - ".save {sp, lr, pc} \n\t" - "sub sp, sp, #16 \n\t" - "stmdb sp!, {r0 - r11} \n\t" - ".pad #52 \n\t" -#endif /* CONFIG_FRAME_POINTER */ - "mov r0, sp \n\t" - "bl trampoline_handler \n\t" - "mov lr, r0 \n\t" - "ldmia sp!, {r0 - r11} \n\t" - "add sp, sp, #16 \n\t" -#ifdef CONFIG_THUMB2_KERNEL - "bx lr \n\t" -#else - "mov pc, lr \n\t" -#endif - : : : "memory"); -} +/*void __kretprobe_trampoline(void);*/
/* Called from __kretprobe_trampoline */ -static __used __kprobes void *trampoline_handler(struct pt_regs *regs) +__kprobes void *trampoline_handler(struct pt_regs *regs) { return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP); } @@ -434,6 +386,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs) void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { + extern void __kretprobe_trampoline(void); + ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr; ri->fp = (void *)regs->TRAMP_FP;
diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S new file mode 100644 index 000000000000..261c99b8c17f --- /dev/null +++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <linux/linkage.h> +#include <asm/unwind.h> +#include <asm-generic/kprobes.h> + +/* + * When a retprobed function returns, trampoline_handler() is called, + * calling the kretprobe's handler. We construct a struct pt_regs to + * give a view of registers r0-r11, sp, lr, and pc to the user + * return-handler. This is not a complete pt_regs structure, but that + * should be enough for stacktrace from the return handler with or + * without pt_regs. + */ +__KPROBE +SYM_FUNC_START(__kretprobe_trampoline) +UNWIND(.fnstart) + ldr lr, =__kretprobe_trampoline +#ifdef CONFIG_FRAME_POINTER + /* __kretprobe_trampoline makes a framepointer on pt_regs. */ +#ifdef CONFIG_CC_IS_CLANG + stmdb sp, {sp, lr, pc} + sub sp, sp, #12 + /* In clang case, pt_regs->ip = lr. */ + stmdb sp!, {r0 - r11, lr} + /* fp points regs->r11 (fp) */ + add fp, sp, #44 +#else /* !CONFIG_CC_IS_CLANG */ + /* In gcc case, pt_regs->ip = fp. */ + stmdb sp, {fp, sp, lr, pc} + sub sp, sp, #16 + stmdb sp!, {r0 - r11} + /* fp points regs->r15 (pc) */ + add fp, sp, #60 +#endif /* CONFIG_CC_IS_CLANG */ +#else /* !CONFIG_FRAME_POINTER */ + /* store SP, LR on stack and add EABI unwind hint */ + stmdb sp, {sp, lr, pc} +UNWIND(.save {sp, lr, pc}) + sub sp, sp, #16 + stmdb sp!, {r0 - r11} +UNWIND(.pad #52) +#endif /* CONFIG_FRAME_POINTER */ + mov r0, sp + bl trampoline_handler + mov lr, r0 + ldmia sp!, {r0 - r11} + add sp, sp, #16 +#ifdef CONFIG_THUMB2_KERNEL + bx lr +#else + mov pc, lr +#endif +UNWIND(.fnend) +SYM_FUNC_END(__kretprobe_trampoline) diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h index 060eab094e5a..1509daa281b8 100644 --- a/include/asm-generic/kprobes.h +++ b/include/asm-generic/kprobes.h @@ -2,7 +2,11 @@ #ifndef _ASM_GENERIC_KPROBES_H #define _ASM_GENERIC_KPROBES_H
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__) +#ifdef __KERNEL__ + +#ifdef __ASSEMBLY__ +# define __KPROBE .section ".kprobes.text", "ax" +#else #ifdef CONFIG_KPROBES /* * Blacklist ganerating macro. Specify functions which is not probed @@ -16,11 +20,12 @@ static unsigned long __used \ /* Use this to forbid a kprobes attach on very low level functions */ # define __kprobes __section(".kprobes.text") # define nokprobe_inline __always_inline -#else +#else /* !defined(CONFIG_KPROBES) */ # define NOKPROBE_SYMBOL(fname) # define __kprobes # define nokprobe_inline inline -#endif -#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */ +#endif /* defined(CONFIG_KPROBES) */ +#endif /* defined(__ASSEMBLY__) */ +#endif /* defined(__KERNEL__) */
#endif /* _ASM_GENERIC_KPROBES_H */
On Mon, Sep 26, 2022 at 11:37 AM Nick Desaulniers ndesaulniers@google.com wrote:
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c index 1435b508aa36..17d7e0259e63 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -375,58 +375,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, return NOTIFY_DONE; }
-/*
- When a retprobed function returns, trampoline_handler() is called,
- calling the kretprobe's handler. We construct a struct pt_regs to
- give a view of registers r0-r11, sp, lr, and pc to the user
- return-handler. This is not a complete pt_regs structure, but that
- should be enough for stacktrace from the return handler with or
- without pt_regs.
- */
-void __naked __kprobes __kretprobe_trampoline(void) -{
__asm__ __volatile__ ("ldr lr, =__kretprobe_trampoline \n\t"-#ifdef CONFIG_FRAME_POINTER
/* __kretprobe_trampoline makes a framepointer on pt_regs. */-#ifdef CONFIG_CC_IS_CLANG
"stmdb sp, {sp, lr, pc} \n\t""sub sp, sp, #12 \n\t"/* In clang case, pt_regs->ip = lr. */"stmdb sp!, {r0 - r11, lr} \n\t"/* fp points regs->r11 (fp) */"add fp, sp, #44 \n\t"-#else /* !CONFIG_CC_IS_CLANG */
/* In gcc case, pt_regs->ip = fp. */"stmdb sp, {fp, sp, lr, pc} \n\t""sub sp, sp, #16 \n\t""stmdb sp!, {r0 - r11} \n\t"/* fp points regs->r15 (pc) */"add fp, sp, #60 \n\t"-#endif /* CONFIG_CC_IS_CLANG */ -#else /* !CONFIG_FRAME_POINTER */
/* store SP, LR on stack and add EABI unwind hint */"stmdb sp, {sp, lr, pc} \n\t"".save {sp, lr, pc} \n\t""sub sp, sp, #16 \n\t""stmdb sp!, {r0 - r11} \n\t"".pad #52 \n\t"-#endif /* CONFIG_FRAME_POINTER */
"mov r0, sp \n\t""bl trampoline_handler \n\t""mov lr, r0 \n\t""ldmia sp!, {r0 - r11} \n\t""add sp, sp, #16 \n\t"-#ifdef CONFIG_THUMB2_KERNEL
"bx lr \n\t"-#else
"mov pc, lr \n\t"-#endif
: : : "memory");-} +/*void __kretprobe_trampoline(void);*/
^ d'oh, that commented out declaration should have been removed. Will wait for feedback wrt. usage of UNWIND and Fixes tag to see if a v2 is necessary, vs what can be modified if/when applied by maintainers.
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^
__kretprobe_trampoline's definition is already entirely inline asm. Move it to out-of-line asm to avoid breaking the build.
Link: https://github.com/llvm/llvm-project/issues/57993 Link: https://github.com/ClangBuiltLinux/linux/issues/1718 Reported-by: Nathan Chancellor nathan@kernel.org Reported-by: Linux Kernel Functional Testing lkft@linaro.org Suggested-by: Logan Chien loganchien@google.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com --- Changes v1 -> v2: * rebase on linux-next again. * drop commented out declaration of __kretprobe_trampoline from v1.
arch/arm/probes/kprobes/Makefile | 1 + arch/arm/probes/kprobes/core.c | 50 +---------------- .../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++ include/asm-generic/kprobes.h | 13 +++-- 4 files changed, 68 insertions(+), 51 deletions(-) create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile index 6159010dac4a..cdbe9dd99e28 100644 --- a/arch/arm/probes/kprobes/Makefile +++ b/arch/arm/probes/kprobes/Makefile @@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n KASAN_SANITIZE_actions-arm.o := n KASAN_SANITIZE_actions-thumb.o := n obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o +obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o test-kprobes-objs := test-core.o
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c index 9090c3a74dcc..53f17529d2cb 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -365,54 +365,8 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, return NOTIFY_DONE; }
-/* - * When a retprobed function returns, trampoline_handler() is called, - * calling the kretprobe's handler. We construct a struct pt_regs to - * give a view of registers r0-r11, sp, lr, and pc to the user - * return-handler. This is not a complete pt_regs structure, but that - * should be enough for stacktrace from the return handler with or - * without pt_regs. - */ -void __naked __kprobes __kretprobe_trampoline(void) -{ - __asm__ __volatile__ ( -#ifdef CONFIG_FRAME_POINTER - "ldr lr, =__kretprobe_trampoline \n\t" - /* __kretprobe_trampoline makes a framepointer on pt_regs. */ -#ifdef CONFIG_CC_IS_CLANG - "stmdb sp, {sp, lr, pc} \n\t" - "sub sp, sp, #12 \n\t" - /* In clang case, pt_regs->ip = lr. */ - "stmdb sp!, {r0 - r11, lr} \n\t" - /* fp points regs->r11 (fp) */ - "add fp, sp, #44 \n\t" -#else /* !CONFIG_CC_IS_CLANG */ - /* In gcc case, pt_regs->ip = fp. */ - "stmdb sp, {fp, sp, lr, pc} \n\t" - "sub sp, sp, #16 \n\t" - "stmdb sp!, {r0 - r11} \n\t" - /* fp points regs->r15 (pc) */ - "add fp, sp, #60 \n\t" -#endif /* CONFIG_CC_IS_CLANG */ -#else /* !CONFIG_FRAME_POINTER */ - "sub sp, sp, #16 \n\t" - "stmdb sp!, {r0 - r11} \n\t" -#endif /* CONFIG_FRAME_POINTER */ - "mov r0, sp \n\t" - "bl trampoline_handler \n\t" - "mov lr, r0 \n\t" - "ldmia sp!, {r0 - r11} \n\t" - "add sp, sp, #16 \n\t" -#ifdef CONFIG_THUMB2_KERNEL - "bx lr \n\t" -#else - "mov pc, lr \n\t" -#endif - : : : "memory"); -} - /* Called from __kretprobe_trampoline */ -static __used __kprobes void *trampoline_handler(struct pt_regs *regs) +__kprobes void *trampoline_handler(struct pt_regs *regs) { return (void *)kretprobe_trampoline_handler(regs, (void *)regs->ARM_fp); } @@ -420,6 +374,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs) void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { + extern void __kretprobe_trampoline(void); + ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr; ri->fp = (void *)regs->ARM_fp;
diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S new file mode 100644 index 000000000000..261c99b8c17f --- /dev/null +++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <linux/linkage.h> +#include <asm/unwind.h> +#include <asm-generic/kprobes.h> + +/* + * When a retprobed function returns, trampoline_handler() is called, + * calling the kretprobe's handler. We construct a struct pt_regs to + * give a view of registers r0-r11, sp, lr, and pc to the user + * return-handler. This is not a complete pt_regs structure, but that + * should be enough for stacktrace from the return handler with or + * without pt_regs. + */ +__KPROBE +SYM_FUNC_START(__kretprobe_trampoline) +UNWIND(.fnstart) + ldr lr, =__kretprobe_trampoline +#ifdef CONFIG_FRAME_POINTER + /* __kretprobe_trampoline makes a framepointer on pt_regs. */ +#ifdef CONFIG_CC_IS_CLANG + stmdb sp, {sp, lr, pc} + sub sp, sp, #12 + /* In clang case, pt_regs->ip = lr. */ + stmdb sp!, {r0 - r11, lr} + /* fp points regs->r11 (fp) */ + add fp, sp, #44 +#else /* !CONFIG_CC_IS_CLANG */ + /* In gcc case, pt_regs->ip = fp. */ + stmdb sp, {fp, sp, lr, pc} + sub sp, sp, #16 + stmdb sp!, {r0 - r11} + /* fp points regs->r15 (pc) */ + add fp, sp, #60 +#endif /* CONFIG_CC_IS_CLANG */ +#else /* !CONFIG_FRAME_POINTER */ + /* store SP, LR on stack and add EABI unwind hint */ + stmdb sp, {sp, lr, pc} +UNWIND(.save {sp, lr, pc}) + sub sp, sp, #16 + stmdb sp!, {r0 - r11} +UNWIND(.pad #52) +#endif /* CONFIG_FRAME_POINTER */ + mov r0, sp + bl trampoline_handler + mov lr, r0 + ldmia sp!, {r0 - r11} + add sp, sp, #16 +#ifdef CONFIG_THUMB2_KERNEL + bx lr +#else + mov pc, lr +#endif +UNWIND(.fnend) +SYM_FUNC_END(__kretprobe_trampoline) diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h index 060eab094e5a..1509daa281b8 100644 --- a/include/asm-generic/kprobes.h +++ b/include/asm-generic/kprobes.h @@ -2,7 +2,11 @@ #ifndef _ASM_GENERIC_KPROBES_H #define _ASM_GENERIC_KPROBES_H
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__) +#ifdef __KERNEL__ + +#ifdef __ASSEMBLY__ +# define __KPROBE .section ".kprobes.text", "ax" +#else #ifdef CONFIG_KPROBES /* * Blacklist ganerating macro. Specify functions which is not probed @@ -16,11 +20,12 @@ static unsigned long __used \ /* Use this to forbid a kprobes attach on very low level functions */ # define __kprobes __section(".kprobes.text") # define nokprobe_inline __always_inline -#else +#else /* !defined(CONFIG_KPROBES) */ # define NOKPROBE_SYMBOL(fname) # define __kprobes # define nokprobe_inline inline -#endif -#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */ +#endif /* defined(CONFIG_KPROBES) */ +#endif /* defined(__ASSEMBLY__) */ +#endif /* defined(__KERNEL__) */
#endif /* _ASM_GENERIC_KPROBES_H */
On Tue, Sep 27, 2022 at 03:28:51PM -0700, Nick Desaulniers wrote:
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
Has it been confirmed that gcc does generate a .fnstart for naked functions?
On Tue, Sep 27, 2022 at 3:35 PM Russell King (Oracle) linux@armlinux.org.uk wrote:
On Tue, Sep 27, 2022 at 03:28:51PM -0700, Nick Desaulniers wrote:
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
Has it been confirmed that gcc does generate a .fnstart for naked functions?
From what I can tell, the presence of __attribute__((naked)) makes no difference with regards to the emission of the .fnstart directive for GCC.
One thing I did notice though: https://godbolt.org/z/Mv5GEobc8 GCC will emit .fnstart directives when -fasynchronous-unwind-tables is specified for C (omitting the directive otherwise), or regardless of -fasynchronous-unwind-tables/-fno-asynchronous-unwind-tables for C++. Clang will unconditionally emit .fnstart directives regardless of language mode.
I don't see -fasynchronous-unwind-tables being specified under arch/arm/. But there are many instances of UNWIND(.fnstart) in various .S files under arch/arm/.
https://sourceware.org/binutils/docs/as/ARM-Unwinding-Tutorial.html https://sourceware.org/binutils/docs/as/ARM-Directives.html#arm_005ffnstart
-- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 40Mbps down 10Mbps up. Decent connectivity at last!
-- Thanks, ~Nick Desaulniers
Hi Nick,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20220923] [cannot apply to arm/for-next rostedt-trace/for-next linus/master v6.0-rc7 v6.0-rc6 v6.0-rc5 v6.0-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nick-Desaulniers/ARM-kprobes-... base: aaa11ce2ffc84166d11c4d2ac88c3fcf75425fbd config: arm-allyesconfig compiler: arm-linux-gnueabi-gcc (GCC) 12.1.0 reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/intel-lab-lkp/linux/commit/4dd7f6ce622448f06b7fd5edd86202... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Nick-Desaulniers/ARM-kprobes-move-__kretprobe_trampoline-to-out-of-line-assembler/20220927-094627 git checkout 4dd7f6ce622448f06b7fd5edd8620235c76152aa # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash arch/arm/probes/kprobes/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
Note: functions only called from assembly code should be annotated with the asmlinkage attribute All warnings (new ones prefixed by >>):
arch/arm/probes/kprobes/core.c:246:16: warning: no previous prototype for 'kprobe_handler' [-Wmissing-prototypes] 246 | void __kprobes kprobe_handler(struct pt_regs *regs) | ^~~~~~~~~~~~~~
arch/arm/probes/kprobes/core.c:381:17: warning: no previous prototype for 'trampoline_handler' [-Wmissing-prototypes]
381 | __kprobes void *trampoline_handler(struct pt_regs *regs) | ^~~~~~~~~~~~~~~~~~
vim +/trampoline_handler +381 arch/arm/probes/kprobes/core.c
238 239 /* 240 * Called with IRQs disabled. IRQs must remain disabled from that point 241 * all the way until processing this kprobe is complete. The current 242 * kprobes implementation cannot process more than one nested level of 243 * kprobe, and that level is reserved for user kprobe handlers, so we can't 244 * risk encountering a new kprobe in an interrupt handler. 245 */
246 void __kprobes kprobe_handler(struct pt_regs *regs)
247 { 248 struct kprobe *p, *cur; 249 struct kprobe_ctlblk *kcb; 250 251 kcb = get_kprobe_ctlblk(); 252 cur = kprobe_running(); 253 254 #ifdef CONFIG_THUMB2_KERNEL 255 /* 256 * First look for a probe which was registered using an address with 257 * bit 0 set, this is the usual situation for pointers to Thumb code. 258 * If not found, fallback to looking for one with bit 0 clear. 259 */ 260 p = get_kprobe((kprobe_opcode_t *)(regs->ARM_pc | 1)); 261 if (!p) 262 p = get_kprobe((kprobe_opcode_t *)regs->ARM_pc); 263 264 #else /* ! CONFIG_THUMB2_KERNEL */ 265 p = get_kprobe((kprobe_opcode_t *)regs->ARM_pc); 266 #endif 267 268 if (p) { 269 if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) { 270 /* 271 * Probe hit but conditional execution check failed, 272 * so just skip the instruction and continue as if 273 * nothing had happened. 274 * In this case, we can skip recursing check too. 275 */ 276 singlestep_skip(p, regs); 277 } else if (cur) { 278 /* Kprobe is pending, so we're recursing. */ 279 switch (kcb->kprobe_status) { 280 case KPROBE_HIT_ACTIVE: 281 case KPROBE_HIT_SSDONE: 282 case KPROBE_HIT_SS: 283 /* A pre- or post-handler probe got us here. */ 284 kprobes_inc_nmissed_count(p); 285 save_previous_kprobe(kcb); 286 set_current_kprobe(p); 287 kcb->kprobe_status = KPROBE_REENTER; 288 singlestep(p, regs, kcb); 289 restore_previous_kprobe(kcb); 290 break; 291 case KPROBE_REENTER: 292 /* A nested probe was hit in FIQ, it is a BUG */ 293 pr_warn("Failed to recover from reentered kprobes.\n"); 294 dump_kprobe(p); 295 fallthrough; 296 default: 297 /* impossible cases */ 298 BUG(); 299 } 300 } else { 301 /* Probe hit and conditional execution check ok. */ 302 set_current_kprobe(p); 303 kcb->kprobe_status = KPROBE_HIT_ACTIVE; 304 305 /* 306 * If we have no pre-handler or it returned 0, we 307 * continue with normal processing. If we have a 308 * pre-handler and it returned non-zero, it will 309 * modify the execution path and no need to single 310 * stepping. Let's just reset current kprobe and exit. 311 */ 312 if (!p->pre_handler || !p->pre_handler(p, regs)) { 313 kcb->kprobe_status = KPROBE_HIT_SS; 314 singlestep(p, regs, kcb); 315 if (p->post_handler) { 316 kcb->kprobe_status = KPROBE_HIT_SSDONE; 317 p->post_handler(p, regs, 0); 318 } 319 } 320 reset_current_kprobe(); 321 } 322 } else { 323 /* 324 * The probe was removed and a race is in progress. 325 * There is nothing we can do about it. Let's restart 326 * the instruction. By the time we can restart, the 327 * real instruction will be there. 328 */ 329 } 330 } 331 332 static int __kprobes kprobe_trap_handler(struct pt_regs *regs, unsigned int instr) 333 { 334 unsigned long flags; 335 local_irq_save(flags); 336 kprobe_handler(regs); 337 local_irq_restore(flags); 338 return 0; 339 } 340 341 int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) 342 { 343 struct kprobe *cur = kprobe_running(); 344 struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); 345 346 switch (kcb->kprobe_status) { 347 case KPROBE_HIT_SS: 348 case KPROBE_REENTER: 349 /* 350 * We are here because the instruction being single 351 * stepped caused a page fault. We reset the current 352 * kprobe and the PC to point back to the probe address 353 * and allow the page fault handler to continue as a 354 * normal page fault. 355 */ 356 regs->ARM_pc = (long)cur->addr; 357 if (kcb->kprobe_status == KPROBE_REENTER) { 358 restore_previous_kprobe(kcb); 359 } else { 360 reset_current_kprobe(); 361 } 362 break; 363 } 364 365 return 0; 366 } 367 368 int __kprobes kprobe_exceptions_notify(struct notifier_block *self, 369 unsigned long val, void *data) 370 { 371 /* 372 * notify_die() is currently never called on ARM, 373 * so this callback is currently empty. 374 */ 375 return NOTIFY_DONE; 376 } 377 378 /*void __kretprobe_trampoline(void);*/ 379 380 /* Called from __kretprobe_trampoline */
381 __kprobes void *trampoline_handler(struct pt_regs *regs)
382 { 383 return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP); 384 } 385
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^
__kretprobe_trampoline's definition is already entirely inline asm. Move it to out-of-line asm to avoid breaking the build. Forward declare trampoline_handler() to avoid -Wmissing-prototypes since it's only called from assembler. Fixes another instance of -Wmissing-prototypes on kprobe_handler() so that arch/arm/probes/kprobes/core.c builds cleanly with W=1.
Link: https://github.com/llvm/llvm-project/issues/57993 Link: https://github.com/ClangBuiltLinux/linux/issues/1718 Reported-by: Nathan Chancellor nathan@kernel.org Reported-by: Linux Kernel Functional Testing lkft@linaro.org Suggested-by: Logan Chien loganchien@google.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com --- Changes v2 -> v3: * Fix -Wmissing-prototypes on trampoline_handler() as reported by kernel test robot. * Update comment above trampoline_handler(). * Fix another pre-existing case of -Wmissing-prototypes on kprobe_handler() so that arch/arm/probes/kprobes/core.c builds cleanly with W=1. * Make note of the above in the commit message.
Changes v1 -> v2: * rebase on linux-next again. * drop commented out declaration of __kretprobe_trampoline from v1.
arch/arm/probes/kprobes/Makefile | 1 + arch/arm/probes/kprobes/core.c | 54 +++--------------- .../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++ include/asm-generic/kprobes.h | 13 +++-- 4 files changed, 72 insertions(+), 51 deletions(-) create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile index 6159010dac4a..cdbe9dd99e28 100644 --- a/arch/arm/probes/kprobes/Makefile +++ b/arch/arm/probes/kprobes/Makefile @@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n KASAN_SANITIZE_actions-arm.o := n KASAN_SANITIZE_actions-thumb.o := n obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o +obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o test-kprobes-objs := test-core.o
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c index 9090c3a74dcc..11159fcf6ba6 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -233,7 +233,7 @@ singlestep(struct kprobe *p, struct pt_regs *regs, struct kprobe_ctlblk *kcb) * kprobe, and that level is reserved for user kprobe handlers, so we can't * risk encountering a new kprobe in an interrupt handler. */ -void __kprobes kprobe_handler(struct pt_regs *regs) +static void __kprobes kprobe_handler(struct pt_regs *regs) { struct kprobe *p, *cur; struct kprobe_ctlblk *kcb; @@ -366,53 +366,11 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, }
/* - * When a retprobed function returns, trampoline_handler() is called, - * calling the kretprobe's handler. We construct a struct pt_regs to - * give a view of registers r0-r11, sp, lr, and pc to the user - * return-handler. This is not a complete pt_regs structure, but that - * should be enough for stacktrace from the return handler with or - * without pt_regs. + * Called from __kretprobe_trampoline in assembler. Forward declare to avoid + * -Wmissing-prototypes. */ -void __naked __kprobes __kretprobe_trampoline(void) -{ - __asm__ __volatile__ ( -#ifdef CONFIG_FRAME_POINTER - "ldr lr, =__kretprobe_trampoline \n\t" - /* __kretprobe_trampoline makes a framepointer on pt_regs. */ -#ifdef CONFIG_CC_IS_CLANG - "stmdb sp, {sp, lr, pc} \n\t" - "sub sp, sp, #12 \n\t" - /* In clang case, pt_regs->ip = lr. */ - "stmdb sp!, {r0 - r11, lr} \n\t" - /* fp points regs->r11 (fp) */ - "add fp, sp, #44 \n\t" -#else /* !CONFIG_CC_IS_CLANG */ - /* In gcc case, pt_regs->ip = fp. */ - "stmdb sp, {fp, sp, lr, pc} \n\t" - "sub sp, sp, #16 \n\t" - "stmdb sp!, {r0 - r11} \n\t" - /* fp points regs->r15 (pc) */ - "add fp, sp, #60 \n\t" -#endif /* CONFIG_CC_IS_CLANG */ -#else /* !CONFIG_FRAME_POINTER */ - "sub sp, sp, #16 \n\t" - "stmdb sp!, {r0 - r11} \n\t" -#endif /* CONFIG_FRAME_POINTER */ - "mov r0, sp \n\t" - "bl trampoline_handler \n\t" - "mov lr, r0 \n\t" - "ldmia sp!, {r0 - r11} \n\t" - "add sp, sp, #16 \n\t" -#ifdef CONFIG_THUMB2_KERNEL - "bx lr \n\t" -#else - "mov pc, lr \n\t" -#endif - : : : "memory"); -} - -/* Called from __kretprobe_trampoline */ -static __used __kprobes void *trampoline_handler(struct pt_regs *regs) +void *trampoline_handler(struct pt_regs *regs); +__kprobes void *trampoline_handler(struct pt_regs *regs) { return (void *)kretprobe_trampoline_handler(regs, (void *)regs->ARM_fp); } @@ -420,6 +378,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs) void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) { + extern void __kretprobe_trampoline(void); + ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr; ri->fp = (void *)regs->ARM_fp;
diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S new file mode 100644 index 000000000000..261c99b8c17f --- /dev/null +++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */ + +#include <linux/linkage.h> +#include <asm/unwind.h> +#include <asm-generic/kprobes.h> + +/* + * When a retprobed function returns, trampoline_handler() is called, + * calling the kretprobe's handler. We construct a struct pt_regs to + * give a view of registers r0-r11, sp, lr, and pc to the user + * return-handler. This is not a complete pt_regs structure, but that + * should be enough for stacktrace from the return handler with or + * without pt_regs. + */ +__KPROBE +SYM_FUNC_START(__kretprobe_trampoline) +UNWIND(.fnstart) + ldr lr, =__kretprobe_trampoline +#ifdef CONFIG_FRAME_POINTER + /* __kretprobe_trampoline makes a framepointer on pt_regs. */ +#ifdef CONFIG_CC_IS_CLANG + stmdb sp, {sp, lr, pc} + sub sp, sp, #12 + /* In clang case, pt_regs->ip = lr. */ + stmdb sp!, {r0 - r11, lr} + /* fp points regs->r11 (fp) */ + add fp, sp, #44 +#else /* !CONFIG_CC_IS_CLANG */ + /* In gcc case, pt_regs->ip = fp. */ + stmdb sp, {fp, sp, lr, pc} + sub sp, sp, #16 + stmdb sp!, {r0 - r11} + /* fp points regs->r15 (pc) */ + add fp, sp, #60 +#endif /* CONFIG_CC_IS_CLANG */ +#else /* !CONFIG_FRAME_POINTER */ + /* store SP, LR on stack and add EABI unwind hint */ + stmdb sp, {sp, lr, pc} +UNWIND(.save {sp, lr, pc}) + sub sp, sp, #16 + stmdb sp!, {r0 - r11} +UNWIND(.pad #52) +#endif /* CONFIG_FRAME_POINTER */ + mov r0, sp + bl trampoline_handler + mov lr, r0 + ldmia sp!, {r0 - r11} + add sp, sp, #16 +#ifdef CONFIG_THUMB2_KERNEL + bx lr +#else + mov pc, lr +#endif +UNWIND(.fnend) +SYM_FUNC_END(__kretprobe_trampoline) diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h index 060eab094e5a..1509daa281b8 100644 --- a/include/asm-generic/kprobes.h +++ b/include/asm-generic/kprobes.h @@ -2,7 +2,11 @@ #ifndef _ASM_GENERIC_KPROBES_H #define _ASM_GENERIC_KPROBES_H
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__) +#ifdef __KERNEL__ + +#ifdef __ASSEMBLY__ +# define __KPROBE .section ".kprobes.text", "ax" +#else #ifdef CONFIG_KPROBES /* * Blacklist ganerating macro. Specify functions which is not probed @@ -16,11 +20,12 @@ static unsigned long __used \ /* Use this to forbid a kprobes attach on very low level functions */ # define __kprobes __section(".kprobes.text") # define nokprobe_inline __always_inline -#else +#else /* !defined(CONFIG_KPROBES) */ # define NOKPROBE_SYMBOL(fname) # define __kprobes # define nokprobe_inline inline -#endif -#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */ +#endif /* defined(CONFIG_KPROBES) */ +#endif /* defined(__ASSEMBLY__) */ +#endif /* defined(__KERNEL__) */
#endif /* _ASM_GENERIC_KPROBES_H */
On Fri, Sep 30, 2022 at 2:15 PM Nick Desaulniers ndesaulniers@google.com wrote:
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^
Chen, I noticed that your patch was discarded; it's not in linux-next today. https://lore.kernel.org/linux-arm-kernel/YzHPGvhLkdQcDYzx@shell.armlinux.org... https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9231/1 How would you like to proceed here?
I think moving this out of line, incorporating Ard's feedback, then putting the UNWIND directives on top might be the way to go. What do you think?
Hi,
Sorry for late reply because I just found this thread before the long vacation so I didn't have much time to deal with it.
On 2022/10/7 4:35, Nick Desaulniers wrote:
On Fri, Sep 30, 2022 at 2:15 PM Nick Desaulniers ndesaulniers@google.com wrote:
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^
Chen, I noticed that your patch was discarded; it's not in linux-next today. https://lore.kernel.org/linux-arm-kernel/YzHPGvhLkdQcDYzx@shell.armlinux.org... https://www.arm.linux.org.uk/developer/patches/viewpatch.php?id=9231/1 How would you like to proceed here?
Since 6.1 is closing now. Let's reorganize everything and queue it up for -next for 6.2
I think moving this out of line, incorporating Ard's feedback, then putting the UNWIND directives on top might be the way to go. What do you think?
This way looks good to me.
How about making a set for this, to make everything more clear:
1. Move this out of line
2. Apply the feature, test with gcc & clang
3. Other cleaning, or merge with 2 if the cleaning is tiny.
I'll send another version for this, rebased to 6.1-rc1
Thanks for your time!
Best,
Chen
Hi Nick,
I love your patch! Perhaps something to improve:
[auto build test WARNING on next-20220923] [cannot apply to arm/for-next rostedt-trace/for-next linus/master v6.0-rc7 v6.0-rc6 v6.0-rc5 v6.0] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nick-Desaulniers/ARM-kprobes-... base: aaa11ce2ffc84166d11c4d2ac88c3fcf75425fbd config: arm-mv78xx0_defconfig compiler: clang version 16.0.0 (https://github.com/llvm/llvm-project 791a7ae1ba3efd6bca96338e10ffde557ba83920) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # install arm cross compiling tool for clang build # apt-get install binutils-arm-linux-gnueabi # https://github.com/intel-lab-lkp/linux/commit/4dd7f6ce622448f06b7fd5edd86202... git remote add linux-review https://github.com/intel-lab-lkp/linux git fetch --no-tags linux-review Nick-Desaulniers/ARM-kprobes-move-__kretprobe_trampoline-to-out-of-line-assembler/20220927-094627 git checkout 4dd7f6ce622448f06b7fd5edd8620235c76152aa # save the config file mkdir build_dir && cp config build_dir/.config COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=arm SHELL=/bin/bash arch/arm/probes/kprobes/
If you fix the issue, kindly add following tag where applicable | Reported-by: kernel test robot lkp@intel.com
Note: functions only called from assembly code should be annotated with the asmlinkage attribute All warnings (new ones prefixed by >>):
arch/arm/probes/kprobes/core.c:246:16: warning: no previous prototype for function 'kprobe_handler' [-Wmissing-prototypes] void __kprobes kprobe_handler(struct pt_regs *regs) ^ arch/arm/probes/kprobes/core.c:246:1: note: declare 'static' if the function is not intended to be used outside of this translation unit void __kprobes kprobe_handler(struct pt_regs *regs) ^ static
arch/arm/probes/kprobes/core.c:381:17: warning: no previous prototype for function 'trampoline_handler' [-Wmissing-prototypes]
__kprobes void *trampoline_handler(struct pt_regs *regs) ^ arch/arm/probes/kprobes/core.c:381:11: note: declare 'static' if the function is not intended to be used outside of this translation unit __kprobes void *trampoline_handler(struct pt_regs *regs) ^ static 2 warnings generated.
vim +/trampoline_handler +381 arch/arm/probes/kprobes/core.c
238 239 /* 240 * Called with IRQs disabled. IRQs must remain disabled from that point 241 * all the way until processing this kprobe is complete. The current 242 * kprobes implementation cannot process more than one nested level of 243 * kprobe, and that level is reserved for user kprobe handlers, so we can't 244 * risk encountering a new kprobe in an interrupt handler. 245 */
246 void __kprobes kprobe_handler(struct pt_regs *regs)
247 { 248 struct kprobe *p, *cur; 249 struct kprobe_ctlblk *kcb; 250 251 kcb = get_kprobe_ctlblk(); 252 cur = kprobe_running(); 253 254 #ifdef CONFIG_THUMB2_KERNEL 255 /* 256 * First look for a probe which was registered using an address with 257 * bit 0 set, this is the usual situation for pointers to Thumb code. 258 * If not found, fallback to looking for one with bit 0 clear. 259 */ 260 p = get_kprobe((kprobe_opcode_t *)(regs->ARM_pc | 1)); 261 if (!p) 262 p = get_kprobe((kprobe_opcode_t *)regs->ARM_pc); 263 264 #else /* ! CONFIG_THUMB2_KERNEL */ 265 p = get_kprobe((kprobe_opcode_t *)regs->ARM_pc); 266 #endif 267 268 if (p) { 269 if (!p->ainsn.insn_check_cc(regs->ARM_cpsr)) { 270 /* 271 * Probe hit but conditional execution check failed, 272 * so just skip the instruction and continue as if 273 * nothing had happened. 274 * In this case, we can skip recursing check too. 275 */ 276 singlestep_skip(p, regs); 277 } else if (cur) { 278 /* Kprobe is pending, so we're recursing. */ 279 switch (kcb->kprobe_status) { 280 case KPROBE_HIT_ACTIVE: 281 case KPROBE_HIT_SSDONE: 282 case KPROBE_HIT_SS: 283 /* A pre- or post-handler probe got us here. */ 284 kprobes_inc_nmissed_count(p); 285 save_previous_kprobe(kcb); 286 set_current_kprobe(p); 287 kcb->kprobe_status = KPROBE_REENTER; 288 singlestep(p, regs, kcb); 289 restore_previous_kprobe(kcb); 290 break; 291 case KPROBE_REENTER: 292 /* A nested probe was hit in FIQ, it is a BUG */ 293 pr_warn("Failed to recover from reentered kprobes.\n"); 294 dump_kprobe(p); 295 fallthrough; 296 default: 297 /* impossible cases */ 298 BUG(); 299 } 300 } else { 301 /* Probe hit and conditional execution check ok. */ 302 set_current_kprobe(p); 303 kcb->kprobe_status = KPROBE_HIT_ACTIVE; 304 305 /* 306 * If we have no pre-handler or it returned 0, we 307 * continue with normal processing. If we have a 308 * pre-handler and it returned non-zero, it will 309 * modify the execution path and no need to single 310 * stepping. Let's just reset current kprobe and exit. 311 */ 312 if (!p->pre_handler || !p->pre_handler(p, regs)) { 313 kcb->kprobe_status = KPROBE_HIT_SS; 314 singlestep(p, regs, kcb); 315 if (p->post_handler) { 316 kcb->kprobe_status = KPROBE_HIT_SSDONE; 317 p->post_handler(p, regs, 0); 318 } 319 } 320 reset_current_kprobe(); 321 } 322 } else { 323 /* 324 * The probe was removed and a race is in progress. 325 * There is nothing we can do about it. Let's restart 326 * the instruction. By the time we can restart, the 327 * real instruction will be there. 328 */ 329 } 330 } 331 332 static int __kprobes kprobe_trap_handler(struct pt_regs *regs, unsigned int instr) 333 { 334 unsigned long flags; 335 local_irq_save(flags); 336 kprobe_handler(regs); 337 local_irq_restore(flags); 338 return 0; 339 } 340 341 int __kprobes kprobe_fault_handler(struct pt_regs *regs, unsigned int fsr) 342 { 343 struct kprobe *cur = kprobe_running(); 344 struct kprobe_ctlblk *kcb = get_kprobe_ctlblk(); 345 346 switch (kcb->kprobe_status) { 347 case KPROBE_HIT_SS: 348 case KPROBE_REENTER: 349 /* 350 * We are here because the instruction being single 351 * stepped caused a page fault. We reset the current 352 * kprobe and the PC to point back to the probe address 353 * and allow the page fault handler to continue as a 354 * normal page fault. 355 */ 356 regs->ARM_pc = (long)cur->addr; 357 if (kcb->kprobe_status == KPROBE_REENTER) { 358 restore_previous_kprobe(kcb); 359 } else { 360 reset_current_kprobe(); 361 } 362 break; 363 } 364 365 return 0; 366 } 367 368 int __kprobes kprobe_exceptions_notify(struct notifier_block *self, 369 unsigned long val, void *data) 370 { 371 /* 372 * notify_die() is currently never called on ARM, 373 * so this callback is currently empty. 374 */ 375 return NOTIFY_DONE; 376 } 377 378 /*void __kretprobe_trampoline(void);*/ 379 380 /* Called from __kretprobe_trampoline */
381 __kprobes void *trampoline_handler(struct pt_regs *regs)
382 { 383 return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP); 384 } 385
Hi NIck,
On Mon, 26 Sept 2022 at 20:37, Nick Desaulniers ndesaulniers@google.com wrote:
commit 1069c1dd20a3 ("ARM: 9231/1: Recover kretprobes return address for EABI stack unwinder") tickled a bug in clang's integrated assembler where the .save and .pad directives must have corresponding .fnstart directives. The integrated assembler is unaware that the compiler will be generating the .fnstart directive.
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^
__kretprobe_trampoline's definition is already entirely inline asm. Move it to out-of-line asm to avoid breaking the build.
I think this is the right approach. I don't think is is even specified what exactly attribute((naked)) entails in the context of unwind information
Some nits below, but regardless:
Acked-by: Ard Biesheuvel ardb@kernel.org
Link: https://github.com/llvm/llvm-project/issues/57993 Link: https://github.com/ClangBuiltLinux/linux/issues/1718 Reported-by: Nathan Chancellor nathan@kernel.org Reported-by: Linux Kernel Functional Testing lkft@linaro.org Suggested-by: Logan Chien loganchien@google.com Signed-off-by: Nick Desaulniers ndesaulniers@google.com
Note: I wasn't quite sure if a Fixes tag against 1069c1dd20a3 was appropriate here? Either way, if 1069c1dd20a3 gets picked up for stable without this, it will break clang builds.
arch/arm/probes/kprobes/Makefile | 1 + arch/arm/probes/kprobes/core.c | 54 ++---------------- .../arm/probes/kprobes/kretprobe-trampoline.S | 55 +++++++++++++++++++ include/asm-generic/kprobes.h | 13 +++-- 4 files changed, 69 insertions(+), 54 deletions(-) create mode 100644 arch/arm/probes/kprobes/kretprobe-trampoline.S
diff --git a/arch/arm/probes/kprobes/Makefile b/arch/arm/probes/kprobes/Makefile index 6159010dac4a..cdbe9dd99e28 100644 --- a/arch/arm/probes/kprobes/Makefile +++ b/arch/arm/probes/kprobes/Makefile @@ -3,6 +3,7 @@ KASAN_SANITIZE_actions-common.o := n KASAN_SANITIZE_actions-arm.o := n KASAN_SANITIZE_actions-thumb.o := n obj-$(CONFIG_KPROBES) += core.o actions-common.o checkers-common.o +obj-$(CONFIG_KPROBES) += kretprobe-trampoline.o obj-$(CONFIG_ARM_KPROBES_TEST) += test-kprobes.o test-kprobes-objs := test-core.o
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c index 1435b508aa36..17d7e0259e63 100644 --- a/arch/arm/probes/kprobes/core.c +++ b/arch/arm/probes/kprobes/core.c @@ -375,58 +375,10 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self, return NOTIFY_DONE; }
-/*
- When a retprobed function returns, trampoline_handler() is called,
- calling the kretprobe's handler. We construct a struct pt_regs to
- give a view of registers r0-r11, sp, lr, and pc to the user
- return-handler. This is not a complete pt_regs structure, but that
- should be enough for stacktrace from the return handler with or
- without pt_regs.
- */
-void __naked __kprobes __kretprobe_trampoline(void) -{
__asm__ __volatile__ ("ldr lr, =__kretprobe_trampoline \n\t"-#ifdef CONFIG_FRAME_POINTER
/* __kretprobe_trampoline makes a framepointer on pt_regs. */-#ifdef CONFIG_CC_IS_CLANG
"stmdb sp, {sp, lr, pc} \n\t""sub sp, sp, #12 \n\t"/* In clang case, pt_regs->ip = lr. */"stmdb sp!, {r0 - r11, lr} \n\t"/* fp points regs->r11 (fp) */"add fp, sp, #44 \n\t"-#else /* !CONFIG_CC_IS_CLANG */
/* In gcc case, pt_regs->ip = fp. */"stmdb sp, {fp, sp, lr, pc} \n\t""sub sp, sp, #16 \n\t""stmdb sp!, {r0 - r11} \n\t"/* fp points regs->r15 (pc) */"add fp, sp, #60 \n\t"-#endif /* CONFIG_CC_IS_CLANG */ -#else /* !CONFIG_FRAME_POINTER */
/* store SP, LR on stack and add EABI unwind hint */"stmdb sp, {sp, lr, pc} \n\t"".save {sp, lr, pc} \n\t""sub sp, sp, #16 \n\t""stmdb sp!, {r0 - r11} \n\t"".pad #52 \n\t"-#endif /* CONFIG_FRAME_POINTER */
"mov r0, sp \n\t""bl trampoline_handler \n\t""mov lr, r0 \n\t""ldmia sp!, {r0 - r11} \n\t""add sp, sp, #16 \n\t"-#ifdef CONFIG_THUMB2_KERNEL
"bx lr \n\t"-#else
"mov pc, lr \n\t"-#endif
: : : "memory");-} +/*void __kretprobe_trampoline(void);*/
/* Called from __kretprobe_trampoline */ -static __used __kprobes void *trampoline_handler(struct pt_regs *regs) +__kprobes void *trampoline_handler(struct pt_regs *regs) { return (void *)kretprobe_trampoline_handler(regs, (void *)regs->TRAMP_FP); } @@ -434,6 +386,8 @@ static __used __kprobes void *trampoline_handler(struct pt_regs *regs) void __kprobes arch_prepare_kretprobe(struct kretprobe_instance *ri, struct pt_regs *regs) {
extern void __kretprobe_trampoline(void);ri->ret_addr = (kprobe_opcode_t *)regs->ARM_lr; ri->fp = (void *)regs->TRAMP_FP;diff --git a/arch/arm/probes/kprobes/kretprobe-trampoline.S b/arch/arm/probes/kprobes/kretprobe-trampoline.S new file mode 100644 index 000000000000..261c99b8c17f --- /dev/null +++ b/arch/arm/probes/kprobes/kretprobe-trampoline.S @@ -0,0 +1,55 @@ +/* SPDX-License-Identifier: GPL-2.0 */
+#include <linux/linkage.h> +#include <asm/unwind.h> +#include <asm-generic/kprobes.h>
+/*
- When a retprobed function returns, trampoline_handler() is called,
- calling the kretprobe's handler. We construct a struct pt_regs to
- give a view of registers r0-r11, sp, lr, and pc to the user
- return-handler. This is not a complete pt_regs structure, but that
- should be enough for stacktrace from the return handler with or
- without pt_regs.
- */
+__KPROBE +SYM_FUNC_START(__kretprobe_trampoline) +UNWIND(.fnstart)
ldr lr, =__kretprobe_trampoline
Nit: adr lr, __kretprobe_trampoline
Now that you made a clean spot, might as well clean this up a bit (although perhaps not in the same patch). I'd merge the !CONFIG_FRAME_POINTER and CC_IS_CLANG cases, and just put the UNWIND() directives in there.
+#ifdef CONFIG_FRAME_POINTER
Drop this
/* __kretprobe_trampoline makes a framepointer on pt_regs. */+#ifdef CONFIG_CC_IS_CLANG
|| !defined(CONFIG_FRAME_POINTER)
stmdb sp, {sp, lr, pc}sub sp, sp, #12
UNWIND(.save {sp, lr, pc})
/* In clang case, pt_regs->ip = lr. */
For .S, it is more idiomatic to put the comments in a column on the right. This reduces visual clutter a lot, so I think it would be worth the effort.
stmdb sp!, {r0 - r11, lr}
UNWIND(.pad #52)
/* fp points regs->r11 (fp) */add fp, sp, #44+#else /* !CONFIG_CC_IS_CLANG */
/* In gcc case, pt_regs->ip = fp. */stmdb sp, {fp, sp, lr, pc}sub sp, sp, #16stmdb sp!, {r0 - r11}/* fp points regs->r15 (pc) */add fp, sp, #60+#endif /* CONFIG_CC_IS_CLANG */ +#else /* !CONFIG_FRAME_POINTER */
/* store SP, LR on stack and add EABI unwind hint */stmdb sp, {sp, lr, pc}+UNWIND(.save {sp, lr, pc})
sub sp, sp, #16stmdb sp!, {r0 - r11}+UNWIND(.pad #52) +#endif /* CONFIG_FRAME_POINTER */
mov r0, spbl trampoline_handlermov lr, r0ldmia sp!, {r0 - r11}add sp, sp, #16+#ifdef CONFIG_THUMB2_KERNEL
bx lr+#else
mov pc, lr+#endif
Please use 'ret lr' here. Note that this code does not even compile for Thumb2, but 'bx lr' should be used at least on v6+ in any case.
+UNWIND(.fnend) +SYM_FUNC_END(__kretprobe_trampoline) diff --git a/include/asm-generic/kprobes.h b/include/asm-generic/kprobes.h index 060eab094e5a..1509daa281b8 100644 --- a/include/asm-generic/kprobes.h +++ b/include/asm-generic/kprobes.h @@ -2,7 +2,11 @@ #ifndef _ASM_GENERIC_KPROBES_H #define _ASM_GENERIC_KPROBES_H
-#if defined(__KERNEL__) && !defined(__ASSEMBLY__) +#ifdef __KERNEL__
+#ifdef __ASSEMBLY__ +# define __KPROBE .section ".kprobes.text", "ax" +#else #ifdef CONFIG_KPROBES /*
- Blacklist ganerating macro. Specify functions which is not probed
@@ -16,11 +20,12 @@ static unsigned long __used \ /* Use this to forbid a kprobes attach on very low level functions */ # define __kprobes __section(".kprobes.text") # define nokprobe_inline __always_inline -#else +#else /* !defined(CONFIG_KPROBES) */ # define NOKPROBE_SYMBOL(fname) # define __kprobes # define nokprobe_inline inline -#endif -#endif /* defined(__KERNEL__) && !defined(__ASSEMBLY__) */ +#endif /* defined(CONFIG_KPROBES) */ +#endif /* defined(__ASSEMBLY__) */ +#endif /* defined(__KERNEL__) */
#endif /* _ASM_GENERIC_KPROBES_H */
2.37.3.998.g577e59143f-goog
Looks to me like a failure caused by Chen Zhongjin's change: "Recover kretprobes return address for EABI stack unwinder"
I will drop this patch; please submit a replacement that builds.
Thanks.
On Mon, Sep 26, 2022 at 06:57:00PM +0530, Naresh Kamboju wrote:
Following build warnings / errors noticed on arm with clang-13 / 14 on Linux next-20220923.
Reported-by: Linux Kernel Functional Testing lkft@linaro.org
Regressions found on arm:
- build-clang-13-bcm2835_defconfig
- build-clang-nightly-imx_v6_v7_defconfig
- build-clang-nightly-orion5x_defconfig
- build-clang-13-keystone_defconfig
- build-clang-13-omap2plus_defconfig
- build-clang-14-imx_v6_v7_defconfig
- build-clang-nightly-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig
- build-clang-nightly-bcm2835_defconfig
- build-clang-13-imx_v6_v7_defconfig
- build-clang-13-imx_v4_v5_defconfig
- build-clang-14-imx_v4_v5_defconfig
- build-clang-13-orion5x_defconfig
- build-clang-14-multi_v5_defconfig-65236a87
- build-clang-14-lkftconfig
- build-clang-nightly-imx_v4_v5_defconfig
- build-clang-13-multi_v5_defconfig
- build-clang-13-lkftconfig
- build-clang-nightly-keystone_defconfig
- build-clang-14-multi_v5_defconfig
- build-clang-14-orion5x_defconfig
- build-clang-14-omap2plus_defconfig
- build-clang-nightly-multi_v5_defconfig-65236a87
- build-clang-14-bcm2835_defconfig
- build-clang-14-keystone_defconfig
- build-clang-nightly-lkftconfig
arch/arm/probes/kprobes/core.c:409:30: error: .fnstart must precede .save or .vsave directives "stmdb sp, {sp, lr, pc} \n\t" ^ <inline asm>:3:2: note: instantiated into assembly here .save {sp, lr, pc} ^ /builds/linux/arch/arm/probes/kprobes/core.c:412:29: error: .fnstart must precede .pad directive "stmdb sp!, {r0 - r11} \n\t" ^ <inline asm>:6:2: note: instantiated into assembly here .pad #52 ^ 2 errors generated. make[5]: *** [/builds/linux/scripts/Makefile.build:250: arch/arm/probes/kprobes/core.o] Error 1
build log: https://builds.tuxbuild.com/2FAyD1qcTlzjIYE7mjrugjCsxu1/
-- Linaro LKFT https://lkft.linaro.org
-
Ard Biesheuvel -
Chen Zhongjin -
kernel test robot -
Naresh Kamboju -
Nathan Chancellor -
Nick Desaulniers -
Russell King (Oracle)