[v7,20/23] aarch64: Process gnu properties in static exe

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

Unlike for BTI, the kernel does not process GCS properties so update
GL(dl_aarch64_gcs) before the GCS status is set.

Co-authored-by: Yury Khrustalev <yury.khrustalev@arm.com>
---
 csu/libc-start.c                             | 12 ++++++++++++
 sysdeps/generic/libc-start.h                 |  1 +
 sysdeps/unix/sysv/linux/aarch64/libc-start.h | 19 +++++++++++++++++++
 3 files changed, 32 insertions(+)
  

On 03/01/25 12:41, Yury Khrustalev wrote:
> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> 
> Unlike for BTI, the kernel does not process GCS properties so update
> GL(dl_aarch64_gcs) before the GCS status is set.
> 
> Co-authored-by: Yury Khrustalev <yury.khrustalev@arm.com>
> ---
>  csu/libc-start.c                             | 12 ++++++++++++
>  sysdeps/generic/libc-start.h                 |  1 +
>  sysdeps/unix/sysv/linux/aarch64/libc-start.h | 19 +++++++++++++++++++
>  3 files changed, 32 insertions(+)
> 
> diff --git a/csu/libc-start.c b/csu/libc-start.c
> index 6f3d52e223..325ef010d4 100644
> --- a/csu/libc-start.c
> +++ b/csu/libc-start.c
> @@ -206,6 +206,10 @@ call_fini (void *unused)
>  
>  #include <libc-start.h>
>  
> +#ifndef ARCH_PROCESS_NOTES
> +# define ARCH_PROCESS_NOTES()
> +#endif
> +

I think it would be better to avoid the old way of defining arch-hooks,
since they are error-prone.  For instance, although is not the case
(since the function is currently empty) if/when we need to add generic
__libc_process_notes code, just adding on the function below will not 
be suffice for x86_64/powerpc because they have its own libc-start.h 
(and this patch did not add an ARCH_PROCESS_NOTES for such ABIs).

Instead, the usual way is to either use the generic definitions
on sysdeps/generic or add a new one.  For this case, I think it would
be better to add new a file with the notes processing function instead
of reuse the dl-prop.h.  

I plan to use it to add the generic memory seal support, which requires 
PT_GNU_PROPERTY for all ABIs, without the need to replicate the same 
code on x86_64 (in fact, I can move the aarch64 definitions on generic 
code and remove aarch64 libc-prop.h).

Something like this diff over your patch:

diff --git a/csu/libc-start.c b/csu/libc-start.c
index 325ef010d4..f19a996bb2 100644
--- a/csu/libc-start.c
+++ b/csu/libc-start.c
@@ -36,7 +36,7 @@
 #include <stdbool.h>
 #include <elf-initfini.h>
 #include <shlib-compat.h>
-
+#include <libc-prop.h>
 #include <elf/dl-tunables.h>
 
 extern void __libc_init_first (int argc, char **argv, char **envp);
@@ -206,10 +206,6 @@ call_fini (void *unused)
 
 #include <libc-start.h>
 
-#ifndef ARCH_PROCESS_NOTES
-# define ARCH_PROCESS_NOTES()
-#endif
-
 STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
 					 MAIN_AUXVEC_DECL),
 			    int argc,
@@ -223,11 +219,6 @@ STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
 			    void *stack_end)
      __attribute__ ((noreturn));
 
-void
-__libc_process_notes (void)
-{
-
-}
 
 /* Note: The init and fini parameters are no longer used.  fini is
    completely unused, init is still called if not NULL, but the
@@ -286,7 +277,7 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
   ARCH_SETUP_IREL ();
 
   /* Process notes: PT_NOTE / PT_GNU_PROPERTY.  */
-  ARCH_PROCESS_NOTES ();
+  __libc_process_gnu_attributes ();
 
   /* The stack guard goes into the TCB, so initialize it early.  */
   ARCH_SETUP_TLS ();
diff --git a/sysdeps/aarch64/libc-prop.h b/sysdeps/aarch64/libc-prop.h
new file mode 100644
index 0000000000..478c21f669
--- /dev/null
+++ b/sysdeps/aarch64/libc-prop.h
@@ -0,0 +1,44 @@
+/* Support for GNU properties for static builds.  AArch64 version.
+   Copyright (C) 2025 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC_PROP_H
+#define _LIBC_PROP_H
+
+#include <dl-prop.h>
+
+/* Called at the start of program execution to handle GNU attribute from
+   PT_NOTE / PT_GNU_PROPERTY.  Must be on a top-level stack frame that does
+   not return.  */
+static __always_inline void
+__libc_process_gnu_attributes (void)
+{
+# ifndef SHARED
+  struct link_map *main_map = _dl_get_dl_main_map ();
+  const ElfW(Phdr) *phdr = GL(dl_phdr);
+  const ElfW(Phdr) *ph;
+  for (ph = phdr; ph < phdr + GL(dl_phnum); ph++)
+    if (ph->p_type == PT_GNU_PROPERTY)
+      {
+	_dl_process_pt_gnu_property (main_map, -1, ph);
+	_rtld_main_check (main_map, _dl_argv[0]);
+	break;
+      }
+# endif
+}
+
+#endif
diff --git a/sysdeps/generic/libc-prop.h b/sysdeps/generic/libc-prop.h
new file mode 100644
index 0000000000..22510a1972
--- /dev/null
+++ b/sysdeps/generic/libc-prop.h
@@ -0,0 +1,29 @@
+/* Support for GNU properties for static builds.  Generic version.
+   Copyright (C) 2025 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _LIBC_PROP_H
+#define _LIBC_PROP_H
+
+/* Called at the start of program executation to handle GNU attribute
+   from PT_NOTE / PT_GNU_PROPERTY.  */
+static __always_inline void
+__libc_process_gnu_attributes (void)
+{
+}
+
+#endif
diff --git a/sysdeps/generic/libc-start.h b/sysdeps/generic/libc-start.h
index 1a421626b5..0b107cf2a1 100644
--- a/sysdeps/generic/libc-start.h
+++ b/sysdeps/generic/libc-start.h
@@ -24,7 +24,6 @@
    initialization, and this means you cannot, without machine
    knowledge, access TLS from an IFUNC resolver.  */
 #define ARCH_SETUP_IREL() apply_irel ()
-#define ARCH_PROCESS_NOTES() __libc_process_notes ()
 #define ARCH_SETUP_TLS() __libc_setup_tls ()
 #define ARCH_APPLY_IREL()
 #endif /* ! SHARED  */
diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.h b/sysdeps/unix/sysv/linux/aarch64/libc-start.h
index 0f80a8abe9..735076c086 100644
--- a/sysdeps/unix/sysv/linux/aarch64/libc-start.h
+++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.h
@@ -21,24 +21,6 @@
 
 #ifndef SHARED
 
-# include <dl-prop.h>
-
-/* Must be on a top-level stack frame that does not return.  */
-static inline void __attribute__((always_inline))
-aarch64_libc_process_notes (void)
-{
-  struct link_map *main_map = _dl_get_dl_main_map ();
-  const ElfW(Phdr) *phdr = GL(dl_phdr);
-  const ElfW(Phdr) *ph;
-  for (ph = phdr; ph < phdr + GL(dl_phnum); ph++)
-    if (ph->p_type == PT_GNU_PROPERTY)
-      {
-	_dl_process_pt_gnu_property (main_map, -1, ph);
-	_rtld_main_check (main_map, _dl_argv[0]);
-	break;
-      }
-}
-
 # ifndef PR_SET_SHADOW_STACK_STATUS
 #  define PR_SET_SHADOW_STACK_STATUS	75
 #  define PR_SHADOW_STACK_ENABLE	(1UL << 0)
@@ -57,7 +39,6 @@ aarch64_libc_setup_tls (void)
 }
 
 # define ARCH_SETUP_IREL() apply_irel ()
-# define ARCH_PROCESS_NOTES() aarch64_libc_process_notes ()
 # define ARCH_SETUP_TLS() aarch64_libc_setup_tls ()
 # define ARCH_APPLY_IREL()
 #endif /* ! SHARED  */


