diff mbox series

[09/17] x86: Check PT_GNU_PROPERTY early

Message ID 20231206172010.1023415-10-hjl.tools@gmail.com
State Committed
Commit 4753e9286858a61d5fbe8742d48d8c9166143354
Headers
Series x86/cet: Update CET kernel interface |

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 fail Patch failed to apply
linaro-tcwg-bot/tcwg_glibc_build--master-arm fail Patch failed to apply

Commit Message

H.J. Lu Dec. 6, 2023, 5:20 p.m. UTC 
  The PT_GNU_PROPERTY segment is scanned before PT_NOTE.  For binaries
with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
the PT_NOTE segment.
---
 sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
 1 file changed, 80 insertions(+), 40 deletions(-)

Comments

Noah Goldstein Dec. 6, 2023, 11:57 p.m. UTC | #1 
On Wed, Dec 6, 2023 at 11:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> The PT_GNU_PROPERTY segment is scanned before PT_NOTE.  For binaries
> with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
> the PT_NOTE segment.
> ---
>  sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
>  1 file changed, 80 insertions(+), 40 deletions(-)
>
> diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> index b2836f3009..d2c53c2182 100644
> --- a/sysdeps/x86/dl-prop.h
> +++ b/sysdeps/x86/dl-prop.h
> @@ -81,6 +81,60 @@ _dl_open_check (struct link_map *m)
>  #endif
>  }
>
> +/* Check the GNU property and return its value.  It returns:
> +   -1: Skip this note.
> +    0: Stop checking.
> +    1: Continue to check.
> + */
> +static inline int
> +_dl_check_gnu_property (unsigned int type, unsigned int datasz,
> +                       void *ptr, unsigned int *feature_1_and,
> +                       unsigned int *needed_1,
> +                       unsigned int *isa_1_needed)
> +{
> +  if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> +      || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> +      || type == GNU_PROPERTY_1_NEEDED)
> +    {
> +      /* The sizes of types which we are searching for are
> +        4 bytes.  There is no point to continue if this
> +        note is ill-formed.  */
> +      if (datasz != 4)
> +       return -1;
> +
> +      /* NB: Stop the scan only after seeing all types which
> +        we are searching for.  */
> +      _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> +                       > GNU_PROPERTY_X86_FEATURE_1_AND)
> +                      && (GNU_PROPERTY_X86_FEATURE_1_AND
> +                          > GNU_PROPERTY_1_NEEDED)),
> +                     "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> +                     "GNU_PROPERTY_X86_FEATURE_1_AND && "
> +                     "GNU_PROPERTY_X86_FEATURE_1_AND > "
> +                     "GNU_PROPERTY_1_NEEDED");
> +      if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> +       *feature_1_and = *(unsigned int *) ptr;
> +      else if (type == GNU_PROPERTY_1_NEEDED)
> +       *needed_1 = *(unsigned int *) ptr;
> +      else
> +       {
> +         *isa_1_needed = *(unsigned int *) ptr;
> +
> +         /* Keep searching for the next GNU property note
> +            generated by the older linker.  */
> +         return 0;
> +       }
> +    }
> +  else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> +    {
> +      /* Stop the scan since property type is in ascending
> +        order.  */
> +      return 0;
> +    }
> +
> +  return 1;
> +}
> +
>  static inline void __attribute__ ((unused))
>  _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
>                            const ElfW(Addr) size, const ElfW(Addr) align)
> @@ -141,45 +195,14 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
>
>               last_type = type;
>
> -             if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> -                 || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> -                 || type == GNU_PROPERTY_1_NEEDED)
> -               {
> -                 /* The sizes of types which we are searching for are
> -                    4 bytes.  There is no point to continue if this
> -                    note is ill-formed.  */
> -                 if (datasz != 4)
> -                   return;
> -
> -                 /* NB: Stop the scan only after seeing all types which
> -                    we are searching for.  */
> -                 _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> -                                   > GNU_PROPERTY_X86_FEATURE_1_AND)
> -                                  && (GNU_PROPERTY_X86_FEATURE_1_AND
> -                                      > GNU_PROPERTY_1_NEEDED)),
> -                                 "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> -                                 "GNU_PROPERTY_X86_FEATURE_1_AND && "
> -                                 "GNU_PROPERTY_X86_FEATURE_1_AND > "
> -                                 "GNU_PROPERTY_1_NEEDED");
> -                 if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> -                   feature_1_and = *(unsigned int *) ptr;
> -                 else if (type == GNU_PROPERTY_1_NEEDED)
> -                   needed_1 = *(unsigned int *) ptr;
> -                 else
> -                   {
> -                     isa_1_needed = *(unsigned int *) ptr;
> -
> -                     /* Keep searching for the next GNU property note
> -                        generated by the older linker.  */
> -                     break;
> -                   }
> -               }
> -             else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> -               {
> -                 /* Stop the scan since property type is in ascending
> -                    order.  */
> -                 break;
> -               }
> +             int result = _dl_check_gnu_property (type, datasz, ptr,
> +                                                  &feature_1_and,
> +                                                  &needed_1,
> +                                                  &isa_1_needed);
> +             if (result == -1)
> +               return;         /* Skip this note.  */
> +             else if (result == 0)
> +               break; /* Stop checking.  */
>
>               /* Check the next property item.  */
>               ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> @@ -217,7 +240,24 @@ static inline int __attribute__ ((always_inline))
>  _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
>                           uint32_t datasz, void *data)
>  {
> -  return 0;
> +  /* This is called on each GNU property.  */
> +  unsigned int needed_1 = 0;
> +  unsigned int feature_1_and = 0;
> +  unsigned int isa_1_needed = 0;
> +  int result = _dl_check_gnu_property (type, datasz, data,
> +                                      &feature_1_and, &needed_1,
> +                                      &isa_1_needed);
> +  if (needed_1 != 0)
> +    l->l_1_needed = needed_1;
> +  if (isa_1_needed != 0)
> +    l->l_x86_isa_1_needed = isa_1_needed;
> +  if (feature_1_and != 0)
> +    l->l_x86_feature_1_and = feature_1_and;
> +  if ((needed_1 | isa_1_needed | feature_1_and) != 0)
> +    l->l_property = lc_property_valid;
> +  else if (l->l_property == lc_property_unknown)
> +    l->l_property = lc_property_none;
> +  return result <= 0 ? 0 : result;
These seems like a seperate change from the refactor mentioned in the
commit message?
>  }
>
>  #endif /* _DL_PROP_H */
> --
> 2.43.0
>
H.J. Lu Dec. 7, 2023, 9:06 p.m. UTC | #2 
On Wed, Dec 6, 2023 at 3:57 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> On Wed, Dec 6, 2023 at 11:20 AM H.J. Lu <hjl.tools@gmail.com> wrote:
> >
> > The PT_GNU_PROPERTY segment is scanned before PT_NOTE.  For binaries
> > with the PT_GNU_PROPERTY segment, we can check it to avoid scan of
> > the PT_NOTE segment.
> > ---
> >  sysdeps/x86/dl-prop.h | 120 ++++++++++++++++++++++++++++--------------
> >  1 file changed, 80 insertions(+), 40 deletions(-)
> >
> > diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
> > index b2836f3009..d2c53c2182 100644
> > --- a/sysdeps/x86/dl-prop.h
> > +++ b/sysdeps/x86/dl-prop.h
> > @@ -81,6 +81,60 @@ _dl_open_check (struct link_map *m)
> >  #endif
> >  }
> >
> > +/* Check the GNU property and return its value.  It returns:
> > +   -1: Skip this note.
> > +    0: Stop checking.
> > +    1: Continue to check.
> > + */
> > +static inline int
> > +_dl_check_gnu_property (unsigned int type, unsigned int datasz,
> > +                       void *ptr, unsigned int *feature_1_and,
> > +                       unsigned int *needed_1,
> > +                       unsigned int *isa_1_needed)
> > +{
> > +  if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> > +      || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> > +      || type == GNU_PROPERTY_1_NEEDED)
> > +    {
> > +      /* The sizes of types which we are searching for are
> > +        4 bytes.  There is no point to continue if this
> > +        note is ill-formed.  */
> > +      if (datasz != 4)
> > +       return -1;
> > +
> > +      /* NB: Stop the scan only after seeing all types which
> > +        we are searching for.  */
> > +      _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> > +                       > GNU_PROPERTY_X86_FEATURE_1_AND)
> > +                      && (GNU_PROPERTY_X86_FEATURE_1_AND
> > +                          > GNU_PROPERTY_1_NEEDED)),
> > +                     "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> > +                     "GNU_PROPERTY_X86_FEATURE_1_AND && "
> > +                     "GNU_PROPERTY_X86_FEATURE_1_AND > "
> > +                     "GNU_PROPERTY_1_NEEDED");
> > +      if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> > +       *feature_1_and = *(unsigned int *) ptr;
> > +      else if (type == GNU_PROPERTY_1_NEEDED)
> > +       *needed_1 = *(unsigned int *) ptr;
> > +      else
> > +       {
> > +         *isa_1_needed = *(unsigned int *) ptr;
> > +
> > +         /* Keep searching for the next GNU property note
> > +            generated by the older linker.  */
> > +         return 0;
> > +       }
> > +    }
> > +  else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> > +    {
> > +      /* Stop the scan since property type is in ascending
> > +        order.  */
> > +      return 0;
> > +    }
> > +
> > +  return 1;
> > +}
> > +
> >  static inline void __attribute__ ((unused))
> >  _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> >                            const ElfW(Addr) size, const ElfW(Addr) align)
> > @@ -141,45 +195,14 @@ _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
> >
> >               last_type = type;
> >
> > -             if (type == GNU_PROPERTY_X86_FEATURE_1_AND
> > -                 || type == GNU_PROPERTY_X86_ISA_1_NEEDED
> > -                 || type == GNU_PROPERTY_1_NEEDED)
> > -               {
> > -                 /* The sizes of types which we are searching for are
> > -                    4 bytes.  There is no point to continue if this
> > -                    note is ill-formed.  */
> > -                 if (datasz != 4)
> > -                   return;
> > -
> > -                 /* NB: Stop the scan only after seeing all types which
> > -                    we are searching for.  */
> > -                 _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
> > -                                   > GNU_PROPERTY_X86_FEATURE_1_AND)
> > -                                  && (GNU_PROPERTY_X86_FEATURE_1_AND
> > -                                      > GNU_PROPERTY_1_NEEDED)),
> > -                                 "GNU_PROPERTY_X86_ISA_1_NEEDED > "
> > -                                 "GNU_PROPERTY_X86_FEATURE_1_AND && "
> > -                                 "GNU_PROPERTY_X86_FEATURE_1_AND > "
> > -                                 "GNU_PROPERTY_1_NEEDED");
> > -                 if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
> > -                   feature_1_and = *(unsigned int *) ptr;
> > -                 else if (type == GNU_PROPERTY_1_NEEDED)
> > -                   needed_1 = *(unsigned int *) ptr;
> > -                 else
> > -                   {
> > -                     isa_1_needed = *(unsigned int *) ptr;
> > -
> > -                     /* Keep searching for the next GNU property note
> > -                        generated by the older linker.  */
> > -                     break;
> > -                   }
> > -               }
> > -             else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
> > -               {
> > -                 /* Stop the scan since property type is in ascending
> > -                    order.  */
> > -                 break;
> > -               }
> > +             int result = _dl_check_gnu_property (type, datasz, ptr,
> > +                                                  &feature_1_and,
> > +                                                  &needed_1,
> > +                                                  &isa_1_needed);
> > +             if (result == -1)
> > +               return;         /* Skip this note.  */
> > +             else if (result == 0)
> > +               break; /* Stop checking.  */
> >
> >               /* Check the next property item.  */
> >               ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
> > @@ -217,7 +240,24 @@ static inline int __attribute__ ((always_inline))
> >  _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
> >                           uint32_t datasz, void *data)
> >  {
> > -  return 0;
> > +  /* This is called on each GNU property.  */
> > +  unsigned int needed_1 = 0;
> > +  unsigned int feature_1_and = 0;
> > +  unsigned int isa_1_needed = 0;
> > +  int result = _dl_check_gnu_property (type, datasz, data,
> > +                                      &feature_1_and, &needed_1,
> > +                                      &isa_1_needed);
> > +  if (needed_1 != 0)
> > +    l->l_1_needed = needed_1;
> > +  if (isa_1_needed != 0)
> > +    l->l_x86_isa_1_needed = isa_1_needed;
> > +  if (feature_1_and != 0)
> > +    l->l_x86_feature_1_and = feature_1_and;
> > +  if ((needed_1 | isa_1_needed | feature_1_and) != 0)
> > +    l->l_property = lc_property_valid;
> > +  else if (l->l_property == lc_property_unknown)
> > +    l->l_property = lc_property_none;
> > +  return result <= 0 ? 0 : result;
> These seems like a seperate change from the refactor mentioned in the
> commit message?

Done.  I sent out a separate patch.

Thanks.

> >  }
> >
> >  #endif /* _DL_PROP_H */
> > --
> > 2.43.0
> >
diff mbox series

Patch

diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index b2836f3009..d2c53c2182 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -81,6 +81,60 @@  _dl_open_check (struct link_map *m)
 #endif
 }
 
+/* Check the GNU property and return its value.  It returns:
+   -1: Skip this note.
+    0: Stop checking.
+    1: Continue to check.
+ */
+static inline int
+_dl_check_gnu_property (unsigned int type, unsigned int datasz,
+			void *ptr, unsigned int *feature_1_and,
+			unsigned int *needed_1,
+			unsigned int *isa_1_needed)
+{
+  if (type == GNU_PROPERTY_X86_FEATURE_1_AND
+      || type == GNU_PROPERTY_X86_ISA_1_NEEDED
+      || type == GNU_PROPERTY_1_NEEDED)
+    {
+      /* The sizes of types which we are searching for are
+	 4 bytes.  There is no point to continue if this
+	 note is ill-formed.  */
+      if (datasz != 4)
+	return -1;
+
+      /* NB: Stop the scan only after seeing all types which
+	 we are searching for.  */
+      _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
+			> GNU_PROPERTY_X86_FEATURE_1_AND)
+		       && (GNU_PROPERTY_X86_FEATURE_1_AND
+			   > GNU_PROPERTY_1_NEEDED)),
+		      "GNU_PROPERTY_X86_ISA_1_NEEDED > "
+		      "GNU_PROPERTY_X86_FEATURE_1_AND && "
+		      "GNU_PROPERTY_X86_FEATURE_1_AND > "
+		      "GNU_PROPERTY_1_NEEDED");
+      if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
+	*feature_1_and = *(unsigned int *) ptr;
+      else if (type == GNU_PROPERTY_1_NEEDED)
+	*needed_1 = *(unsigned int *) ptr;
+      else
+	{
+	  *isa_1_needed = *(unsigned int *) ptr;
+
+	  /* Keep searching for the next GNU property note
+	     generated by the older linker.  */
+	  return 0;
+	}
+    }
+  else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
+    {
+      /* Stop the scan since property type is in ascending
+	 order.  */
+      return 0;
+    }
+
+  return 1;
+}
+
 static inline void __attribute__ ((unused))
 _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
 			   const ElfW(Addr) size, const ElfW(Addr) align)
