linux-kernel - Re: [PATCH v2] ARM: unwind: improve unwinders for noreturn case

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: <18f5c5fd-4a5d-0c33-9d73-7e94c1e42f3f@huawei.com>
Date: Fri, 22 Mar 2024 20:54:09 +0800
From: Jiangfeng Xiao <xiaojiangfeng@...wei.com>
To: "Russell King (Oracle)" <linux@...linux.org.uk>, David Laight
	<David.Laight@...lab.com>
CC: Ard Biesheuvel <ardb@...nel.org>, "arnd@...db.de" <arnd@...db.de>,
	"keescook@...omium.org" <keescook@...omium.org>, "haibo.li@...iatek.com"
	<haibo.li@...iatek.com>, "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>, "amergnat@...libre.com"
	<amergnat@...libre.com>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>, "douzhaolei@...wei.com"
	<douzhaolei@...wei.com>, "gustavoars@...nel.org" <gustavoars@...nel.org>,
	"jpoimboe@...nel.org" <jpoimboe@...nel.org>, "kepler.chenxin@...wei.com"
	<kepler.chenxin@...wei.com>, "kirill.shutemov@...ux.intel.com"
	<kirill.shutemov@...ux.intel.com>, "linux-hardening@...r.kernel.org"
	<linux-hardening@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-mm@...ck.org" <linux-mm@...ck.org>,
	"linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "nixiaoming@...wei.com"
	<nixiaoming@...wei.com>, "peterz@...radead.org" <peterz@...radead.org>,
	"wangbing6@...wei.com" <wangbing6@...wei.com>, "wangfangpeng1@...wei.com"
	<wangfangpeng1@...wei.com>, "jannh@...gle.com" <jannh@...gle.com>,
	"willy@...radead.org" <willy@...radead.org>
Subject: Re: [PATCH v2] ARM: unwind: improve unwinders for noreturn case



On 2024/3/22 17:52, Russell King (Oracle) wrote:
> On Fri, Mar 22, 2024 at 09:24:20AM +0000, David Laight wrote:
>> From: Russell King
>>> Sent: 22 March 2024 00:09
>>>
>>> On Thu, Mar 21, 2024 at 11:43:41PM +0100, Ard Biesheuvel wrote:
>>>> Given that this particular issue would just disappear if the compiler
>>>> would just insert a BRK after the BL, I'd prefer to explore first
>>>> whether we can get this fixed on the compiler side.
>>>
>>> Arm32 doesn't have a BRK instruction. What would be appropriate after
>>> the no-return BL would be OS specific.
>>
>> It would need to depend on what was being compiled.
> 
> Yes, but as for the rest...
> 
>> For the kernel it could be much the same as BUG().
>> (Probably without any extra data.)
>> I suspect that arm32 could use 'swi' in kernel space,
>> but you wouldn't want to use that in userspace.
>>
>> Looks like armv5 has a bkpt instruction - could that be used?
>> Or does the kernel need to support armv4?
>>
>> The last arm I wrote anything for was a strongarm.
> 
> Thank you David, but remember - I have programmed 32-bit Arm since 1992,
> and wrote the majority of the 32-bit Arm kernel support. I think I know
> what I'm walking about by now.
> 
> The compiler can't do the same as BUG() - that is a kernel specific
> construct and not an architecture one. It is an undefined instruction
> specifically chosen to be undefined on both 32-bit and 16-bit Arm ISAs.
> 
> As for your idea of using "swi" in kernel space, no that's never going
> to happen - to shoe-horn that into the SWI exception path for the sake
> of the compiler would be totally idiotic - it would cause userspace
> performance regressions for something that never happens. Moreover,
> with EABI the "comment" field in the "swi" instruction is ignored so
> all SWIs under EABI are treated the same. So no, that's not going to
> work without causing inefficiencies - again - for a case that will
> likely never happen.
> 
> Whereas we already provide an abort() function because iirc the
> compiler used to emit branches to that due to noreturn functions. If
> correct, there's previous convention for doing this - and abort() is
> still exists in the kernel and in userspace since it's part of ANSI
> C. This would be a more reliable and portable solution, but probably
> not for embedded platforms - and that's probably why it got removed.
> 
> There isn't going to be a single solution to this which satisfies
> everyone, and I don't blame the compiler people for deciding to
> basically give up with putting any instruction after a call to a
> no-return function - because there isn't an instruction defined in
> the architecture that _could_ be put there that would work everywhere.
> 


If the compiler inserts (a branch to 'abort') behind (no-return BL)
that does not apply to ARM32 embedded platforms, do you think the
 "[PATCH v3] ARM: unwind: improve unwinders for noreturn case"
submitted the day before yesterday can be used as a
complementary solution?

2) we're unwinding a frame that has been created because of a branch,
   where the PC points at the next instruction _after_ that callsite.

When we hit the second type of frame, "pc-1" is closer to callsite,
and no problem is introduced. In addition, the issue of the 'noreturn'
can be solved.