Patchwork i386: Use _dl_runtime_[resolve|profile]_shstk for SHSTK [BZ #23716]

login
register
mail settings
Submitter H.J. Lu
Date Sept. 26, 2018, 5:17 p.m.
Message ID <20180926171711.29435-1-hjl.tools@gmail.com>
Download mbox | patch
Permalink /patch/29554/
State New
Headers show

Comments

H.J. Lu - Sept. 26, 2018, 5:17 p.m.
When elf_machine_runtime_setup is called to set up resolver, it should
use _dl_runtime_resolve_shstk or _dl_runtime_profile_shstk if SHSTK is
enabled by kernel.

Tested on i686 with and without --enable-cet using the following patch:
Florian Weimer - Sept. 26, 2018, 5:23 p.m.
* H. J. Lu:

> diff --git a/sysdeps/i386/dl-trampoline.S b/sysdeps/i386/dl-trampoline.S
> index 6dc0319216..9734f9c981 100644
> --- a/sysdeps/i386/dl-trampoline.S
> +++ b/sysdeps/i386/dl-trampoline.S
> @@ -33,6 +33,7 @@
>  _dl_runtime_resolve:
>  	cfi_adjust_cfa_offset (8)
>  	_CET_ENDBR
> +	hlt
>  	pushl %eax		# Preserve registers otherwise clobbered.
>  	cfi_adjust_cfa_offset (4)
>  	pushl %ecx

That doesn't look right. 8-)

Florian
H.J. Lu - Sept. 26, 2018, 5:30 p.m.
On Wed, Sep 26, 2018 at 10:23 AM, Florian Weimer <fweimer@redhat.com> wrote:
> * H. J. Lu:
>
>> diff --git a/sysdeps/i386/dl-trampoline.S b/sysdeps/i386/dl-trampoline.S
>> index 6dc0319216..9734f9c981 100644
>> --- a/sysdeps/i386/dl-trampoline.S
>> +++ b/sysdeps/i386/dl-trampoline.S
>> @@ -33,6 +33,7 @@
>>  _dl_runtime_resolve:
>>       cfi_adjust_cfa_offset (8)
>>       _CET_ENDBR
>> +     hlt
>>       pushl %eax              # Preserve registers otherwise clobbered.
>>       cfi_adjust_cfa_offset (4)
>>       pushl %ecx
>
> That doesn't look right. 8-)
>

This is the change I used to test my fix to verify that the SHSTK resolver
is used if SHSTK is enabled by kernel.  It isn't the part of the fix.
H.J. Lu - Sept. 28, 2018, 2:33 p.m.
On Wed, Sep 26, 2018 at 10:30 AM, H.J. Lu <hjl.tools@gmail.com> wrote:
> On Wed, Sep 26, 2018 at 10:23 AM, Florian Weimer <fweimer@redhat.com> wrote:
>> * H. J. Lu:
>>
>>> diff --git a/sysdeps/i386/dl-trampoline.S b/sysdeps/i386/dl-trampoline.S
>>> index 6dc0319216..9734f9c981 100644
>>> --- a/sysdeps/i386/dl-trampoline.S
>>> +++ b/sysdeps/i386/dl-trampoline.S
>>> @@ -33,6 +33,7 @@
>>>  _dl_runtime_resolve:
>>>       cfi_adjust_cfa_offset (8)
>>>       _CET_ENDBR
>>> +     hlt
>>>       pushl %eax              # Preserve registers otherwise clobbered.
>>>       cfi_adjust_cfa_offset (4)
>>>       pushl %ecx
>>
>> That doesn't look right. 8-)
>>
>
> This is the change I used to test my fix to verify that the SHSTK resolver
> is used if SHSTK is enabled by kernel.  It isn't the part of the fix.
>

We verified that the fix worked on CET simulator.  If there is no objection,
I will check it later today.

Thanks.

Patch

diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
index f6cfb90e21..d4a737ba0d 100644
--- a/sysdeps/i386/dl-machine.h
+++ b/sysdeps/i386/dl-machine.h
@@ -73,6 +73,7 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
   bool shstk_enabled
     = (GL(dl_x86_feature_1)[0] & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0;

+  shstk_enabled = true;
   if (l->l_info[DT_JMPREL] && lazy)
     {
       /* The GOT entries for functions in the PLT have not yet been filled
diff --git a/sysdeps/i386/dl-trampoline.S b/sysdeps/i386/dl-trampoline.S
index 6dc0319216..9734f9c981 100644
--- a/sysdeps/i386/dl-trampoline.S
+++ b/sysdeps/i386/dl-trampoline.S
@@ -33,6 +33,7 @@ 
 _dl_runtime_resolve:
 	cfi_adjust_cfa_offset (8)
 	_CET_ENDBR
+	hlt
 	pushl %eax		# Preserve registers otherwise clobbered.
 	cfi_adjust_cfa_offset (4)
 	pushl %ecx
@@ -130,6 +131,7 @@  _dl_runtime_profile_shstk:
 _dl_runtime_profile:
 	cfi_adjust_cfa_offset (8)
 	_CET_ENDBR
+	hlt
 	pushl %esp
 	cfi_adjust_cfa_offset (4)
 	addl $8, (%esp)		# Account for the pushed PLT data

	[BZ #23716]
	* sysdeps/i386/dl-cet.c: Removed.
	* sysdeps/i386/dl-machine.h (_dl_runtime_resolve_shstk): New
	prototype.
	(_dl_runtime_profile_shstk): Likewise.
	(elf_machine_runtime_setup): Use _dl_runtime_profile_shstk or
	_dl_runtime_resolve_shstk if SHSTK is enabled by kernel.

Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
---
 sysdeps/i386/dl-cet.c     | 67 ---------------------------------------
 sysdeps/i386/dl-machine.h | 13 ++++++--
 2 files changed, 11 insertions(+), 69 deletions(-)
 delete mode 100644 sysdeps/i386/dl-cet.c

diff --git a/sysdeps/i386/dl-cet.c b/sysdeps/i386/dl-cet.c
deleted file mode 100644
index 5d9a4e8d51..0000000000
--- a/sysdeps/i386/dl-cet.c
+++ /dev/null
@@ -1,67 +0,0 @@ 
-/* Linux/i386 CET initializers function.
-   Copyright (C) 2018 Free Software Foundation, Inc.
-
-   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
-   <http://www.gnu.org/licenses/>.  */
-
-
-#define LINKAGE static inline
-#define _dl_cet_check cet_check
-#include <sysdeps/x86/dl-cet.c>
-#undef _dl_cet_check
-
-#ifdef SHARED
-void
-_dl_cet_check (struct link_map *main_map, const char *program)
-{
-  cet_check (main_map, program);
-
-  if ((GL(dl_x86_feature_1)[0] & GNU_PROPERTY_X86_FEATURE_1_SHSTK))
-    {
-      /* Replace _dl_runtime_resolve and _dl_runtime_profile with
-         _dl_runtime_resolve_shstk and _dl_runtime_profile_shstk,
-	 respectively if SHSTK is enabled.  */
-      extern void _dl_runtime_resolve (Elf32_Word) attribute_hidden;
-      extern void _dl_runtime_resolve_shstk (Elf32_Word) attribute_hidden;
-      extern void _dl_runtime_profile (Elf32_Word) attribute_hidden;
-      extern void _dl_runtime_profile_shstk (Elf32_Word) attribute_hidden;
-      unsigned int i;
-      struct link_map *l;
-      Elf32_Addr *got;
-
-      if (main_map->l_info[DT_JMPREL])
-	{
-	  got = (Elf32_Addr *) D_PTR (main_map, l_info[DT_PLTGOT]);
-	  if (got[2] == (Elf32_Addr) &_dl_runtime_resolve)
-	    got[2] = (Elf32_Addr) &_dl_runtime_resolve_shstk;
-	  else if (got[2] == (Elf32_Addr) &_dl_runtime_profile)
-	    got[2] = (Elf32_Addr) &_dl_runtime_profile_shstk;
-	}
-
-      i = main_map->l_searchlist.r_nlist;
-      while (i-- > 0)
-	{
-	  l = main_map->l_initfini[i];
-	  if (l->l_info[DT_JMPREL])
-	    {
-	      got = (Elf32_Addr *) D_PTR (l, l_info[DT_PLTGOT]);
-	      if (got[2] == (Elf32_Addr) &_dl_runtime_resolve)
-		got[2] = (Elf32_Addr) &_dl_runtime_resolve_shstk;
-	      else if (got[2] == (Elf32_Addr) &_dl_runtime_profile)
-		got[2] = (Elf32_Addr) &_dl_runtime_profile_shstk;
-	    }
-	}
-    }
-}
-#endif
diff --git a/sysdeps/i386/dl-machine.h b/sysdeps/i386/dl-machine.h
index 1afdcbd9ea..f6cfb90e21 100644
--- a/sysdeps/i386/dl-machine.h
+++ b/sysdeps/i386/dl-machine.h
@@ -67,6 +67,11 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
   Elf32_Addr *got;
   extern void _dl_runtime_resolve (Elf32_Word) attribute_hidden;
   extern void _dl_runtime_profile (Elf32_Word) attribute_hidden;
+  extern void _dl_runtime_resolve_shstk (Elf32_Word) attribute_hidden;
+  extern void _dl_runtime_profile_shstk (Elf32_Word) attribute_hidden;
+  /* Check if SHSTK is enabled by kernel.  */
+  bool shstk_enabled
+    = (GL(dl_x86_feature_1)[0] & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0;
 
   if (l->l_info[DT_JMPREL] && lazy)
     {
@@ -93,7 +98,9 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
 	 end in this function.  */
       if (__glibc_unlikely (profile))
 	{
-	  got[2] = (Elf32_Addr) &_dl_runtime_profile;
+	  got[2] = (shstk_enabled
+		    ? (Elf32_Addr) &_dl_runtime_profile_shstk
+		    : (Elf32_Addr) &_dl_runtime_profile);
 
 	  if (GLRO(dl_profile) != NULL
 	      && _dl_name_match_p (GLRO(dl_profile), l))
@@ -104,7 +111,9 @@  elf_machine_runtime_setup (struct link_map *l, int lazy, int profile)
       else
 	/* This function will get called to fix up the GOT entry indicated by
 	   the offset on the stack, and then jump to the resolved address.  */
-	got[2] = (Elf32_Addr) &_dl_runtime_resolve;
+	got[2] = (shstk_enabled
+		  ? (Elf32_Addr) &_dl_runtime_resolve_shstk
+		  : (Elf32_Addr) &_dl_runtime_resolve);
     }
 
   return lazy;