diff mbox

V2 [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509]

Message ID CAMe9rOqOx9qPuuQ9+jEODwLVzq5JKm7-PTtKODu-ZP1RqeMyDQ@mail.gmail.com
State New, archived
Headers show

Commit Message

H.J. Lu Nov. 8, 2018, 6:57 p.m. UTC
On Thu, Nov 8, 2018 at 9:41 AM Florian Weimer <fweimer@redhat.com> wrote:
>
> * H. J. Lu:
>
> > Here is the updated patch with
> >
> > Linkers group input note sections with the same name into one output
> > note section with the same name.  One output note section is placed in
> > one PT_NOTE segment.  Since new linkers merge input .note.gnu.property
> > sections into one output .note.gnu.property section, there is only
> > one NT_GNU_PROPERTY_TYPE_0 note in one PT_NOTE segment with new linkers.
> > Since older linkers treat input .note.gnu.property section as a generic
> > note section and just concatenate all input .note.gnu.property sections
> > into one output .note.gnu.property section without merging them, we may
> > see multiple NT_GNU_PROPERTY_TYPE_0 notes in one PT_NOTE segment with
> > older linkers.
>
> Thanks.  This hypothesis regarding older linkers seems reasonable, and
> the explanation is very clear now.  The code looks okay to me.

I checked it into master branch.

> Would you please backport this to the 2.28 release branch as well?
>

I am backporting this patch to 2.28 branch.
diff mbox

Patch

