diff mbox series

[v2,2/4] arm64, scs: save scs_sp values per-cpu when switching stacks

Message ID f75c58b17bfaa419f84286cd174e3a08f971b779.1648049113.git.andreyknvl@google.com (mailing list archive)
State New
Headers show
Series kasan, arm64, scs, stacktrace: collect stack traces from Shadow Call Stack | expand

Commit Message

andrey.konovalov@linux.dev March 23, 2022, 3:32 p.m. UTC 
From: Andrey Konovalov <andreyknvl@google.com>

When an interrupt happens, the current Shadow Call Stack (SCS) pointer
is switched to a per-interrupt one stored in a per-CPU variable. The old
pointer is then saved on the normal stack and restored when the interrupt
is handled.

To collect the current stack trace based on SCS when the interrupt is
being handled, we need to know the SCS pointers that belonged to the
task and potentially other interrupts that were interrupted.

Instead of trying to retrieve the SCS pointers from the stack, change
interrupt handlers (for hard IRQ, Normal and Critical SDEI) to save the
previous SCS pointer in a per-CPU variable.

Note that interrupts stack. A task can be interrupted by a hard IRQ,
which then can interrupted by a normal SDEI, etc. This is handled by
using a separate per-CPU variable for each interrupt type.

Also reset the saved SCS pointer when exiting the interrupt. This allows
checking whether we should include any interrupt frames when collecting
the stack trace. While we could use in_hardirq(), there seems to be no
easy way to check whether we are in an SDEI handler. Directly checking
the per-CPU variables for being non-zero is more resilient.

Also expose both the the added saved SCS variables and the existing SCS
base variables in arch/arm64/include/asm/scs.h so that the stack trace
collection impementation can use them.

Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
---
 arch/arm64/include/asm/assembler.h | 12 ++++++++++++
 arch/arm64/include/asm/scs.h       | 13 ++++++++++++-
 arch/arm64/kernel/entry.S          | 28 ++++++++++++++++++++++++----
 arch/arm64/kernel/irq.c            |  4 +---
 arch/arm64/kernel/sdei.c           |  5 ++---
 5 files changed, 51 insertions(+), 11 deletions(-)

Comments

kernel test robot March 24, 2022, 11:08 a.m. UTC | #1 
Hi,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20220323]
[also build test ERROR on v5.17]
[cannot apply to arm64/for-next/core hnaz-mm/master linus/master v5.17 v5.17-rc8 v5.17-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]

url:    https://github.com/0day-ci/linux/commits/andrey-konovalov-linux-dev/kasan-arm64-scs-stacktrace-collect-stack-traces-from-Shadow-Call-Stack/20220323-233436
base:    b61581ae229d8eb9f21f8753be3f4011f7692384
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20220324/202203241922.UDw4JHPD-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.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/0day-ci/linux/commit/da5bedb1ac7aa0b303f6d996d306e675860b6e12
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review andrey-konovalov-linux-dev/kasan-arm64-scs-stacktrace-collect-stack-traces-from-Shadow-Call-Stack/20220323-233436
        git checkout da5bedb1ac7aa0b303f6d996d306e675860b6e12
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   In file included from include/asm-generic/percpu.h:7,
                    from arch/arm64/include/asm/percpu.h:248,
                    from include/linux/irqflags.h:17,
                    from include/linux/spinlock.h:58,
                    from include/linux/irq.h:14,
                    from arch/arm64/kernel/irq.c:13:
   arch/arm64/kernel/irq.c: In function 'init_irq_scs':
