Re: [PATCH v2 07/10] arm64: asm: Provide a mechanism for generating ELF note for BTI

From: Dave Martin <Dave.Martin@arm•com>
To: Will Deacon <will@kernel•org>
Cc: Catalin Marinas <catalin.marinas@arm•com>,
	Mark Brown <broonie@kernel•org>,
	Vincenzo Frascino <Vincenzo.Frascino@arm•com>,
	Kees Cook <keescook@chromium•org>,
	linux-arm-kernel@lists•infradead.org
Subject: Re: [PATCH v2 07/10] arm64: asm: Provide a mechanism for generating ELF note for BTI
Date: Tue, 5 May 2020 17:51:28 +0100	[thread overview]
Message-ID: <20200505165128.GR30377@arm.com> (raw)
In-Reply-To: <20200505145858.GB24239@willie-the-truck>

On Tue, May 05, 2020 at 03:58:59PM +0100, Will Deacon wrote:
> [+Dave]
> 
> On Wed, Apr 29, 2020 at 10:16:38PM +0100, Mark Brown wrote:
> > ELF files built for BTI should have a program property note section which
> > identifies them as such. The linker expects to find this note in all
> > object files it is linking into a BTI annotated output, the compiler will
> > ensure that this happens for C files but for assembler files we need to do
> > this in the source so provide a macro which can be used for this purpose.
> > 
> > This is mainly for use in the VDSO which should be a normal ELF shared
> > library and should therefore include BTI annotations when built for BTI.
> > 
> > Signed-off-by: Mark Brown <broonie@kernel•org>
> > ---
> >  arch/arm64/include/asm/assembler.h | 41 ++++++++++++++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> > 
> > diff --git a/arch/arm64/include/asm/assembler.h b/arch/arm64/include/asm/assembler.h
> > index 0bff325117b4..85a88df2d0fe 100644
> > --- a/arch/arm64/include/asm/assembler.h
> > +++ b/arch/arm64/include/asm/assembler.h
> > @@ -736,4 +736,45 @@ USER(\label, ic	ivau, \tmp2)			// invalidate I line PoU
> >  .Lyield_out_\@ :
> >  	.endm
> >  
> > +/*
> > + * This macro emits a program property note section identifying
> > + * architecture features which require special handling, mainly for
> > + * use in assembly files included in the VDSO.
> > + */
> > +
> > +#ifdef CONFIG_ARM64_BTI_KERNEL
> > +
> > +#define NT_GNU_PROPERTY_TYPE_0  5
> > +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND      0xc0000000
> > +
> > +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI      (1U << 0)
> > +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC      (1U << 1)
> > +
> > +.macro emit_aarch64_feature_1_and
> 
> Might be useful to take the features as a macro argument, so we can
> re-use this when extra features get added in the future.

Probably a good idea, though I hope this doesn't crop up too often...

> > +	.pushsection .note.gnu.property, "a"
> > +	.align  3
> > +	.long   2f - 1f
> > +	.long   6f - 3f
> > +	.long   NT_GNU_PROPERTY_TYPE_0
> > +1:      .string "GNU"
> > +2:
> > +	.align  3
> > +3:      .long   GNU_PROPERTY_AARCH64_FEATURE_1_AND
> > +	.long   5f - 4f
> > +4:
> > +	.long   GNU_PROPERTY_AARCH64_FEATURE_1_PAC | \
> > +		GNU_PROPERTY_AARCH64_FEATURE_1_BTI
> 
> Hmm. The Linux ABI doc [1] says this field is:
> 
> 	unsigned char pr_data[PR_DATASZ];
> 
> but the AArch64 PCS [2] says:
> 
> 	"It has a single 32-bit value for the pr_data field."
> 
> What does this mean for endianness?

I think this means it's poorly specified.

The spirit of this is that each property is a container for a random ELF
structure, whose elements are encoded with endianness matching that of 
the ELF file.

Because these structures are variably sized, they can't be described
properly using a C type.  The pseudo-C in [1] is illustrative but a bit
of a bodge IMHO.  Attempting to do things this way for real would also
get you into trouble with strict-aliasing IIUC.

This ship has already sailed: someone should check what comes out of
cc -mbig-endian -mbranch-protection=standard.  (Extra points scored if
you can persuade gcc and clang to do something different!)

Cheers
---Dave

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