>  STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
>  					 MAIN_AUXVEC_DECL),
>  			    int argc,
> @@ -219,6 +223,11 @@ STATIC int LIBC_START_MAIN (int (*main) (int, char **, char **
>  			    void *stack_end)
>       __attribute__ ((noreturn));
>  
> +void
> +__libc_process_notes (void)
> +{
> +
> +}
>  
>  /* Note: The init and fini parameters are no longer used.  fini is
>     completely unused, init is still called if not NULL, but the
> @@ -276,6 +285,9 @@ LIBC_START_MAIN (int (*main) (int, char **, char ** MAIN_AUXVEC_DECL),
>    /* Perform IREL{,A} relocations.  */
>    ARCH_SETUP_IREL ();
>  
> +  /* Process notes: PT_NOTE / PT_GNU_PROPERTY.  */
> +  ARCH_PROCESS_NOTES ();
> +
>    /* The stack guard goes into the TCB, so initialize it early.  */
>    ARCH_SETUP_TLS ();
>  
> diff --git a/sysdeps/generic/libc-start.h b/sysdeps/generic/libc-start.h
> index 0b107cf2a1..1a421626b5 100644
> --- a/sysdeps/generic/libc-start.h
> +++ b/sysdeps/generic/libc-start.h
> @@ -24,6 +24,7 @@
>     initialization, and this means you cannot, without machine
>     knowledge, access TLS from an IFUNC resolver.  */
>  #define ARCH_SETUP_IREL() apply_irel ()
> +#define ARCH_PROCESS_NOTES() __libc_process_notes ()
>  #define ARCH_SETUP_TLS() __libc_setup_tls ()
>  #define ARCH_APPLY_IREL()
>  #endif /* ! SHARED  */
> diff --git a/sysdeps/unix/sysv/linux/aarch64/libc-start.h b/sysdeps/unix/sysv/linux/aarch64/libc-start.h
> index 735076c086..0f80a8abe9 100644
> --- a/sysdeps/unix/sysv/linux/aarch64/libc-start.h
> +++ b/sysdeps/unix/sysv/linux/aarch64/libc-start.h
> @@ -21,6 +21,24 @@
>  
>  #ifndef SHARED
>  
> +# include <dl-prop.h>
> +
> +/* Must be on a top-level stack frame that does not return.  */
> +static inline void __attribute__((always_inline))
> +aarch64_libc_process_notes (void)
> +{
> +  struct link_map *main_map = _dl_get_dl_main_map ();
> +  const ElfW(Phdr) *phdr = GL(dl_phdr);
> +  const ElfW(Phdr) *ph;
> +  for (ph = phdr; ph < phdr + GL(dl_phnum); ph++)
> +    if (ph->p_type == PT_GNU_PROPERTY)
> +      {
> +	_dl_process_pt_gnu_property (main_map, -1, ph);
> +	_rtld_main_check (main_map, _dl_argv[0]);
> +	break;
> +      }
> +}
> +
>  # ifndef PR_SET_SHADOW_STACK_STATUS
>  #  define PR_SET_SHADOW_STACK_STATUS	75
>  #  define PR_SHADOW_STACK_ENABLE	(1UL << 0)
> @@ -39,6 +57,7 @@ aarch64_libc_setup_tls (void)
>  }
>  
>  # define ARCH_SETUP_IREL() apply_irel ()
> +# define ARCH_PROCESS_NOTES() aarch64_libc_process_notes ()
>  # define ARCH_SETUP_TLS() aarch64_libc_setup_tls ()
>  # define ARCH_APPLY_IREL()
>  #endif /* ! SHARED  */
  