>> arch/arm64/kernel/irq.c:44:25: error: 'irq_shadow_call_stack_ptr' undeclared (first use in this function)
      44 |                 per_cpu(irq_shadow_call_stack_ptr, cpu) =
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:219:54: note: in definition of macro '__verify_pcpu_ptr'
     219 |         const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL;    \
         |                                                      ^~~
   include/linux/percpu-defs.h:269:35: note: in expansion of macro 'per_cpu_ptr'
     269 | #define per_cpu(var, cpu)       (*per_cpu_ptr(&(var), cpu))
         |                                   ^~~~~~~~~~~
   arch/arm64/kernel/irq.c:44:17: note: in expansion of macro 'per_cpu'
      44 |                 per_cpu(irq_shadow_call_stack_ptr, cpu) =
         |                 ^~~~~~~
   arch/arm64/kernel/irq.c:44:25: note: each undeclared identifier is reported only once for each function it appears in
      44 |                 per_cpu(irq_shadow_call_stack_ptr, cpu) =
         |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
   include/linux/percpu-defs.h:219:54: note: in definition of macro '__verify_pcpu_ptr'
     219 |         const void __percpu *__vpp_verify = (typeof((ptr) + 0))NULL;    \
         |                                                      ^~~
   include/linux/percpu-defs.h:269:35: note: in expansion of macro 'per_cpu_ptr'
     269 | #define per_cpu(var, cpu)       (*per_cpu_ptr(&(var), cpu))
         |                                   ^~~~~~~~~~~
   arch/arm64/kernel/irq.c:44:17: note: in expansion of macro 'per_cpu'
      44 |                 per_cpu(irq_shadow_call_stack_ptr, cpu) =
         |                 ^~~~~~~
   arch/arm64/kernel/irq.c: At top level:
   arch/arm64/kernel/irq.c:105:13: warning: no previous prototype for 'init_IRQ' [-Wmissing-prototypes]
     105 | void __init init_IRQ(void)
         |             ^~~~~~~~


vim +/irq_shadow_call_stack_ptr +44 arch/arm64/kernel/irq.c

ac20ffbb0279aa Sami Tolvanen 2020-11-30  35  
ac20ffbb0279aa Sami Tolvanen 2020-11-30  36  static void init_irq_scs(void)
ac20ffbb0279aa Sami Tolvanen 2020-11-30  37  {
ac20ffbb0279aa Sami Tolvanen 2020-11-30  38  	int cpu;
ac20ffbb0279aa Sami Tolvanen 2020-11-30  39  
ac20ffbb0279aa Sami Tolvanen 2020-11-30  40  	if (!IS_ENABLED(CONFIG_SHADOW_CALL_STACK))
ac20ffbb0279aa Sami Tolvanen 2020-11-30  41  		return;
ac20ffbb0279aa Sami Tolvanen 2020-11-30  42  
ac20ffbb0279aa Sami Tolvanen 2020-11-30  43  	for_each_possible_cpu(cpu)
ac20ffbb0279aa Sami Tolvanen 2020-11-30 @44  		per_cpu(irq_shadow_call_stack_ptr, cpu) =
ac20ffbb0279aa Sami Tolvanen 2020-11-30  45  			scs_alloc(cpu_to_node(cpu));
ac20ffbb0279aa Sami Tolvanen 2020-11-30  46  }
ac20ffbb0279aa Sami Tolvanen 2020-11-30  47
kernel test robot March 24, 2022, 9:39 p.m. UTC | #2 
Hi,

I love your patch! Yet something to improve:

[auto build test ERROR on next-20220323]
[also build test ERROR on v5.17]
[cannot apply to arm64/for-next/core hnaz-mm/master linus/master v5.17 v5.17-rc8 v5.17-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]

