Subject: Re: [PATCH 1/3] arm64: vdso: Don't prefix sigreturn trampoline with a BTI C instruction

From: Dave Martin <[email-masked]>
Date: Tue, 19 May 2020 14:25:38 +0100

On Tue, May 19, 2020 at 01:38:43PM +0100, Mark Brown wrote:
> On Tue, May 19, 2020 at 01:18:16PM +0100, Will Deacon wrote:
> 
> > Fixes: 714a8d02ca4d ("arm64: asm: Override SYM_FUNC_START when building the kernel with BTI")
> 
> I'd say it's the annotation conversion not this, and also that the
> bikeshed would be most fetching in orange.
> 
> c91db232da484851 (arm64: vdso: Convert to modern assembler annotations)
> 
> > -SYM_FUNC_START(__kernel_rt_sigreturn)
> > +/*
> > + * GDB relies on being able to identify the sigreturn instruction sequence to
> > + * unwind from signal handlers. We cannot, therefore, use SYM_FUNC_START()
> > + * here, as it will emit a BTI C instruction and break the unwinder. Thankfully,
> > + * this function is only ever called from a RET and so omitting the landing pad
> > + * is perfectly fine.
> > + */
> > +SYM_START(__kernel_rt_sigreturn, SYM_L_GLOBAL, SYM_A_ALIGN)
> 
> Shouldn't this be a SYM_CODE_START()?  It's the same thing as above
> currently and we'll break an awful lot more if we change what it does in
> a way that affects the code, plus the use of CODE basically says the
> above - it's a "this is non-standard and we know exactly what we're
> doing, don't mess with it" annotation.  If not then it'd be good to
> cover that in the comment since otherwise this seems like it's asking
> for a cleanup, we shouldn't really have raw SYM_START() in code.
> 
> I guess we also ought to annotate the 32 bit sigreturns as CODE too,
> though it's academic there without BTI.

Relating to this, we explicitly don't support calls to
__kernel_rt_sigreturn.

Rather, the "ret lr" that jumps here is supposed to be authenticated via
pointer auth in the caller.

If BTI {nothing} allows this while disallowing all BR/BLR then we could
use that (I can't remember what BTI {nothing} is useful for, if anything).

Otherwise, it's less clear what we should have here.

Cheers
---Dave

_______________________________________________
linux-arm-kernel mailing list
[email-masked]
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel