* V2 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
@ 2020-03-09 18:18 H.J. Lu
2020-03-10 13:13 ` Florian Weimer
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-03-09 18:18 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
[-- Attachment #1: Type: text/plain, Size: 2386 bytes --]
On Mon, Mar 9, 2020 at 3:05 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> * H. J. Lu:
>
> > diff --git a/sysdeps/x86/dl-cet.c b/sysdeps/x86/dl-cet.c
> > index ca3b5849bc..2be7a3a49f 100644
> > --- a/sysdeps/x86/dl-cet.c
> > +++ b/sysdeps/x86/dl-cet.c
>
> > + /* 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);
>
> As a general comment, I think we should add more logging as to way
> processes lose protection. But that does not need to happen with this
> patch.
>
> > + /* When IBT is enabled, we can't dlopening a shared
> > + object without IBT. */
>
> “we cannot dlopen”
Fixed.
> > + if (enable_ibt != ibt_enabled)
> > + _dl_signal_error (EINVAL, l->l_name, "dlopen",
> > + N_("indirect branch tracking isn't enabled"));
>
> 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
>
> > + /* 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"));
>
> See above.
Also fixed.
> > diff --git a/sysdeps/x86/tst-cet-legacy-7.c b/sysdeps/x86/tst-cet-legacy-7.c
> > new file mode 100644
> > index 0000000000..e6f5d6f145
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-7.c
>
> > + funcp = mmap (NULL, 0x1000, PROT_EXEC | PROT_READ | PROT_WRITE,
> > + MAP_ANONYMOUS | MAP_PRIVATE, -1, 0);
> > + if (funcp == MAP_FAILED)
> > + FAIL_EXIT1 ("mmap failed (errno=%d)", errno);
>
> You could use xmmap here.
Fixed,
> Rest of the patch looks okay to me.
Here is the updated patch. OK for master?
Thanks.
--
H.J.
[-- Attachment #2: 0001-x86-Remove-ARCH_CET_LEGACY_BITMAP-BZ-25397.patch --]
[-- Type: text/x-patch, Size: 16437 bytes --]
From 3fdfdca97bc103f633ff1f9ee7aa0a5bc9fa5748 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 25 Jun 2019 09:50:52 -0700
Subject: [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
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-7.c | 36 +++
7 files changed, 109 insertions(+), 206 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
-}
-
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
#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
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
$(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..f527a1414c 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;
-}
-
/* 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;
/* 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;
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;
}
/* 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)
{
- /* 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;
}
-
- /* 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;
}
}
@@ -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 (EINVAL,
+ m->l_initfini[ibt_legacy]->l_name,
+ "dlopen",
+ N_("indirect branch tracking isn't 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 (EINVAL,
+ m->l_initfini[shstk_legacy]->l_name,
+ "dlopen",
+ N_("shadow stack isn't 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)
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"));
+ }
}
/* 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);
+ }
+ /* 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
#endif
diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
index a77078afc9..da1f3d1e0d 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, "indirect branch tracking isn't enabled"))
+ 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-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>
+
+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;
+}
+
+#include <support/test-driver.c>
--
2.24.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: V2 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-09 18:18 V2 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397] H.J. Lu
@ 2020-03-10 13:13 ` Florian Weimer
2020-03-10 13:36 ` V3 " H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: Florian Weimer @ 2020-03-10 13:13 UTC (permalink / raw)
To: H.J. Lu; +Cc: GNU C Library
* 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.)
^ permalink raw reply [flat|nested] 10+ messages in thread
* V3 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-10 13:13 ` Florian Weimer
@ 2020-03-10 13:36 ` H.J. Lu
2020-03-13 13:25 ` PING: " H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-03-10 13:36 UTC (permalink / raw)
To: Florian Weimer; +Cc: GNU C Library
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
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-7.c | 36 +++
7 files changed, 109 insertions(+), 206 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
-}
-
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
#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
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
$(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;
-}
-
/* 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;
/* 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;
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;
}
/* 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)
{
- /* 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;
}
-
- /* 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;
}
}
@@ -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"));
+ }
+
+ 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"));
+ }
+
+ 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)
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"));
+ }
}
/* 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);
+ }
+ /* 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
#endif
diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
index a77078afc9..da1f3d1e0d 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, "indirect branch tracking isn't enabled"))
+ 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-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>
+
+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;
+}
+
+#include <support/test-driver.c>
--
2.24.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* PING: V3 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-10 13:36 ` V3 " H.J. Lu
@ 2020-03-13 13:25 ` H.J. Lu
2020-03-16 22:04 ` PING^2: " H.J. Lu
2020-03-16 23:41 ` PING: " Carlos O'Donell
0 siblings, 2 replies; 10+ messages in thread
From: H.J. Lu @ 2020-03-13 13:25 UTC (permalink / raw)
To: Florian Weimer, GNU C Library
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
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
-}
-
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
#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
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
$(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;
-}
-
/* 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;
/* 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;
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;
}
/* 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)
{
- /* 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;
}
-
- /* 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;
}
}
@@ -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"));
+ }
+
+ 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"));
+ }
+
+ 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)
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"));
+ }
}
/* 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);
+ }
+ /* 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
#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"))
+ 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)
{
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)
{
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>
+
+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;
+}
+
+#include <support/test-driver.c>
--
2.24.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* PING^2: V3 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-13 13:25 ` PING: " H.J. Lu
@ 2020-03-16 22:04 ` H.J. Lu
2020-03-16 23:41 ` PING: " Carlos O'Donell
1 sibling, 0 replies; 10+ messages in thread
From: H.J. Lu @ 2020-03-16 22:04 UTC (permalink / raw)
To: Florian Weimer, GNU C Library
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
--
H.J.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: PING: V3 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-13 13:25 ` PING: " H.J. Lu
2020-03-16 22:04 ` PING^2: " H.J. Lu
@ 2020-03-16 23:41 ` Carlos O'Donell
2020-03-17 0:58 ` V4 " H.J. Lu
1 sibling, 1 reply; 10+ messages in thread
From: Carlos O'Donell @ 2020-03-16 23:41 UTC (permalink / raw)
To: H.J. Lu, Florian Weimer, GNU C Library
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
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 10+ messages in thread
* V4 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-16 23:41 ` PING: " Carlos O'Donell
@ 2020-03-17 0:58 ` H.J. Lu
2020-03-18 3:20 ` Carlos O'Donell
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-03-17 0:58 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: Florian Weimer, GNU C Library
On Mon, Mar 16, 2020 at 07:41:54PM -0400, Carlos O'Donell wrote:
> 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 :-)
>
Leftover from my old version :-).
> > ---
> > 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"
Fixed.
>
> > +
> > + 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"
>
Fixed.
> > +
> > + 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).
>
Fixed.
> > + 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)
Fixed.
>
> > {
> > 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)
Fixed.
>
> > {
> > 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.
>
Done.
> I expect this test is supposed to just pass and not fail because it's
> run with -fcf-protection=none.
Yes.
>
> What about the equivalent test to test for failure?
Added.
>
> > +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
>
>
OK for master?
Thanks.
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 | 20 --
.../unix/sysv/linux/x86/include/asm/prctl.h | 5 -
sysdeps/x86/Makefile | 9 +-
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 | 3 +-
sysdeps/x86/tst-cet-legacy-6.c | 3 +-
sysdeps/x86/tst-cet-legacy-7.c | 38 +++
sysdeps/x86/tst-cet-legacy-8.c | 55 +++++
10 files changed, 172 insertions(+), 208 deletions(-)
create mode 100644 sysdeps/x86/tst-cet-legacy-7.c
create mode 100644 sysdeps/x86/tst-cet-legacy-8.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
-}
-
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
#endif /* ARCH_CET_STATUS */
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 95182a508c..4ffa699e5f 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -20,7 +20,8 @@ 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 \
+ tst-cet-legacy-8
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 +44,16 @@ 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
+CFLAGS-tst-cet-legacy-8.c += -mshstk
$(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..c7029f1b51 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;
-}
-
/* 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;
/* 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;
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;
}
/* 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)
{
- /* 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;
}
-
- /* 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;
}
}
@@ -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_("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_("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)
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"));
+ }
}
/* 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);
+ }
+ /* 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
#endif
diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
index a77078afc9..ee54b878ed 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, "rebuild shared object with IBT support enabled"))
+ 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..e2e95b6749 100644
--- a/sysdeps/x86/tst-cet-legacy-5.c
+++ b/sysdeps/x86/tst-cet-legacy-5.c
@@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
+ == NULL)
{
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..bdbbb9075f 100644
--- a/sysdeps/x86/tst-cet-legacy-6.c
+++ b/sysdeps/x86/tst-cet-legacy-6.c
@@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
+ == NULL)
{
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..58bcb29a5f
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-7.c
@@ -0,0 +1,38 @@
+/* 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>
+
+/* Check that mmapped legacy code works with -fcf-protection=none. */
+
+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;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
new file mode 100644
index 0000000000..b14b2a7290
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-8.c
@@ -0,0 +1,55 @@
+/* Check incompatibility with legacy 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 <stdlib.h>
+#include <x86intrin.h>
+#include <sys/mman.h>
+#include <support/test-driver.h>
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+
+/* Check that mmapped legacy code trigges segfault with -fcf-protection. */
+
+static void
+segfault_handler (int signo)
+{
+ exit (0);
+}
+
+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;
+
+ xsignal (SIGSEGV, segfault_handler);
+
+ 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 EXIT_FAILURE;
+}
+
+#include <support/test-driver.c>
--
2.24.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: V4 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-17 0:58 ` V4 " H.J. Lu
@ 2020-03-18 3:20 ` Carlos O'Donell
2020-03-18 11:34 ` H.J. Lu
0 siblings, 1 reply; 10+ messages in thread
From: Carlos O'Donell @ 2020-03-18 3:20 UTC (permalink / raw)
To: H.J. Lu; +Cc: Florian Weimer, GNU C Library
On 3/16/20 8:58 PM, H.J. Lu wrote:
> On Mon, Mar 16, 2020 at 07:41:54PM -0400, Carlos O'Donell wrote:
>> 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 :-)
>>
>
> Leftover from my old version :-).
>
>>> ---
>>> 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"
>
> Fixed.
>
>>
>>> +
>>> + 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"
>>
>
> Fixed.
>
>>> +
>>> + 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).
>>
>
> Fixed.
>
>>> + 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)
>
> Fixed.
>
>>
>>> {
>>> 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)
>
> Fixed.
>
>>
>>> {
>>> 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.
>>
>
> Done.
>
>> I expect this test is supposed to just pass and not fail because it's
>> run with -fcf-protection=none.
>
> Yes.
>
>>
>> What about the equivalent test to test for failure?
>
> Added.
>
>>
>>> +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
>>
>>
>
> OK for master?
Thank you for the extra test.
OK if you use EXPECTED_SIGNAL for the new test.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
> Thanks.
>
> 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 | 20 --
> .../unix/sysv/linux/x86/include/asm/prctl.h | 5 -
> sysdeps/x86/Makefile | 9 +-
> 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 | 3 +-
> sysdeps/x86/tst-cet-legacy-6.c | 3 +-
> sysdeps/x86/tst-cet-legacy-7.c | 38 +++
> sysdeps/x86/tst-cet-legacy-8.c | 55 +++++
> 10 files changed, 172 insertions(+), 208 deletions(-)
> create mode 100644 sysdeps/x86/tst-cet-legacy-7.c
> create mode 100644 sysdeps/x86/tst-cet-legacy-8.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
> -}
> -
> 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
> #endif /* ARCH_CET_STATUS */
> diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> index 95182a508c..4ffa699e5f 100644
> --- a/sysdeps/x86/Makefile
> +++ b/sysdeps/x86/Makefile
> @@ -20,7 +20,8 @@ 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 \
> + tst-cet-legacy-8
> 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 +44,16 @@ 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
> +CFLAGS-tst-cet-legacy-8.c += -mshstk
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..c7029f1b51 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;
> -}
> -
> /* 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;
>
> /* 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;
>
> 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;
> }
>
> /* 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)
> {
> - /* 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;
> }
> -
> - /* 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;
> }
> }
>
> @@ -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_("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_("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)
> 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"));
> + }
> }
>
> /* 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);
> + }
>
> + /* 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
> #endif
> diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
> index a77078afc9..ee54b878ed 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, "rebuild shared object with IBT support enabled"))
> + 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..e2e95b6749 100644
> --- a/sysdeps/x86/tst-cet-legacy-5.c
> +++ b/sysdeps/x86/tst-cet-legacy-5.c
> @@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
> + == NULL)
> {
> 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..bdbbb9075f 100644
> --- a/sysdeps/x86/tst-cet-legacy-6.c
> +++ b/sysdeps/x86/tst-cet-legacy-6.c
> @@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
> + == NULL)
> {
> 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..58bcb29a5f
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-7.c
> @@ -0,0 +1,38 @@
> +/* 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>
> +
> +/* Check that mmapped legacy code works with -fcf-protection=none. */
> +
> +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;
> +}
> +
> +#include <support/test-driver.c>
> diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
> new file mode 100644
> index 0000000000..b14b2a7290
> --- /dev/null
> +++ b/sysdeps/x86/tst-cet-legacy-8.c
> @@ -0,0 +1,55 @@
> +/* Check incompatibility with legacy 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 <stdlib.h>
> +#include <x86intrin.h>
> +#include <sys/mman.h>
> +#include <support/test-driver.h>
> +#include <support/xsignal.h>
> +#include <support/xunistd.h>
> +
> +/* Check that mmapped legacy code trigges segfault with -fcf-protection. */
> +
> +static void
> +segfault_handler (int signo)
> +{
> + exit (0);
> +}
> +
> +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;
> +
> + xsignal (SIGSEGV, segfault_handler);
Please use EXPECTED_SIGNAL.
> +
> + 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 EXIT_FAILURE;
> +}
> +
> +#include <support/test-driver.c>
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: V4 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-18 3:20 ` Carlos O'Donell
@ 2020-03-18 11:34 ` H.J. Lu
2020-03-18 11:44 ` Carlos O'Donell
0 siblings, 1 reply; 10+ messages in thread
From: H.J. Lu @ 2020-03-18 11:34 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: Florian Weimer, GNU C Library
[-- Attachment #1: Type: text/plain, Size: 49699 bytes --]
On Tue, Mar 17, 2020 at 8:20 PM Carlos O'Donell <carlos@redhat.com> wrote:
>
> On 3/16/20 8:58 PM, H.J. Lu wrote:
> > On Mon, Mar 16, 2020 at 07:41:54PM -0400, Carlos O'Donell wrote:
> >> 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 :-)
> >>
> >
> > Leftover from my old version :-).
> >
> >>> ---
> >>> 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"
> >
> > Fixed.
> >
> >>
> >>> +
> >>> + 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"
> >>
> >
> > Fixed.
> >
> >>> +
> >>> + 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).
> >>
> >
> > Fixed.
> >
> >>> + 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)
> >
> > Fixed.
> >
> >>
> >>> {
> >>> 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)
> >
> > Fixed.
> >
> >>
> >>> {
> >>> 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.
> >>
> >
> > Done.
> >
> >> I expect this test is supposed to just pass and not fail because it's
> >> run with -fcf-protection=none.
> >
> > Yes.
> >
> >>
> >> What about the equivalent test to test for failure?
> >
> > Added.
> >
> >>
> >>> +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
> >>
> >>
> >
> > OK for master?
>
> Thank you for the extra test.
>
> OK if you use EXPECTED_SIGNAL for the new test.
>
> Reviewed-by: Carlos O'Donell <carlos@redhat.com>
>
> > Thanks.
> >
> > 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 | 20 --
> > .../unix/sysv/linux/x86/include/asm/prctl.h | 5 -
> > sysdeps/x86/Makefile | 9 +-
> > 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 | 3 +-
> > sysdeps/x86/tst-cet-legacy-6.c | 3 +-
> > sysdeps/x86/tst-cet-legacy-7.c | 38 +++
> > sysdeps/x86/tst-cet-legacy-8.c | 55 +++++
> > 10 files changed, 172 insertions(+), 208 deletions(-)
> > create mode 100644 sysdeps/x86/tst-cet-legacy-7.c
> > create mode 100644 sysdeps/x86/tst-cet-legacy-8.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
> > -}
> > -
> > 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
> > #endif /* ARCH_CET_STATUS */
> > diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
> > index 95182a508c..4ffa699e5f 100644
> > --- a/sysdeps/x86/Makefile
> > +++ b/sysdeps/x86/Makefile
> > @@ -20,7 +20,8 @@ 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 \
> > + tst-cet-legacy-8
> > 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 +44,16 @@ 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
> > +CFLAGS-tst-cet-legacy-8.c += -mshstk
>
> 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..c7029f1b51 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;
> > -}
> > -
> > /* 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;
> >
> > /* 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;
> >
> > 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;
> > }
> >
> > /* 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)
> > {
> > - /* 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;
> > }
> > -
> > - /* 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;
> > }
> > }
> >
> > @@ -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_("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_("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)
> > 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"));
> > + }
> > }
> >
> > /* 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);
> > + }
> >
> > + /* 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
> > #endif
> > diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
> > index a77078afc9..ee54b878ed 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, "rebuild shared object with IBT support enabled"))
> > + 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..e2e95b6749 100644
> > --- a/sysdeps/x86/tst-cet-legacy-5.c
> > +++ b/sysdeps/x86/tst-cet-legacy-5.c
> > @@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
> > + == NULL)
> > {
> > 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..bdbbb9075f 100644
> > --- a/sysdeps/x86/tst-cet-legacy-6.c
> > +++ b/sysdeps/x86/tst-cet-legacy-6.c
> > @@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
> > + == NULL)
> > {
> > 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..58bcb29a5f
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-7.c
> > @@ -0,0 +1,38 @@
> > +/* 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>
> > +
> > +/* Check that mmapped legacy code works with -fcf-protection=none. */
> > +
> > +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;
> > +}
> > +
> > +#include <support/test-driver.c>
> > diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
> > new file mode 100644
> > index 0000000000..b14b2a7290
> > --- /dev/null
> > +++ b/sysdeps/x86/tst-cet-legacy-8.c
> > @@ -0,0 +1,55 @@
> > +/* Check incompatibility with legacy 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 <stdlib.h>
> > +#include <x86intrin.h>
> > +#include <sys/mman.h>
> > +#include <support/test-driver.h>
> > +#include <support/xsignal.h>
> > +#include <support/xunistd.h>
> > +
> > +/* Check that mmapped legacy code trigges segfault with -fcf-protection. */
> > +
> > +static void
> > +segfault_handler (int signo)
> > +{
> > + exit (0);
> > +}
> > +
> > +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;
> > +
> > + xsignal (SIGSEGV, segfault_handler);
>
> Please use EXPECTED_SIGNAL.
>
> > +
> > + 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 EXIT_FAILURE;
> > +}
> > +
> > +#include <support/test-driver.c>
> >
This is the patch I am checking in.
Thanks.
--
H.J.
[-- Attachment #2: 0001-x86-Remove-ARCH_CET_LEGACY_BITMAP-BZ-25397.patch --]
[-- Type: text/x-patch, Size: 19016 bytes --]
From 2efe420d675e366adcb50ee6865f6839ad062359 Mon Sep 17 00:00:00 2001
From: "H.J. Lu" <hjl.tools@gmail.com>
Date: Tue, 25 Jun 2019 09:50:52 -0700
Subject: [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
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.
Tested on CET Linux/x86-64 and non-CET Linux/x86-64.
Reviewed-by: Carlos O'Donell <carlos@redhat.com>
---
sysdeps/unix/sysv/linux/x86/dl-cet.h | 20 --
.../unix/sysv/linux/x86/include/asm/prctl.h | 5 -
sysdeps/x86/Makefile | 9 +-
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 | 3 +-
sysdeps/x86/tst-cet-legacy-6.c | 3 +-
sysdeps/x86/tst-cet-legacy-7.c | 38 +++
sysdeps/x86/tst-cet-legacy-8.c | 48 ++++
10 files changed, 165 insertions(+), 208 deletions(-)
create mode 100644 sysdeps/x86/tst-cet-legacy-7.c
create mode 100644 sysdeps/x86/tst-cet-legacy-8.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
-}
-
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
#endif /* ARCH_CET_STATUS */
diff --git a/sysdeps/x86/Makefile b/sysdeps/x86/Makefile
index 95182a508c..4ffa699e5f 100644
--- a/sysdeps/x86/Makefile
+++ b/sysdeps/x86/Makefile
@@ -20,7 +20,8 @@ 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 \
+ tst-cet-legacy-8
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 +44,16 @@ 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
+CFLAGS-tst-cet-legacy-8.c += -mshstk
$(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..c7029f1b51 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;
-}
-
/* 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;
/* 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;
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;
}
/* 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)
{
- /* 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;
}
-
- /* 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;
}
}
@@ -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_("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_("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)
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"));
+ }
}
/* 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);
+ }
+ /* 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
#endif
diff --git a/sysdeps/x86/tst-cet-legacy-4.c b/sysdeps/x86/tst-cet-legacy-4.c
index a77078afc9..ee54b878ed 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, "rebuild shared object with IBT support enabled"))
+ 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..e2e95b6749 100644
--- a/sysdeps/x86/tst-cet-legacy-5.c
+++ b/sysdeps/x86/tst-cet-legacy-5.c
@@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
+ == NULL)
{
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..bdbbb9075f 100644
--- a/sysdeps/x86/tst-cet-legacy-6.c
+++ b/sysdeps/x86/tst-cet-legacy-6.c
@@ -35,7 +35,8 @@ 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, "rebuild shared object with SHSTK support enabled")
+ == NULL)
{
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..58bcb29a5f
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-7.c
@@ -0,0 +1,38 @@
+/* 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>
+
+/* Check that mmapped legacy code works with -fcf-protection=none. */
+
+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;
+}
+
+#include <support/test-driver.c>
diff --git a/sysdeps/x86/tst-cet-legacy-8.c b/sysdeps/x86/tst-cet-legacy-8.c
new file mode 100644
index 0000000000..11e811588c
--- /dev/null
+++ b/sysdeps/x86/tst-cet-legacy-8.c
@@ -0,0 +1,48 @@
+/* Check incompatibility with legacy 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 <stdlib.h>
+#include <x86intrin.h>
+#include <sys/mman.h>
+#include <support/test-driver.h>
+#include <support/xsignal.h>
+#include <support/xunistd.h>
+
+/* Check that mmapped legacy code trigges segfault with -fcf-protection. */
+
+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);
+ printf ("mmap = %p\n", funcp);
+ /* Write RET instruction. */
+ *(char *) funcp = 0xc3;
+ funcp ();
+ return EXIT_FAILURE;
+}
+
+#define EXPECTED_SIGNAL (_get_ssp () == 0 ? 0 : SIGSEGV)
+#include <support/test-driver.c>
--
2.25.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: V4 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397]
2020-03-18 11:34 ` H.J. Lu
@ 2020-03-18 11:44 ` Carlos O'Donell
0 siblings, 0 replies; 10+ messages in thread
From: Carlos O'Donell @ 2020-03-18 11:44 UTC (permalink / raw)
To: H.J. Lu; +Cc: Florian Weimer, GNU C Library
On 3/18/20 7:34 AM, H.J. Lu wrote:
> This is the patch I am checking in.
LGTM. Thanks for continuing to add more tests for these behaviours! :-)
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-03-18 11:44 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-09 18:18 V2 [PATCH] x86: Remove ARCH_CET_LEGACY_BITMAP [BZ #25397] H.J. Lu
2020-03-10 13:13 ` Florian Weimer
2020-03-10 13:36 ` V3 " H.J. Lu
2020-03-13 13:25 ` PING: " H.J. Lu
2020-03-16 22:04 ` PING^2: " H.J. Lu
2020-03-16 23:41 ` PING: " Carlos O'Donell
2020-03-17 0:58 ` V4 " H.J. Lu
2020-03-18 3:20 ` Carlos O'Donell
2020-03-18 11:34 ` H.J. Lu
2020-03-18 11:44 ` Carlos O'Donell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).