[v4,06/22] aarch64: Add GCS support to vfork

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

---
 sysdeps/unix/sysv/linux/aarch64/vfork.S | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)
  

On 11/29/24 11:37 AM, Yury Khrustalev wrote:
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> 
> ---

This makes all aarch64 builds jump to the child via br,
is there any consequence to that?

>  sysdeps/unix/sysv/linux/aarch64/vfork.S | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/sysdeps/unix/sysv/linux/aarch64/vfork.S b/sysdeps/unix/sysv/linux/aarch64/vfork.S
> index e71e492da3..cfaf4a1ffb 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/vfork.S
> +++ b/sysdeps/unix/sysv/linux/aarch64/vfork.S
> @@ -33,8 +33,14 @@ ENTRY (__vfork)
>  
>  	cmn	x0, #4095
>  	b.cs    .Lsyscall_error
> +	cbz	x0, L(child)

OK. Child sees zero.

>  	RET
> -
> +L(child):
> +	/* Return with indirect branch in the child to support GCS.
> +	   Clear x30 to crash early if the child tries to ret.  */
> +	mov	x1, x30
> +	mov	x30, 0
> +	br	x1

OK. Always use br in child.

>  PSEUDO_END (__vfork)
>  libc_hidden_def (__vfork)
>
  

On Mon, Dec 02, 2024 at 04:47:48PM -0500, Carlos O'Donell wrote:
> On 11/29/24 11:37 AM, Yury Khrustalev wrote:
> > From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > 
> > ---
> 
> This makes all aarch64 builds jump to the child via br,
> is there any consequence to that?

There are no consequences to that except the intended compatibility
with GCS. Strictly speaking, from the child perspective using RET
is not meaningful as the child hasn't called any function yet, so
we could say that using BR makes code a bit more clear.

I will also remove MOV instructions in v5 of this patch as they
would mostly be unnecessary.

> >  sysdeps/unix/sysv/linux/aarch64/vfork.S | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/sysdeps/unix/sysv/linux/aarch64/vfork.S b/sysdeps/unix/sysv/linux/aarch64/vfork.S
> > index e71e492da3..cfaf4a1ffb 100644
> > --- a/sysdeps/unix/sysv/linux/aarch64/vfork.S
> > +++ b/sysdeps/unix/sysv/linux/aarch64/vfork.S
> > @@ -33,8 +33,14 @@ ENTRY (__vfork)
> >  
> >  	cmn	x0, #4095
> >  	b.cs    .Lsyscall_error
> > +	cbz	x0, L(child)
> 
> OK. Child sees zero.
> 
> >  	RET
> > -
> > +L(child):
> > +	/* Return with indirect branch in the child to support GCS.
> > +	   Clear x30 to crash early if the child tries to ret.  */
> > +	mov	x1, x30
> > +	mov	x30, 0
> > +	br	x1
> 
> OK. Always use br in child.
> 
> >  PSEUDO_END (__vfork)
> >  libc_hidden_def (__vfork)
> >  

Kind regards,
Yury
  

On 02/12/2024 21:47, Carlos O'Donell wrote:
> On 11/29/24 11:37 AM, Yury Khrustalev wrote:
>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>
>> ---
> 
> This makes all aarch64 builds jump to the child via br,
> is there any consequence to that?
> 
>>   sysdeps/unix/sysv/linux/aarch64/vfork.S | 8 +++++++-
>>   1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/sysdeps/unix/sysv/linux/aarch64/vfork.S b/sysdeps/unix/sysv/linux/aarch64/vfork.S
>> index e71e492da3..cfaf4a1ffb 100644
>> --- a/sysdeps/unix/sysv/linux/aarch64/vfork.S
>> +++ b/sysdeps/unix/sysv/linux/aarch64/vfork.S
>> @@ -33,8 +33,14 @@ ENTRY (__vfork)
>>   
>>   	cmn	x0, #4095
>>   	b.cs    .Lsyscall_error
>> +	cbz	x0, L(child)
> 
> OK. Child sees zero.
> 
>>   	RET
>> -
>> +L(child):
>> +	/* Return with indirect branch in the child to support GCS.
>> +	   Clear x30 to crash early if the child tries to ret.  */
>> +	mov	x1, x30
>> +	mov	x30, 0
>> +	br	x1
> 
> OK. Always use br in child.

Has this been tested on targets that use BTI?  RET will bypass BTI 
checking, but BR will not, so if the caller has not put a BTI 
instruction after the call this will crash.

R.
> 
>>   PSEUDO_END (__vfork)
>>   libc_hidden_def (__vfork)
>>   
>
  

Hi Richard,

>> OK. Always use br in child.
>
> Has this been tested on targets that use BTI?  RET will bypass BTI
> checking, but BR will not, so if the caller has not put a BTI
> instruction after the call this will crash.

GCC will insert BTI after calls that may return differently. Setjmp and vfork
are some examples (they are internally marked as returns_twice).

Cheers,
Wilco
  

On 11/12/2024 13:03, Wilco Dijkstra wrote:
> Hi Richard,
> 
>>> OK. Always use br in child.
>>
>> Has this been tested on targets that use BTI?  RET will bypass BTI
>> checking, but BR will not, so if the caller has not put a BTI
>> instruction after the call this will crash.
> 
> GCC will insert BTI after calls that may return differently. Setjmp and vfork
> are some examples (they are internally marked as returns_twice).
> 
> Cheers,
> Wilco

Great.  I wasn't aware that vfork was already correctly marked.

R.