linux-toradex.git/arch/riscv/kernel/stacktrace.c, branch v6.10

riscv: stacktrace: fix usage of ftrace_graph_ret_addr()

2024-07-03T20:10:03+00:00

ftrace_graph_ret_addr() takes an `idx` integer pointer that is used to
optimize the stack unwinding. Pass it a valid pointer to utilize the
optimizations that might be available in the future.

The commit is making riscv's usage of ftrace_graph_ret_addr() match
x86_64.

Signed-off-by: Puranjay Mohan 
Reviewed-by: Steven Rostedt (Google) 
Link: https://lore.kernel.org/r/20240618145820.62112-1-puranjay@kernel.org
Signed-off-by: Palmer Dabbelt

riscv: stacktrace: convert arch_stack_walk() to noinstr

2024-06-26T14:37:59+00:00

arch_stack_walk() is called intensively in function_graph when the
kernel is compiled with CONFIG_TRACE_IRQFLAGS. As a result, the kernel
logs a lot of arch_stack_walk and its sub-functions into the ftrace
buffer. However, these functions should not appear on the trace log
because they are part of the ftrace itself. This patch references what
arm64 does for the smae function. So it further prevent the re-enter
kprobe issue, which is also possible on riscv.

Related-to: commit 0fbcd8abf337 ("arm64: Prohibit instrumentation on arch_stack_walk()")
Fixes: 680341382da5 ("riscv: add CALLER_ADDRx support")
Signed-off-by: Andy Chiu 
Reviewed-by: Alexandre Ghiti 
Link: https://lore.kernel.org/r/20240613-dev-andyc-dyn-ftrace-v4-v1-1-1a538e12c01e@sifive.com
Signed-off-by: Palmer Dabbelt

riscv: stacktrace: fixed walk_stackframe()

2024-05-22T23:12:49+00:00

If the load access fault occures in a leaf function (with
CONFIG_FRAME_POINTER=y), when wrong stack trace will be displayed:

[] regmap_mmio_read32le+0xe/0x1c
---[ end trace 0000000000000000 ]---

Registers dump:
    ra     0xffffffff80485758 
    sp     0xffffffc80200b9a0
    fp     0xffffffc80200b9b0
    pc     0xffffffff804853ba 

Stack dump:
    0xffffffc80200b9a0:  0xffffffc80200b9e0  0xffffffc80200b9e0
    0xffffffc80200b9b0:  0xffffffff8116d7e8  0x0000000000000100
    0xffffffc80200b9c0:  0xffffffd8055b9400  0xffffffd8055b9400
    0xffffffc80200b9d0:  0xffffffc80200b9f0  0xffffffff8047c526
    0xffffffc80200b9e0:  0xffffffc80200ba30  0xffffffff8047fe9a

The assembler dump of the function preambula:
    add     sp,sp,-16
    sd      s0,8(sp)
    add     s0,sp,16

In the fist stack frame, where ra is not stored on the stack we can
observe:

        0(sp)                  8(sp)
        .---------------------------------------------.
    sp->|       frame->fp      | frame->ra (saved fp) |
        |---------------------------------------------|
    fp->|         ....         |         ....         |
        |---------------------------------------------|
        |                      |                      |

and in the code check is performed:
	if (regs && (regs->epc == pc) && (frame->fp & 0x7))

I see no reason to check frame->fp value at all, because it is can be
uninitialized value on the stack. A better way is to check frame->ra to
be an address on the stack. After the stacktrace shows as expect:

[] regmap_mmio_read32le+0xe/0x1c
[] regmap_mmio_read+0x24/0x52
[] _regmap_bus_reg_read+0x1a/0x22
[] _regmap_read+0x5c/0xea
[] _regmap_update_bits+0x76/0xc0
...
---[ end trace 0000000000000000 ]---
As pointed by Samuel Holland it is incorrect to remove check of the stackframe
entirely.

Changes since v2 [2]:
 - Add accidentally forgotten curly brace

Changes since v1 [1]:
 - Instead of just dropping frame->fp check, replace it with validation of
   frame->ra, which should be a stack address.
 - Move frame pointer validation into the separate function.

[1] https://lore.kernel.org/linux-riscv/20240426072701.6463-1-dev.mbstr@gmail.com/
[2] https://lore.kernel.org/linux-riscv/20240521131314.48895-1-dev.mbstr@gmail.com/

Fixes: f766f77a74f5 ("riscv/stacktrace: Fix stack output without ra on the stack top")
Signed-off-by: Matthew Bystrin 
Reviewed-by: Samuel Holland 
Link: https://lore.kernel.org/r/20240521191727.62012-1-dev.mbstr@gmail.com
Signed-off-by: Palmer Dabbelt

