[v2] riscv: stacktrace: Fix missing the first frame

[PATCH v2] riscv: stacktrace: Fix missing the first frame

Posted by Liu Shixin 2 years, 9 months ago

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 <liushixin2@huawei.com>
---
v1->v2: Fix the incorrect Fixes tag found by Conor.

 arch/riscv/kernel/stacktrace.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/kernel/stacktrace.c b/arch/riscv/kernel/stacktrace.c
index 08d11a53f39e..5fe2ae4cf135 100644
--- a/arch/riscv/kernel/stacktrace.c
+++ b/arch/riscv/kernel/stacktrace.c
@@ -30,6 +30,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		fp = (unsigned long)__builtin_frame_address(0);
 		sp = current_stack_pointer;
 		pc = (unsigned long)walk_stackframe;
+		level = -1;
 	} else {
 		/* task blocked in __switch_to */
 		fp = task->thread.s[0];
@@ -41,7 +42,7 @@ void notrace walk_stackframe(struct task_struct *task, struct pt_regs *regs,
 		unsigned long low, high;
 		struct stackframe *frame;
 
-		if (unlikely(!__kernel_text_address(pc) || (level++ >= 1 && !fn(arg, pc))))
+		if (unlikely(!__kernel_text_address(pc) || (level++ >= 0 && !fn(arg, pc))))
 			break;
 
 		/* Validate frame pointer */
-- 
2.25.1

Re: [PATCH v2] riscv: stacktrace: Fix missing the first frame

Posted by patchwork-bot+linux-riscv@kernel.org 2 years, 7 months ago

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:

On Wed, 7 Dec 2022 10:50:38 +0800 you wrote:
> 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:
> 
> [...]

Here is the summary with links:
  - [v2] riscv: stacktrace: Fix missing the first frame
    https://git.kernel.org/riscv/c/cb80242cc679

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v2] riscv: stacktrace: Fix missing the first frame

Posted by Samuel Holland 2 years, 8 months ago

On 12/6/22 20:50, Liu Shixin wrote:
> 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 <liushixin2@huawei.com>
> ---
> v1->v2: Fix the incorrect Fixes tag found by Conor.
> 
>  arch/riscv/kernel/stacktrace.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)

Tested-by: Samuel Holland <samuel@sholland.org>

Before:

WARNING: CPU: 0 PID: 162 at drivers/regulator/core.c:5700
regulator_unregister+0xf0/0x106
...
[<ffffffff803a0794>] devm_rdev_release+0xe/0x16
[<ffffffff80427bac>] release_nodes+0x3c/0x98
[<ffffffff80428cac>] devres_release_all+0x72/0x9e
[<ffffffff80424054>] device_unbind_cleanup+0x10/0x4a
...

After:

WARNING: CPU: 0 PID: 165 at drivers/regulator/core.c:5700
regulator_unregister+0xf0/0x106
...
[<ffffffff8039d194>] regulator_unregister+0xf0/0x106
[<ffffffff803a0798>] devm_rdev_release+0xe/0x16
[<ffffffff80427bb0>] release_nodes+0x3c/0x98
[<ffffffff80428cb0>] devres_release_all+0x72/0x9e
[<ffffffff80424058>] device_unbind_cleanup+0x10/0x4a
...