Skip to contentSkip to conversation

ARM: stacktrace: disregard .entry.text when looking for exception frames

PATCH In review Ard Biesheuvel 2 participants 3 messages Oct 29, 2020 – Nov 5, 2020 Arm (32-bit)

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 ARM: stacktrace: disregard .entry.text when looking for exception frames

Commit c608906165355089 ("ARM: probes: avoid adding kprobes to sensitive kernel-entry/exit code") reorganized the section layout to prevent entry code from being instrumented by kprobes.

This resulted in stack frames referring to back to entry code to be misidentified as exception frames, resulting in splats like the below when KASAN is enabled:

==================================================================

  BUG: KASAN: stack-out-of-bounds in save_trace+0xc1/0xf8
  Read of size 4 at addr df01f89c by task bash/3421

  CPU: 12 PID: 3421 Comm: bash Not tainted 5.10.0-rc1-kasan+ #219
  Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
  [<c0411765>] (unwind_backtrace) from [<c040ba8b>] (show_stack+0xb/0xc)
  [<c040ba8b>] (show_stack) from [<c121c5e5>] (dump_stack+0x8d/0xa0)
  [<c121c5e5>] (dump_stack) from [<c05bd87b>] (print_address_description.constprop.3+0x2b/0x368)
  [<c05bd87b>] (print_address_description.constprop.3) from [<c05bdd2d>] (kasan_report+0xfd/0x114)
  [<c05bdd2d>] (kasan_report) from [<c040b629>] (save_trace+0xc1/0xf8)
  [<c040b629>] (save_trace) from [<c040b563>] (walk_stackframe+0x1b/0x20)
  [<c040b563>] (walk_stackframe) from [<c040b759>] (__save_stack_trace+0xf9/0x100)
  [<c040b759>] (__save_stack_trace) from [<c04cb169>] (stack_trace_save+0x71/0x88)
  [<c04cb169>] (stack_trace_save) from [<c05bcd9d>] (kasan_save_stack+0x11/0x28)
  [<c05bcd9d>] (kasan_save_stack) from [<c05bcdd1>] (kasan_set_track+0x1d/0x20)
  [<c05bcdd1>] (kasan_set_track) from [<c05be421>] (kasan_set_free_info+0x19/0x20)
  [<c05be421>] (kasan_set_free_info) from [<c05bcd67>] (__kasan_slab_free+0xa7/0xcc)
  [<c05bcd67>] (__kasan_slab_free) from [<c05bbe41>] (kmem_cache_free+0x59/0x21c)
  [<c05bbe41>] (kmem_cache_free) from [<c04c1fdb>] (rcu_core+0x2d7/0x988)
  [<c04c1fdb>] (rcu_core) from [<c04014ab>] (__do_softirq+0x133/0x41c)
  [<c04014ab>] (__do_softirq) from [<c044d4c9>] (irq_exit+0xb5/0xcc)
  [<c044d4c9>] (irq_exit) from [<c04a8503>] (__handle_domain_irq+0x5f/0xa8)
  [<c04a8503>] (__handle_domain_irq) from [<c08d5cb5>] (gic_handle_irq+0x3d/0x8c)
  [<c08d5cb5>] (gic_handle_irq) from [<c0400c11>] (__irq_svc+0x51/0x7c)
  Exception stack(0xdf01f7f8 to 0xdf01f840)
  f7e0:                                                       df01f8e0 df01f8a0
  f800: 00000008 df01fc7c 000000ad df01f960 df01f8e0 00000008 bae03f10 df01fc58
  f820: df01f8a0 0000000a c040b660 df01f848 c04114eb c04110b8 20000033 ffffffff
  [<c0400c11>] (__irq_svc) from [<c04110b8>] (unwind_pop_register+0x0/0x58)
  [<c04110b8>] (unwind_pop_register) from [<c0400161>] (ret_fast_syscall+0x1/0x5a)
  Exception stack(0xdf01f860 to 0xdf01f8a8)
  f860: 41b58ab3 c1f939e0 c04111ec 00000000 d129e800 c0411311 df01fa44 df01fa4c
  f880: 41b58ab3 c1f939e0 c04111ec 00000001 00000003 c040b569 df01f8c0 df01c000
  f8a0: df01fc7c df01ffa8
  ...

addr df01f89c is located in stack of task bash/3421 at offset 28 in frame:
unwind_frame+0x0/0x578

Here, the last entry represents a call to unwind_pop_register frame() with the return address set to ret_fast_syscall, and since in_entry_text() returns true for that return address, save_trace() treats it as an exception frame, and attempts to dereference the struct pt_regs pointer to access the ARM_pc value. With KASAN instrumentation enabled, this results in a read from an address which is annotated as out of bounds, resulting in the splat.

(Note that the KASAN response is triggered inside the KASAN machinery itself, which records stack traces for memory allocation and free actions. While recording such a stack trace, the out of bounds access triggered the response above, resulting in yet another walk of the call stack, but this time KASAN was no longer mediating the memory accesses. The same stack frame is misidentified a second time, which is why the trace above contains 'Exception stack(0xdf01f860 to 0xdf01f8a8)' which is not really an exception stack at all.)

