[v2,03/16] x86/cet: Don't assume that SHSTK implies IBT

Message ID 20231216165325.2584919-4-hjl.tools@gmail.com
State Committed
Commit 442983319ba70de801fc856e8dd4748fba8f7f1b
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

Commit Message

H.J. Lu Dec. 16, 2023, 4:53 p.m. UTC
  Since shadow stack (SHSTK) is enabled in the Linux kernel without
enabling indirect branch tracking (IBT), don't assume that SHSTK
implies IBT.  Use "CPU_FEATURE_ACTIVE (IBT)" to check if IBT is active
and "CPU_FEATURE_ACTIVE (SHSTK)" to check if SHSTK is active.
---
 sysdeps/x86/Makefile            |  1 -
 sysdeps/x86/tst-cet-legacy-10.c |  6 +++---
 sysdeps/x86/tst-cet-legacy-8.c  | 15 ++++++++-------
 3 files changed, 11 insertions(+), 11 deletions(-)
  

Comments

H.J. Lu Dec. 17, 2023, 10:28 p.m. UTC | #1
On Sat, Dec 16, 2023 at 8:53 AM H.J. Lu <hjl.tools@gmail.com> wrote:
>
> Since shadow stack (SHSTK) is enabled in the Linux kernel without
> enabling indirect branch tracking (IBT), don't assume that SHSTK
> implies IBT.  Use "CPU_FEATURE_ACTIVE (IBT)" to check if IBT is active
> and "CPU_FEATURE_ACTIVE (SHSTK)" to check if SHSTK is active.
> ---
>  sysdeps/x86/Makefile            |  1 -
>  sysdeps/x86/tst-cet-legacy-10.c |  6 +++---
>  sysdeps/x86/tst-cet-legacy-8.c  | 15 ++++++++-------
>  3 files changed, 11 insertions(+), 11 deletions(-)
>
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index 5631a59a26..3d936ed537 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -209,7 +209,6 @@ 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
> -CFLAGS-tst-cet-legacy-8.c += -mshstk
>  CFLAGS-tst-cet-legacy-10.c += -mshstk
>  CFLAGS-tst-cet-legacy-10-static.c += -mshstk
>
> diff --git a/sysdeps/x86/tst-cet-legacy-10.c b/sysdeps/x86/tst-cet-legacy-10.c
> index a85cdc3171..ae2c34de3e 100644
> --- a/sysdeps/x86/tst-cet-legacy-10.c
> +++ b/sysdeps/x86/tst-cet-legacy-10.c
> @@ -21,19 +21,19 @@
>  #include <support/test-driver.h>
>  #include <support/xunistd.h>
>
> -/* Check that CPU_FEATURE_ACTIVE on IBT and SHSTK matches _get_ssp.  */
> +/* Check that CPU_FEATURE_ACTIVE on SHSTK matches _get_ssp.  */
>
>  static int
>  do_test (void)
>  {
>    if (_get_ssp () != 0)
>      {
> -      if (CPU_FEATURE_ACTIVE (IBT) && CPU_FEATURE_ACTIVE (SHSTK))
> +      if (CPU_FEATURE_ACTIVE (SHSTK))
>         return EXIT_SUCCESS;
>      }
>    else
>      {
> -      if (!CPU_FEATURE_ACTIVE (IBT) && !CPU_FEATURE_ACTIVE (SHSTK))
> +      if (!CPU_FEATURE_ACTIVE (SHSTK))
>         return EXIT_SUCCESS;
>      }
>
> diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
> index 5d8d9ba7dc..77d77a5408 100644
> --- a/sysdeps/x86/tst-cet-legacy-8.c
> +++ b/sysdeps/x86/tst-cet-legacy-8.c
> @@ -18,7 +18,7 @@
>
>  #include <stdio.h>
>  #include <stdlib.h>
> -#include <x86intrin.h>
> +#include <sys/platform/x86.h>
>  #include <sys/mman.h>
>  #include <support/test-driver.h>
>  #include <support/xsignal.h>
> @@ -29,11 +29,6 @@
>  static int
>  do_test (void)
>  {
> -  /* NB: This test should trigger SIGSEGV on CET platforms.  If SHSTK
> -     is disabled, assuming IBT is also disabled.  */
> -  if (_get_ssp () == 0)
> -    return EXIT_UNSUPPORTED;
> -
>    void (*funcp) (void);
>    funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
>                  MAP_ANONYMOUS | MAP_PRIVATE, -1);
> @@ -41,8 +36,14 @@ do_test (void)
>    /* Write RET instruction.  */
>    *(char *) funcp = 0xc3;
>    funcp ();
> +
> +  /* NB: This test should trigger SIGSEGV when IBT is active.  We should
> +     reach here if IBT isn't active.  */
> +  if (!CPU_FEATURE_ACTIVE (IBT))
> +    return EXIT_UNSUPPORTED;
> +
>    return EXIT_FAILURE;
>  }
>
> -#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV)
> +#define EXPECTED_SIGNAL (CPU_FEATURE_ACTIVE (IBT) ? SIGSEGV : 0)
>  #include <support/test-driver.c>
> --
> 2.43.0
>

