Re: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code

[Adhemerval Zanella <adhemerval.zanella@linaro.org>]

On 11/05/2020 08:38, Szabolcs Nagy wrote:
> The 05/07/2020 13:55, Adhemerval Zanella via Libc-alpha wrote:
>> On 30/04/2020 14:39, Szabolcs Nagy wrote:
>>> From 550fe66ed93e13c0f063955e81bfcb8db386413c Mon Sep 17 00:00:00 2001
>>> From: Sudakshina Das <sudi.das@arm.com>
>>> Date: Tue, 17 Mar 2020 15:44:18 +0000
>>> Subject: [PATCH 03/12] aarch64: Add BTI landing pads to assembly code
>>>
>>> Adding the landing pads and the ELF markings are required if
>>> glibc is built with branch protection. For the handful of asm
>>> files this is done unconditionally, this simplifies maintenance
>>> and avoids complications where code layout is carefully aligned
>>> such that conditionally turning BTI off may cause performance
>>> regression (e.g. string functions).
>>
>> I am seeing the warning the cover letter warned on old binutils:
>>
>>   unsupported GNU_PROPERTY_TYPE (5) type: 0xc0000000
>>
>> In this case, how exactly binutils handles it? Does it still emit
>> the expected correct notes, and if so, is the expected layout?
> 
> ok, v2 takes care of this (with config checks).

Ack.

> 
> (linkers that don't handle the bti property will either
> emit a note that is not valid so ignored at runtime or
> not emit any note, so the warnings don't mean broken
> behaviour just turned off security feature)
> 
>>> +/* Branch Target Identitication support.  */
>>> +#define BTI_C		hint	34
>>> +#define BTI_J		hint	36
>>> +
>>> +#define FEATURE_1_BTI 1
>>> +#define FEATURE_1_PAC 2
>>
>> There are GNU_PROPERTY_AARCH64_FEATURE_1_BTI and GNU_PROPERTY_AARCH64_FEATURE_1_PAC,
>> why not use them?
> 
> i need definitions suitable for inclusion
> into assembly code.
> (i shortened the name so it does not go
> off the screen, but i can use the same
> name as in elf.h if that's less confusing)

Ack.

> 
>>> +#define END_FILE GNU_PROPERTY(FEATURE_1_BTI)
>>
>> END_FILE name does not really give much information.  Would be better
>> to use ADD_GNU_PROPERTIES or something?
> 
> the assumption was that we may need to
> add other things to asm files and don't
> want to touch all asm files when that
> happens, just reuse the END_FILE hook.
> 
> on the other hand i'm not sure how much
> value this hook adds: i can do what arm
> does for attributes: add them in sysdep.h
> at the top of asm files that include it.
> 
> the sysdep.h solution means less changes
> but may be easier to miss when new asm
> files are added: we need to ensure that
> all newly added asm code is compatible
> with the added properties, including asm
> like syscall-template.S that is shared
> across targets. Explicitly adding END_FILE
> implies the person who added it verified
> the necessary properties, but it's not a
> strong assurance and you could say the
> same for adding 'include <sysdep.h>'.
> 

I was referring to the define name in fact. But now that you brought
the different ways of define it, maybe setting it as arm attributes
might be a better alternative.

Although not required, current practice is to use sysdep.h macros
to define function entrypoints. So I think it should be feasible
to enforce its inclusion for all assembly implementations.

But I don't have a strong opinion here which mechanism is better.
What I think it would be useful is a way at build time to certify 
that if BTI is enabled, the linker warns if a object has a missing
property.