PING: V3 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]

  On Tue, Mar 10, 2020 at 06:36:54AM -0700, H.J. Lu wrote:
> On Tue, Mar 10, 2020 at 02:13:37PM +0100, Florian Weimer wrote:
> > * H. J. Lu:
> > 
> > >> I think the error message should say *where* indirect branch tracking
> > >> is not enabled.  I assume this is a property of the loaded object.
> > >
> > > Fixed.   Now I got
> > >
> > > .... libvirtd[1048]: internal error: Failed to load module
> > > '/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so':
> > > /usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't
> > > enabled: Invalid argument
> > 
> > What I meant is that the error message should say that the object
> > needs to be rebuilt with IBT/SHSTK support.  In the above, it's
> > unclear whether the process or the object has to enable IBT.
> > 
> > (The Invalid Argument part should also be suppressed.)
> 
> Like this?  The diff against V2 patch is:
> 
> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> index f527a1414c..b2843488be 100644
> --- a/sysdeps/x86/dl-cet.c
> +++ b/sysdeps/x86/dl-cet.c
> @@ -142,10 +142,10 @@ dl_cet_check (struct link_map *m, const char *program)
>  		  /* When IBT is enabled, we cannot dlopen a shared
>  		     object without IBT.  */
>  		  if (found_ibt_legacy)
> -		    _dl_signal_error (EINVAL,
> +		    _dl_signal_error (0,
>  				      m->l_initfini[ibt_legacy]->l_name,
>  				      "dlopen",
> -				      N_("indirect branch tracking isn't enabled"));
> +				      N_("rebuilt with IBT support needed"));
>  		}
>  
>  	      if (enable_shstk_type != CET_PERMISSIVE)
> @@ -153,10 +153,10 @@ dl_cet_check (struct link_map *m, const char *program)
>  		  /* When SHSTK is enabled, we cannot dlopen a shared
>  		     object without SHSTK.  */
>  		  if (found_shstk_legacy)
> -		    _dl_signal_error (EINVAL,
> +		    _dl_signal_error (0,
>  				      m->l_initfini[shstk_legacy]->l_name,
>  				      "dlopen",
> -				      N_("shadow stack isn't enabled"));
> +				      N_("rebuilt with SHSTK support needed"));
>  		}
>  
>  	      if (enable_ibt_type != CET_PERMISSIVE
> 

PING.

H.J.
---
Since legacy bitmap doesn't cover jitted code generated by legacy JIT
engine, it isn't very useful.  This patch removes ARCH_CET_LEGACY_BITMAP
and treats indirect branch tracking similar to shadow stack by removing
legacy bitmap support.

	* sysdeps/unix/sysv/linux/x86/dl-cet.h
	(dl_cet_allocate_legacy_bitmap): Removed.
	* sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
	(ARCH_CET_LEGACY_BITMAP): Removed.
	* sysdeps/x86/Makefile (tests): Add tst-cet-legacy-7.
	(CFLAGS-tst-cet-legacy-mod-5a.c): Changed to
	-fcf-protection=branch.
	(CFLAGS-tst-cet-legacy-mod-6a.c): Likewise.
	(CFLAGS-tst-cet-legacy-7.c): New.
	* sysdeps/x86/dl-cet.c (dl_cet_mark_legacy_region): Removed.
	(dl_cet_check): Remove legacy bitmap support.  Don't allow
	dlopening legacy shared library when IBT is enabled.  Set
	feature_1 if IBT or SHSTK is enabled in executable.
	* sysdeps/x86/dl-procruntime.c (_dl_x86_legacy_bitmap): Removed.
	* sysdeps/x86/tst-cet-legacy-4.c: Include <string.h> and
	<support/check.h>.
	(do_test): Check indirect branch tracking error.  Use
	FAIL_EXIT1.
	* sysdeps/x86/tst-cet-legacy-7.c: New file.
---
 sysdeps/unix/sysv/linux/x86/dl-cet.h          |  20 --
 .../unix/sysv/linux/x86/include/asm/prctl.h   |   5 -
 sysdeps/x86/Makefile                          |   7 +-
 sysdeps/x86/dl-cet.c                          | 217 +++++-------------
 sysdeps/x86/dl-procruntime.c                  |  11 -
 sysdeps/x86/tst-cet-legacy-4.c                |  19 +-
 sysdeps/x86/tst-cet-legacy-5.c                |   2 +-
 sysdeps/x86/tst-cet-legacy-6.c                |   2 +-
 sysdeps/x86/tst-cet-legacy-7.c                |  36 +++
 9 files changed, 111 insertions(+), 208 deletions(-)
 create mode 100644 sysdeps/x86/tst-cet-legacy-7.c
  

On Fri, Mar 13, 2020 at 6:25 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> On Tue, Mar 10, 2020 at 06:36:54AM -0700, H.J. Lu wrote:
> > On Tue, Mar 10, 2020 at 02:13:37PM +0100, Florian Weimer wrote:
> > > * H. J. Lu:
> > >
> > > >> I think the error message should say *where* indirect branch tracking
> > > >> is not enabled.  I assume this is a property of the loaded object.
> > > >
> > > > Fixed.   Now I got
> > > >
> > > > .... libvirtd[1048]: internal error: Failed to load module
> > > > '/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so':
> > > > /usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't
> > > > enabled: Invalid argument
> > >
> > > What I meant is that the error message should say that the object
> > > needs to be rebuilt with IBT/SHSTK support.  In the above, it's
> > > unclear whether the process or the object has to enable IBT.
> > >
> > > (The Invalid Argument part should also be suppressed.)
> >
> > Like this?  The diff against V2 patch is:
> >
> > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> > index f527a1414c..b2843488be 100644
> > --- a/sysdeps/x86/dl-cet.c
> > +++ b/sysdeps/x86/dl-cet.c
> > @@ -142,10 +142,10 @@ dl_cet_check (struct link_map *m, const char *program)
> >                 /* When IBT is enabled, we cannot dlopen a shared
> >                    object without IBT.  */
> >                 if (found_ibt_legacy)
> > -                 _dl_signal_error (EINVAL,
> > +                 _dl_signal_error (0,
> >                                     m->l_initfini[ibt_legacy]->l_name,
> >                                     "dlopen",
> > -                                   N_("indirect branch tracking isn't enabled"));
> > +                                   N_("rebuilt with IBT support needed"));
> >               }
> >
> >             if (enable_shstk_type != CET_PERMISSIVE)
> > @@ -153,10 +153,10 @@ dl_cet_check (struct link_map *m, const char *program)
> >                 /* When SHSTK is enabled, we cannot dlopen a shared
> >                    object without SHSTK.  */
> >                 if (found_shstk_legacy)
> > -                 _dl_signal_error (EINVAL,
> > +                 _dl_signal_error (0,
> >                                     m->l_initfini[shstk_legacy]->l_name,
> >                                     "dlopen",
> > -                                   N_("shadow stack isn't enabled"));
> > +                                   N_("rebuilt with SHSTK support needed"));
> >               }
> >
> >             if (enable_ibt_type != CET_PERMISSIVE
> >
>
> PING.
>

PING:

https://sourceware.org/pipermail/libc-alpha/2020-March/111873.html
  

On 3/13/20 9:25 AM, H.J. Lu via Libc-alpha wrote:
> On Tue, Mar 10, 2020 at 06:36:54AM -0700, H.J. Lu wrote:
>> On Tue, Mar 10, 2020 at 02:13:37PM +0100, Florian Weimer wrote:
>>> * H. J. Lu:
>>>
>>>>> I think the error message should say *where* indirect branch tracking
>>>>> is not enabled.  I assume this is a property of the loaded object.
>>>> Fixed.   Now I got
>>>>
>>>> .... libvirtd[1048]: internal error: Failed to load module
>>>> '/usr/lib64/libvirt/storage-backend/libvirt_storage_backend_rbd.so':
>>>> /usr/lib64/ceph/libceph-common.so.0: indirect branch tracking isn't
>>>> enabled: Invalid argument
>>> What I meant is that the error message should say that the object
>>> needs to be rebuilt with IBT/SHSTK support.  In the above, it's
>>> unclear whether the process or the object has to enable IBT.
>>>
>>> (The Invalid Argument part should also be suppressed.)
>> Like this?  The diff against V2 patch is:
>>
>> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
>> index f527a1414c..b2843488be 100644
>> --- a/sysdeps/x86/dl-cet.c
>> +++ b/sysdeps/x86/dl-cet.c
>> @@ -142,10 +142,10 @@ dl_cet_check (struct link_map *m, const char *program)
>>  		  /* When IBT is enabled, we cannot dlopen a shared
>>  		     object without IBT.  */
>>  		  if (found_ibt_legacy)
>> -		    _dl_signal_error (EINVAL,
>> +		    _dl_signal_error (0,
>>  				      m->l_initfini[ibt_legacy]->l_name,
>>  				      "dlopen",
>> -				      N_("indirect branch tracking isn't enabled"));
>> +				      N_("rebuilt with IBT support needed"));
>>  		}
>>  
>>  	      if (enable_shstk_type != CET_PERMISSIVE)
>> @@ -153,10 +153,10 @@ dl_cet_check (struct link_map *m, const char *program)
>>  		  /* When SHSTK is enabled, we cannot dlopen a shared
>>  		     object without SHSTK.  */
>>  		  if (found_shstk_legacy)
>> -		    _dl_signal_error (EINVAL,
>> +		    _dl_signal_error (0,
>>  				      m->l_initfini[shstk_legacy]->l_name,
>>  				      "dlopen",
>> -				      N_("shadow stack isn't enabled"));
>> +				      N_("rebuilt with SHSTK support needed"));
>>  		}
>>  
>>  	      if (enable_ibt_type != CET_PERMISSIVE
>>
> PING.

Please post v4 with adjusted error message, and expanded comment in new test,
and I'll review that quickly.

> H.J.
> ---
> Since legacy bitmap doesn't cover jitted code generated by legacy JIT
> engine, it isn't very useful.  This patch removes ARCH_CET_LEGACY_BITMAP
> and treats indirect branch tracking similar to shadow stack by removing
> legacy bitmap support.

OK.


> 
> 	* sysdeps/unix/sysv/linux/x86/dl-cet.h
> 	(dl_cet_allocate_legacy_bitmap): Removed.
> 	* sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
> 	(ARCH_CET_LEGACY_BITMAP): Removed.
> 	* sysdeps/x86/Makefile (tests): Add tst-cet-legacy-7.
> 	(CFLAGS-tst-cet-legacy-mod-5a.c): Changed to
> 	-fcf-protection=branch.
> 	(CFLAGS-tst-cet-legacy-mod-6a.c): Likewise.
> 	(CFLAGS-tst-cet-legacy-7.c): New.
> 	* sysdeps/x86/dl-cet.c (dl_cet_mark_legacy_region): Removed.
> 	(dl_cet_check): Remove legacy bitmap support.  Don't allow
> 	dlopening legacy shared library when IBT is enabled.  Set
> 	feature_1 if IBT or SHSTK is enabled in executable.
> 	* sysdeps/x86/dl-procruntime.c (_dl_x86_legacy_bitmap): Removed.
> 	* sysdeps/x86/tst-cet-legacy-4.c: Include <string.h> and
> 	<support/check.h>.
> 	(do_test): Check indirect branch tracking error.  Use
> 	FAIL_EXIT1.
> 	* sysdeps/x86/tst-cet-legacy-7.c: New file.

Hopefully you don't plan to include this in the commit log :-)

> ---
>  sysdeps/unix/sysv/linux/x86/dl-cet.h          |  20 --
>  .../unix/sysv/linux/x86/include/asm/prctl.h   |   5 -
>  sysdeps/x86/Makefile                          |   7 +-
>  sysdeps/x86/dl-cet.c                          | 217 +++++-------------
>  sysdeps/x86/dl-procruntime.c                  |  11 -
>  sysdeps/x86/tst-cet-legacy-4.c                |  19 +-
>  sysdeps/x86/tst-cet-legacy-5.c                |   2 +-
>  sysdeps/x86/tst-cet-legacy-6.c                |   2 +-
>  sysdeps/x86/tst-cet-legacy-7.c                |  36 +++
>  9 files changed, 111 insertions(+), 208 deletions(-)
>  create mode 100644 sysdeps/x86/tst-cet-legacy-7.c
> 
> diff --git a/sysdeps/unix/sysv/linux/x86/dl-cet.h b/sysdeps/unix/sysv/linux/x86/dl-cet.h
> index 9b2aaa238c..ae97a433a2 100644
> --- a/sysdeps/unix/sysv/linux/x86/dl-cet.h
> +++ b/sysdeps/unix/sysv/linux/x86/dl-cet.h
> @@ -18,26 +18,6 @@
>  #include <sys/prctl.h>
>  #include <asm/prctl.h>
>  
> -static inline int __attribute__ ((always_inline))
> -dl_cet_allocate_legacy_bitmap (unsigned long *legacy_bitmap)
> -{
> -  /* Allocate legacy bitmap.  */
> -#ifdef __LP64__
> -  return (int) INTERNAL_SYSCALL_CALL (arch_prctl,
> -				      ARCH_CET_LEGACY_BITMAP, legacy_bitmap);
> -#else
> -  unsigned long long legacy_bitmap_u64[2];
> -  int res = INTERNAL_SYSCALL_CALL (arch_prctl,
> -				   ARCH_CET_LEGACY_BITMAP, legacy_bitmap_u64);
> -  if (res == 0)
> -    {
> -      legacy_bitmap[0] = legacy_bitmap_u64[0];
> -      legacy_bitmap[1] = legacy_bitmap_u64[1];
> -    }
> -  return res;
> -#endif
> -}
> -

OK.

>  static inline int __attribute__ ((always_inline))
>  dl_cet_disable_cet (unsigned int cet_feature)
>  {
> diff --git a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
> index f67f3299b9..45ad0b052f 100644
> --- a/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
> +++ b/sysdeps/unix/sysv/linux/x86/include/asm/prctl.h
> @@ -24,9 +24,4 @@
>       OUT: allocated shadow stack address: *addr.
>   */
>  # define ARCH_CET_ALLOC_SHSTK	0x3004
> -/* Return legacy region bitmap info in unsigned long long *addr:
> -     address: addr[0].
> -     size: addr[1].
> - */
> -# define ARCH_CET_LEGACY_BITMAP	0x3005

OK.

>  #endif /* ARCH_CET_STATUS */
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index 95182a508c..ec96b59a78 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -20,7 +20,7 @@ sysdep-dl-routines += dl-cet
>  
>  tests += tst-cet-legacy-1 tst-cet-legacy-1a tst-cet-legacy-2 \
>  	 tst-cet-legacy-2a tst-cet-legacy-3 tst-cet-legacy-4 \
> -	 tst-cet-legacy-5a tst-cet-legacy-6a
> +	 tst-cet-legacy-5a tst-cet-legacy-6a tst-cet-legacy-7

OK.

>  tst-cet-legacy-1a-ARGS = -- $(host-test-program-cmd)
>  ifneq (no,$(have-tunables))
>  tests += tst-cet-legacy-4a tst-cet-legacy-4b tst-cet-legacy-4c \
> @@ -43,14 +43,15 @@ CFLAGS-tst-cet-legacy-4b.c += -fcf-protection
>  CFLAGS-tst-cet-legacy-mod-4.c += -fcf-protection=none
>  CFLAGS-tst-cet-legacy-5a.c += -fcf-protection
>  CFLAGS-tst-cet-legacy-5b.c += -fcf-protection
> -CFLAGS-tst-cet-legacy-mod-5a.c += -fcf-protection=none
> +CFLAGS-tst-cet-legacy-mod-5a.c += -fcf-protection=branch
>  CFLAGS-tst-cet-legacy-mod-5b.c += -fcf-protection
>  CFLAGS-tst-cet-legacy-mod-5c.c += -fcf-protection
>  CFLAGS-tst-cet-legacy-6a.c += -fcf-protection
>  CFLAGS-tst-cet-legacy-6b.c += -fcf-protection
> -CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=none
> +CFLAGS-tst-cet-legacy-mod-6a.c += -fcf-protection=branch
>  CFLAGS-tst-cet-legacy-mod-6b.c += -fcf-protection
>  CFLAGS-tst-cet-legacy-mod-6c.c += -fcf-protection
> +CFLAGS-tst-cet-legacy-7.c += -fcf-protection=none

OK.

>  
>  $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \
>  		       $(objpfx)tst-cet-legacy-mod-2.so
> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> index ca3b5849bc..b2843488be 100644
> --- a/sysdeps/x86/dl-cet.c
> +++ b/sysdeps/x86/dl-cet.c
> @@ -33,63 +33,6 @@
>  # error GNU_PROPERTY_X86_FEATURE_1_SHSTK != X86_FEATURE_1_SHSTK
>  #endif
>  
> -static int
> -dl_cet_mark_legacy_region (struct link_map *l)
> -{
> -  /* Mark PT_LOAD segments with PF_X in legacy code page bitmap.  */
> -  size_t i, phnum = l->l_phnum;
> -  const ElfW(Phdr) *phdr = l->l_phdr;
> -#ifdef __x86_64__
> -  typedef unsigned long long word_t;
> -#else
> -  typedef unsigned long word_t;
> -#endif
> -  unsigned int bits_to_set;
> -  word_t mask_to_set;
> -#define BITS_PER_WORD (sizeof (word_t) * 8)
> -#define BITMAP_FIRST_WORD_MASK(start) \
> -  (~((word_t) 0) << ((start) & (BITS_PER_WORD - 1)))
> -#define BITMAP_LAST_WORD_MASK(nbits) \
> -  (~((word_t) 0) >> (-(nbits) & (BITS_PER_WORD - 1)))
> -
> -  word_t *bitmap = (word_t *) GL(dl_x86_legacy_bitmap)[0];
> -  word_t bitmap_size = GL(dl_x86_legacy_bitmap)[1];
> -  word_t *p;
> -  size_t page_size = GLRO(dl_pagesize);
> -
> -  for (i = 0; i < phnum; i++)
> -    if (phdr[i].p_type == PT_LOAD && (phdr[i].p_flags & PF_X))
> -      {
> -	/* One bit in legacy bitmap represents a page.  */
> -	ElfW(Addr) start = (phdr[i].p_vaddr + l->l_addr) / page_size;
> -	ElfW(Addr) len = (phdr[i].p_memsz + page_size - 1) / page_size;
> -	ElfW(Addr) end = start + len;
> -
> -	if ((end / 8) > bitmap_size)
> -	  return -EINVAL;
> -
> -	p = bitmap + (start / BITS_PER_WORD);
> -	bits_to_set = BITS_PER_WORD - (start % BITS_PER_WORD);
> -	mask_to_set = BITMAP_FIRST_WORD_MASK (start);
> -
> -	while (len >= bits_to_set)
> -	  {
> -	    *p |= mask_to_set;
> -	    len -= bits_to_set;
> -	    bits_to_set = BITS_PER_WORD;
> -	    mask_to_set = ~((word_t) 0);
> -	    p++;
> -	  }
> -	if (len)
> -	  {
> -	    mask_to_set &= BITMAP_LAST_WORD_MASK (end);
> -	    *p |= mask_to_set;
> -	  }
> -      }
> -
> -  return 0;
> -}
> -

OK.

>  /* Check if object M is compatible with CET.  */
>  
>  static void
> @@ -117,6 +60,8 @@ dl_cet_check (struct link_map *m, const char *program)
>    if (ibt_enabled || shstk_enabled)
>      {
>        struct link_map *l = NULL;
> +      unsigned int ibt_legacy = 0, shstk_legacy = 0;
> +      bool found_ibt_legacy = false, found_shstk_legacy = false;

OK.

>  
>        /* Check if IBT and SHSTK are enabled in object.  */
>        bool enable_ibt = (ibt_enabled
> @@ -142,10 +87,7 @@ dl_cet_check (struct link_map *m, const char *program)
>  	 support IBT nor SHSTK.  */
>        if (enable_ibt || enable_shstk)
>  	{
> -	  int res;
>  	  unsigned int i;
> -	  unsigned int first_legacy, last_legacy;
> -	  bool need_legacy_bitmap = false;

OK.

>  
>  	  i = m->l_searchlist.r_nlist;
>  	  while (i-- > 0)
> @@ -167,91 +109,25 @@ dl_cet_check (struct link_map *m, const char *program)
>  		continue;
>  #endif
>  
> -	      if (enable_ibt
> -		  && enable_ibt_type != CET_ALWAYS_ON
> -		  && !(l->l_cet & lc_ibt))
> +	      /* IBT is enabled only if it is enabled in executable as
> +		 well as all shared objects.  */
> +	      enable_ibt &= (enable_ibt_type == CET_ALWAYS_ON
> +			     || (l->l_cet & lc_ibt) != 0);
> +	      if (!found_ibt_legacy && enable_ibt != ibt_enabled)
>  		{
> -		  /* Remember the first and last legacy objects.  */
> -		  if (!need_legacy_bitmap)
> -		    last_legacy = i;
> -		  first_legacy = i;
> -		  need_legacy_bitmap = true;
> +		  found_ibt_legacy = true;
> +		  ibt_legacy = i;

OK.

>  		}
>  
>  	      /* SHSTK is enabled only if it is enabled in executable as
>  		 well as all shared objects.  */
>  	      enable_shstk &= (enable_shstk_type == CET_ALWAYS_ON
>  			       || (l->l_cet & lc_shstk) != 0);
> -	    }
> -
> -	  if (need_legacy_bitmap)
> -	    {
> -	      if (GL(dl_x86_legacy_bitmap)[0])
> -		{
> -		  /* Change legacy bitmap to writable.  */
> -		  if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0],
> -				  GL(dl_x86_legacy_bitmap)[1],
> -				  PROT_READ | PROT_WRITE) < 0)
> -		    {
> -mprotect_failure:
> -		      if (program)
> -			_dl_fatal_printf ("%s: mprotect legacy bitmap failed\n",
> -					  l->l_name);
> -		      else
> -			_dl_signal_error (EINVAL, l->l_name, "dlopen",
> -					  N_("mprotect legacy bitmap failed"));
> -		    }
> -		}
> -	      else
> +	      if (enable_shstk != shstk_enabled)

OK.

>  		{
> -		  /* Allocate legacy bitmap.  */
> -		  int res = dl_cet_allocate_legacy_bitmap
> -		    (GL(dl_x86_legacy_bitmap));
> -		  if (res != 0)
> -		    {
> -		      if (program)
> -			_dl_fatal_printf ("%s: legacy bitmap isn't available\n",
> -					  l->l_name);
> -		      else
> -			_dl_signal_error (EINVAL, l->l_name, "dlopen",
> -					  N_("legacy bitmap isn't available"));
> -		    }
> +		  found_shstk_legacy = true;
> +		  shstk_legacy = i;

OK.

>  		}
> -
> -	      /* Put legacy shared objects in legacy bitmap.  */
> -	      for (i = first_legacy; i <= last_legacy; i++)
> -		{
> -		  l = m->l_initfini[i];
> -
> -		  if (l->l_init_called || (l->l_cet & lc_ibt))
> -		    continue;
> -
> -#ifdef SHARED
> -		  if (l == &GL(dl_rtld_map)
> -		      ||  l->l_real == &GL(dl_rtld_map)
> -		      || (program && l == m))
> -		    continue;
> -#endif
> -
> -		  /* If IBT is enabled in executable and IBT isn't enabled
> -		     in this shard object, mark PT_LOAD segments with PF_X
> -		     in legacy code page bitmap.  */
> -		  res = dl_cet_mark_legacy_region (l);
> -		  if (res != 0)
> -		    {
> -		      if (program)
> -			_dl_fatal_printf ("%s: failed to mark legacy code region\n",
> -					  l->l_name);
> -		      else
> -			_dl_signal_error (-res, l->l_name, "dlopen",
> -					  N_("failed to mark legacy code region"));
> -		    }
> -		}
> -
> -	      /* Change legacy bitmap to read-only.  */
> -	      if (__mprotect ((void *) GL(dl_x86_legacy_bitmap)[0],
> -			      GL(dl_x86_legacy_bitmap)[1], PROT_READ) < 0)
> -		goto mprotect_failure;

OK.

>  	    }
>  	}
>  
> @@ -259,23 +135,40 @@ mprotect_failure:
>  
>        if (enable_ibt != ibt_enabled || enable_shstk != shstk_enabled)
>  	{
> -	  if (!program
> -	      && enable_shstk_type != CET_PERMISSIVE)
> +	  if (!program)
>  	    {
> -	      /* When SHSTK is enabled, we can't dlopening a shared
> -		 object without SHSTK.  */
> -	      if (enable_shstk != shstk_enabled)
> -		_dl_signal_error (EINVAL, l->l_name, "dlopen",
> -				  N_("shadow stack isn't enabled"));
> -	      return;
> +	      if (enable_ibt_type != CET_PERMISSIVE)
> +		{
> +		  /* When IBT is enabled, we cannot dlopen a shared
> +		     object without IBT.  */
> +		  if (found_ibt_legacy)
> +		    _dl_signal_error (0,
> +				      m->l_initfini[ibt_legacy]->l_name,
> +				      "dlopen",
> +				      N_("rebuilt with IBT support needed"));
> +		}

Suggest:
"rebuild shared object with IBT support enabled"

> +
> +	      if (enable_shstk_type != CET_PERMISSIVE)
> +		{
> +		  /* When SHSTK is enabled, we cannot dlopen a shared
> +		     object without SHSTK.  */
> +		  if (found_shstk_legacy)
> +		    _dl_signal_error (0,
> +				      m->l_initfini[shstk_legacy]->l_name,
> +				      "dlopen",
> +				      N_("rebuilt with SHSTK support needed"));
> +		}

Suggest:
"rebuild shared object with SHSTK support enabled"

> +
> +	      if (enable_ibt_type != CET_PERMISSIVE
> +		  && enable_shstk_type != CET_PERMISSIVE)
> +		return;
>  	    }
>  
>  	  /* Disable IBT and/or SHSTK if they are enabled by kernel, but
>  	     disabled in executable or shared objects.  */
>  	  unsigned int cet_feature = 0;
>  
> -	  /* Disable IBT only during program startup.  */
> -	  if (program && !enable_ibt)
> +	  if (!enable_ibt)

OK.

>  	    cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
>  	  if (!enable_shstk)
>  	    cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
> @@ -286,8 +179,14 @@ mprotect_failure:
>  	      if (program)
>  		_dl_fatal_printf ("%s: can't disable CET\n", program);
>  	      else
> -		_dl_signal_error (-res, l->l_name, "dlopen",
> -				  N_("can't disable CET"));
> +		{
> +		  if (found_ibt_legacy)
> +		    l = m->l_initfini[ibt_legacy];
> +		  else
> +		    l = m->l_initfini[shstk_legacy];
> +		  _dl_signal_error (-res, l->l_name, "dlopen",
> +				    N_("can't disable CET"));

OK.

> +		}
>  	    }
>  
>  	  /* Clear the disabled bits in dl_x86_feature_1.  */
> @@ -297,17 +196,21 @@ mprotect_failure:
>  	}
>  
>  #ifdef SHARED
> -      if (program
> -	  && (!shstk_enabled
> -	      || enable_shstk_type != CET_PERMISSIVE)
> -	  && (ibt_enabled || shstk_enabled))
> +      if (program && (ibt_enabled || shstk_enabled))
>  	{
> -	  /* Lock CET if IBT or SHSTK is enabled in executable.  Don't
> -	     lock CET if SHSTK is enabled permissively.  */
> -	  int res = dl_cet_lock_cet ();
> -	  if (res != 0)
> -	    _dl_fatal_printf ("%s: can't lock CET\n", program);
> +	  if ((!ibt_enabled
> +	       || enable_ibt_type != CET_PERMISSIVE)
> +	      && (!shstk_enabled
> +		  || enable_shstk_type != CET_PERMISSIVE))
> +	    {
> +	      /* Lock CET if IBT or SHSTK is enabled in executable unless
> +	         IBT or SHSTK is enabled permissively.  */
> +	      int res = dl_cet_lock_cet ();
> +	      if (res != 0)
> +		_dl_fatal_printf ("%s: can't lock CET\n", program);

OK.

> +	    }
>  
> +	  /* Set feature_1 if IBT or SHSTK is enabled in executable.  */
>  	  cet_feature_changed = true;
>  	}
>  #endif
> diff --git a/sysdeps/x86/dl-procruntime.c b/sysdeps/x86/dl-procruntime.c
> index fb36801f3e..5e39a38133 100644
> --- a/sysdeps/x86/dl-procruntime.c
> +++ b/sysdeps/x86/dl-procruntime.c
> @@ -54,15 +54,4 @@ PROCINFO_CLASS unsigned int _dl_x86_feature_1[2]
>  # else
>  ,
>  # endif
> -
> -# if !defined PROCINFO_DECL && defined SHARED
> -  ._dl_x86_legacy_bitmap
> -# else
> -PROCINFO_CLASS unsigned long _dl_x86_legacy_bitmap[2]
> -# endif
> -# if !defined SHARED || defined PROCINFO_DECL
> -;
> -# else
> -,
> -# endif

