2022-12-07 02:17:38

by Liu Shixin

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

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 <[email protected]>
---
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


2023-01-03 18:28:22

by Samuel Holland

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

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 <[email protected]>
> ---
> 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 <[email protected]>

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
...

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

Hello:

This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <[email protected]>:

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