| Message ID | 20221205132936.493245-1-liushixin2@huawei.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: stacktrace: Fix missing the first frame | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/patch_count | success | Link |
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/alphanumeric_selects | success | Out of order selects before the patch: 57 and now 57 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/build_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | warning | WARNING: Unknown commit id 'a7c5c7e8ff78', maybe rebased or not pulled? |
| conchuod/source_inline | success | Was 0 now: 0 |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | fail | Problems with Fixes tag: 1 |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
On Mon, Dec 05, 2022 at 09:29:36PM +0800, 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. Patch a7c5c7e8ff78 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: a7c5c7e8ff78 ("riscv: eliminate unreliable __builtin_frame_address(1)") This fixes tag is not right, did checkpatch not warn about it? The correct fixes tag would be: Fixes: 6a00ef449370 ("riscv: eliminate unreliable __builtin_frame_address(1)") Maybe consider automating the creation of fixes tags, like so: git log -1 --format='Fixes: %h (\"%s\")' Thanks, Conor. > Signed-off-by: Liu Shixin <liushixin2@huawei.com> > --- > 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 >
On 2022/12/7 0:15, Conor Dooley wrote: > On Mon, Dec 05, 2022 at 09:29:36PM +0800, 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. Patch a7c5c7e8ff78 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: a7c5c7e8ff78 ("riscv: eliminate unreliable __builtin_frame_address(1)") > This fixes tag is not right, did checkpatch not warn about it? Yes, there are no warn. Maybe it's because I do have this commit in my local repository. I confused the original commit with my local commit. Thanks for your reminder. I'll fix it as soon as possible. > The correct fixes tag would be: > Fixes: 6a00ef449370 ("riscv: eliminate unreliable __builtin_frame_address(1)") > > Maybe consider automating the creation of fixes tags, like so: > git log -1 --format='Fixes: %h (\"%s\")' Thanks, it seems to be convenient. > > Thanks, > Conor. > >> Signed-off-by: Liu Shixin <liushixin2@huawei.com> >> --- >> 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 >>
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 */
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. Patch a7c5c7e8ff78 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: a7c5c7e8ff78 ("riscv: eliminate unreliable __builtin_frame_address(1)") Signed-off-by: Liu Shixin <liushixin2@huawei.com> --- arch/riscv/kernel/stacktrace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)