url:    https://github.com/0day-ci/linux/commits/andrey-konovalov-linux-dev/kasan-arm64-scs-stacktrace-collect-stack-traces-from-Shadow-Call-Stack/20220323-233436
base:    b61581ae229d8eb9f21f8753be3f4011f7692384
config: arm64-allyesconfig (https://download.01.org/0day-ci/archive/20220325/202203250512.yMAPu8rv-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 11.2.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/0day-ci/linux/commit/da5bedb1ac7aa0b303f6d996d306e675860b6e12
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review andrey-konovalov-linux-dev/kasan-arm64-scs-stacktrace-collect-stack-traces-from-Shadow-Call-Stack/20220323-233436
        git checkout da5bedb1ac7aa0b303f6d996d306e675860b6e12
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm64 SHELL=/bin/bash

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

   arch/arm64/kernel/sdei.c: In function 'free_sdei_scs':
>> arch/arm64/kernel/sdei.c:124:33: error: 'sdei_shadow_call_stack_normal_ptr' undeclared (first use in this function)
     124 |                 _free_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu);
         |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/arm64/kernel/sdei.c:124:33: note: each undeclared identifier is reported only once for each function it appears in
>> arch/arm64/kernel/sdei.c:125:33: error: 'sdei_shadow_call_stack_critical_ptr' undeclared (first use in this function); did you mean 'sdei_stack_critical_ptr'?
     125 |                 _free_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu);
         |                                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                 sdei_stack_critical_ptr
   arch/arm64/kernel/sdei.c: In function 'init_sdei_scs':
   arch/arm64/kernel/sdei.c:150:39: error: 'sdei_shadow_call_stack_normal_ptr' undeclared (first use in this function)
     150 |                 err = _init_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu);
         |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   arch/arm64/kernel/sdei.c:153:39: error: 'sdei_shadow_call_stack_critical_ptr' undeclared (first use in this function); did you mean 'sdei_stack_critical_ptr'?
     153 |                 err = _init_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu);
         |                                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
         |                                       sdei_stack_critical_ptr


vim +/sdei_shadow_call_stack_normal_ptr +124 arch/arm64/kernel/sdei.c

ac20ffbb0279aae Sami Tolvanen 2020-11-30  118  
ac20ffbb0279aae Sami Tolvanen 2020-11-30  119  static void free_sdei_scs(void)
ac20ffbb0279aae Sami Tolvanen 2020-11-30  120  {
ac20ffbb0279aae Sami Tolvanen 2020-11-30  121  	int cpu;
ac20ffbb0279aae Sami Tolvanen 2020-11-30  122  
ac20ffbb0279aae Sami Tolvanen 2020-11-30  123  	for_each_possible_cpu(cpu) {
ac20ffbb0279aae Sami Tolvanen 2020-11-30 @124  		_free_sdei_scs(&sdei_shadow_call_stack_normal_ptr, cpu);
ac20ffbb0279aae Sami Tolvanen 2020-11-30 @125  		_free_sdei_scs(&sdei_shadow_call_stack_critical_ptr, cpu);
ac20ffbb0279aae Sami Tolvanen 2020-11-30  126  	}
ac20ffbb0279aae Sami Tolvanen 2020-11-30  127  }
ac20ffbb0279aae Sami Tolvanen 2020-11-30  128
Mark Rutland March 31, 2022, 9:24 a.m. UTC | #3 
On Wed, Mar 23, 2022 at 04:32:53PM +0100, andrey.konovalov@linux.dev wrote:
> From: Andrey Konovalov <andreyknvl@google.com>
> 
> When an interrupt happens, the current Shadow Call Stack (SCS) pointer
> is switched to a per-interrupt one stored in a per-CPU variable. The old
> pointer is then saved on the normal stack and restored when the interrupt
> is handled.
> 
> To collect the current stack trace based on SCS when the interrupt is
> being handled, we need to know the SCS pointers that belonged to the
> task and potentially other interrupts that were interrupted.
> 
> Instead of trying to retrieve the SCS pointers from the stack, change
> interrupt handlers (for hard IRQ, Normal and Critical SDEI) to save the
> previous SCS pointer in a per-CPU variable.

I'm *really* not keen on *always* poking this in the entry code for the
uncommon case of unwind. It complicates the entry code and means we're always
paying a cost for potentially no benefit. At a high-level, I don't think this
is the right approach.

