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