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: <Zf1UyxlDf/oCjXxr@shell.armlinux.org.uk>
Date: Fri, 22 Mar 2024 09:52:11 +0000
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: David Laight <David.Laight@...lab.com>
Cc: Ard Biesheuvel <ardb@...nel.org>,
	Jiangfeng Xiao <xiaojiangfeng@...wei.com>,
	"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 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.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!