From 3e8d8dd5afba18a847ff7a80f473336f777cc329 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Thu, 8 Nov 2018 10:06:58 -0800
Subject: [PATCH] Check multiple NT_GNU_PROPERTY_TYPE_0 notes [BZ #23509]

Linkers group input note sections with the same name into one output
note section with the same name.  One output note section is placed in
one PT_NOTE segment.  Since new linkers merge input .note.gnu.property
sections into one output .note.gnu.property section, there is only
one NT_GNU_PROPERTY_TYPE_0 note in one PT_NOTE segment with new linkers.
Since older linkers treat input .note.gnu.property section as a generic
note section and just concatenate all input .note.gnu.property sections
into one output .note.gnu.property section without merging them, we may
see multiple NT_GNU_PROPERTY_TYPE_0 notes in one PT_NOTE segment with
older linkers.

When an older linker is used to created the program on CET-enabled OS,
the linker output has a single .note.gnu.property section with multiple
NT_GNU_PROPERTY_TYPE_0 notes, some of which have IBT and SHSTK enable
bits set even if the program isn't CET enabled.  Such programs will
crash on CET-enabled machines.  This patch updates the note parser:

1. Skip note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed.
2. Check multiple NT_GNU_PROPERTY_TYPE_0 notes.

	[BZ #23509]
	* sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Skip
	note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed.
	Update the l_cet field when processing NT_GNU_PROPERTY_TYPE_0 note.
	Check multiple NT_GNU_PROPERTY_TYPE_0 notes.
	* sysdeps/x86/link_map.h (l_cet): Expand to 3 bits,  Add
	lc_unknown.

(cherry picked from commit d524fa6c35e675eedbd8fe6cdf4db0b49c658026)
---
 ChangeLog              | 10 +++++++++
 NEWS                   |  1 +
 sysdeps/x86/dl-prop.h  | 51 +++++++++++++++++++++++++++++++++---------
 sysdeps/x86/link_map.h |  9 ++++----
 4 files changed, 57 insertions(+), 14 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index db4ac3b76a..86bdf17989 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,13 @@ 
+2018-11-08  H.J. Lu  <hongjiu.lu@intel.com>
+
+	[BZ #23509]
+	* sysdeps/x86/dl-prop.h (_dl_process_cet_property_note): Skip
+	note parsing if a NT_GNU_PROPERTY_TYPE_0 note has been processed.
+	Update the l_cet field when processing NT_GNU_PROPERTY_TYPE_0 note.
+	Check multiple NT_GNU_PROPERTY_TYPE_0 notes.
+	* sysdeps/x86/link_map.h (l_cet): Expand to 3 bits,  Add
+	lc_unknown.
+
 2018-11-05  Andreas Schwab  <schwab@suse.de>
 
 	[BZ #22927]
diff --git a/NEWS b/NEWS
index b85be4a9c1..e5ca5903ec 100644
--- a/NEWS
+++ b/NEWS
@@ -13,6 +13,7 @@  The following bugs are resolved with this release:
   [22927] libanl: properly cleanup if first helper thread creation failed
   [23400] stdlib/test-bz22786.c creates temporary files in glibc source tree
   [23497] readdir64@GLIBC_2.1 cannot parse the kernel directory stream
+  [23509] CET enabled glibc is incompatible with the older linker
   [23521] nss_files aliases database file stream leak
   [23538] pthread_cond_broadcast: Fix waiters-after-spinning case
   [23562] signal: Use correct type for si_band in siginfo_t
diff --git a/sysdeps/x86/dl-prop.h b/sysdeps/x86/dl-prop.h
index 26c3131ac5..9ab890d12b 100644
--- a/sysdeps/x86/dl-prop.h
+++ b/sysdeps/x86/dl-prop.h
@@ -49,6 +49,10 @@  _dl_process_cet_property_note (struct link_map *l,
 			      const ElfW(Addr) align)
 {
 #if CET_ENABLED
+  /* Skip if we have seen a NT_GNU_PROPERTY_TYPE_0 note before.  */
+  if (l->l_cet != lc_unknown)
+    return;
+
   /* The NT_GNU_PROPERTY_TYPE_0 note must be aliged to 4 bytes in
      32-bit objects and to 8 bytes in 64-bit objects.  Skip notes
      with incorrect alignment.  */
@@ -57,6 +61,9 @@  _dl_process_cet_property_note (struct link_map *l,
 
   const ElfW(Addr) start = (ElfW(Addr)) note;
 
+  unsigned int feature_1 = 0;
+  unsigned int last_type = 0;
+
   while ((ElfW(Addr)) (note + 1) - start < size)
     {
       /* Find the NT_GNU_PROPERTY_TYPE_0 note.  */
@@ -64,10 +71,18 @@  _dl_process_cet_property_note (struct link_map *l,
 	  && note->n_type == NT_GNU_PROPERTY_TYPE_0
 	  && memcmp (note + 1, "GNU", 4) == 0)
 	{
+	  /* Stop if we see more than one GNU property note which may
+	     be generated by the older linker.  */
+	  if (l->l_cet != lc_unknown)
+	    return;
+
+	  /* Check CET status now.  */
+	  l->l_cet = lc_none;
+
 	  /* Check for invalid property.  */
 	  if (note->n_descsz < 8
 	      || (note->n_descsz % sizeof (ElfW(Addr))) != 0)
-	    break;
+	    return;
 
 	  /* Start and end of property array.  */
 	  unsigned char *ptr = (unsigned char *) (note + 1) + 4;
@@ -78,9 +93,15 @@  _dl_process_cet_property_note (struct link_map *l,
 	      unsigned int type = *(unsigned int *) ptr;
 	      unsigned int datasz = *(unsigned int *) (ptr + 4);
 
+	      /* Property type must be in ascending order.  */
+	      if (type < last_type)
+		return;
+
 	      ptr += 8;
 	      if ((ptr + datasz) > ptr_end)
-		break;
+		return;
+
+	      last_type = type;
 
 	      if (type == GNU_PROPERTY_X86_FEATURE_1_AND)
 		{
@@ -89,14 +110,18 @@  _dl_process_cet_property_note (struct link_map *l,
 		     we stop the search regardless if its size is correct
 		     or not.  There is no point to continue if this note
 		     is ill-formed.  */
-		  if (datasz == 4)
-		    {
-		      unsigned int feature_1 = *(unsigned int *) ptr;
-		      if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
-			l->l_cet |= lc_ibt;
-		      if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
-			l->l_cet |= lc_shstk;
-		    }
+		  if (datasz != 4)
+		    return;
+
+		  feature_1 = *(unsigned int *) ptr;
+
+		  /* Keep searching for the next GNU property note
+		     generated by the older linker.  */
+		  break;
+		}
+	      else if (type > GNU_PROPERTY_X86_FEATURE_1_AND)
+		{
+		  /* Stop since property type is in ascending order.  */
 		  return;
 		}
 
@@ -112,6 +137,12 @@  _dl_process_cet_property_note (struct link_map *l,
 	      + ELF_NOTE_NEXT_OFFSET (note->n_namesz, note->n_descsz,
 				      align));
     }
+
+  /* We get here only if there is one or no GNU property note.  */
+  if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_IBT))
+    l->l_cet |= lc_ibt;
+  if ((feature_1 & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
+    l->l_cet |= lc_shstk;
 #endif
 }
 
diff --git a/sysdeps/x86/link_map.h b/sysdeps/x86/link_map.h
index ef1206a9d2..9367ed0889 100644
--- a/sysdeps/x86/link_map.h
+++ b/sysdeps/x86/link_map.h
@@ -19,8 +19,9 @@ 
 /* If this object is enabled with CET.  */
 enum
   {
-    lc_none = 0,			 /* Not enabled with CET.  */
-    lc_ibt = 1 << 0,			 /* Enabled with IBT.  */
-    lc_shstk = 1 << 1,			 /* Enabled with STSHK.  */
+    lc_unknown = 0,			 /* Unknown CET status.  */
+    lc_none = 1 << 0,			 /* Not enabled with CET.  */
+    lc_ibt = 1 << 1,			 /* Enabled with IBT.  */
+    lc_shstk = 1 << 2,			 /* Enabled with STSHK.  */
     lc_ibt_and_shstk = lc_ibt | lc_shstk /* Enabled with both.  */
-  } l_cet:2;
+  } l_cet:3;
-- 
2.19.1