[v7,14/23] aarch64: Mark objects with GCS property note

  From: Szabolcs Nagy <szabolcs.nagy@arm.com>

Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
---
 sysdeps/aarch64/sysdep.h | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)
  

On 03/01/25 12:41, Yury Khrustalev wrote:
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> 
> Reviewed-by: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> ---
>  sysdeps/aarch64/sysdep.h | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sysdeps/aarch64/sysdep.h b/sysdeps/aarch64/sysdep.h
> index 4df120ad80..036eb12527 100644
> --- a/sysdeps/aarch64/sysdep.h
> +++ b/sysdeps/aarch64/sysdep.h
> @@ -85,6 +85,7 @@ strip_pac (void *p)
>  #define FEATURE_1_AND 0xc0000000
>  #define FEATURE_1_BTI 1
>  #define FEATURE_1_PAC 2
> +#define FEATURE_1_GCS 4
>  
>  /* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
>  #define GNU_PROPERTY(type, value)	\
> @@ -103,9 +104,9 @@ strip_pac (void *p)
>  /* Add GNU property note with the supported features to all asm code
>     where sysdep.h is included.  */
>  #if HAVE_AARCH64_BTI && HAVE_AARCH64_PAC_RET

Both HAVE_AARCH64_BTI and HAVE_AARCH64_PAC_RET have configure tests 
to check if -mbranch-protection={bti,pac-ret} is used.  Why do assume
GCS support for this case? Should it have a similar check for
-mbranch-protection=gcs?

> -GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC)
> +GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_PAC|FEATURE_1_GCS)
>  #elif HAVE_AARCH64_BTI
> -GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI)
> +GNU_PROPERTY (FEATURE_1_AND, FEATURE_1_BTI|FEATURE_1_GCS)
>  #endif
>  
>  /* Define an entry point visible from C.  */
  

On Tue, Jan 07, 2025 at 01:55:50PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> >  #define FEATURE_1_AND 0xc0000000
> >  #define FEATURE_1_BTI 1
> >  #define FEATURE_1_PAC 2
> > +#define FEATURE_1_GCS 4
> >  
> >  /* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
> >  #define GNU_PROPERTY(type, value)	\
> > @@ -103,9 +104,9 @@ strip_pac (void *p)
> >  /* Add GNU property note with the supported features to all asm code
> >     where sysdep.h is included.  */
> >  #if HAVE_AARCH64_BTI && HAVE_AARCH64_PAC_RET
> 
> Both HAVE_AARCH64_BTI and HAVE_AARCH64_PAC_RET have configure tests 
> to check if -mbranch-protection={bti,pac-ret} is used.  Why do assume
> GCS support for this case? Should it have a similar check for
> -mbranch-protection=gcs?

The short answer is no, we don't need a similar check for GCS and the way
it is currenty implement is correct though not ideal. However, I can see
how this could be confusing.

First of all, this part only affects hand written assembly and object that
are compiled from it.

Secondly, checks for BTI and PAC had been historically added this way and
this might be amended in the future (e.g. we might want to have BTI always
enabled by default), but this problem is not relevant for GCS.

We want GCS markings to be always enabled simply because it doesn't break
anything and is convenient because we don't want to support every possible
combination (it would have no practical value). Having GCS added as above
is the simplest possible solution with no downsides (except for it looking
a bit confusing at first glance).

Thanks,
Yury
  

On 08/01/25 12:34, Yury Khrustalev wrote:
> On Tue, Jan 07, 2025 at 01:55:50PM -0300, Adhemerval Zanella Netto wrote:
>>
>>
>>>  #define FEATURE_1_AND 0xc0000000
>>>  #define FEATURE_1_BTI 1
>>>  #define FEATURE_1_PAC 2
>>> +#define FEATURE_1_GCS 4
>>>  
>>>  /* Add a NT_GNU_PROPERTY_TYPE_0 note.  */
>>>  #define GNU_PROPERTY(type, value)	\
>>> @@ -103,9 +104,9 @@ strip_pac (void *p)
>>>  /* Add GNU property note with the supported features to all asm code
>>>     where sysdep.h is included.  */
>>>  #if HAVE_AARCH64_BTI && HAVE_AARCH64_PAC_RET
>>
>> Both HAVE_AARCH64_BTI and HAVE_AARCH64_PAC_RET have configure tests 
>> to check if -mbranch-protection={bti,pac-ret} is used.  Why do assume
>> GCS support for this case? Should it have a similar check for
>> -mbranch-protection=gcs?
> 
> The short answer is no, we don't need a similar check for GCS and the way
> it is currenty implement is correct though not ideal. However, I can see
> how this could be confusing.
> 
> First of all, this part only affects hand written assembly and object that
> are compiled from it.
> 
> Secondly, checks for BTI and PAC had been historically added this way and
> this might be amended in the future (e.g. we might want to have BTI always
> enabled by default), but this problem is not relevant for GCS.
> 
> We want GCS markings to be always enabled simply because it doesn't break
> anything and is convenient because we don't want to support every possible
> combination (it would have no practical value). Having GCS added as above
> is the simplest possible solution with no downsides (except for it looking
> a bit confusing at first glance).

Fair enough, should it be enable regardless of HAVE_AARCH64_BTI/HAVE_AARCH64_PAC_RET
then?