riscv: Use READ_ONCE_NOCHECK in imprecise unwinding stack mode

2023-03-09T22:50:35+00:00

When CONFIG_FRAME_POINTER is unset, the stack unwinding function
walk_stackframe randomly reads the stack and then, when KASAN is enabled,
it can lead to the following backtrace:

[    0.000000] ==================================================================
[    0.000000] BUG: KASAN: stack-out-of-bounds in walk_stackframe+0xa6/0x11a
[    0.000000] Read of size 8 at addr ffffffff81807c40 by task swapper/0
[    0.000000]
[    0.000000] CPU: 0 PID: 0 Comm: swapper Not tainted 6.2.0-12919-g24203e6db61f #43
[    0.000000] Hardware name: riscv-virtio,qemu (DT)
[    0.000000] Call Trace:
[    0.000000] [] walk_stackframe+0x0/0x11a
[    0.000000] [] init_param_lock+0x26/0x2a
[    0.000000] [] walk_stackframe+0xa2/0x11a
[    0.000000] [] dump_stack_lvl+0x22/0x36
[    0.000000] [] print_report+0x198/0x4a8
[    0.000000] [] init_param_lock+0x26/0x2a
[    0.000000] [] walk_stackframe+0xa2/0x11a
[    0.000000] [] kasan_report+0x9a/0xc8
[    0.000000] [] walk_stackframe+0xa2/0x11a
[    0.000000] [] walk_stackframe+0xa2/0x11a
[    0.000000] [] desc_make_final+0x80/0x84
[    0.000000] [] stack_trace_save+0x88/0xa6
[    0.000000] [] filter_irq_stacks+0x72/0x76
[    0.000000] [] devkmsg_read+0x32a/0x32e
[    0.000000] [] kasan_save_stack+0x28/0x52
[    0.000000] [] desc_make_final+0x7c/0x84
[    0.000000] [] stack_trace_save+0x84/0xa6
[    0.000000] [] kasan_set_track+0x12/0x20
[    0.000000] [] __kasan_slab_alloc+0x58/0x5e
[    0.000000] [] __kmem_cache_create+0x21e/0x39a
[    0.000000] [] create_boot_cache+0x70/0x9c
[    0.000000] [] kmem_cache_init+0x6c/0x11e
[    0.000000] [] mm_init+0xd8/0xfe
[    0.000000] [] start_kernel+0x190/0x3ca
[    0.000000]
[    0.000000] The buggy address belongs to stack of task swapper/0
[    0.000000]  and is located at offset 0 in frame:
[    0.000000]  stack_trace_save+0x0/0xa6
[    0.000000]
[    0.000000] This frame has 1 object:
[    0.000000]  [32, 56) 'c'
[    0.000000]
[    0.000000] The buggy address belongs to the physical page:
[    0.000000] page:(____ptrval____) refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x81a07
[    0.000000] flags: 0x1000(reserved|zone=0)
[    0.000000] raw: 0000000000001000 ff600003f1e3d150 ff600003f1e3d150 0000000000000000
[    0.000000] raw: 0000000000000000 0000000000000000 00000001ffffffff
[    0.000000] page dumped because: kasan: bad access detected
[    0.000000]
[    0.000000] Memory state around the buggy address:
[    0.000000]  ffffffff81807b00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    0.000000]  ffffffff81807b80: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    0.000000] >ffffffff81807c00: 00 00 00 00 00 00 00 00 f1 f1 f1 f1 00 00 00 f3
[    0.000000]                                            ^
[    0.000000]  ffffffff81807c80: f3 f3 f3 f3 00 00 00 00 00 00 00 00 00 00 00 00
[    0.000000]  ffffffff81807d00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[    0.000000] ==================================================================

Fix that by using READ_ONCE_NOCHECK when reading the stack in imprecise
mode.

Fixes: 5d8544e2d007 ("RISC-V: Generic library routines and assembly")
Reported-by: Chathura Rajapaksha 
Link: https://lore.kernel.org/all/CAD7mqryDQCYyJ1gAmtMm8SASMWAQ4i103ptTb0f6Oda=tPY2=A@mail.gmail.com/
Suggested-by: Dmitry Vyukov 
Signed-off-by: Alexandre Ghiti 
Link: https://lore.kernel.org/r/20230308091639.602024-1-alexghiti@rivosinc.com
Signed-off-by: Palmer Dabbelt

riscv: stacktrace: Fix missing the first frame

2023-02-03T03:33:05+00:00

When running kfence_test, I found some testcases failed like this:

 # test_out_of_bounds_read: EXPECTATION FAILED at mm/kfence/kfence_test.c:346
 Expected report_matches(&expect) to be true, but is false
 not ok 1 - test_out_of_bounds_read

