diff mbox series

V2 [PATCH] x86: Properly set usable CET feature bits [BZ #26625]

Message ID CAMe9rOo5xUnqKGWgDoLV5mCyh_U1MEHJn8cjyJqdx+FinJOOvQ@mail.gmail.com
State Committed
Headers
Series V2 [PATCH] x86: Properly set usable CET feature bits [BZ #26625] |

Commit Message

H.J. Lu Jan. 28, 2021, 6:57 p.m. UTC 
  On Thu, Jan 28, 2021 at 9:51 AM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 27/01/2021 09:27, H.J. Lu wrote:
> > commit 94cd37ebb293321115a36a422b091fdb72d2fb08
> > Author: H.J. Lu <hjl.tools@gmail.com>
> > Date:   Wed Sep 16 05:27:32 2020 -0700
> >
> >     x86: Use HAS_CPU_FEATURE with IBT and SHSTK [BZ #26625]
> >
> > broke
> >
> > GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> >
> > since it can no longer disable IBT nor SHSTK.  Handle IBT and SHSTK with:
> >
> > 1. Revert commit 94cd37ebb293321115a36a422b091fdb72d2fb08.
> > 2. Clears the usable CET feature bits if kernel doesn't support CET.
> > 3. Add GLIBC_TUNABLES tests without dlopen.
> > 4. Add tests to verify that CPU_FEATURE_USABLE on IBT and SHSTK matches
> > _get_ssp.
> > 5. Update GLIBC_TUNABLES tests with dlopen to verify that CET is disabled
> > with GLIBC_TUNABLES.
>
> Patch looks ok with some minot nit below, but I am kind worried it would
> require a lot of x86 testing where there are already results posted on
> release wiki.

I have tested the fixed glibc on i686, x32, x86-64 with --enable-cet
on Tiger Lake
and legacy CPU.  There are no differences in test results.

> Carlos and Florian, what do you think? Should we push it for the 2.33 release
> on backport on the release branch?

I prefer to fix it for glibc 2.33.  It has happened multiple times to
me when I need

$ export GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK

to update OS on Tiger Lake.

> > ---
> >  sysdeps/x86/Makefile                   |  8 ++++-
> >  sysdeps/x86/cpu-features.c             | 11 +++++--
> >  sysdeps/x86/dl-cet.c                   |  4 +--
> >  sysdeps/x86/tst-cet-legacy-10-static.c |  1 +
> >  sysdeps/x86/tst-cet-legacy-10.c        | 43 ++++++++++++++++++++++++++
> >  sysdeps/x86/tst-cet-legacy-5.c         | 11 ++++---
> >  sysdeps/x86/tst-cet-legacy-6.c         | 11 ++++---
> >  sysdeps/x86/tst-cet-legacy-9-static.c  |  1 +
> >  sysdeps/x86/tst-cet-legacy-9.c         | 41 ++++++++++++++++++++++++
> >  sysdeps/x86/tst-get-cpu-features.c     |  2 ++
> >  10 files changed, 120 insertions(+), 13 deletions(-)
> >  create mode 100644 sysdeps/x86/tst-cet-legacy-10-static.c
> >  create mode 100644 sysdeps/x86/tst-cet-legacy-10.c
> >  create mode 100644 sysdeps/x86/tst-cet-legacy-9-static.c
> >  create mode 100644 sysdeps/x86/tst-cet-legacy-9.c
> >
> > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> > index 7549507a9a..dd82674342 100644
> > --- a/sysdeps/x86/Makefile
> > +++ b/sysdeps/x86/Makefile
> > @@ -82,7 +82,9 @@ 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-7 \
> > -      tst-cet-legacy-8
> > +      tst-cet-legacy-8 tst-cet-legacy-9 tst-cet-legacy-9-static \
> > +      tst-cet-legacy-10 tst-cet-legacy-10-static
> > +tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static
>
> Do we need to stat the static variant to 'tests' rule as well?

I always add the static variants to 'tests'.

> >  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 \
> > @@ -123,6 +125,8 @@ 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
> >  CFLAGS-tst-cet-legacy-8.c += -mshstk
> > +CFLAGS-tst-cet-legacy-10.c += -mshstk
> > +CFLAGS-tst-cet-legacy-10-static.c += -mshstk
> >
> >  $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \
> >                      $(objpfx)tst-cet-legacy-mod-2.so
>
> Ok.
>
> > @@ -163,6 +167,8 @@ $(objpfx)tst-cet-legacy-6b: $(libdl)
> >  $(objpfx)tst-cet-legacy-6b.out: $(objpfx)tst-cet-legacy-mod-6a.so \
> >                               $(objpfx)tst-cet-legacy-mod-6b.so
> >  tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> > +tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> > +tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> >  endif
> >  endif
> >
>
> Ok.
>
> > diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> > index 6496512a0d..73b0a4dc9a 100644
> > --- a/sysdeps/x86/cpu-features.c
> > +++ b/sysdeps/x86/cpu-features.c
> > @@ -75,6 +75,7 @@ update_usable (struct cpu_features *cpu_features)
> >    CPU_FEATURE_SET_USABLE (cpu_features, PREFETCHWT1);
> >    CPU_FEATURE_SET_USABLE (cpu_features, OSPKE);
> >    CPU_FEATURE_SET_USABLE (cpu_features, WAITPKG);
> > +  CPU_FEATURE_SET_USABLE (cpu_features, SHSTK);
> >    CPU_FEATURE_SET_USABLE (cpu_features, GFNI);
> >    CPU_FEATURE_SET_USABLE (cpu_features, RDPID);
> >    CPU_FEATURE_SET_USABLE (cpu_features, RDRAND);
> > @@ -84,6 +85,7 @@ update_usable (struct cpu_features *cpu_features)
> >    CPU_FEATURE_SET_USABLE (cpu_features, FSRM);
> >    CPU_FEATURE_SET_USABLE (cpu_features, SERIALIZE);
> >    CPU_FEATURE_SET_USABLE (cpu_features, TSXLDTRK);
> > +  CPU_FEATURE_SET_USABLE (cpu_features, IBT);
> >    CPU_FEATURE_SET_USABLE (cpu_features, LAHF64_SAHF64);
> >    CPU_FEATURE_SET_USABLE (cpu_features, LZCNT);
> >    CPU_FEATURE_SET_USABLE (cpu_features, SSE4A);
>
> Ok.
>
> > @@ -705,6 +707,11 @@ no_cpuid:
> >    /* Check CET status.  */
> >    unsigned int cet_status = get_cet_status ();
> >
> > +  if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
> > +    CPU_FEATURE_UNSET (cpu_features, IBT)
> > +  if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
> > +    CPU_FEATURE_UNSET (cpu_features, SHSTK)
> > +
> >    if (cet_status)
> >      {
> >        GL(dl_x86_feature_1) = cet_status;
>
> Ok
>
> > @@ -720,9 +727,9 @@ no_cpuid:
> >            GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> >          */
> >         unsigned int cet_feature = 0;
> > -       if (!HAS_CPU_FEATURE (IBT))
> > +       if (!CPU_FEATURE_USABLE (IBT))
> >           cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
> > -       if (!HAS_CPU_FEATURE (SHSTK))
> > +       if (!CPU_FEATURE_USABLE (SHSTK))
> >           cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
> >
> >         if (cet_feature)
>
> Ok, it is changing the check from 'cpuid' field to 'usable'.
>
> > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> > index a63b9c7164..c74e577289 100644
> > --- a/sysdeps/x86/dl-cet.c
> > +++ b/sysdeps/x86/dl-cet.c
> > @@ -77,11 +77,11 @@ dl_cet_check (struct link_map *m, const char *program)
> >
> >            GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> >          */
> > -       enable_ibt &= (HAS_CPU_FEATURE (IBT)
> > +       enable_ibt &= (CPU_FEATURE_USABLE (IBT)
> >                        && (enable_ibt_type == cet_always_on
> >                            || (m->l_x86_feature_1_and
> >                                & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0));
> > -       enable_shstk &= (HAS_CPU_FEATURE (SHSTK)
> > +       enable_shstk &= (CPU_FEATURE_USABLE (SHSTK)
> >                          && (enable_shstk_type == cet_always_on
> >                              || (m->l_x86_feature_1_and
> >                                  & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0));
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-cet-legacy-10-static.c b/sysdeps/x86/tst-cet-legacy-10-static.c
> > new file mode 100644
> > index 0000000000..ecc1208e35
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-10-static.c
> > @@ -0,0 +1 @@
> > +#include "tst-cet-legacy-10.c"
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-cet-legacy-10.c b/sysdeps/x86/tst-cet-legacy-10.c
> > new file mode 100644
> > index 0000000000..a618557f45
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-10.c
> > @@ -0,0 +1,43 @@
> > +/* Check CPU_FEATURE_USABLE on IBT and SHSTK.
> > +   Copyright (C) 2021 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 <x86intrin.h>
> > +#include <sys/platform/x86.h>
> > +#include <support/test-driver.h>
> > +#include <support/xunistd.h>
> > +
> > +/* Check that CPU_FEATURE_USABLE on IBT and SHSTK matches _get_ssp.  */
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  if (_get_ssp () != 0)
> > +    {
> > +      if (CPU_FEATURE_USABLE (IBT) && CPU_FEATURE_USABLE (SHSTK))
> > +     return EXIT_SUCCESS;
> > +    }
> > +  else
> > +    {
> > +      if (!CPU_FEATURE_USABLE (IBT) && !CPU_FEATURE_USABLE (SHSTK))
> > +     return EXIT_SUCCESS;
> > +    }
> > +
> > +  return EXIT_FAILURE;
> > +}
> > +
> > +#include <support/test-driver.c>
>
> Ok. Checking on the _get_ssp documentation the idea is on hardware without CET
> support it returns 0, right?

Correct.

> > diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c
> > index e3efeb1f4e..d870de3eae 100644
> > --- a/sysdeps/x86/tst-cet-legacy-5.c
> > +++ b/sysdeps/x86/tst-cet-legacy-5.c
> > @@ -37,6 +37,12 @@ do_test_1 (const char *modname, bool fail)
> >    int (*fp) (void);
> >    void *h;
> >
> > +  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> > +     disabled, assuming IBT is also disabled.  */
> > +  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> > +  if (!cet_enabled)
> > +    fail = false;
> > +
> >    h = dlopen (modname, RTLD_LAZY);
> >    if (h == NULL)
> >      {
> > @@ -53,10 +59,7 @@ do_test_1 (const char *modname, bool fail)
> >        FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err);
> >      }
> >
> > -  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> > -     disabled, assuming IBT is also disabled.  */
> > -  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> > -  if (fail && cet_enabled)
> > +  if (fail)
> >      FAIL_EXIT1 ("dlopen should have failed\n");
> >
> >    fp = dlsym (h, "test");
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c
> > index 44b2ef5c7a..8e82ee2246 100644
> > --- a/sysdeps/x86/tst-cet-legacy-6.c
> > +++ b/sysdeps/x86/tst-cet-legacy-6.c
> > @@ -37,6 +37,12 @@ do_test_1 (const char *modname, bool fail)
> >    int (*fp) (void);
> >    void *h;
> >
> > +  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> > +     disabled, assuming IBT is also disabled.  */
> > +  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> > +  if (!cet_enabled)
> > +    fail = false;
> > +
> >    h = dlopen (modname, RTLD_LAZY);
> >    if (h == NULL)
> >      {
> > @@ -53,10 +59,7 @@ do_test_1 (const char *modname, bool fail)
> >        FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err);
> >      }
> >
> > -  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> > -     disabled, assuming IBT is also disabled.  */
> > -  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> > -  if (fail && cet_enabled)
> > +  if (fail)
> >      FAIL_EXIT1 ("dlopen should have failed\n");
> >
> >    fp = dlsym (h, "test");
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-cet-legacy-9-static.c b/sysdeps/x86/tst-cet-legacy-9-static.c
> > new file mode 100644
> > index 0000000000..f9a8518b99
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-9-static.c
> > @@ -0,0 +1 @@
> > +#include "tst-cet-legacy-9.c"
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-cet-legacy-9.c b/sysdeps/x86/tst-cet-legacy-9.c
> > new file mode 100644
> > index 0000000000..2b526c9055
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-9.c
> > @@ -0,0 +1,41 @@
> > +/* Check CET compatibility with legacy JIT engine via GLIBC_TUNABLES.
> > +   Copyright (C) 2021 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 <stdlib.h>
> > +#include <sys/mman.h>
> > +#include <support/test-driver.h>
> > +#include <support/xunistd.h>
> > +
> > +/* Check that mmapped legacy code won't trigger segfault with
> > +   -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK.  */
> > +
> > +static int
> > +do_test (void)
> > +{
> > +  void (*funcp) (void);
> > +  funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
> > +              MAP_ANONYMOUS | MAP_PRIVATE, -1);
>
> Although x86_64 only support 4k page currently (not counting large
> pages, but which requires an additional mmap flag anyway), I think it
> should use xsysconf (_SC_PAGESIZE) to be future-proof.

Fixed.

> > +  printf ("mmap = %p\n", funcp);
> > +  /* Write RET instruction.  */
> > +  *(char *) funcp = 0xc3;
> > +  funcp ();
> > +  return EXIT_SUCCESS;
> > +}
> > +
> > +#include <support/test-driver.c>
>
> Ok.
>
> > diff --git a/sysdeps/x86/tst-get-cpu-features.c b/sysdeps/x86/tst-get-cpu-features.c
> > index dcdb86bb93..b5e7f6e7b0 100644
> > --- a/sysdeps/x86/tst-get-cpu-features.c
> > +++ b/sysdeps/x86/tst-get-cpu-features.c
> > @@ -301,6 +301,7 @@ do_test (void)
> >    CHECK_CPU_FEATURE_USABLE (OSPKE);
> >    CHECK_CPU_FEATURE_USABLE (WAITPKG);
> >    CHECK_CPU_FEATURE_USABLE (AVX512_VBMI2);
> > +  CHECK_CPU_FEATURE_USABLE (SHSTK);
> >    CHECK_CPU_FEATURE_USABLE (GFNI);
> >    CHECK_CPU_FEATURE_USABLE (VAES);
> >    CHECK_CPU_FEATURE_USABLE (VPCLMULQDQ);
> > @@ -324,6 +325,7 @@ do_test (void)
> >    CHECK_CPU_FEATURE_USABLE (HYBRID);
> >    CHECK_CPU_FEATURE_USABLE (TSXLDTRK);
> >    CHECK_CPU_FEATURE_USABLE (PCONFIG);
> > +  CHECK_CPU_FEATURE_USABLE (IBT);
> >    CHECK_CPU_FEATURE_USABLE (AMX_BF16);
> >    CHECK_CPU_FEATURE_USABLE (AVX512_FP16);
> >    CHECK_CPU_FEATURE_USABLE (AMX_TILE);
> >
>
> Ok.+

Here is the updated patch.  OK for master?

Thanks/

Comments

Carlos O'Donell Jan. 29, 2021, 2:42 a.m. UTC | #1 
On 1/28/21 1:57 PM, H.J. Lu wrote:
> Here is the updated patch.  OK for master?

Yes.

I don't think this patch will have any impact on the x86_64 and i686 testing
that we've done on RHEL7, RHEL8, and versions under development. It should
improve our ability to test different scenarios on Tigerlake hardware. We
should include this in the release to support the hardware. The change is
fairly minor, it just looks like a lot because we have to make the change
in several places and add a regression test.

Reviewed-by: Carlos O'Donell <carlos@redhat.com>
 
> From 65a9ca9c20db7ffd61f3defb4083b513589c7231 Mon Sep 17 00:00:00 2001
> From: "H.J. Lu" <hjl.tools@gmail.com>
> Date: Tue, 26 Jan 2021 20:48:45 -0800
> Subject: [PATCH] x86: Properly set usable CET feature bits [BZ #26625]
> 
> commit 94cd37ebb293321115a36a422b091fdb72d2fb08
> Author: H.J. Lu <hjl.tools@gmail.com>
> Date:   Wed Sep 16 05:27:32 2020 -0700
> 
>     x86: Use HAS_CPU_FEATURE with IBT and SHSTK [BZ #26625]
> 
> broke
> 
> GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> 
> since it can no longer disable IBT nor SHSTK.  Handle IBT and SHSTK with:
> 
> 1. Revert commit 94cd37ebb293321115a36a422b091fdb72d2fb08.
> 2. Clears the usable CET feature bits if kernel doesn't support CET.
> 3. Add GLIBC_TUNABLES tests without dlopen.
> 4. Add tests to verify that CPU_FEATURE_USABLE on IBT and SHSTK matches
> _get_ssp.
> 5. Update GLIBC_TUNABLES tests with dlopen to verify that CET is disabled
> with GLIBC_TUNABLES.
> ---
>  sysdeps/x86/Makefile                   |  8 ++++-
>  sysdeps/x86/cpu-features.c             | 11 +++++--
>  sysdeps/x86/dl-cet.c                   |  4 +--
>  sysdeps/x86/tst-cet-legacy-10-static.c |  1 +
>  sysdeps/x86/tst-cet-legacy-10.c        | 43 ++++++++++++++++++++++++++
>  sysdeps/x86/tst-cet-legacy-5.c         | 11 ++++---
>  sysdeps/x86/tst-cet-legacy-6.c         | 11 ++++---
>  sysdeps/x86/tst-cet-legacy-9-static.c  |  1 +
>  sysdeps/x86/tst-cet-legacy-9.c         | 42 +++++++++++++++++++++++++
>  sysdeps/x86/tst-get-cpu-features.c     |  2 ++
>  10 files changed, 121 insertions(+), 13 deletions(-)
>  create mode 100644 sysdeps/x86/tst-cet-legacy-10-static.c
>  create mode 100644 sysdeps/x86/tst-cet-legacy-10.c
>  create mode 100644 sysdeps/x86/tst-cet-legacy-9-static.c
>  create mode 100644 sysdeps/x86/tst-cet-legacy-9.c
> 
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index 7549507a9a..dd82674342 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -82,7 +82,9 @@ 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-7 \
> -	 tst-cet-legacy-8
> +	 tst-cet-legacy-8 tst-cet-legacy-9 tst-cet-legacy-9-static \
> +	 tst-cet-legacy-10 tst-cet-legacy-10-static

OK. Add 2 new testes with static variants.

> +tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static

OK. List static variants.

>  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 \
> @@ -123,6 +125,8 @@ 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
>  CFLAGS-tst-cet-legacy-8.c += -mshstk
> +CFLAGS-tst-cet-legacy-10.c += -mshstk
> +CFLAGS-tst-cet-legacy-10-static.c += -mshstk

OK. Build the 10th test with shadow stack enabled explicitly (maybe
the current toolchain doesn't do that).

>  
>  $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \
>  		       $(objpfx)tst-cet-legacy-mod-2.so
> @@ -163,6 +167,8 @@ $(objpfx)tst-cet-legacy-6b: $(libdl)
>  $(objpfx)tst-cet-legacy-6b.out: $(objpfx)tst-cet-legacy-mod-6a.so \
>  				$(objpfx)tst-cet-legacy-mod-6b.so
>  tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> +tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
> +tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK

OK. Run a test with IBT and SHSTK disabled by tunables.

>  endif
>  endif
>  
> diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
> index 6496512a0d..73b0a4dc9a 100644
> --- a/sysdeps/x86/cpu-features.c
> +++ b/sysdeps/x86/cpu-features.c
> @@ -75,6 +75,7 @@ update_usable (struct cpu_features *cpu_features)
>    CPU_FEATURE_SET_USABLE (cpu_features, PREFETCHWT1);
>    CPU_FEATURE_SET_USABLE (cpu_features, OSPKE);
>    CPU_FEATURE_SET_USABLE (cpu_features, WAITPKG);
> +  CPU_FEATURE_SET_USABLE (cpu_features, SHSTK);

OK. Copy SHSTK bit from cpuid.

>    CPU_FEATURE_SET_USABLE (cpu_features, GFNI);
>    CPU_FEATURE_SET_USABLE (cpu_features, RDPID);
>    CPU_FEATURE_SET_USABLE (cpu_features, RDRAND);
> @@ -84,6 +85,7 @@ update_usable (struct cpu_features *cpu_features)
>    CPU_FEATURE_SET_USABLE (cpu_features, FSRM);
>    CPU_FEATURE_SET_USABLE (cpu_features, SERIALIZE);
>    CPU_FEATURE_SET_USABLE (cpu_features, TSXLDTRK);
> +  CPU_FEATURE_SET_USABLE (cpu_features, IBT);

OK. Copy IBT bit from cpuid.

>    CPU_FEATURE_SET_USABLE (cpu_features, LAHF64_SAHF64);
>    CPU_FEATURE_SET_USABLE (cpu_features, LZCNT);
>    CPU_FEATURE_SET_USABLE (cpu_features, SSE4A);
> @@ -705,6 +707,11 @@ no_cpuid:
>    /* Check CET status.  */
>    unsigned int cet_status = get_cet_status ();
>  
> +  if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
> +    CPU_FEATURE_UNSET (cpu_features, IBT)
> +  if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
> +    CPU_FEATURE_UNSET (cpu_features, SHSTK)

OK. Disable the bit in cpu_features if process has CET disabled
(uses prctl ARCH_CET_STATUS).

> +
>    if (cet_status)
>      {
>        GL(dl_x86_feature_1) = cet_status;
> @@ -720,9 +727,9 @@ no_cpuid:
>  	     GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>  	   */
>  	  unsigned int cet_feature = 0;
> -	  if (!HAS_CPU_FEATURE (IBT))
> +	  if (!CPU_FEATURE_USABLE (IBT))

OK. Use CPU_FEATURE_USABLE to check IBT bit because we are
consistently using CPU_FEATURE_* macros. The dyanmic loader
will have recorded tunable preferences in GLRO(dl_x86_cpu_features)
and we check them here.

>  	    cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
> -	  if (!HAS_CPU_FEATURE (SHSTK))
> +	  if (!CPU_FEATURE_USABLE (SHSTK))

OK.

>  	    cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
>  
>  	  if (cet_feature)
> diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> index a63b9c7164..c74e577289 100644
> --- a/sysdeps/x86/dl-cet.c
> +++ b/sysdeps/x86/dl-cet.c
> @@ -77,11 +77,11 @@ dl_cet_check (struct link_map *m, const char *program)
>  
>  	     GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
>  	   */
> -	  enable_ibt &= (HAS_CPU_FEATURE (IBT)
> +	  enable_ibt &= (CPU_FEATURE_USABLE (IBT)

OK.

>  			 && (enable_ibt_type == cet_always_on
>  			     || (m->l_x86_feature_1_and
>  				 & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0));
> -	  enable_shstk &= (HAS_CPU_FEATURE (SHSTK)
> +	  enable_shstk &= (CPU_FEATURE_USABLE (SHSTK)

OK.

>  			   && (enable_shstk_type == cet_always_on
>  			       || (m->l_x86_feature_1_and
>  				   & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0));
> diff --git a/sysdeps/x86/tst-cet-legacy-10-static.c b/sysdeps/x86/tst-cet-legacy-10-static.c
> new file mode 100644
> index 0000000000..ecc1208e35
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-10-static.c
> @@ -0,0 +1 @@
> +#include "tst-cet-legacy-10.c"

OK.

> diff --git a/sysdeps/x86/tst-cet-legacy-10.c b/sysdeps/x86/tst-cet-legacy-10.c
> new file mode 100644
> index 0000000000..a618557f45
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-10.c
> @@ -0,0 +1,43 @@
> +/* Check CPU_FEATURE_USABLE on IBT and SHSTK.
> +   Copyright (C) 2021 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 <x86intrin.h>
> +#include <sys/platform/x86.h>
> +#include <support/test-driver.h>
> +#include <support/xunistd.h>
> +
> +/* Check that CPU_FEATURE_USABLE on IBT and SHSTK matches _get_ssp.  */
> +
> +static int
> +do_test (void)
> +{
> +  if (_get_ssp () != 0)
> +    {
> +      if (CPU_FEATURE_USABLE (IBT) && CPU_FEATURE_USABLE (SHSTK))
> +	return EXIT_SUCCESS;
> +    }
> +  else
> +    {
> +      if (!CPU_FEATURE_USABLE (IBT) && !CPU_FEATURE_USABLE (SHSTK))
> +	return EXIT_SUCCESS;
> +    }
> +
> +  return EXIT_FAILURE;
> +}

OK. Test looks good and uses _get_ssp (CET intrinsic) from the compiler
to test what glibc sees matches what the hardware reports.

> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c
> index e3efeb1f4e..d870de3eae 100644
> --- a/sysdeps/x86/tst-cet-legacy-5.c
> +++ b/sysdeps/x86/tst-cet-legacy-5.c
> @@ -37,6 +37,12 @@ do_test_1 (const char *modname, bool fail)
>    int (*fp) (void);
>    void *h;
>  
> +  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> +     disabled, assuming IBT is also disabled.  */
> +  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> +  if (!cet_enabled)
> +    fail = false;

OK. Move *AHEAD* of use of fail in h==NULL case.

> +
>    h = dlopen (modname, RTLD_LAZY);
>    if (h == NULL)
>      {
> @@ -53,10 +59,7 @@ do_test_1 (const char *modname, bool fail)
>        FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err);
>      }
>  
> -  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> -     disabled, assuming IBT is also disabled.  */
> -  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> -  if (fail && cet_enabled)
> +  if (fail)

OK.

>      FAIL_EXIT1 ("dlopen should have failed\n");
>  
>    fp = dlsym (h, "test");
> diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c
> index 44b2ef5c7a..8e82ee2246 100644
> --- a/sysdeps/x86/tst-cet-legacy-6.c
> +++ b/sysdeps/x86/tst-cet-legacy-6.c
> @@ -37,6 +37,12 @@ do_test_1 (const char *modname, bool fail)
>    int (*fp) (void);
>    void *h;
>  
> +  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> +     disabled, assuming IBT is also disabled.  */
> +  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> +  if (!cet_enabled)
> +    fail = false;

OK. Move fail setting ahead of use.

> +
>    h = dlopen (modname, RTLD_LAZY);
>    if (h == NULL)
>      {
> @@ -53,10 +59,7 @@ do_test_1 (const char *modname, bool fail)
>        FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err);
>      }
>  
> -  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
> -     disabled, assuming IBT is also disabled.  */
> -  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
> -  if (fail && cet_enabled)
> +  if (fail)

OK.

>      FAIL_EXIT1 ("dlopen should have failed\n");
>  
>    fp = dlsym (h, "test");
> diff --git a/sysdeps/x86/tst-cet-legacy-9-static.c b/sysdeps/x86/tst-cet-legacy-9-static.c
> new file mode 100644
> index 0000000000..f9a8518b99
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-9-static.c
> @@ -0,0 +1 @@
> +#include "tst-cet-legacy-9.c"

OK.

> diff --git a/sysdeps/x86/tst-cet-legacy-9.c b/sysdeps/x86/tst-cet-legacy-9.c
> new file mode 100644
> index 0000000000..fe4d8e73c8
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-9.c
> @@ -0,0 +1,42 @@
> +/* Check CET compatibility with legacy JIT engine via GLIBC_TUNABLES.
> +   Copyright (C) 2021 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 <stdlib.h>
> +#include <sys/mman.h>
> +#include <support/test-driver.h>
> +#include <support/xunistd.h>
> +
> +/* Check that mmapped legacy code won't trigger segfault with
> +   -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK.  */
> +
> +static int
> +do_test (void)
> +{
> +  void (*funcp) (void);
> +  size_t page_size = xsysconf (_SC_PAGESIZE);
> +  funcp = xmmap (NULL, page_size, PROT_EXEC | PROT_READ | PROT_WRITE,
> +		 MAP_ANONYMOUS | MAP_PRIVATE, -1);
> +  printf ("mmap = %p\n", funcp);
> +  /* Write RET instruction.  */
> +  *(char *) funcp = 0xc3;
> +  funcp ();
> +  return EXIT_SUCCESS;

OK.

> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/x86/tst-get-cpu-features.c b/sysdeps/x86/tst-get-cpu-features.c
> index dcdb86bb93..b5e7f6e7b0 100644
> --- a/sysdeps/x86/tst-get-cpu-features.c
> +++ b/sysdeps/x86/tst-get-cpu-features.c
> @@ -301,6 +301,7 @@ do_test (void)
>    CHECK_CPU_FEATURE_USABLE (OSPKE);
>    CHECK_CPU_FEATURE_USABLE (WAITPKG);
>    CHECK_CPU_FEATURE_USABLE (AVX512_VBMI2);
> +  CHECK_CPU_FEATURE_USABLE (SHSTK);

OK.

>    CHECK_CPU_FEATURE_USABLE (GFNI);
>    CHECK_CPU_FEATURE_USABLE (VAES);
>    CHECK_CPU_FEATURE_USABLE (VPCLMULQDQ);
> @@ -324,6 +325,7 @@ do_test (void)
>    CHECK_CPU_FEATURE_USABLE (HYBRID);
>    CHECK_CPU_FEATURE_USABLE (TSXLDTRK);
>    CHECK_CPU_FEATURE_USABLE (PCONFIG);
> +  CHECK_CPU_FEATURE_USABLE (IBT);

OK.

>    CHECK_CPU_FEATURE_USABLE (AMX_BF16);
>    CHECK_CPU_FEATURE_USABLE (AVX512_FP16);
>    CHECK_CPU_FEATURE_USABLE (AMX_TILE);
> -- 
> 2.29.2
>
Adhemerval Zanella Jan. 29, 2021, 11:45 a.m. UTC | #2 
On 28/01/2021 23:42, Carlos O'Donell wrote:
> On 1/28/21 1:57 PM, H.J. Lu wrote:
>> Here is the updated patch.  OK for master?
> 
> Yes.
> 
> I don't think this patch will have any impact on the x86_64 and i686 testing
> that we've done on RHEL7, RHEL8, and versions under development. It should
> improve our ability to test different scenarios on Tigerlake hardware. We
> should include this in the release to support the hardware. The change is
> fairly minor, it just looks like a lot because we have to make the change
> in several places and add a regression test.
> 
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>

Thanks, this patch is ok for 2.33.
diff mbox series

Patch

From 65a9ca9c20db7ffd61f3defb4083b513589c7231 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 26 Jan 2021 20:48:45 -0800
Subject: [PATCH] x86: Properly set usable CET feature bits [BZ #26625]

commit 94cd37ebb293321115a36a422b091fdb72d2fb08
Author: H.J. Lu <hjl.tools@gmail.com>
Date:   Wed Sep 16 05:27:32 2020 -0700

    x86: Use HAS_CPU_FEATURE with IBT and SHSTK [BZ #26625]

broke

GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK

since it can no longer disable IBT nor SHSTK.  Handle IBT and SHSTK with:

1. Revert commit 94cd37ebb293321115a36a422b091fdb72d2fb08.
2. Clears the usable CET feature bits if kernel doesn't support CET.
3. Add GLIBC_TUNABLES tests without dlopen.
4. Add tests to verify that CPU_FEATURE_USABLE on IBT and SHSTK matches
_get_ssp.
5. Update GLIBC_TUNABLES tests with dlopen to verify that CET is disabled
with GLIBC_TUNABLES.
---
 sysdeps/x86/Makefile                   |  8 ++++-
 sysdeps/x86/cpu-features.c             | 11 +++++--
 sysdeps/x86/dl-cet.c                   |  4 +--
 sysdeps/x86/tst-cet-legacy-10-static.c |  1 +
 sysdeps/x86/tst-cet-legacy-10.c        | 43 ++++++++++++++++++++++++++
 sysdeps/x86/tst-cet-legacy-5.c         | 11 ++++---
 sysdeps/x86/tst-cet-legacy-6.c         | 11 ++++---
 sysdeps/x86/tst-cet-legacy-9-static.c  |  1 +
 sysdeps/x86/tst-cet-legacy-9.c         | 42 +++++++++++++++++++++++++
 sysdeps/x86/tst-get-cpu-features.c     |  2 ++
 10 files changed, 121 insertions(+), 13 deletions(-)
 create mode 100644 sysdeps/x86/tst-cet-legacy-10-static.c
 create mode 100644 sysdeps/x86/tst-cet-legacy-10.c
 create mode 100644 sysdeps/x86/tst-cet-legacy-9-static.c
 create mode 100644 sysdeps/x86/tst-cet-legacy-9.c

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 7549507a9a..dd82674342 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -82,7 +82,9 @@  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-7 \
-	 tst-cet-legacy-8
+	 tst-cet-legacy-8 tst-cet-legacy-9 tst-cet-legacy-9-static \
+	 tst-cet-legacy-10 tst-cet-legacy-10-static
+tests-static += tst-cet-legacy-9-static tst-cet-legacy-10-static
 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 \
@@ -123,6 +125,8 @@  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
 CFLAGS-tst-cet-legacy-8.c += -mshstk
+CFLAGS-tst-cet-legacy-10.c += -mshstk
+CFLAGS-tst-cet-legacy-10-static.c += -mshstk
 
 $(objpfx)tst-cet-legacy-1: $(objpfx)tst-cet-legacy-mod-1.so \
 		       $(objpfx)tst-cet-legacy-mod-2.so
@@ -163,6 +167,8 @@  $(objpfx)tst-cet-legacy-6b: $(libdl)
 $(objpfx)tst-cet-legacy-6b.out: $(objpfx)tst-cet-legacy-mod-6a.so \
 				$(objpfx)tst-cet-legacy-mod-6b.so
 tst-cet-legacy-6b-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
+tst-cet-legacy-9-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
+tst-cet-legacy-9-static-ENV = GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
 endif
 endif
 
diff --git a/sysdeps/x86/cpu-features.c b/sysdeps/x86/cpu-features.c
index 6496512a0d..73b0a4dc9a 100644
--- a/sysdeps/x86/cpu-features.c
+++ b/sysdeps/x86/cpu-features.c
@@ -75,6 +75,7 @@  update_usable (struct cpu_features *cpu_features)
   CPU_FEATURE_SET_USABLE (cpu_features, PREFETCHWT1);
   CPU_FEATURE_SET_USABLE (cpu_features, OSPKE);
   CPU_FEATURE_SET_USABLE (cpu_features, WAITPKG);
+  CPU_FEATURE_SET_USABLE (cpu_features, SHSTK);
   CPU_FEATURE_SET_USABLE (cpu_features, GFNI);
   CPU_FEATURE_SET_USABLE (cpu_features, RDPID);
   CPU_FEATURE_SET_USABLE (cpu_features, RDRAND);
@@ -84,6 +85,7 @@  update_usable (struct cpu_features *cpu_features)
   CPU_FEATURE_SET_USABLE (cpu_features, FSRM);
   CPU_FEATURE_SET_USABLE (cpu_features, SERIALIZE);
   CPU_FEATURE_SET_USABLE (cpu_features, TSXLDTRK);
+  CPU_FEATURE_SET_USABLE (cpu_features, IBT);
   CPU_FEATURE_SET_USABLE (cpu_features, LAHF64_SAHF64);
   CPU_FEATURE_SET_USABLE (cpu_features, LZCNT);
   CPU_FEATURE_SET_USABLE (cpu_features, SSE4A);
@@ -705,6 +707,11 @@  no_cpuid:
   /* Check CET status.  */
   unsigned int cet_status = get_cet_status ();
 
+  if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_IBT) == 0)
+    CPU_FEATURE_UNSET (cpu_features, IBT)
+  if ((cet_status & GNU_PROPERTY_X86_FEATURE_1_SHSTK) == 0)
+    CPU_FEATURE_UNSET (cpu_features, SHSTK)
+
   if (cet_status)
     {
       GL(dl_x86_feature_1) = cet_status;
@@ -720,9 +727,9 @@  no_cpuid:
 	     GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
 	   */
 	  unsigned int cet_feature = 0;
-	  if (!HAS_CPU_FEATURE (IBT))
+	  if (!CPU_FEATURE_USABLE (IBT))
 	    cet_feature |= GNU_PROPERTY_X86_FEATURE_1_IBT;
-	  if (!HAS_CPU_FEATURE (SHSTK))
+	  if (!CPU_FEATURE_USABLE (SHSTK))
 	    cet_feature |= GNU_PROPERTY_X86_FEATURE_1_SHSTK;
 
 	  if (cet_feature)
diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
index a63b9c7164..c74e577289 100644
--- a/sysdeps/x86/dl-cet.c
+++ b/sysdeps/x86/dl-cet.c
@@ -77,11 +77,11 @@  dl_cet_check (struct link_map *m, const char *program)
 
 	     GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK
 	   */
-	  enable_ibt &= (HAS_CPU_FEATURE (IBT)
+	  enable_ibt &= (CPU_FEATURE_USABLE (IBT)
 			 && (enable_ibt_type == cet_always_on
 			     || (m->l_x86_feature_1_and
 				 & GNU_PROPERTY_X86_FEATURE_1_IBT) != 0));
-	  enable_shstk &= (HAS_CPU_FEATURE (SHSTK)
+	  enable_shstk &= (CPU_FEATURE_USABLE (SHSTK)
 			   && (enable_shstk_type == cet_always_on
 			       || (m->l_x86_feature_1_and
 				   & GNU_PROPERTY_X86_FEATURE_1_SHSTK) != 0));
diff --git a/sysdeps/x86/tst-cet-legacy-10-static.c b/sysdeps/x86/tst-cet-legacy-10-static.c
new file mode 100644
index 0000000000..ecc1208e35
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-10-static.c
@@ -0,0 +1 @@ 
+#include "tst-cet-legacy-10.c"
diff --git a/sysdeps/x86/tst-cet-legacy-10.c b/sysdeps/x86/tst-cet-legacy-10.c
new file mode 100644
index 0000000000..a618557f45
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-10.c
@@ -0,0 +1,43 @@ 
+/* Check CPU_FEATURE_USABLE on IBT and SHSTK.
+   Copyright (C) 2021 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 <x86intrin.h>
+#include <sys/platform/x86.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+
+/* Check that CPU_FEATURE_USABLE on IBT and SHSTK matches _get_ssp.  */
+
+static int
+do_test (void)
+{
+  if (_get_ssp () != 0)
+    {
+      if (CPU_FEATURE_USABLE (IBT) && CPU_FEATURE_USABLE (SHSTK))
+	return EXIT_SUCCESS;
+    }
+  else
+    {
+      if (!CPU_FEATURE_USABLE (IBT) && !CPU_FEATURE_USABLE (SHSTK))
+	return EXIT_SUCCESS;
+    }
+
+  return EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-cet-legacy-5.c b/sysdeps/x86/tst-cet-legacy-5.c
index e3efeb1f4e..d870de3eae 100644
--- a/sysdeps/x86/tst-cet-legacy-5.c
+++ b/sysdeps/x86/tst-cet-legacy-5.c
@@ -37,6 +37,12 @@  do_test_1 (const char *modname, bool fail)
   int (*fp) (void);
   void *h;
 
+  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
+     disabled, assuming IBT is also disabled.  */
+  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
+  if (!cet_enabled)
+    fail = false;
+
   h = dlopen (modname, RTLD_LAZY);
   if (h == NULL)
     {
@@ -53,10 +59,7 @@  do_test_1 (const char *modname, bool fail)
       FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err);
     }
 
-  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
-     disabled, assuming IBT is also disabled.  */
-  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
-  if (fail && cet_enabled)
+  if (fail)
     FAIL_EXIT1 ("dlopen should have failed\n");
 
   fp = dlsym (h, "test");
diff --git a/sysdeps/x86/tst-cet-legacy-6.c b/sysdeps/x86/tst-cet-legacy-6.c
index 44b2ef5c7a..8e82ee2246 100644
--- a/sysdeps/x86/tst-cet-legacy-6.c
+++ b/sysdeps/x86/tst-cet-legacy-6.c
@@ -37,6 +37,12 @@  do_test_1 (const char *modname, bool fail)
   int (*fp) (void);
   void *h;
 
+  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
+     disabled, assuming IBT is also disabled.  */
+  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
+  if (!cet_enabled)
+    fail = false;
+
   h = dlopen (modname, RTLD_LAZY);
   if (h == NULL)
     {
@@ -53,10 +59,7 @@  do_test_1 (const char *modname, bool fail)
       FAIL_EXIT1 ("cannot open '%s': %s\n", modname, err);
     }
 
-  /* NB: dlopen should never fail on non-CET platforms.  If SHSTK is
-     disabled, assuming IBT is also disabled.  */
-  bool cet_enabled = _get_ssp () != 0 && !CET_MAYBE_DISABLED;
-  if (fail && cet_enabled)
+  if (fail)
     FAIL_EXIT1 ("dlopen should have failed\n");
 
   fp = dlsym (h, "test");
diff --git a/sysdeps/x86/tst-cet-legacy-9-static.c b/sysdeps/x86/tst-cet-legacy-9-static.c
new file mode 100644
index 0000000000..f9a8518b99
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-9-static.c
@@ -0,0 +1 @@ 
+#include "tst-cet-legacy-9.c"
diff --git a/sysdeps/x86/tst-cet-legacy-9.c b/sysdeps/x86/tst-cet-legacy-9.c
new file mode 100644
index 0000000000..fe4d8e73c8
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-9.c
@@ -0,0 +1,42 @@ 
+/* Check CET compatibility with legacy JIT engine via GLIBC_TUNABLES.
+   Copyright (C) 2021 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 <stdlib.h>
+#include <sys/mman.h>
+#include <support/test-driver.h>
+#include <support/xunistd.h>
+
+/* Check that mmapped legacy code won't trigger segfault with
+   -fcf-protection and GLIBC_TUNABLES=glibc.cpu.hwcaps=-IBT,-SHSTK.  */
+
+static int
+do_test (void)
+{
+  void (*funcp) (void);
+  size_t page_size = xsysconf (_SC_PAGESIZE);
+  funcp = xmmap (NULL, page_size, PROT_EXEC | PROT_READ | PROT_WRITE,
+		 MAP_ANONYMOUS | MAP_PRIVATE, -1);
+  printf ("mmap = %p\n", funcp);
+  /* Write RET instruction.  */
+  *(char *) funcp = 0xc3;
+  funcp ();
+  return EXIT_SUCCESS;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-get-cpu-features.c b/sysdeps/x86/tst-get-cpu-features.c
index dcdb86bb93..b5e7f6e7b0 100644
--- a/sysdeps/x86/tst-get-cpu-features.c
+++ b/sysdeps/x86/tst-get-cpu-features.c
@@ -301,6 +301,7 @@  do_test (void)
   CHECK_CPU_FEATURE_USABLE (OSPKE);
   CHECK_CPU_FEATURE_USABLE (WAITPKG);
   CHECK_CPU_FEATURE_USABLE (AVX512_VBMI2);
+  CHECK_CPU_FEATURE_USABLE (SHSTK);
   CHECK_CPU_FEATURE_USABLE (GFNI);
   CHECK_CPU_FEATURE_USABLE (VAES);
   CHECK_CPU_FEATURE_USABLE (VPCLMULQDQ);
@@ -324,6 +325,7 @@  do_test (void)
   CHECK_CPU_FEATURE_USABLE (HYBRID);
   CHECK_CPU_FEATURE_USABLE (TSXLDTRK);
   CHECK_CPU_FEATURE_USABLE (PCONFIG);
+  CHECK_CPU_FEATURE_USABLE (IBT);
   CHECK_CPU_FEATURE_USABLE (AMX_BF16);
   CHECK_CPU_FEATURE_USABLE (AVX512_FP16);
   CHECK_CPU_FEATURE_USABLE (AMX_TILE);
-- 
2.29.2