@@ -141,45 +195,14 @@  _dl_process_property_note (struct link_map *l, const ElfW(Nhdr) *note,
 
 	      last_type = type;
 
-	      if (type == GNU_PROPERTY_X86_FEATURE_1_AND
-		  || type == GNU_PROPERTY_X86_ISA_1_NEEDED
-		  || type == GNU_PROPERTY_1_NEEDED)
-		{
-		  /* The sizes of types which we are searching for are
-		     4 bytes.  There is no point to continue if this
-		     note is ill-formed.  */
-		  if (datasz != 4)
-		    return;
-
-		  /* NB: Stop the scan only after seeing all types which
-		     we are searching for.  */
-		  _Static_assert (((GNU_PROPERTY_X86_ISA_1_NEEDED
-				    > GNU_PROPERTY_X86_FEATURE_1_AND)
-				   && (GNU_PROPERTY_X86_FEATURE_1_AND
-				       > GNU_PROPERTY_1_NEEDED)),
-				  "GNU_PROPERTY_X86_ISA_1_NEEDED > "
-				  "GNU_PROPERTY_X86_FEATURE_1_AND && "
-				  "GNU_PROPERTY_X86_FEATURE_1_AND > "
-				  "GNU_PROPERTY_1_NEEDED");
-		  if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
-		    feature_1_and = *(unsigned int *) ptr;
-		  else if (type == GNU_PROPERTY_1_NEEDED)
-		    needed_1 = *(unsigned int *) ptr;
-		  else
-		    {
-		      isa_1_needed = *(unsigned int *) ptr;
-
-		      /* Keep searching for the next GNU property note
-			 generated by the older linker.  */
-		      break;
-		    }
-		}
-	      else if (type > GNU_PROPERTY_X86_ISA_1_NEEDED)
-		{
-		  /* Stop the scan since property type is in ascending
-		     order.  */
-		  break;
-		}
+	      int result = _dl_check_gnu_property (type, datasz, ptr,
+						   &feature_1_and,
+						   &needed_1,
+						   &isa_1_needed);
+	      if (result == -1)
+		return;		/* Skip this note.  */
+	      else if (result == 0)
+		break; /* Stop checking.  */
 
 	      /* Check the next property item.  */
 	      ptr += ALIGN_UP (datasz, sizeof (ElfW(Addr)));
@@ -217,7 +240,24 @@  static inline int __attribute__ ((always_inline))
 _dl_process_gnu_property (struct link_map *l, int fd, uint32_t type,
 			  uint32_t datasz, void *data)
 {
-  return 0;
+  /* This is called on each GNU property.  */
+  unsigned int needed_1 = 0;
+  unsigned int feature_1_and = 0;
+  unsigned int isa_1_needed = 0;
+  int result = _dl_check_gnu_property (type, datasz, data,
+				       &feature_1_and, &needed_1,
+				       &isa_1_needed);
+  if (needed_1 != 0)
+    l->l_1_needed = needed_1;
+  if (isa_1_needed != 0)
+    l->l_x86_isa_1_needed = isa_1_needed;
+  if (feature_1_and != 0)
+    l->l_x86_feature_1_and = feature_1_and;
+  if ((needed_1 | isa_1_needed | feature_1_and) != 0)
+    l->l_property = lc_property_valid;
+  else if (l->l_property == lc_property_unknown)
+    l->l_property = lc_property_none;
+  return result <= 0 ? 0 : result;
 }
 
 #endif /* _DL_PROP_H */