For the regular unwinder, I want to rework things such that we can identify
exception boundaries and look into the regs (e.g. so that we can recover the
PC+LR+FP and avoid duplicating part of this in a frame record), and I'd much
prefer that we did the same here.

Thanks,
Mark.

> Note that interrupts stack. A task can be interrupted by a hard IRQ,
> which then can interrupted by a normal SDEI, etc. This is handled by
> using a separate per-CPU variable for each interrupt type.
> 
> Also reset the saved SCS pointer when exiting the interrupt. This allows
> checking whether we should include any interrupt frames when collecting
> the stack trace. While we could use in_hardirq(), there seems to be no
> easy way to check whether we are in an SDEI handler. Directly checking
> the per-CPU variables for being non-zero is more resilient.
> 
> Also expose both the the added saved SCS variables and the existing SCS
> base variables in arch/arm64/include/asm/scs.h so that the stack trace
> collection impementation can use them.
> 
> Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> ---
>  arch/arm64/include/asm/assembler.h | 12 ++++++++++++
>  arch/arm64/include/asm/scs.h       | 13 ++++++++++++-
>  arch/arm64/kernel/entry.S          | 28 ++++++++++++++++++++++++----
>  arch/arm64/kernel/irq.c            |  4 +---
>  arch/arm64/kernel/sdei.c           |  5 ++---
>  5 files changed, 51 insertions(+), 11 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> index 8c5a61aeaf8e..ca018e981d13 100644
> --- a/arch/arm64/include/asm/assembler.h
> +++ b/arch/arm64/include/asm/assembler.h
> @@ -270,6 +270,18 @@ alternative_endif
>  	ldr	\dst, [\dst, \tmp]
>  	.endm
>  
> +	/*
> +	 * @src: Register whose value gets stored in sym
> +	 * @sym: The name of the per-cpu variable
> +	 * @tmp0: Scratch register
> +	 * @tmp1: Another scratch register
> +	 */
> +	.macro str_this_cpu src, sym, tmp0, tmp1
> +	adr_l	\tmp0, \sym
> +	get_this_cpu_offset \tmp1
> +	str	\src, [\tmp0, \tmp1]
> +	.endm
> +
>  /*
>   * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
>   */
> diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
> index 8297bccf0784..2bb2b32f787b 100644
> --- a/arch/arm64/include/asm/scs.h
> +++ b/arch/arm64/include/asm/scs.h
> @@ -24,6 +24,17 @@
>  	.endm
>  #endif /* CONFIG_SHADOW_CALL_STACK */
>  
> -#endif /* __ASSEMBLY __ */
> +#else /* __ASSEMBLY__ */
> +
> +#include <linux/percpu.h>
> +
> +DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> +DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_saved_ptr);
> +DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
> +DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_saved_ptr);
> +DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
> +DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_saved_ptr);
> +
> +#endif /* __ASSEMBLY__ */
>  
>  #endif /* _ASM_SCS_H */
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index ede028dee81b..1c62fecda172 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -880,7 +880,8 @@ NOKPROBE(ret_from_fork)
>   */
>  SYM_FUNC_START(call_on_irq_stack)
>  #ifdef CONFIG_SHADOW_CALL_STACK
> -	stp	scs_sp, xzr, [sp, #-16]!
> +	/* Save the current SCS pointer and load the per-IRQ one. */
> +	str_this_cpu scs_sp, irq_shadow_call_stack_saved_ptr, x15, x17
>  	ldr_this_cpu scs_sp, irq_shadow_call_stack_ptr, x17
>  #endif
>  	/* Create a frame record to save our LR and SP (implicit in FP) */
> @@ -902,7 +903,9 @@ SYM_FUNC_START(call_on_irq_stack)
>  	mov	sp, x29
>  	ldp	x29, x30, [sp], #16
>  #ifdef CONFIG_SHADOW_CALL_STACK
> -	ldp	scs_sp, xzr, [sp], #16
> +	/* Restore saved SCS pointer and reset the saved value. */
> +	ldr_this_cpu scs_sp, irq_shadow_call_stack_saved_ptr, x17
> +	str_this_cpu xzr, irq_shadow_call_stack_saved_ptr, x15, x17
>  #endif
>  	ret
>  SYM_FUNC_END(call_on_irq_stack)
> @@ -1024,11 +1027,16 @@ SYM_CODE_START(__sdei_asm_handler)
>  #endif
>  
>  #ifdef CONFIG_SHADOW_CALL_STACK
> -	/* Use a separate shadow call stack for normal and critical events */
> +	/*
> +	 * Use a separate shadow call stack for normal and critical events.
> +	 * Save the current SCS pointer and load the per-SDEI one.
> +	 */
>  	cbnz	w4, 3f
> +	str_this_cpu src=scs_sp, sym=sdei_shadow_call_stack_normal_saved_ptr, tmp0=x5, tmp1=x6
>  	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_normal_ptr, tmp=x6
>  	b	4f
> -3:	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_ptr, tmp=x6
> +3:	str_this_cpu src=scs_sp, sym=sdei_shadow_call_stack_critical_saved_ptr, tmp0=x5, tmp1=x6
> +	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_ptr, tmp=x6
>  4:
>  #endif
>  
> @@ -1062,6 +1070,18 @@ SYM_CODE_START(__sdei_asm_handler)
>  	ldp	lr, x1, [x4, #SDEI_EVENT_INTREGS + S_LR]
>  	mov	sp, x1
>  
> +#ifdef CONFIG_SHADOW_CALL_STACK
> +	/* Restore saved SCS pointer and reset the saved value. */
> +	ldrb	w5, [x4, #SDEI_EVENT_PRIORITY]
> +	cbnz	w5, 5f
> +	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_normal_saved_ptr, tmp=x6
> +	str_this_cpu src=xzr, sym=sdei_shadow_call_stack_normal_saved_ptr, tmp0=x5, tmp1=x6
> +	b	6f
> +5:	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_saved_ptr, tmp=x6
> +	str_this_cpu src=xzr, sym=sdei_shadow_call_stack_critical_saved_ptr, tmp0=x5, tmp1=x6
> +6:
> +#endif
> +
>  	mov	x1, x0			// address to complete_and_resume
>  	/* x0 = (x0 <= SDEI_EV_FAILED) ?
>  	 * EVENT_COMPLETE:EVENT_COMPLETE_AND_RESUME
> diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
> index bda49430c9ea..4199f900714a 100644
> --- a/arch/arm64/kernel/irq.c
> +++ b/arch/arm64/kernel/irq.c
> @@ -28,11 +28,9 @@ DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
>  
>  DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
>  
> -
> -DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> -
>  #ifdef CONFIG_SHADOW_CALL_STACK
>  DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
> +DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_saved_ptr);
>  #endif
>  
>  static void init_irq_scs(void)
> diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
> index d20620a1c51a..269adcb9e854 100644
> --- a/arch/arm64/kernel/sdei.c
> +++ b/arch/arm64/kernel/sdei.c
> @@ -39,12 +39,11 @@ DEFINE_PER_CPU(unsigned long *, sdei_stack_normal_ptr);
>  DEFINE_PER_CPU(unsigned long *, sdei_stack_critical_ptr);
>  #endif
>  
> -DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
> -DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
> -
>  #ifdef CONFIG_SHADOW_CALL_STACK
>  DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
> +DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_saved_ptr);
>  DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
> +DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_saved_ptr);
>  #endif
>  
>  static void _free_sdei_stack(unsigned long * __percpu *ptr, int cpu)
> -- 
> 2.25.1
>
Andrey Konovalov April 5, 2022, 3:22 p.m. UTC | #4 
On Thu, Mar 31, 2022 at 11:24 AM Mark Rutland <mark.rutland@arm.com> wrote:
>
> On Wed, Mar 23, 2022 at 04:32:53PM +0100, andrey.konovalov@linux.dev wrote:
> > From: Andrey Konovalov <andreyknvl@google.com>
> >
> > Instead of trying to retrieve the SCS pointers from the stack, change
> > interrupt handlers (for hard IRQ, Normal and Critical SDEI) to save the
> > previous SCS pointer in a per-CPU variable.
>
> I'm *really* not keen on *always* poking this in the entry code for the
> uncommon case of unwind. It complicates the entry code and means we're always
> paying a cost for potentially no benefit. At a high-level, I don't think this
> is the right approach.

This also gives a 5% slowdown, which is not acceptable.

What we can do instead, is to not collect frames from the higher
exception levels at all. This would leave SCS-based stack collection
method impaired, but this is probably fine for KASAN's use case:
currently, stack depot filters out higher-level frames anyway, so
KASAN never saves them. And the lower-level part of the stack trace is
enough to identify the allocation.

Thanks!


> For the regular unwinder, I want to rework things such that we can identify
> exception boundaries and look into the regs (e.g. so that we can recover the
> PC+LR+FP and avoid duplicating part of this in a frame record), and I'd much
> prefer that we did the same here.
diff mbox series

Patch

diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
index 8c5a61aeaf8e..ca018e981d13 100644
--- a/arch/arm64/include/asm/assembler.h
+++ b/arch/arm64/include/asm/assembler.h
@@ -270,6 +270,18 @@  alternative_endif
 	ldr	\dst, [\dst, \tmp]
 	.endm
 
+	/*
+	 * @src: Register whose value gets stored in sym
+	 * @sym: The name of the per-cpu variable
+	 * @tmp0: Scratch register
+	 * @tmp1: Another scratch register
+	 */
+	.macro str_this_cpu src, sym, tmp0, tmp1
+	adr_l	\tmp0, \sym
+	get_this_cpu_offset \tmp1
+	str	\src, [\tmp0, \tmp1]
+	.endm
+
 /*
  * vma_vm_mm - get mm pointer from vma pointer (vma->vm_mm)
  */
diff --git a/arch/arm64/include/asm/scs.h b/arch/arm64/include/asm/scs.h
index 8297bccf0784..2bb2b32f787b 100644
--- a/arch/arm64/include/asm/scs.h
+++ b/arch/arm64/include/asm/scs.h
@@ -24,6 +24,17 @@ 
 	.endm
 #endif /* CONFIG_SHADOW_CALL_STACK */
 
-#endif /* __ASSEMBLY __ */
+#else /* __ASSEMBLY__ */
+
+#include <linux/percpu.h>
+
+DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
+DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_saved_ptr);
+DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
+DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_saved_ptr);
+DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
+DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_saved_ptr);
+
+#endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_SCS_H */
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index ede028dee81b..1c62fecda172 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -880,7 +880,8 @@  NOKPROBE(ret_from_fork)
  */
 SYM_FUNC_START(call_on_irq_stack)
 #ifdef CONFIG_SHADOW_CALL_STACK
