Make __kretprobe_trampoline() as a pure asm function same as the x86
code does. Anyway, it is safe to define the symbol in the asm code
instead of accessing a C symbol from the inline asm.
Without this fix, building arm kernel with GCC-11 may cause below
error.
/tmp/ccIWiggX.s: Assembler messages:
>> /tmp/ccIWiggX.s:22: Error: invalid literal constant: pool needs to be closer
This fixes the error reported by 0day build bot.
Reported-by: kernel test robot <[email protected]>
Fixes: 7e9bf33b8124 ("ARM: kprobes: Make a frame pointer on __kretprobe_trampoline")
Signed-off-by: Masami Hiramatsu <[email protected]>
---
arch/arm/probes/kprobes/core.c | 57 +++++++++++++++++++++-------------------
1 file changed, 30 insertions(+), 27 deletions(-)
diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
index 4848404ba51b..51f1438456ae 100644
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -373,43 +373,46 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
* should be enough for stacktrace from the return handler with or
* without pt_regs.
*/
-void __naked __kprobes __kretprobe_trampoline(void)
-{
- __asm__ __volatile__ (
+asm(
+ ".text\n"
+ ".global __kretprobe_trampoline\n"
+ ".type __kretprobe_trampoline, %function\n"
+ "__kretprobe_trampoline:\n"
#ifdef CONFIG_FRAME_POINTER
- "ldr lr, =__kretprobe_trampoline \n\t"
+ "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"
+ "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"
+ /* 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"
+ "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"
+ "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"
+ "bx lr \n\t"
#else
- "mov pc, lr \n\t"
+ "mov pc, lr \n\t"
#endif
- : : : "memory");
-}
+ ".size __kretprobe_trampoline, .-__kretprobe_trampoline\n"
+);
+NOKPROBE_SYMBOL(__kretprobe_trampoline);
/* Called from __kretprobe_trampoline */
static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
--- a/arch/arm/probes/kprobes/core.c
+++ b/arch/arm/probes/kprobes/core.c
@@ -377,7 +377,7 @@ void __naked __kprobes __kretprobe_trampoline(void)
{
__asm__ __volatile__ (
#ifdef CONFIG_FRAME_POINTER
- "ldr lr, =__kretprobe_trampoline \n\t"
+ "ldr lr, .L__kretprobe_trampoline \n\t"
/* __kretprobe_trampoline makes a framepointer on pt_regs. */
#ifdef CONFIG_CC_IS_CLANG
"stmdb sp, {sp, lr, pc} \n\t"
@@ -407,6 +407,11 @@ void __naked __kprobes __kretprobe_trampoline(void)
"bx lr \n\t"
#else
"mov pc, lr \n\t"
+#endif
+#ifdef CONFIG_FRAME_POINTER
+ ".align 2 \n\t"
+ ".L__kretprobe_trampoline: \n\t"
+ ".long __kretprobe_trampoline \n\t"
#endif
: : : "memory");
}
On Thu, 24 Mar 2022 at 14:23, Masami Hiramatsu <[email protected]> wrote:
>
> On Thu, 24 Mar 2022 15:21:08 +0900
> Masami Hiramatsu <[email protected]> wrote:
>
> > Hi,
> >
> > I found a better solution for this issue from Ard :-)
> >
> > https://lore.kernel.org/all/[email protected]/T/#u
> >
> > I should use mov_l instead of ldr for loading the symbol address.
>
> Hm, these macros are only for the pure assembly file (.S), so we have to
> split this in a asm file to use that.
>
What about the below?
On Thu, 24 Mar 2022 at 14:34, Ard Biesheuvel <[email protected]> wrote:
>
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -377,7 +377,7 @@ void __naked __kprobes __kretprobe_trampoline(void)
> {
> __asm__ __volatile__ (
> #ifdef CONFIG_FRAME_POINTER
> - "ldr lr, =__kretprobe_trampoline \n\t"
> + "ldr lr, .L__kretprobe_trampoline \n\t"
> /* __kretprobe_trampoline makes a framepointer on pt_regs. */
> #ifdef CONFIG_CC_IS_CLANG
> "stmdb sp, {sp, lr, pc} \n\t"
> @@ -407,6 +407,11 @@ void __naked __kprobes __kretprobe_trampoline(void)
> "bx lr \n\t"
> #else
> "mov pc, lr \n\t"
> +#endif
> +#ifdef CONFIG_FRAME_POINTER
> + ".align 2 \n\t"
> + ".L__kretprobe_trampoline: \n\t"
> + ".long __kretprobe_trampoline \n\t"
> #endif
> : : : "memory");
> }
>
>
Pardon the copy/paste error.
> What about the below?
was meant to refer to the diff above.
> On Thu, 24 Mar 2022 at 14:23, Masami Hiramatsu <[email protected]> wrote:
> >
> > On Thu, 24 Mar 2022 15:21:08 +0900
> > Masami Hiramatsu <[email protected]> wrote:
> >
> > > Hi,
> > >
> > > I found a better solution for this issue from Ard :-)
> > >
> > > https://lore.kernel.org/all/[email protected]/T/#u
> > >
> > > I should use mov_l instead of ldr for loading the symbol address.
> >
> > Hm, these macros are only for the pure assembly file (.S), so we have to
> > split this in a asm file to use that.
> >
>
>
> What about the below?
Hi Ard,
On Thu, 24 Mar 2022 14:34:46 +0100
Ard Biesheuvel <[email protected]> wrote:
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -377,7 +377,7 @@ void __naked __kprobes __kretprobe_trampoline(void)
> {
> __asm__ __volatile__ (
> #ifdef CONFIG_FRAME_POINTER
> - "ldr lr, =__kretprobe_trampoline \n\t"
> + "ldr lr, .L__kretprobe_trampoline \n\t"
> /* __kretprobe_trampoline makes a framepointer on pt_regs. */
> #ifdef CONFIG_CC_IS_CLANG
> "stmdb sp, {sp, lr, pc} \n\t"
> @@ -407,6 +407,11 @@ void __naked __kprobes __kretprobe_trampoline(void)
> "bx lr \n\t"
> #else
> "mov pc, lr \n\t"
> +#endif
> +#ifdef CONFIG_FRAME_POINTER
> + ".align 2 \n\t"
> + ".L__kretprobe_trampoline: \n\t"
> + ".long __kretprobe_trampoline \n\t"
> #endif
> : : : "memory");
> }
Yes, I confirmed this works too :-)
Can you send the patch with my Tested-by ?
Thank you!
>
>
> On Thu, 24 Mar 2022 at 14:23, Masami Hiramatsu <[email protected]> wrote:
> >
> > On Thu, 24 Mar 2022 15:21:08 +0900
> > Masami Hiramatsu <[email protected]> wrote:
> >
> > > Hi,
> > >
> > > I found a better solution for this issue from Ard :-)
> > >
> > > https://lore.kernel.org/all/[email protected]/T/#u
> > >
> > > I should use mov_l instead of ldr for loading the symbol address.
> >
> > Hm, these macros are only for the pure assembly file (.S), so we have to
> > split this in a asm file to use that.
> >
>
>
> What about the below?
--
Masami Hiramatsu <[email protected]>
On Thu, 24 Mar 2022 15:21:08 +0900
Masami Hiramatsu <[email protected]> wrote:
> Hi,
>
> I found a better solution for this issue from Ard :-)
>
> https://lore.kernel.org/all/[email protected]/T/#u
>
> I should use mov_l instead of ldr for loading the symbol address.
Hm, these macros are only for the pure assembly file (.S), so we have to
split this in a asm file to use that.
Thanks,
>
> Thank you,
>
> On Tue, 15 Mar 2022 18:52:16 +0900
> Masami Hiramatsu <[email protected]> wrote:
>
> > Make __kretprobe_trampoline() as a pure asm function same as the x86
> > code does. Anyway, it is safe to define the symbol in the asm code
> > instead of accessing a C symbol from the inline asm.
> >
> > Without this fix, building arm kernel with GCC-11 may cause below
> > error.
> >
> > /tmp/ccIWiggX.s: Assembler messages:
> > >> /tmp/ccIWiggX.s:22: Error: invalid literal constant: pool needs to be closer
> >
> > This fixes the error reported by 0day build bot.
> >
> > Reported-by: kernel test robot <[email protected]>
> > Fixes: 7e9bf33b8124 ("ARM: kprobes: Make a frame pointer on __kretprobe_trampoline")
> > Signed-off-by: Masami Hiramatsu <[email protected]>
> > ---
> > arch/arm/probes/kprobes/core.c | 57 +++++++++++++++++++++-------------------
> > 1 file changed, 30 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> > index 4848404ba51b..51f1438456ae 100644
> > --- a/arch/arm/probes/kprobes/core.c
> > +++ b/arch/arm/probes/kprobes/core.c
> > @@ -373,43 +373,46 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> > * should be enough for stacktrace from the return handler with or
> > * without pt_regs.
> > */
> > -void __naked __kprobes __kretprobe_trampoline(void)
> > -{
> > - __asm__ __volatile__ (
> > +asm(
> > + ".text\n"
> > + ".global __kretprobe_trampoline\n"
> > + ".type __kretprobe_trampoline, %function\n"
> > + "__kretprobe_trampoline:\n"
> > #ifdef CONFIG_FRAME_POINTER
> > - "ldr lr, =__kretprobe_trampoline \n\t"
> > + "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"
> > + "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"
> > + /* 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"
> > + "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"
> > + "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"
> > + "bx lr \n\t"
> > #else
> > - "mov pc, lr \n\t"
> > + "mov pc, lr \n\t"
> > #endif
> > - : : : "memory");
> > -}
> > + ".size __kretprobe_trampoline, .-__kretprobe_trampoline\n"
> > +);
> > +NOKPROBE_SYMBOL(__kretprobe_trampoline);
> >
> > /* Called from __kretprobe_trampoline */
> > static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
> >
>
>
> --
> Masami Hiramatsu <[email protected]>
--
Masami Hiramatsu <[email protected]>
Hi,
I found a better solution for this issue from Ard :-)
https://lore.kernel.org/all/[email protected]/T/#u
I should use mov_l instead of ldr for loading the symbol address.
Thank you,
On Tue, 15 Mar 2022 18:52:16 +0900
Masami Hiramatsu <[email protected]> wrote:
> Make __kretprobe_trampoline() as a pure asm function same as the x86
> code does. Anyway, it is safe to define the symbol in the asm code
> instead of accessing a C symbol from the inline asm.
>
> Without this fix, building arm kernel with GCC-11 may cause below
> error.
>
> /tmp/ccIWiggX.s: Assembler messages:
> >> /tmp/ccIWiggX.s:22: Error: invalid literal constant: pool needs to be closer
>
> This fixes the error reported by 0day build bot.
>
> Reported-by: kernel test robot <[email protected]>
> Fixes: 7e9bf33b8124 ("ARM: kprobes: Make a frame pointer on __kretprobe_trampoline")
> Signed-off-by: Masami Hiramatsu <[email protected]>
> ---
> arch/arm/probes/kprobes/core.c | 57 +++++++++++++++++++++-------------------
> 1 file changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/arch/arm/probes/kprobes/core.c b/arch/arm/probes/kprobes/core.c
> index 4848404ba51b..51f1438456ae 100644
> --- a/arch/arm/probes/kprobes/core.c
> +++ b/arch/arm/probes/kprobes/core.c
> @@ -373,43 +373,46 @@ int __kprobes kprobe_exceptions_notify(struct notifier_block *self,
> * should be enough for stacktrace from the return handler with or
> * without pt_regs.
> */
> -void __naked __kprobes __kretprobe_trampoline(void)
> -{
> - __asm__ __volatile__ (
> +asm(
> + ".text\n"
> + ".global __kretprobe_trampoline\n"
> + ".type __kretprobe_trampoline, %function\n"
> + "__kretprobe_trampoline:\n"
> #ifdef CONFIG_FRAME_POINTER
> - "ldr lr, =__kretprobe_trampoline \n\t"
> + "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"
> + "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"
> + /* 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"
> + "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"
> + "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"
> + "bx lr \n\t"
> #else
> - "mov pc, lr \n\t"
> + "mov pc, lr \n\t"
> #endif
> - : : : "memory");
> -}
> + ".size __kretprobe_trampoline, .-__kretprobe_trampoline\n"
> +);
> +NOKPROBE_SYMBOL(__kretprobe_trampoline);
>
> /* Called from __kretprobe_trampoline */
> static __used __kprobes void *trampoline_handler(struct pt_regs *regs)
>
--
Masami Hiramatsu <[email protected]>