Re: [PATCH] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE

From: Mark Rutland <mark.rutland@arm.com>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: Mark Brown <broonie@kernel.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] arm64: asmlinkage: Enable use of BTI_C macro in SYM_CODE
Date: Mon, 6 Dec 2021 17:11:20 +0000	[thread overview]
Message-ID: <Ya5EOLruTzBhjQ3P@FVFF77S0Q05N> (raw)
In-Reply-To: <CAMj1kXFXPQWMoAw3NbfBXfKyZsKM=Qpv1vkDNbCkBY6G9LMiuA@mail.gmail.com>

On Mon, Dec 06, 2021 at 05:52:16PM +0100, Ard Biesheuvel wrote:
> On Mon, 6 Dec 2021 at 17:30, Mark Rutland <mark.rutland@arm.com> wrote:
> >
> > On Fri, Dec 03, 2021 at 01:03:35PM +0000, Mark Brown wrote:
> > > A couple of SYM_CODE sections have added usage of BTI_C which is
> > > currently only defined when building for BTI.  This means that the
> > > users have ugly ifdefs for the case where BTI is disabled so let's
> > > provide an empty definition in that case and remove the ifdefs.
> > >
> > > Signed-off-by: Mark Brown <broonie@kernel.org>
> > > ---
> > >  arch/arm64/include/asm/linkage.h | 4 ++++
> > >  arch/arm64/kernel/entry-ftrace.S | 4 ----
> > >  arch/arm64/lib/kasan_sw_tags.S   | 2 --
> > >  3 files changed, 4 insertions(+), 6 deletions(-)
> >
> > Looking around, there are other places that open-code `hint 34`, e.g.
> > arch/arm64/crypto/aes-modes.S. Those are unconditional, so we should probably
> > figure out whether we want those to be conditional (or if we're happy to make
> > the other cases similarly unconditional).
> >
> > I'd argue we should probably place BTIs in assembly unconditionally, on the
> > assumption that they shouldn't have an measureable performance impact in
> > practice (as we're already assuming that when CONFIG_ARM64_BTI_KERNEL is
> > selected anyhow). Thoughts?
> 
> From the top of my head, I can't say for sure but there are some
> computed gotos in the crypto code where removing an instruction may
> throw off the calculation. So keeping the hint unconditionally makes
> sense to me, and by the same reasoning, it would be better not to
> introduce macros that shadow existing instructions if they may resolve
> to a sequence of a different size.

As an aside, while that code works, it's using `BTI C` in an odd way, since the
those are internal labels rather than callable function entry points (but the
`BR` works because of the X16/X17 exemption that makes PLTs work).

For consistency, we might want to make that use `BTI J`, and an X register
other than X16/X17, which is what the compiler should generate for cases like
this. That would minimize the set of targets the `BR` can legitimately hit
(though AFAICT this cannot be gadgetized anyway, so this'd just be for
consistency/lack-of-surprise).

Thanks,
Mark.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel