Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Subject: Re: [PATCH] riscv: stacktrace: Fix missing the first frame
To:     Conor Dooley <conor@kernel.org>
References: <20221205132936.493245-1-liushixin2@huawei.com>
 <Y49qvHb3LVas1cvO@spud>
CC:     Paul Walmsley <paul.walmsley@sifive.com>,
        Palmer Dabbelt <palmer@dabbelt.com>,
        Albert Ou <aou@eecs.berkeley.edu>,
        Changbin Du <changbin.du@intel.com>,
        <linux-riscv@lists.infradead.org>, <linux-kernel@vger.kernel.org>
From:   Liu Shixin <liushixin2@huawei.com>
Message-ID: <e3bace03-b0eb-9e58-f39e-6ecf8589491c@huawei.com>
Date:   Wed, 7 Dec 2022 10:02:23 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.1
MIME-Version: 1.0
In-Reply-To: <Y49qvHb3LVas1cvO@spud>
Content-Type: text/plain; charset="windows-1252"
Content-Transfer-Encoding: 7bit
Precedence: bulk


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