-	stp	scs_sp, xzr, [sp, #-16]!
+	/* Save the current SCS pointer and load the per-IRQ one. */
+	str_this_cpu scs_sp, irq_shadow_call_stack_saved_ptr, x15, x17
 	ldr_this_cpu scs_sp, irq_shadow_call_stack_ptr, x17
 #endif
 	/* Create a frame record to save our LR and SP (implicit in FP) */
@@ -902,7 +903,9 @@  SYM_FUNC_START(call_on_irq_stack)
 	mov	sp, x29
 	ldp	x29, x30, [sp], #16
 #ifdef CONFIG_SHADOW_CALL_STACK
-	ldp	scs_sp, xzr, [sp], #16
+	/* Restore saved SCS pointer and reset the saved value. */
+	ldr_this_cpu scs_sp, irq_shadow_call_stack_saved_ptr, x17
+	str_this_cpu xzr, irq_shadow_call_stack_saved_ptr, x15, x17
 #endif
 	ret
 SYM_FUNC_END(call_on_irq_stack)
@@ -1024,11 +1027,16 @@  SYM_CODE_START(__sdei_asm_handler)
 #endif
 
 #ifdef CONFIG_SHADOW_CALL_STACK
-	/* Use a separate shadow call stack for normal and critical events */
+	/*
+	 * Use a separate shadow call stack for normal and critical events.
+	 * Save the current SCS pointer and load the per-SDEI one.
+	 */
 	cbnz	w4, 3f
+	str_this_cpu src=scs_sp, sym=sdei_shadow_call_stack_normal_saved_ptr, tmp0=x5, tmp1=x6
 	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_normal_ptr, tmp=x6
 	b	4f
-3:	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_ptr, tmp=x6
+3:	str_this_cpu src=scs_sp, sym=sdei_shadow_call_stack_critical_saved_ptr, tmp0=x5, tmp1=x6
+	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_ptr, tmp=x6
 4:
 #endif
 
@@ -1062,6 +1070,18 @@  SYM_CODE_START(__sdei_asm_handler)
 	ldp	lr, x1, [x4, #SDEI_EVENT_INTREGS + S_LR]
 	mov	sp, x1
 
+#ifdef CONFIG_SHADOW_CALL_STACK
+	/* Restore saved SCS pointer and reset the saved value. */
+	ldrb	w5, [x4, #SDEI_EVENT_PRIORITY]
+	cbnz	w5, 5f
+	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_normal_saved_ptr, tmp=x6
+	str_this_cpu src=xzr, sym=sdei_shadow_call_stack_normal_saved_ptr, tmp0=x5, tmp1=x6
+	b	6f
+5:	ldr_this_cpu dst=scs_sp, sym=sdei_shadow_call_stack_critical_saved_ptr, tmp=x6
+	str_this_cpu src=xzr, sym=sdei_shadow_call_stack_critical_saved_ptr, tmp0=x5, tmp1=x6
+6:
+#endif
+
 	mov	x1, x0			// address to complete_and_resume
 	/* x0 = (x0 <= SDEI_EV_FAILED) ?
 	 * EVENT_COMPLETE:EVENT_COMPLETE_AND_RESUME
diff --git a/arch/arm64/kernel/irq.c b/arch/arm64/kernel/irq.c
index bda49430c9ea..4199f900714a 100644
--- a/arch/arm64/kernel/irq.c
+++ b/arch/arm64/kernel/irq.c
@@ -28,11 +28,9 @@  DEFINE_PER_CPU(struct nmi_ctx, nmi_contexts);
 
 DEFINE_PER_CPU(unsigned long *, irq_stack_ptr);
 
-
-DECLARE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
-
 #ifdef CONFIG_SHADOW_CALL_STACK
 DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_ptr);
+DEFINE_PER_CPU(unsigned long *, irq_shadow_call_stack_saved_ptr);
 #endif
 
 static void init_irq_scs(void)
diff --git a/arch/arm64/kernel/sdei.c b/arch/arm64/kernel/sdei.c
index d20620a1c51a..269adcb9e854 100644
--- a/arch/arm64/kernel/sdei.c
+++ b/arch/arm64/kernel/sdei.c
@@ -39,12 +39,11 @@  DEFINE_PER_CPU(unsigned long *, sdei_stack_normal_ptr);
 DEFINE_PER_CPU(unsigned long *, sdei_stack_critical_ptr);
 #endif
 
-DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
-DECLARE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
-
 #ifdef CONFIG_SHADOW_CALL_STACK
 DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_ptr);
+DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_normal_saved_ptr);
 DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_ptr);
+DEFINE_PER_CPU(unsigned long *, sdei_shadow_call_stack_critical_saved_ptr);
 #endif
 
 static void _free_sdei_stack(unsigned long * __percpu *ptr, int cpu)