linux-kernel - Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87eac880-d1c0-272c-8a9e-98cb4ac8541e@loongson.cn>
Date: Sun, 14 Dec 2025 21:15:29 +0800
From: Tiezhu Yang <yangtiezhu@...ngson.cn>
To: Huacai Chen <chenhuacai@...nel.org>
Cc: Bibo Mao <maobibo@...ngson.cn>, loongarch@...ts.linux.dev,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1] LoongArch: Remove unnecessary checks in bt_address()

On 12/13/25 20:24, Huacai Chen wrote:
> On Wed, Dec 10, 2025 at 4:00 PM Tiezhu Yang <yangtiezhu@...ngson.cn> wrote:
>>
>> On 2025/12/10 上午10:25, Bibo Mao wrote:
>>>
>>>
>>> On 2025/12/10 上午9:28, Tiezhu Yang wrote:
>>>> On 2025/12/9 下午4:30, Huacai Chen wrote:
>>>>> Hi, Tiezhu,
>>>>>
>>>>> On Tue, Dec 9, 2025 at 2:18 PM Tiezhu Yang <yangtiezhu@...ngson.cn>
>>>>> wrote:
>>>>>>
>>>> ...
>>>>>>           extern unsigned long eentry;
>>>>>>
>>>>>> -       if (__kernel_text_address(ra))
>>>>>> -               return ra;
>>>>>> -
>>>>>> -       if (__module_text_address(ra))
>>>>>> -               return ra;
>>>>> I think the correct way is to remove the __module_text_address()
>>>>> condition but keep the __kernel_text_address() condition. Then return
>>>>> 0 at the end of this function, and remove the __kernel_text_address()
>>>>> condition out of this function.
>>>>
>>>> It can not remove the check of __kernel_text_address() after calling
>>>> bt_address() because it needs to validate the calculated address for
>>>> exception, then no need to keep the __kernel_text_address() condition
>>>> in bt_address() because it will check the PC outside bt_address().
>>>           state->pc = bt_address(pc);
>>>           if (!state->pc) {
>>>                   pr_err("cannot find unwind pc at %p\n", (void *)pc);
>>>                   goto err;
>>>           }
>>>
>>>           if (!__kernel_text_address(state->pc))
>>>                   goto err;
>>> I guess that you both comes from different views :) one treats these
>>> piece of code into one, one only views function bt_address().
>>>
>>> Especially with if (!state->pc), how could this happen?
>>
>> IMO, state->pc will be not 0 forever in practice, this check is just an
>> error path and can be removed too if possible.
> bt_address() return ra both in "good path" and "bad path" is strange.
> I still suggest using my method, but move __kernel_text_address()
> after the "if (ra >= eentry && ra < eentry +  EXCCODE_INT_END *
> VECSIZE)" block to verify the modified ra.

I am OK if you mean like this:

static inline unsigned long bt_address(unsigned long ra)
{
	extern unsigned long eentry;

	if (ra >= eentry && ra < eentry +  EXCCODE_INT_END * VECSIZE) {
		...
		ra = func + offset;
	}

	if (__kernel_text_address(ra))
		return ra;

	return 0;
}

bool unwind_next_frame(struct unwind_state *state)
{
	...
	state->pc = bt_address(pc);
	if (!state->pc) {
		pr_err("cannot find unwind pc at %p\n", (void *)pc);
		goto err;
	}

	return true;
	...
}

If so, I will send v2 later.

Thanks,
Tiezhu