So the correct thing to do here is to disregard .entry.text, and only take true exception frames into account. So we need to use __in_irqentry_text() instead, in two places: in save_trace(), which KASAN uses to record the stack traces, and in dump_backtrace_entry(), which prints the exception stack to the kernel log like above.

Fixes c608906165355089 ("ARM: probes: avoid adding kprobes to sensitive ...")
---
 arch/arm/kernel/stacktrace.c | 2 +-
 arch/arm/kernel/traps.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/kernel/stacktrace.c +1 -1
index 76ea4178a55c..56a7abdc1b96 100644
@@ -112,7 +112,7 @@ static int save_trace(struct stackframe *frame, void *d)
 	if (trace->nr_entries >= trace->max_entries)
 		return 1;
 
-	if (!in_entry_text(frame->pc))
+	if (!__in_irqentry_text(frame->pc))
 		return 0;
 
 	regs = (struct pt_regs *)frame->sp;
arch/arm/kernel/traps.c +1 -1
index 17d5a785df28..30628daa80b4 100644
@@ -75,7 +75,7 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
 		loglvl, where, from);
 #endif
 
-	if (in_entry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
+	if (__in_irqentry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
 		dump_mem(loglvl, "Exception stack", frame + 4, end);
 }
 
Signature · 7 lines
2.17.1


_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
#1 lore raw
Quoted 96 lines · Ard Biesheuvel

Commit c608906165355089 ("ARM: probes: avoid adding kprobes to sensitive kernel-entry/exit code") reorganized the section layout to prevent entry code from being instrumented by kprobes.

This resulted in stack frames referring to back to entry code to be misidentified as exception frames, resulting in splats like the below when KASAN is enabled:

==================================================================

  BUG: KASAN: stack-out-of-bounds in save_trace+0xc1/0xf8
  Read of size 4 at addr df01f89c by task bash/3421

  CPU: 12 PID: 3421 Comm: bash Not tainted 5.10.0-rc1-kasan+ #219
  Hardware name: QEMU KVM Virtual Machine, BIOS 0.0.0 02/06/2015
  [<c0411765>] (unwind_backtrace) from [<c040ba8b>] (show_stack+0xb/0xc)
  [<c040ba8b>] (show_stack) from [<c121c5e5>] (dump_stack+0x8d/0xa0)
  [<c121c5e5>] (dump_stack) from [<c05bd87b>] (print_address_description.constprop.3+0x2b/0x368)
  [<c05bd87b>] (print_address_description.constprop.3) from [<c05bdd2d>] (kasan_report+0xfd/0x114)
  [<c05bdd2d>] (kasan_report) from [<c040b629>] (save_trace+0xc1/0xf8)
  [<c040b629>] (save_trace) from [<c040b563>] (walk_stackframe+0x1b/0x20)
  [<c040b563>] (walk_stackframe) from [<c040b759>] (__save_stack_trace+0xf9/0x100)
  [<c040b759>] (__save_stack_trace) from [<c04cb169>] (stack_trace_save+0x71/0x88)
  [<c04cb169>] (stack_trace_save) from [<c05bcd9d>] (kasan_save_stack+0x11/0x28)
  [<c05bcd9d>] (kasan_save_stack) from [<c05bcdd1>] (kasan_set_track+0x1d/0x20)
  [<c05bcdd1>] (kasan_set_track) from [<c05be421>] (kasan_set_free_info+0x19/0x20)
  [<c05be421>] (kasan_set_free_info) from [<c05bcd67>] (__kasan_slab_free+0xa7/0xcc)
  [<c05bcd67>] (__kasan_slab_free) from [<c05bbe41>] (kmem_cache_free+0x59/0x21c)
  [<c05bbe41>] (kmem_cache_free) from [<c04c1fdb>] (rcu_core+0x2d7/0x988)
  [<c04c1fdb>] (rcu_core) from [<c04014ab>] (__do_softirq+0x133/0x41c)
  [<c04014ab>] (__do_softirq) from [<c044d4c9>] (irq_exit+0xb5/0xcc)
  [<c044d4c9>] (irq_exit) from [<c04a8503>] (__handle_domain_irq+0x5f/0xa8)
  [<c04a8503>] (__handle_domain_irq) from [<c08d5cb5>] (gic_handle_irq+0x3d/0x8c)
  [<c08d5cb5>] (gic_handle_irq) from [<c0400c11>] (__irq_svc+0x51/0x7c)
  Exception stack(0xdf01f7f8 to 0xdf01f840)
  f7e0:                                                       df01f8e0 df01f8a0
  f800: 00000008 df01fc7c 000000ad df01f960 df01f8e0 00000008 bae03f10 df01fc58
  f820: df01f8a0 0000000a c040b660 df01f848 c04114eb c04110b8 20000033 ffffffff
  [<c0400c11>] (__irq_svc) from [<c04110b8>] (unwind_pop_register+0x0/0x58)
  [<c04110b8>] (unwind_pop_register) from [<c0400161>] (ret_fast_syscall+0x1/0x5a)
  Exception stack(0xdf01f860 to 0xdf01f8a8)
  f860: 41b58ab3 c1f939e0 c04111ec 00000000 d129e800 c0411311 df01fa44 df01fa4c
  f880: 41b58ab3 c1f939e0 c04111ec 00000001 00000003 c040b569 df01f8c0 df01c000
  f8a0: df01fc7c df01ffa8
  ...

addr df01f89c is located in stack of task bash/3421 at offset 28 in frame:

   unwind_frame+0x0/0x578

Here, the last entry represents a call to unwind_pop_register frame() with the return address set to ret_fast_syscall, and since in_entry_text() returns true for that return address, save_trace() treats it as an exception frame, and attempts to dereference the struct pt_regs pointer to access the ARM_pc value. With KASAN instrumentation enabled, this results in a read from an address which is annotated as out of bounds, resulting in the splat.

(Note that the KASAN response is triggered inside the KASAN machinery itself, which records stack traces for memory allocation and free actions. While recording such a stack trace, the out of bounds access triggered the response above, resulting in yet another walk of the call stack, but this time KASAN was no longer mediating the memory accesses. The same stack frame is misidentified a second time, which is why the trace above contains 'Exception stack(0xdf01f860 to 0xdf01f8a8)' which is not really an exception stack at all.)

So the correct thing to do here is to disregard .entry.text, and only take true exception frames into account. So we need to use __in_irqentry_text() instead, in two places: in save_trace(), which KASAN uses to record the stack traces, and in dump_backtrace_entry(), which prints the exception stack to the kernel log like above.

Fixes c608906165355089 ("ARM: probes: avoid adding kprobes to sensitive ...")
---
 arch/arm/kernel/stacktrace.c | 2 +-
 arch/arm/kernel/traps.c      | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
arch/arm/kernel/stacktrace.c +1 -1
index 76ea4178a55c..56a7abdc1b96 100644
@@ -112,7 +112,7 @@ static int save_trace(struct stackframe *frame, void *d)
        if (trace->nr_entries >= trace->max_entries)
                return 1;
 
-       if (!in_entry_text(frame->pc))
+       if (!__in_irqentry_text(frame->pc))
                return 0;
 
        regs = (struct pt_regs *)frame->sp;
arch/arm/kernel/traps.c +0 -0
index 17d5a785df28..30628daa80b4 100644

arch/arm/kernel/traps.c void dump_backtrace_entry(unsigned long where, unsigned long from, · line 81

Quoted 8 lines
arch/arm/kernel/traps.c +1 -1
@@ -75,7 +75,7 @@ void dump_backtrace_entry(unsigned long where, unsigned long from,
                loglvl, where, from);
 #endif
 
-       if (in_entry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
+       if (__in_irqentry_text(from) && end <= ALIGN(frame, THREAD_SIZE))
                dump_mem(loglvl, "Exception stack", frame + 4, end);
 }
 

This is not quite correct - identifying the exception frame should be based on 'where' not 'from' when relying on .irqentry.text annotations but we don't emit those unless CONFIG_FUNCTION_GRAPH_TRACER is enabled.

I don't understand this well enough to disentangle it, and other KASAN-enabled architectures simply disable instrumentation for the stack traversal code, so I propose we do the same for ARM

I.e.,

arch/arm/kernel/Makefile +3 -0
index 6a44ffd9c7b4..dad77b0bb734 100644
@@ -21,6 +21,9 @@ obj-y         := elf.o entry-common.o irq.o opcodes.o \
                   setup.o signal.o sigreturn_codes.o \
                   stacktrace.o sys_arm.o time.o traps.o
 
+KASAN_SANITIZE_stacktrace.o := n
+KASAN_SANITIZE_traps.o := n
+
 ifneq ($(CONFIG_ARM_UNWIND),y)
 obj-$(CONFIG_FRAME_POINTER)    += return_address.o
 endif

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Reviewed-by
#2 lore raw
Quoted 16 lines · Ard Biesheuvel

This is not quite correct - identifying the exception frame should be based on 'where' not 'from' when relying on .irqentry.text annotations but we don't emit those unless CONFIG_FUNCTION_GRAPH_TRACER is enabled.

I don't understand this well enough to disentangle it, and other KASAN-enabled architectures simply disable instrumentation for the stack traversal code, so I propose we do the same for ARM

I.e.,

arch/arm/kernel/Makefile +0 -0
index 6a44ffd9c7b4..dad77b0bb734 100644

arch/arm/kernel/Makefile obj-y := elf.o entry-common.o irq.o opcodes.o \ · line 25

Quoted 5 lines
arch/arm/kernel/Makefile +2 -0
@@ -21,6 +21,9 @@ obj-y         := elf.o entry-common.o irq.o opcodes.o \
                   setup.o signal.o sigreturn_codes.o \
                   stacktrace.o sys_arm.o time.o traps.o
 
+KASAN_SANITIZE_stacktrace.o := n
+KASAN_SANITIZE_traps.o := n

for this approach. Just cook this into a proper patch and then put it in Russell's patch tracker I'd say.

If people really want to probe around this code then they can invest the time to analyze the code paths.

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel