diff mbox series

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

Message ID 20241129163721.2385847-7-yury.khrustalev@arm.com
State Superseded
Delegated to: Carlos O'Donell
Headers
Series aarch64: Add support for Guarded Control Stack extension |

Checks

Context Check Description
redhat-pt-bot/TryBot-apply_patch success Patch applied to master at the time it was sent
linaro-tcwg-bot/tcwg_glibc_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-aarch64 success Test passed
linaro-tcwg-bot/tcwg_glibc_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_glibc_check--master-arm success Test passed

Commit Message

Yury Khrustalev Nov. 29, 2024, 4:37 p.m. UTC 
  From: Szabolcs Nagy <szabolcs.nagy@arm.com>

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

Comments

Carlos O'Donell Dec. 2, 2024, 9:47 p.m. UTC | #1 
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)
>
Yury Khrustalev Dec. 6, 2024, 10:54 a.m. UTC | #2 
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
diff mbox series

Patch

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)
 	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
 PSEUDO_END (__vfork)
 libc_hidden_def (__vfork)