The corresponding call-trace is:

 BUG: KFENCE: out-of-bounds read in kunit_try_run_case+0x38/0x84

 Out-of-bounds read at 0x(____ptrval____) (32B right of kfence-#10):
  kunit_try_run_case+0x38/0x84
  kunit_generic_run_threadfn_adapter+0x12/0x1e
  kthread+0xc8/0xde
  ret_from_exception+0x0/0xc

The kfence_test using the first frame of call trace to check whether the
testcase is succeed or not. Commit 6a00ef449370 ("riscv: eliminate
unreliable __builtin_frame_address(1)") skip first frame for all
case, which results the kfence_test failed. Indeed, we only need to skip
the first frame for case (task==NULL || task==current).

With this patch, the call-trace will be:

 BUG: KFENCE: out-of-bounds read in test_out_of_bounds_read+0x88/0x19e

 Out-of-bounds read at 0x(____ptrval____) (1B left of kfence-#7):
  test_out_of_bounds_read+0x88/0x19e
  kunit_try_run_case+0x38/0x84
  kunit_generic_run_threadfn_adapter+0x12/0x1e
  kthread+0xc8/0xde
  ret_from_exception+0x0/0xc

Fixes: 6a00ef449370 ("riscv: eliminate unreliable __builtin_frame_address(1)")
Signed-off-by: Liu Shixin 
Tested-by: Samuel Holland 
Link: https://lore.kernel.org/r/20221207025038.1022045-1-liushixin2@huawei.com
Signed-off-by: Palmer Dabbelt

riscv: stacktrace: Make walk_stackframe cross pt_regs frame

2022-12-06T02:13:34+00:00

The current walk_stackframe with FRAME_POINTER would stop unwinding at
ret_from_exception:
  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [] walk_stackframe+0x0/0xee
  [] show_stack+0x32/0x4a
  [] dump_stack_lvl+0x72/0x8e
  [] dump_stack+0x14/0x1c
  [] ___might_sleep+0x12e/0x138
  [] __might_sleep+0x10/0x18
  [] down_read+0x22/0xa4
  [] do_page_fault+0xb0/0x2fe
  [] ret_from_exception+0x0/0xc

The optimization would help walk_stackframe cross the pt_regs frame and
get more backtrace of debug info:
  BUG: sleeping function called from invalid context at kernel/locking/rwsem.c:1518
  in_atomic(): 0, irqs_disabled(): 1, non_block: 0, pid: 1, name: init
  CPU: 0 PID: 1 Comm: init Not tainted 5.10.113-00021-g15c15974895c-dirty #192
  Call Trace:
  [] walk_stackframe+0x0/0xee
  [] show_stack+0x32/0x4a
  [] dump_stack_lvl+0x72/0x8e
  [] dump_stack+0x14/0x1c
  [] ___might_sleep+0x12e/0x138
  [] __might_sleep+0x10/0x18
  [] down_read+0x22/0xa4
  [] do_page_fault+0xb0/0x2fe
  [] ret_from_exception+0x0/0xc
  [] riscv_intc_irq+0x1a/0x72
  [] ret_from_exception+0x0/0xc
  [] vma_link+0x54/0x160
  [] mmap_region+0x2cc/0x4d0
  [] do_mmap+0x2d8/0x3ac
  [] vm_mmap_pgoff+0x70/0xb8
  [] vm_mmap+0x2a/0x36
  [] elf_map+0x72/0x84
  [] load_elf_binary+0x69a/0xec8
  [] bprm_execve+0x246/0x53a
  [] kernel_execve+0xe8/0x124
  [] run_init_process+0xfa/0x10c
  [] try_to_run_init_process+0x12/0x3c
  [] kernel_init+0xb4/0xf8
  [] ret_from_exception+0x0/0xc

Here is the error injection test code for the above output:
 drivers/irqchip/irq-riscv-intc.c:
 static asmlinkage void riscv_intc_irq(struct pt_regs *regs)
 {
        unsigned long cause = regs->cause & ~CAUSE_IRQ_FLAG;
+       u32 tmp; __get_user(tmp, (u32 *)0);

Signed-off-by: Guo Ren 
Signed-off-by: Guo Ren 
Link: https://lore.kernel.org/r/20221109064937.3643993-3-guoren@kernel.org
[Palmer: use SYM_CODE_*]
Signed-off-by: Palmer Dabbelt

riscv: stacktrace: Fixup ftrace_graph_ret_addr retp argument

2022-12-06T00:58:01+00:00

The 'retp' is a pointer to the return address on the stack, so we
must pass the current return address pointer as the 'retp'
argument to ftrace_push_return_trace(). Not parent function's
return address on the stack.

Fixes: b785ec129bd9 ("riscv/ftrace: Add HAVE_FUNCTION_GRAPH_RET_ADDR_PTR support")
Signed-off-by: Guo Ren 
Signed-off-by: Guo Ren 
Link: https://lore.kernel.org/r/20221109064937.3643993-2-guoren@kernel.org
Cc: stable@vger.kernel.org
Signed-off-by: Palmer Dabbelt

riscv: Rename "sp_in_global" to "current_stack_pointer"

2022-03-30T22:15:27+00:00

To follow the existing per-arch conventions, rename "sp_in_global" to
"current_stack_pointer". This will let it be used in non-arch places
(like HARDENED_USERCOPY).

Signed-off-by: Kees Cook 
Signed-off-by: Palmer Dabbelt

riscv: eliminate unreliable __builtin_frame_address(1)

2022-02-04T18:12:32+00:00

I tried different pieces of code which uses __builtin_frame_address(1)
(with both gcc version 7.5.0 and 10.3.0) to verify whether it works as
expected on riscv64. The result is negative.

What the compiler had generated is as below:
31                      fp = (unsigned long)__builtin_frame_address(1);
   0xffffffff80006024 <+200>:   ld      s1,0(s0)

It takes '0(s0)' as the address of frame 1 (caller), but the actual address
should be '-16(s0)'.

          |       ...       | <-+
          +-----------------+   |
          | return address  |   |
          | previous fp     |   |
          | saved registers |   |
          | local variables |   |
  $fp --> |       ...       |   |
          +-----------------+   |
          | return address  |   |
          | previous fp --------+
          | saved registers |
  $sp --> | local variables |
          +-----------------+

This leads the kernel can not dump the full stack trace on riscv.

[    7.222126][    T1] Call Trace:
[    7.222804][    T1] [] dump_backtrace+0x2c/0x3a

This problem is not exposed on most riscv builds just because the '0(s0)'
occasionally is the address frame 2 (caller's caller), if only ra and fp
are stored in frame 1 (caller).

          |       ...       | <-+
          +-----------------+   |
          | return address  |   |
  $fp --> | previous fp     |   |
          +-----------------+   |
          | return address  |   |
          | previous fp --------+
          | saved registers |
  $sp --> | local variables |
          +-----------------+

This could be a *bug* of gcc that should be fixed. But as noted in gcc
manual "Calling this function with a nonzero argument can have
unpredictable effects, including crashing the calling program.", let's
remove the '__builtin_frame_address(1)' in backtrace code.

With this fix now it can show full stack trace:
[   10.444838][    T1] Call Trace:
[   10.446199][    T1] [] dump_backtrace+0x2c/0x3a
[   10.447711][    T1] [] show_stack+0x32/0x3e
[   10.448710][    T1] [] dump_stack_lvl+0x58/0x7a
[   10.449941][    T1] [] dump_stack+0x14/0x1c
[   10.450929][    T1] [] ubsan_epilogue+0x10/0x5a
[   10.451869][    T1] [] __ubsan_handle_load_invalid_value+0x6c/0x78
[   10.453049][    T1] [] __pagevec_release+0x62/0x64
[   10.455476][    T1] [] truncate_inode_pages_range+0x132/0x5be
[   10.456798][    T1] [] truncate_inode_pages+0x24/0x30
[   10.457853][    T1] [] kill_bdev+0x32/0x3c
...

Signed-off-by: Changbin Du 
Fixes: eac2f3059e02 ("riscv: stacktrace: fix the riscv stacktrace when CONFIG_FRAME_POINTER enabled")
Cc: stable@vger.kernel.org
Signed-off-by: Palmer Dabbelt

arch: Make ARCH_STACKWALK independent of STACKTRACE

2021-12-10T14:06:03+00:00

Make arch_stack_walk() available for ARCH_STACKWALK architectures
without it being entangled in STACKTRACE.

Link: https://lore.kernel.org/lkml/20211022152104.356586621@infradead.org/
Signed-off-by: Peter Zijlstra (Intel) 
[Mark: rebase, drop unnecessary arm change]
Signed-off-by: Mark Rutland 
Cc: Albert Ou 
Cc: Borislav Petkov 
Cc: Christian Borntraeger 
Cc: Dave Hansen 
Cc: Heiko Carstens 
Cc: Ingo Molnar 
Cc: Michael Ellerman 
Cc: Palmer Dabbelt 
Cc: Paul Walmsley 
Cc: Thomas Gleixner 
Cc: Vasily Gorbik 
Link: https://lore.kernel.org/r/20211129142849.3056714-2-mark.rutland@arm.com
Signed-off-by: Catalin Marinas