I will check it in tomorrow if there is no objection.
  

Patch

diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 5631a59a26..3d936ed537 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -209,7 +209,6 @@  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
-CFLAGS-tst-cet-legacy-8.c += -mshstk
 CFLAGS-tst-cet-legacy-10.c += -mshstk
 CFLAGS-tst-cet-legacy-10-static.c += -mshstk
 
diff --git a/sysdeps/x86/tst-cet-legacy-10.c b/sysdeps/x86/tst-cet-legacy-10.c
index a85cdc3171..ae2c34de3e 100644
--- a/sysdeps/x86/tst-cet-legacy-10.c
+++ b/sysdeps/x86/tst-cet-legacy-10.c
@@ -21,19 +21,19 @@ 
 #include <support/test-driver.h>
 #include <support/xunistd.h>
 
-/* Check that CPU_FEATURE_ACTIVE on IBT and SHSTK matches _get_ssp.  */
+/* Check that CPU_FEATURE_ACTIVE on SHSTK matches _get_ssp.  */
 
 static int
 do_test (void)
 {
   if (_get_ssp () != 0)
     {
-      if (CPU_FEATURE_ACTIVE (IBT) && CPU_FEATURE_ACTIVE (SHSTK))
+      if (CPU_FEATURE_ACTIVE (SHSTK))
 	return EXIT_SUCCESS;
     }
   else
     {
-      if (!CPU_FEATURE_ACTIVE (IBT) && !CPU_FEATURE_ACTIVE (SHSTK))
+      if (!CPU_FEATURE_ACTIVE (SHSTK))
 	return EXIT_SUCCESS;
     }
 
diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
index 5d8d9ba7dc..77d77a5408 100644
--- a/sysdeps/x86/tst-cet-legacy-8.c
+++ b/sysdeps/x86/tst-cet-legacy-8.c
@@ -18,7 +18,7 @@ 
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <x86intrin.h>
+#include <sys/platform/x86.h>
 #include <sys/mman.h>
 #include <support/test-driver.h>
 #include <support/xsignal.h>
@@ -29,11 +29,6 @@ 
 static int
 do_test (void)
 {
-  /* NB: This test should trigger SIGSEGV on CET platforms.  If SHSTK
-     is disabled, assuming IBT is also disabled.  */
-  if (_get_ssp () == 0)
-    return EXIT_UNSUPPORTED;
-
   void (*funcp) (void);
   funcp = xmmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
 		 MAP_ANONYMOUS | MAP_PRIVATE, -1);
@@ -41,8 +36,14 @@  do_test (void)
   /* Write RET instruction.  */
   *(char *) funcp = 0xc3;
   funcp ();
+
+  /* NB: This test should trigger SIGSEGV when IBT is active.  We should
+     reach here if IBT isn't active.  */
+  if (!CPU_FEATURE_ACTIVE (IBT))
+    return EXIT_UNSUPPORTED;
+
   return EXIT_FAILURE;
 }
 
-#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV)
+#define EXPECTED_SIGNAL (CPU_FEATURE_ACTIVE (IBT) ? SIGSEGV : 0)
 #include <support/test-driver.c>