OK.

>  #endif
> diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
> index a77078afc9..a922339333 100644
> --- a/sysdeps/x86/tst-cet-legacy-4.c
> +++ b/sysdeps/x86/tst-cet-legacy-4.c
> @@ -20,6 +20,9 @@
>  #include <dlfcn.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <string.h>
> +
> +#include <support/check.h>
>  
>  static int
>  do_test (void)
> @@ -31,22 +34,18 @@ do_test (void)
>    h = dlopen (modname, RTLD_LAZY);
>    if (h == NULL)
>      {
> -      printf ("cannot open '%s': %s\n", modname, dlerror ());
> -      exit (1);
> +      const char *err = dlerror ();
> +      if (!strstr (err, "rebuilt with IBT support needed"))

OK (needs adjustment if string is changed).

> +	FAIL_EXIT1 ("incorrect dlopen '%s' error: %s\n", modname, err);
> +      return 0;
>      }
>  
>    fp = dlsym (h, "test");
>    if (fp == NULL)
> -    {
> -      printf ("cannot get symbol 'test': %s\n", dlerror ());
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("cannot get symbol 'test': %s\n", dlerror ());
>  
>    if (fp () != 0)
> -    {
> -      puts ("test () != 0");
> -      exit (1);
> -    }
> +    FAIL_EXIT1 ("test () != 0");
>  
>    dlclose (h);
>  
> diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c
> index 6c9bba06f5..350c7e41df 100644
> --- a/sysdeps/x86/tst-cet-legacy-5.c
> +++ b/sysdeps/x86/tst-cet-legacy-5.c
> @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail)
>        if (fail)
>  	{
>  	  const char *err = dlerror ();
> -	  if (strstr (err, "shadow stack isn't enabled") == NULL)
> +	  if (strstr (err, "rebuilt with SHSTK support needed") == NULL)

OK (also needs adjustment)

>  	    {
>  	      printf ("incorrect dlopen '%s' error: %s\n", modname,
>  		      err);
> diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c
> index 877e77747d..9f2748fcb6 100644
> --- a/sysdeps/x86/tst-cet-legacy-6.c
> +++ b/sysdeps/x86/tst-cet-legacy-6.c
> @@ -35,7 +35,7 @@ do_test_1 (const char *modname, bool fail)
>        if (fail)
>  	{
>  	  const char *err = dlerror ();
> -	  if (strstr (err, "shadow stack isn't enabled") == NULL)
> +	  if (strstr (err, "rebuilt with SHSTK support needed") == NULL)

OK. (also needs adjustment)

>  	    {
>  	      printf ("incorrect dlopen '%s' error: %s\n", modname,
>  		      err);
> diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c
> new file mode 100644
> index 0000000000..cf4c2779ce
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-7.c
> @@ -0,0 +1,36 @@
> +/* Check compatibility of legacy executable with a JIT engine.
> +   Copyright (C) 2020 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/>.  */
> +
> +#include <stdio.h>
> +#include <sys/mman.h>
> +#include <support/xunistd.h>
> +

Needs a pargraph explaining what this test is supposed to do.

I expect this test is supposed to just pass and not fail because it's
run with -fcf-protection=none.

What about the equivalent test to test for failure?

> +static int
> +do_test (void)
> +{
> +  void (*funcp) (void);
> +  funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
> +		 MAP_ANONYMOUS | MAP_PRIVATE, -1);
> +  printf ("mmap = %p\n", funcp);
> +  /* Write RET instruction.  */
> +  *(char *) funcp = 0xc3;
> +  funcp ();
> +  return 0;

OK.

> +}
> +
> +#include <support/test-driver.c>
> -- 2.24.1