On Thu, Jan 09, 2025 at 03:01:37PM -0300, Adhemerval Zanella Netto wrote:
> 
> 
> On 03/01/25 12:41, Yury Khrustalev wrote:
> > From: Szabolcs Nagy <szabolcs.nagy@arm.com>
> > 
> > Unlike for BTI, the kernel does not process GCS properties so update
> > GL(dl_aarch64_gcs) before the GCS status is set.
> > 
> > Co-authored-by: Yury Khrustalev <yury.khrustalev@arm.com>
> > ---
> >  csu/libc-start.c                             | 12 ++++++++++++
> >  sysdeps/generic/libc-start.h                 |  1 +
> >  sysdeps/unix/sysv/linux/aarch64/libc-start.h | 19 +++++++++++++++++++
> >  3 files changed, 32 insertions(+)
> > 
> > diff --git a/csu/libc-start.c b/csu/libc-start.c
> > index 6f3d52e223..325ef010d4 100644
> > --- a/csu/libc-start.c
> > +++ b/csu/libc-start.c
> > @@ -206,6 +206,10 @@ call_fini (void *unused)
> >  
> >  #include <libc-start.h>
> >  
> > +#ifndef ARCH_PROCESS_NOTES
> > +# define ARCH_PROCESS_NOTES()
> > +#endif
> > +
> 
> I think it would be better to avoid the old way of defining arch-hooks,
> since they are error-prone.  For instance, although is not the case
> (since the function is currently empty) if/when we need to add generic
> __libc_process_notes code, just adding on the function below will not 
> be suffice for x86_64/powerpc because they have its own libc-start.h 
> (and this patch did not add an ARCH_PROCESS_NOTES for such ABIs).
> 
> Instead, the usual way is to either use the generic definitions
> on sysdeps/generic or add a new one.  For this case, I think it would
> be better to add new a file with the notes processing function instead
> of reuse the dl-prop.h.  
> 
> I plan to use it to add the generic memory seal support, which requires 
> PT_GNU_PROPERTY for all ABIs, without the need to replicate the same 
> code on x86_64 (in fact, I can move the aarch64 definitions on generic 
> code and remove aarch64 libc-prop.h).

In the interest of time, would it be OK if I revert this patch to its
original version [1] so that we could make property processing generic
later (as this is not require for GCS and would delay merging of this
patch series)?

[1] https://inbox.sourceware.org/libc-alpha/20241129163721.2385847-20-yury.khrustalev@arm.com/

Thanks,
Yury
  

On 10/01/25 07:30, Yury Khrustalev wrote:
> On Thu, Jan 09, 2025 at 03:01:37PM -0300, Adhemerval Zanella Netto wrote:
>>
>>
>> On 03/01/25 12:41, Yury Khrustalev wrote:
>>> From: Szabolcs Nagy <szabolcs.nagy@arm.com>
>>>
>>> Unlike for BTI, the kernel does not process GCS properties so update
>>> GL(dl_aarch64_gcs) before the GCS status is set.
>>>
>>> Co-authored-by: Yury Khrustalev <yury.khrustalev@arm.com>
>>> ---
>>>  csu/libc-start.c                             | 12 ++++++++++++
>>>  sysdeps/generic/libc-start.h                 |  1 +
>>>  sysdeps/unix/sysv/linux/aarch64/libc-start.h | 19 +++++++++++++++++++
>>>  3 files changed, 32 insertions(+)
>>>
>>> diff --git a/csu/libc-start.c b/csu/libc-start.c
>>> index 6f3d52e223..325ef010d4 100644
>>> --- a/csu/libc-start.c
>>> +++ b/csu/libc-start.c
>>> @@ -206,6 +206,10 @@ call_fini (void *unused)
>>>  
>>>  #include <libc-start.h>
>>>  
>>> +#ifndef ARCH_PROCESS_NOTES
>>> +# define ARCH_PROCESS_NOTES()
>>> +#endif
>>> +
>>
>> I think it would be better to avoid the old way of defining arch-hooks,
>> since they are error-prone.  For instance, although is not the case
>> (since the function is currently empty) if/when we need to add generic
>> __libc_process_notes code, just adding on the function below will not 
>> be suffice for x86_64/powerpc because they have its own libc-start.h 
>> (and this patch did not add an ARCH_PROCESS_NOTES for such ABIs).
>>
>> Instead, the usual way is to either use the generic definitions
>> on sysdeps/generic or add a new one.  For this case, I think it would
>> be better to add new a file with the notes processing function instead
>> of reuse the dl-prop.h.  
>>
>> I plan to use it to add the generic memory seal support, which requires 
>> PT_GNU_PROPERTY for all ABIs, without the need to replicate the same 
>> code on x86_64 (in fact, I can move the aarch64 definitions on generic 
>> code and remove aarch64 libc-prop.h).
> 
> In the interest of time, would it be OK if I revert this patch to its
> original version [1] so that we could make property processing generic
> later (as this is not require for GCS and would delay merging of this
> patch series)?
> 
> [1] https://inbox.sourceware.org/libc-alpha/20241129163721.2385847-20-yury.khrustalev@arm.com/
> 

Alright, sounds fair.