All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI
@ 2019-06-05 20:57 Richard Henderson
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signals Richard Henderson
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

Dave Martin has recently posted a kernel patch set for 
supporting ARMv8.5 Branch Target Identification in userland.

  http://lists.infradead.org/pipermail/linux-arm-kernel/2019-May/654654.html

While that support is not yet in the upstream kernel, it looks
to be close to its final form.  Note that the patch set spells
this PROT_BTI_GUARDED, but review suggested to rename to PROT_BTI.

Changes since v5:
  * New function to validate the target PROT parameter for mmap/mprotect.
  * Require BTI in the cpu for PROT_BTI set.
  * Set PSTATE.BTYPE=2 for the signal handler.
    Adjust the smoke test to match.
  * Tidy up the note parsing.


r~


Richard Henderson (6):
  linux-user/aarch64: Reset btype for syscalls and signals
  linux-user: Validate mmap/mprotect prot value
  linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI
  include/elf: Add defines related to notes for GNU systems
  linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes
  tests/tcg/aarch64: Add bti smoke test

 include/elf.h                     |  48 ++++++++++++
 include/exec/cpu-all.h            |   2 +
 linux-user/syscall_defs.h         |   4 +
 linux-user/aarch64/cpu_loop.c     |   7 ++
 linux-user/aarch64/signal.c       |  10 ++-
 linux-user/elfload.c              |  83 ++++++++++++++++++--
 linux-user/mmap.c                 | 122 ++++++++++++++++++++++--------
 target/arm/translate-a64.c        |   6 +-
 tests/tcg/aarch64/bti-1.c         |  77 +++++++++++++++++++
 tests/tcg/aarch64/bti-crt.inc.c   |  69 +++++++++++++++++
 tests/tcg/aarch64/Makefile.target |   3 +
 11 files changed, 387 insertions(+), 44 deletions(-)
 create mode 100644 tests/tcg/aarch64/bti-1.c
 create mode 100644 tests/tcg/aarch64/bti-crt.inc.c

-- 
2.17.1



^ permalink raw reply	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signals
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
@ 2019-06-05 20:57 ` Richard Henderson
  2019-06-06  9:53   ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signalsy Dave Martin
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value Richard Henderson
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

The value of btype for syscalls is CONSTRAINED UNPREDICTABLE,
so we need to make sure that the value is 0 before clone,
fork, or syscall return.

The kernel sets btype for the signal handler as if for a call.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/aarch64/cpu_loop.c |  7 +++++++
 linux-user/aarch64/signal.c   | 10 ++++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
index 2f2f63e3e8..1f68b13168 100644
--- a/linux-user/aarch64/cpu_loop.c
+++ b/linux-user/aarch64/cpu_loop.c
@@ -86,6 +86,13 @@ void cpu_loop(CPUARMState *env)
 
         switch (trapnr) {
         case EXCP_SWI:
+            /*
+             * The state of BTYPE on syscall entry is CONSTRAINED
+             * UNPREDICTABLE.  The real kernel will need to tidy this up
+             * as well.  Do this before syscalls so that the value is
+             * correct on return from syscall (especially clone & fork).
+             */
+            env->btype = 0;
             ret = do_syscall(env,
                              env->xregs[8],
                              env->xregs[0],
diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
index f84a9cf28a..5605d404b3 100644
--- a/linux-user/aarch64/signal.c
+++ b/linux-user/aarch64/signal.c
@@ -506,10 +506,16 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
             + offsetof(struct target_rt_frame_record, tramp);
     }
     env->xregs[0] = usig;
-    env->xregs[31] = frame_addr;
     env->xregs[29] = frame_addr + fr_ofs;
-    env->pc = ka->_sa_handler;
     env->xregs[30] = return_addr;
+    env->xregs[31] = frame_addr;
+    env->pc = ka->_sa_handler;
+
+    /* Invoke the signal handler as if by indirect call.  */
+    if (cpu_isar_feature(aa64_bti, arm_env_get_cpu(env))) {
+        env->btype = 2;
+    }
+
     if (info) {
         tswap_siginfo(&frame->info, info);
         env->xregs[1] = frame_addr + offsetof(struct target_rt_sigframe, info);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signals Richard Henderson
@ 2019-06-05 20:57 ` Richard Henderson
  2019-06-06 11:24   ` Aleksandar Markovic
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 3/6] linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI Richard Henderson
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

The kernel will return -EINVAL for bits set in the prot argument
that are unknown or invalid.  Previously we were simply cropping
out the bits that we care about.

Introduce validate_prot_to_pageflags to perform this check in a
single place between the two syscalls.  Differentiate between
the target and host versions of prot.  Compute the qemu internal
page_flags value at the same time.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/mmap.c | 106 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 75 insertions(+), 31 deletions(-)

diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index af41339d57..3117f57fd8 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -61,11 +61,38 @@ void mmap_fork_end(int child)
         pthread_mutex_unlock(&mmap_mutex);
 }
 
+/*
+ * Validate target prot bitmask.
+ * Return the prot bitmask for the host in *HOST_PROT.
+ * Return 0 if the target prot bitmask is invalid, otherwise
+ * the internal qemu page_flags (which will include PAGE_VALID).
+ */
+static int validate_prot_to_pageflags(int *host_prot, int prot)
+{
+    int valid = PROT_READ | PROT_WRITE | PROT_EXEC | TARGET_PROT_SEM;
+    int page_flags = (prot & PAGE_BITS) | PAGE_VALID;
+
+    /*
+     * For the host, we need not pass anything except read/write/exec.
+     * While PROT_SEM is allowed by all hosts, it is also ignored, so
+     * don't bother transforming guest bit to host bit.  Any other
+     * target-specific prot bits will not be understood by the host
+     * and will need to be encoded into page_flags for qemu emulation.
+     *
+     * TODO: We do not actually have to map guest pages as executable,
+     * since they will not be directly executed by the host.  We only
+     * need to remember exec within page_flags.
+     */
+    *host_prot = prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
+
+    return prot & ~valid ? 0 : page_flags;
+}
+
 /* NOTE: all the constants are the HOST ones, but addresses are target. */
-int target_mprotect(abi_ulong start, abi_ulong len, int prot)
+int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
 {
     abi_ulong end, host_start, host_end, addr;
-    int prot1, ret;
+    int prot1, ret, page_flags, host_prot;
 
 #ifdef DEBUG_MMAP
     printf("mprotect: start=0x" TARGET_ABI_FMT_lx
@@ -75,56 +102,65 @@ int target_mprotect(abi_ulong start, abi_ulong len, int prot)
            prot & PROT_EXEC ? 'x' : '-');
 #endif
 
-    if ((start & ~TARGET_PAGE_MASK) != 0)
+    if ((start & ~TARGET_PAGE_MASK) != 0) {
         return -TARGET_EINVAL;
+    }
+    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
+    if (!page_flags) {
+        return -TARGET_EINVAL;
+    }
     len = TARGET_PAGE_ALIGN(len);
     end = start + len;
     if (!guest_range_valid(start, len)) {
         return -TARGET_ENOMEM;
     }
-    prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
-    if (len == 0)
+    if (len == 0) {
         return 0;
+    }
 
     mmap_lock();
     host_start = start & qemu_host_page_mask;
     host_end = HOST_PAGE_ALIGN(end);
     if (start > host_start) {
         /* handle host page containing start */
-        prot1 = prot;
-        for(addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
+        prot1 = host_prot;
+        for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
         if (host_end == host_start + qemu_host_page_size) {
-            for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
+            for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
                 prot1 |= page_get_flags(addr);
             }
             end = host_end;
         }
-        ret = mprotect(g2h(host_start), qemu_host_page_size, prot1 & PAGE_BITS);
-        if (ret != 0)
+        ret = mprotect(g2h(host_start), qemu_host_page_size,
+                       prot1 & PAGE_BITS);
+        if (ret != 0) {
             goto error;
+        }
         host_start += qemu_host_page_size;
     }
     if (end < host_end) {
-        prot1 = prot;
-        for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
+        prot1 = host_prot;
+        for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
             prot1 |= page_get_flags(addr);
         }
-        ret = mprotect(g2h(host_end - qemu_host_page_size), qemu_host_page_size,
-                       prot1 & PAGE_BITS);
-        if (ret != 0)
+        ret = mprotect(g2h(host_end - qemu_host_page_size),
+                       qemu_host_page_size, prot1 & PAGE_BITS);
+        if (ret != 0) {
             goto error;
+        }
         host_end -= qemu_host_page_size;
     }
 
     /* handle the pages in the middle */
     if (host_start < host_end) {
-        ret = mprotect(g2h(host_start), host_end - host_start, prot);
-        if (ret != 0)
+        ret = mprotect(g2h(host_start), host_end - host_start, host_prot);
+        if (ret != 0) {
             goto error;
+        }
     }
-    page_set_flags(start, start + len, prot | PAGE_VALID);
+    page_set_flags(start, start + len, page_flags);
     mmap_unlock();
     return 0;
 error:
@@ -364,10 +400,11 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong size, abi_ulong align)
 }
 
 /* NOTE: all the constants are the HOST ones */
-abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
+abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
                      int flags, int fd, abi_ulong offset)
 {
     abi_ulong ret, end, real_start, real_end, retaddr, host_offset, host_len;
+    int page_flags, host_prot;
 
     mmap_lock();
 #ifdef DEBUG_MMAP
@@ -402,6 +439,12 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         goto fail;
     }
 
+    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
+    if (!page_flags) {
+        errno = EINVAL;
+        goto fail;
+    }
+
     /* Also check for overflows... */
     len = TARGET_PAGE_ALIGN(len);
     if (!len) {
@@ -467,14 +510,15 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         /* Note: we prefer to control the mapping address. It is
            especially important if qemu_host_page_size >
            qemu_real_host_page_size */
-        p = mmap(g2h(start), host_len, prot,
+        p = mmap(g2h(start), host_len, host_prot,
                  flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
-        if (p == MAP_FAILED)
+        if (p == MAP_FAILED) {
             goto fail;
+        }
         /* update start so that it points to the file position at 'offset' */
         host_start = (unsigned long)p;
         if (!(flags & MAP_ANONYMOUS)) {
-            p = mmap(g2h(start), len, prot,
+            p = mmap(g2h(start), len, host_prot,
                      flags | MAP_FIXED, fd, host_offset);
             if (p == MAP_FAILED) {
                 munmap(g2h(start), host_len);
@@ -508,19 +552,19 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             /* msync() won't work here, so we return an error if write is
                possible while it is a shared mapping */
             if ((flags & MAP_TYPE) == MAP_SHARED &&
-                (prot & PROT_WRITE)) {
+                (host_prot & PROT_WRITE)) {
                 errno = EINVAL;
                 goto fail;
             }
-            retaddr = target_mmap(start, len, prot | PROT_WRITE,
+            retaddr = target_mmap(start, len, target_prot | PROT_WRITE,
                                   MAP_FIXED | MAP_PRIVATE | MAP_ANONYMOUS,
                                   -1, 0);
             if (retaddr == -1)
                 goto fail;
             if (pread(fd, g2h(start), len, offset) == -1)
                 goto fail;
-            if (!(prot & PROT_WRITE)) {
-                ret = target_mprotect(start, len, prot);
+            if (!(host_prot & PROT_WRITE)) {
+                ret = target_mprotect(start, len, target_prot);
                 assert(ret == 0);
             }
             goto the_end;
@@ -531,13 +575,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             if (real_end == real_start + qemu_host_page_size) {
                 /* one single host page */
                 ret = mmap_frag(real_start, start, end,
-                                prot, flags, fd, offset);
+                                host_prot, flags, fd, offset);
                 if (ret == -1)
                     goto fail;
                 goto the_end1;
             }
             ret = mmap_frag(real_start, start, real_start + qemu_host_page_size,
-                            prot, flags, fd, offset);
+                            host_prot, flags, fd, offset);
             if (ret == -1)
                 goto fail;
             real_start += qemu_host_page_size;
@@ -546,7 +590,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
         if (end < real_end) {
             ret = mmap_frag(real_end - qemu_host_page_size,
                             real_end - qemu_host_page_size, end,
-                            prot, flags, fd,
+                            host_prot, flags, fd,
                             offset + real_end - qemu_host_page_size - start);
             if (ret == -1)
                 goto fail;
@@ -562,13 +606,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
             else
                 offset1 = offset + real_start - start;
             p = mmap(g2h(real_start), real_end - real_start,
-                     prot, flags, fd, offset1);
+                     host_prot, flags, fd, offset1);
             if (p == MAP_FAILED)
                 goto fail;
         }
     }
  the_end1:
-    page_set_flags(start, start + len, prot | PAGE_VALID);
+    page_set_flags(start, start + len, page_flags);
  the_end:
 #ifdef DEBUG_MMAP
     printf("ret=0x" TARGET_ABI_FMT_lx "\n", start);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v6 3/6] linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signals Richard Henderson
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value Richard Henderson
@ 2019-06-05 20:57 ` Richard Henderson
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems Richard Henderson
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

Transform the prot bit to a qemu internal page bit, and save
it in the page tables.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/exec/cpu-all.h     |  2 ++
 linux-user/syscall_defs.h  |  4 ++++
 linux-user/mmap.c          | 16 ++++++++++++++++
 target/arm/translate-a64.c |  6 +++---
 4 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/include/exec/cpu-all.h b/include/exec/cpu-all.h
index da07ce311f..e65530acae 100644
--- a/include/exec/cpu-all.h
+++ b/include/exec/cpu-all.h
@@ -249,6 +249,8 @@ extern intptr_t qemu_host_page_mask;
 /* FIXME: Code that sets/uses this is broken and needs to go away.  */
 #define PAGE_RESERVED  0x0020
 #endif
+/* Target-specific bits that will be used via page_get_flags().  */
+#define PAGE_TARGET_1  0x0080
 
 #if defined(CONFIG_USER_ONLY)
 void page_dump(FILE *f);
diff --git a/linux-user/syscall_defs.h b/linux-user/syscall_defs.h
index 7f141f699c..9a8a14e81e 100644
--- a/linux-user/syscall_defs.h
+++ b/linux-user/syscall_defs.h
@@ -1097,6 +1097,10 @@ struct target_winsize {
 #define TARGET_PROT_SEM         0x08
 #endif
 
+#ifdef TARGET_AARCH64
+#define TARGET_PROT_BTI         0x10
+#endif
+
 /* Common */
 #define TARGET_MAP_SHARED	0x01		/* Share changes */
 #define TARGET_MAP_PRIVATE	0x02		/* Changes are private */
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index 3117f57fd8..def64a41d5 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -85,6 +85,22 @@ static int validate_prot_to_pageflags(int *host_prot, int prot)
      */
     *host_prot = prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
 
+#ifdef TARGET_AARCH64
+    /*
+     * The PROT_BTI bit is only accepted if the cpu supports the feature.
+     * Since this is the unusual case, don't bother checking unless
+     * the bit has been requested.  If set and valid, record the bit
+     * within QEMU's page_flags as PAGE_TARGET_1.
+     */
+    if (prot & TARGET_PROT_BTI) {
+        ARMCPU *cpu = ARM_CPU(thread_cpu);
+        if (cpu_isar_feature(aa64_bti, cpu)) {
+            valid |= TARGET_PROT_BTI;
+            page_flags |= PAGE_TARGET_1;
+        }
+    }
+#endif
+
     return prot & ~valid ? 0 : page_flags;
 }
 
diff --git a/target/arm/translate-a64.c b/target/arm/translate-a64.c
index 092f0df3c4..5043344eba 100644
--- a/target/arm/translate-a64.c
+++ b/target/arm/translate-a64.c
@@ -14115,10 +14115,10 @@ static void disas_data_proc_simd_fp(DisasContext *s, uint32_t insn)
  */
 static bool is_guarded_page(CPUARMState *env, DisasContext *s)
 {
-#ifdef CONFIG_USER_ONLY
-    return false;  /* FIXME */
-#else
     uint64_t addr = s->base.pc_first;
+#ifdef CONFIG_USER_ONLY
+    return page_get_flags(addr) & PAGE_TARGET_1;
+#else
     int mmu_idx = arm_to_core_mmu_idx(s->mmu_idx);
     unsigned int index = tlb_index(env, mmu_idx, addr);
     CPUTLBEntry *entry = tlb_entry(env, mmu_idx, addr);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
                   ` (2 preceding siblings ...)
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 3/6] linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI Richard Henderson
@ 2019-06-05 20:57 ` Richard Henderson
  2019-06-05 21:17   ` Aleksandar Markovic
  2019-06-06  5:45   ` Aleksandar Markovic
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes Richard Henderson
                   ` (2 subsequent siblings)
  6 siblings, 2 replies; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

This is a collection of related defines for notes, copied
from glibc's <elf.h>.  We're not going to use all of these
right away, but it seemed foolish to cherry-pick only the
ones we need now.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 include/elf.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 48 insertions(+)

diff --git a/include/elf.h b/include/elf.h
index ea7708a4ea..6f3eada36f 100644
--- a/include/elf.h
+++ b/include/elf.h
@@ -1650,6 +1650,54 @@ typedef struct elf64_shdr {
 #define NT_ARM_HW_WATCH 0x403           /* ARM hardware watchpoint registers */
 #define NT_ARM_SYSTEM_CALL      0x404   /* ARM system call number */
 
+/* Defined note types for GNU systems.  */
+
+#define NT_GNU_ABI_TAG          1       /* ABI information */
+#define NT_GNU_HWCAP            2       /* Synthetic hwcap information */
+#define NT_GNU_BUILD_ID         3       /* Build ID */
+#define NT_GNU_GOLD_VERSION     4       /* Version of ld.gold */
+#define NT_GNU_PROPERTY_TYPE_0  5       /* Program property */
+
+/* Values used in GNU .note.gnu.property notes (NT_GNU_PROPERTY_TYPE_0).  */
+
+#define GNU_PROPERTY_STACK_SIZE                 1
+#define GNU_PROPERTY_NO_COPY_ON_PROTECTED       2
+
+#define GNU_PROPERTY_LOPROC                     0xc0000000
+#define GNU_PROPERTY_HIPROC                     0xdfffffff
+#define GNU_PROPERTY_LOUSER                     0xe0000000
+#define GNU_PROPERTY_HIUSER                     0xffffffff
+
+#define GNU_PROPERTY_X86_ISA_1_USED             0xc0000000
+#define GNU_PROPERTY_X86_ISA_1_NEEDED           0xc0000001
+
+#define GNU_PROPERTY_X86_ISA_1_486              (1U << 0)
+#define GNU_PROPERTY_X86_ISA_1_586              (1U << 1)
+#define GNU_PROPERTY_X86_ISA_1_686              (1U << 2)
+#define GNU_PROPERTY_X86_ISA_1_SSE              (1U << 3)
+#define GNU_PROPERTY_X86_ISA_1_SSE2             (1U << 4)
+#define GNU_PROPERTY_X86_ISA_1_SSE3             (1U << 5)
+#define GNU_PROPERTY_X86_ISA_1_SSSE3            (1U << 6)
+#define GNU_PROPERTY_X86_ISA_1_SSE4_1           (1U << 7)
+#define GNU_PROPERTY_X86_ISA_1_SSE4_2           (1U << 8)
+#define GNU_PROPERTY_X86_ISA_1_AVX              (1U << 9)
+#define GNU_PROPERTY_X86_ISA_1_AVX2             (1U << 10)
+#define GNU_PROPERTY_X86_ISA_1_AVX512F          (1U << 11)
+#define GNU_PROPERTY_X86_ISA_1_AVX512CD         (1U << 12)
+#define GNU_PROPERTY_X86_ISA_1_AVX512ER         (1U << 13)
+#define GNU_PROPERTY_X86_ISA_1_AVX512PF         (1U << 14)
+#define GNU_PROPERTY_X86_ISA_1_AVX512VL         (1U << 15)
+#define GNU_PROPERTY_X86_ISA_1_AVX512DQ         (1U << 16)
+#define GNU_PROPERTY_X86_ISA_1_AVX512BW         (1U << 17)
+
+#define GNU_PROPERTY_X86_FEATURE_1_AND          0xc0000002
+#define GNU_PROPERTY_X86_FEATURE_1_IBT          (1U << 0)
+#define GNU_PROPERTY_X86_FEATURE_1_SHSTK        (1U << 1)
+
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND      0xc0000000
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI      (1u << 0)
+#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC      (1u << 1)
+
 /*
  * Physical entry point into the kernel.
  *
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
                   ` (3 preceding siblings ...)
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems Richard Henderson
@ 2019-06-05 20:57 ` Richard Henderson
  2019-06-06 10:12   ` Dave Martin
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test Richard Henderson
  2019-06-05 22:15 ` [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI no-reply
  6 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

For aarch64, this includes the GNU_PROPERTY_AARCH64_FEATURE_1_BTI bit,
which indicates that the image should be mapped with guarded pages.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 linux-user/elfload.c | 83 +++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 75 insertions(+), 8 deletions(-)

diff --git a/linux-user/elfload.c b/linux-user/elfload.c
index a57b7049dd..1a12c60a33 100644
--- a/linux-user/elfload.c
+++ b/linux-user/elfload.c
@@ -2253,7 +2253,7 @@ static void load_elf_image(const char *image_name, int image_fd,
     struct elfhdr *ehdr = (struct elfhdr *)bprm_buf;
     struct elf_phdr *phdr;
     abi_ulong load_addr, load_bias, loaddr, hiaddr, error;
-    int i, retval;
+    int i, retval, prot_exec = PROT_EXEC;
     const char *errmsg;
 
     /* First of all, some simple consistency checks */
@@ -2288,17 +2288,78 @@ static void load_elf_image(const char *image_name, int image_fd,
     loaddr = -1, hiaddr = 0;
     info->alignment = 0;
     for (i = 0; i < ehdr->e_phnum; ++i) {
-        if (phdr[i].p_type == PT_LOAD) {
-            abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset;
+        struct elf_phdr *eppnt = phdr + i;
+
+        if (eppnt->p_type == PT_LOAD) {
+            abi_ulong a = eppnt->p_vaddr - eppnt->p_offset;
             if (a < loaddr) {
                 loaddr = a;
             }
-            a = phdr[i].p_vaddr + phdr[i].p_memsz;
+            a = eppnt->p_vaddr + eppnt->p_memsz;
             if (a > hiaddr) {
                 hiaddr = a;
             }
             ++info->nsegs;
-            info->alignment |= phdr[i].p_align;
+            info->alignment |= eppnt->p_align;
+        } else if (eppnt->p_type == PT_NOTE) {
+#ifdef TARGET_AARCH64
+            /*
+             * Process NT_GNU_PROPERTY_TYPE_0.
+             *
+             * TODO: The only item that is AArch64 specific is the
+             * GNU_PROPERTY_AARCH64_FEATURE_1_AND processing at the end.
+             * If we were to ever process GNU_PROPERTY_X86_*, all of the
+             * code through checking the gnu0 magic number is sharable.
+             * But for now, since this *is* only used by AArch64, don't
+             * process the note elsewhere.
+             */
+            const uint32_t gnu0_magic = const_le32('G' | 'N' << 8 | 'U' << 16);
+            uint32_t note[7];
+
+            /*
+             * The note contents are 7 words, but depending on LP64 vs ILP32
+             * there may be an 8th padding word at the end.  Check for and
+             * read the minimum size.  Further checks below will validate
+             * that the sizes of everything involved are as we expect.
+             */
+            if (eppnt->p_filesz < sizeof(note)) {
+                continue;
+            }
+            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
+                memcpy(note, bprm_buf + eppnt->p_offset, sizeof(note));
+            } else {
+                retval = pread(image_fd, note, sizeof(note), eppnt->p_offset);
+                if (retval != sizeof(note)) {
+                    goto exit_perror;
+                }
+            }
+#ifdef BSWAP_NEEDED
+            for (i = 0; i < ARRAY_SIZE(note); ++i) {
+                bswap32s(note + i);
+            }
+#endif
+            /*
+             * Check that this is a NT_GNU_PROPERTY_TYPE_0 note.
+             * Again, descsz includes padding.  Full size validation
+             * awaits checking the final payload.
+             */
+            if (note[0] != 4 ||                       /* namesz */
+                note[1] < 12 ||                       /* descsz */
+                note[2] != NT_GNU_PROPERTY_TYPE_0 ||  /* type */
+                note[3] != gnu0_magic) {              /* name */
+                continue;
+            }
+            /*
+             * Check for the BTI feature.  If present, this indicates
+             * that all the executable pages of the binary should be
+             * mapped with PROT_BTI, so that branch targets are enforced.
+             */
+            if (note[4] == GNU_PROPERTY_AARCH64_FEATURE_1_AND &&
+                note[5] == 4 &&
+                (note[6] & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) {
+                prot_exec |= TARGET_PROT_BTI;
+            }
+#endif /* TARGET_AARCH64 */
         }
     }
 
@@ -2358,9 +2419,15 @@ static void load_elf_image(const char *image_name, int image_fd,
             abi_ulong vaddr, vaddr_po, vaddr_ps, vaddr_ef, vaddr_em, vaddr_len;
             int elf_prot = 0;
 
-            if (eppnt->p_flags & PF_R) elf_prot =  PROT_READ;
-            if (eppnt->p_flags & PF_W) elf_prot |= PROT_WRITE;
-            if (eppnt->p_flags & PF_X) elf_prot |= PROT_EXEC;
+            if (eppnt->p_flags & PF_R) {
+                elf_prot |= PROT_READ;
+            }
+            if (eppnt->p_flags & PF_W) {
+                elf_prot |= PROT_WRITE;
+            }
+            if (eppnt->p_flags & PF_X) {
+                elf_prot |= prot_exec;
+            }
 
             vaddr = load_bias + eppnt->p_vaddr;
             vaddr_po = TARGET_ELF_PAGEOFFSET(vaddr);
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
                   ` (4 preceding siblings ...)
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes Richard Henderson
@ 2019-06-05 20:57 ` Richard Henderson
  2019-06-06 10:21   ` Dave Martin
  2019-06-05 22:15 ` [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI no-reply
  6 siblings, 1 reply; 16+ messages in thread
From: Richard Henderson @ 2019-06-05 20:57 UTC (permalink / raw)
  To: qemu-devel; +Cc: peter.maydell, qemu-arm, Dave.Martin

This will build with older toolchains, without the upstream support
for -mbranch-protection.  Such a toolchain will produce a warning
in such cases,

ld: warning: /tmp/ccyZt0kq.o: unsupported GNU_PROPERTY_TYPE (5) \
type: 0xc0000000

but the still places the note at the correct location in the binary
for processing by the runtime loader.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 tests/tcg/aarch64/bti-1.c         | 77 +++++++++++++++++++++++++++++++
 tests/tcg/aarch64/bti-crt.inc.c   | 69 +++++++++++++++++++++++++++
 tests/tcg/aarch64/Makefile.target |  3 ++
 3 files changed, 149 insertions(+)
 create mode 100644 tests/tcg/aarch64/bti-1.c
 create mode 100644 tests/tcg/aarch64/bti-crt.inc.c

diff --git a/tests/tcg/aarch64/bti-1.c b/tests/tcg/aarch64/bti-1.c
new file mode 100644
index 0000000000..2aee57ea7a
--- /dev/null
+++ b/tests/tcg/aarch64/bti-1.c
@@ -0,0 +1,77 @@
+/*
+ * Branch target identification, basic notskip cases.
+ */
+
+#include "bti-crt.inc.c"
+
+/*
+ * Work around lack of -mbranch-protection=standard in older toolchains.
+ * The signal handler is invoked by the kernel with PSTATE.BTYPE=2, which
+ * means that the handler must begin with a marker like BTI_C.
+ */
+asm("skip2_sigill1:\n\
+	hint	#34\n\
+	b	skip2_sigill2\n\
+.type skip2_sigill1,%function\n\
+.size skip2_sigill1,8");
+
+extern void skip2_sigill1(int sig, siginfo_t *info, ucontext_t *uc)
+    __attribute__((visibility("hidden")));
+
+static void __attribute__((used))
+skip2_sigill2(int sig, siginfo_t *info, ucontext_t *uc)
+{
+    uc->uc_mcontext.pc += 8;
+    uc->uc_mcontext.pstate = 1;
+}
+
+#define NOP       "nop"
+#define BTI_N     "hint #32"
+#define BTI_C     "hint #34"
+#define BTI_J     "hint #36"
+#define BTI_JC    "hint #38"
+
+#define BTYPE_1(DEST) \
+    asm("mov %0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %0,#0" \
+        : "=r"(skipped) : : "x16")
+
+#define BTYPE_2(DEST) \
+    asm("mov %0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %0,#0" \
+        : "=r"(skipped) : : "x16", "x30")
+
+#define BTYPE_3(DEST) \
+    asm("mov %0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %0,#0" \
+        : "=r"(skipped) : : "x15")
+
+#define TEST(WHICH, DEST, EXPECT) \
+    do { WHICH(DEST); fail += skipped ^ EXPECT; } while (0)
+
+
+int main()
+{
+    int fail = 0;
+    int skipped;
+
+    /* Signal-like with SA_SIGINFO.  */
+    signal_info(SIGILL, skip2_sigill1);
+
+    TEST(BTYPE_1, NOP, 1);
+    TEST(BTYPE_1, BTI_N, 1);
+    TEST(BTYPE_1, BTI_C, 0);
+    TEST(BTYPE_1, BTI_J, 0);
+    TEST(BTYPE_1, BTI_JC, 0);
+
+    TEST(BTYPE_2, NOP, 1);
+    TEST(BTYPE_2, BTI_N, 1);
+    TEST(BTYPE_2, BTI_C, 0);
+    TEST(BTYPE_2, BTI_J, 1);
+    TEST(BTYPE_2, BTI_JC, 0);
+
+    TEST(BTYPE_3, NOP, 1);
+    TEST(BTYPE_3, BTI_N, 1);
+    TEST(BTYPE_3, BTI_C, 1);
+    TEST(BTYPE_3, BTI_J, 0);
+    TEST(BTYPE_3, BTI_JC, 0);
+
+    return fail;
+}
diff --git a/tests/tcg/aarch64/bti-crt.inc.c b/tests/tcg/aarch64/bti-crt.inc.c
new file mode 100644
index 0000000000..bb363853de
--- /dev/null
+++ b/tests/tcg/aarch64/bti-crt.inc.c
@@ -0,0 +1,69 @@
+/*
+ * Minimal user-environment for testing BTI.
+ *
+ * Normal libc is not built with BTI support enabled, and so could
+ * generate a BTI TRAP before ever reaching main.
+ */
+
+#include <stdlib.h>
+#include <signal.h>
+#include <ucontext.h>
+#include <asm/unistd.h>
+
+int main(void);
+
+void _start(void)
+{
+    exit(main());
+}
+
+void exit(int ret)
+{
+    register int x0 __asm__("x0") = ret;
+    register int x8 __asm__("x8") = __NR_exit;
+
+    asm volatile("svc #0" : : "r"(x0), "r"(x8));
+    __builtin_unreachable();
+}
+
+/*
+ * Irritatingly, the user API struct sigaction does not match the
+ * kernel API struct sigaction.  So for simplicity, isolate the
+ * kernel ABI here, and make this act like signal.
+ */
+void signal_info(int sig, void (*fn)(int, siginfo_t *, ucontext_t *))
+{
+    struct kernel_sigaction {
+        void (*handler)(int, siginfo_t *, ucontext_t *);
+        unsigned long flags;
+        unsigned long restorer;
+        unsigned long mask;
+    } sa = { fn, SA_SIGINFO, 0, 0 };
+
+    register int x0 __asm__("x0") = sig;
+    register void *x1 __asm__("x1") = &sa;
+    register void *x2 __asm__("x2") = 0;
+    register int x3 __asm__("x3") = sizeof(unsigned long);
+    register int x8 __asm__("x8") = __NR_rt_sigaction;
+
+    asm volatile("svc #0"
+                 : : "r"(x0), "r"(x1), "r"(x2), "r"(x3), "r"(x8) : "memory");
+}
+
+/*
+ * Create the PT_NOTE that will enable BTI in the page tables.
+ * This will be created by the compiler with -mbranch-protection=standard,
+ * but as of 2019-03-29, this is has not been committed to gcc mainline.
+ * This will probably be in GCC10.
+ */
+asm(".section .note.gnu.property,\"a\"\n\
+	.align	3\n\
+	.long	4\n\
+        .long	16\n\
+        .long	5\n\
+        .string	\"GNU\"\n\
+	.long	0xc0000000\n\
+	.long	4\n\
+	.long	1\n\
+        .align  3\n\
+	.previous");
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index 2bb914975b..21da3bc37f 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -18,4 +18,7 @@ run-fcvt: fcvt
 AARCH64_TESTS += pauth-1
 run-pauth-%: QEMU += -cpu max
 
+AARCH64_TESTS += bti-1
+bti-1: LDFLAGS += -nostartfiles -nodefaultlibs -nostdlib
+
 TESTS:=$(AARCH64_TESTS)
-- 
2.17.1



^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems Richard Henderson
@ 2019-06-05 21:17   ` Aleksandar Markovic
  2019-06-06  5:45   ` Aleksandar Markovic
  1 sibling, 0 replies; 16+ messages in thread
From: Aleksandar Markovic @ 2019-06-05 21:17 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-arm, qemu-devel, Dave.Martin

On Jun 5, 2019 11:03 PM, "Richard Henderson" <richard.henderson@linaro.org>
wrote:
>
> This is a collection of related defines for notes, copied
> from glibc's <elf.h>.  We're not going to use all of these
> right away, but it seemed foolish to cherry-pick only the
> ones we need now.
>

But you are doing exactly that: cherry-picking, but this time according to
some undisclosed criterium. Why don't you state your motivation in the
commit message in a clear way? Why only these items are chosen? Confusing
and confused.

Regards,
Aleksandar

> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/elf.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/include/elf.h b/include/elf.h
> index ea7708a4ea..6f3eada36f 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -1650,6 +1650,54 @@ typedef struct elf64_shdr {
>  #define NT_ARM_HW_WATCH 0x403           /* ARM hardware watchpoint
registers */
>  #define NT_ARM_SYSTEM_CALL      0x404   /* ARM system call number */
>
> +/* Defined note types for GNU systems.  */
> +
> +#define NT_GNU_ABI_TAG          1       /* ABI information */
> +#define NT_GNU_HWCAP            2       /* Synthetic hwcap information */
> +#define NT_GNU_BUILD_ID         3       /* Build ID */
> +#define NT_GNU_GOLD_VERSION     4       /* Version of ld.gold */
> +#define NT_GNU_PROPERTY_TYPE_0  5       /* Program property */
> +
> +/* Values used in GNU .note.gnu.property notes
(NT_GNU_PROPERTY_TYPE_0).  */
> +
> +#define GNU_PROPERTY_STACK_SIZE                 1
> +#define GNU_PROPERTY_NO_COPY_ON_PROTECTED       2
> +
> +#define GNU_PROPERTY_LOPROC                     0xc0000000
> +#define GNU_PROPERTY_HIPROC                     0xdfffffff
> +#define GNU_PROPERTY_LOUSER                     0xe0000000
> +#define GNU_PROPERTY_HIUSER                     0xffffffff
> +
> +#define GNU_PROPERTY_X86_ISA_1_USED             0xc0000000
> +#define GNU_PROPERTY_X86_ISA_1_NEEDED           0xc0000001
> +
> +#define GNU_PROPERTY_X86_ISA_1_486              (1U << 0)
> +#define GNU_PROPERTY_X86_ISA_1_586              (1U << 1)
> +#define GNU_PROPERTY_X86_ISA_1_686              (1U << 2)
> +#define GNU_PROPERTY_X86_ISA_1_SSE              (1U << 3)
> +#define GNU_PROPERTY_X86_ISA_1_SSE2             (1U << 4)
> +#define GNU_PROPERTY_X86_ISA_1_SSE3             (1U << 5)
> +#define GNU_PROPERTY_X86_ISA_1_SSSE3            (1U << 6)
> +#define GNU_PROPERTY_X86_ISA_1_SSE4_1           (1U << 7)
> +#define GNU_PROPERTY_X86_ISA_1_SSE4_2           (1U << 8)
> +#define GNU_PROPERTY_X86_ISA_1_AVX              (1U << 9)
> +#define GNU_PROPERTY_X86_ISA_1_AVX2             (1U << 10)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512F          (1U << 11)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512CD         (1U << 12)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512ER         (1U << 13)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512PF         (1U << 14)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512VL         (1U << 15)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512DQ         (1U << 16)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512BW         (1U << 17)
> +
> +#define GNU_PROPERTY_X86_FEATURE_1_AND          0xc0000002
> +#define GNU_PROPERTY_X86_FEATURE_1_IBT          (1U << 0)
> +#define GNU_PROPERTY_X86_FEATURE_1_SHSTK        (1U << 1)
> +
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND      0xc0000000
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI      (1u << 0)
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC      (1u << 1)
> +
>  /*
>   * Physical entry point into the kernel.
>   *
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI
  2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
                   ` (5 preceding siblings ...)
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test Richard Henderson
@ 2019-06-05 22:15 ` no-reply
  6 siblings, 0 replies; 16+ messages in thread
From: no-reply @ 2019-06-05 22:15 UTC (permalink / raw)
  To: richard.henderson; +Cc: peter.maydell, qemu-arm, qemu-devel, Dave.Martin

Patchew URL: https://patchew.org/QEMU/20190605205706.569-1-richard.henderson@linaro.org/



Hi,

This series seems to have some coding style problems. See output below for
more information:

Subject: [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI
Type: series
Message-id: 20190605205706.569-1-richard.henderson@linaro.org

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

From https://github.com/patchew-project/qemu
 * [new tag]               patchew/20190605205706.569-1-richard.henderson@linaro.org -> patchew/20190605205706.569-1-richard.henderson@linaro.org
Switched to a new branch 'test'
6038ce2e0e tests/tcg/aarch64: Add bti smoke test
b16ad5bd7b linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes
68fefef717 include/elf: Add defines related to notes for GNU systems
951669d523 linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI
6ec44edda5 linux-user: Validate mmap/mprotect prot value
4eb77556ad linux-user/aarch64: Reset btype for syscalls and signals

=== OUTPUT BEGIN ===
1/6 Checking commit 4eb77556ad1e (linux-user/aarch64: Reset btype for syscalls and signals)
2/6 Checking commit 6ec44edda5a0 (linux-user: Validate mmap/mprotect prot value)
3/6 Checking commit 951669d5231b (linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI)
4/6 Checking commit 68fefef717be (include/elf: Add defines related to notes for GNU systems)
5/6 Checking commit b16ad5bd7b5c (linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes)
6/6 Checking commit 6038ce2e0e44 (tests/tcg/aarch64: Add bti smoke test)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#33: 
new file mode 100644

ERROR: code indent should never use tabs
#50: FILE: tests/tcg/aarch64/bti-1.c:13:
+^Ihint^I#34\n\$

ERROR: code indent should never use tabs
#51: FILE: tests/tcg/aarch64/bti-1.c:14:
+^Ib^Iskip2_sigill2\n\$

ERROR: externs should be avoided in .c files
#133: FILE: tests/tcg/aarch64/bti-crt.inc.c:13:
+int main(void);

ERROR: code indent should never use tabs
#180: FILE: tests/tcg/aarch64/bti-crt.inc.c:60:
+^I.align^I3\n\$

ERROR: code indent should never use tabs
#181: FILE: tests/tcg/aarch64/bti-crt.inc.c:61:
+^I.long^I4\n\$

ERROR: code indent should never use tabs
#182: FILE: tests/tcg/aarch64/bti-crt.inc.c:62:
+        .long^I16\n\$

ERROR: code indent should never use tabs
#183: FILE: tests/tcg/aarch64/bti-crt.inc.c:63:
+        .long^I5\n\$

ERROR: code indent should never use tabs
#184: FILE: tests/tcg/aarch64/bti-crt.inc.c:64:
+        .string^I\"GNU\"\n\$

ERROR: code indent should never use tabs
#185: FILE: tests/tcg/aarch64/bti-crt.inc.c:65:
+^I.long^I0xc0000000\n\$

ERROR: code indent should never use tabs
#186: FILE: tests/tcg/aarch64/bti-crt.inc.c:66:
+^I.long^I4\n\$

ERROR: code indent should never use tabs
#187: FILE: tests/tcg/aarch64/bti-crt.inc.c:67:
+^I.long^I1\n\$

ERROR: code indent should never use tabs
#189: FILE: tests/tcg/aarch64/bti-crt.inc.c:69:
+^I.previous");$

total: 12 errors, 1 warnings, 153 lines checked

Patch 6/6 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

=== OUTPUT END ===

Test command exited with code: 1


The full log is available at
http://patchew.org/logs/20190605205706.569-1-richard.henderson@linaro.org/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems Richard Henderson
  2019-06-05 21:17   ` Aleksandar Markovic
@ 2019-06-06  5:45   ` Aleksandar Markovic
  2019-06-06  8:42     ` Markus Armbruster
  1 sibling, 1 reply; 16+ messages in thread
From: Aleksandar Markovic @ 2019-06-06  5:45 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-arm, qemu-devel, Dave.Martin

On Jun 5, 2019 11:03 PM, "Richard Henderson" <richard.henderson@linaro.org>
wrote:
>
> This is a collection of related

Related to what?

> defines for notes, copied
> from glibc's <elf.h>.  We're not going to use all of these
> right away, but it seemed foolish

I don't think this an appropriate word for a commit message.

> to cherry-pick only the
> ones we need now.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  include/elf.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 48 insertions(+)
>
> diff --git a/include/elf.h b/include/elf.h
> index ea7708a4ea..6f3eada36f 100644
> --- a/include/elf.h
> +++ b/include/elf.h
> @@ -1650,6 +1650,54 @@ typedef struct elf64_shdr {
>  #define NT_ARM_HW_WATCH 0x403           /* ARM hardware watchpoint
registers */
>  #define NT_ARM_SYSTEM_CALL      0x404   /* ARM system call number */
>
> +/* Defined note types for GNU systems.  */
> +
> +#define NT_GNU_ABI_TAG          1       /* ABI information */
> +#define NT_GNU_HWCAP            2       /* Synthetic hwcap information */
> +#define NT_GNU_BUILD_ID         3       /* Build ID */
> +#define NT_GNU_GOLD_VERSION     4       /* Version of ld.gold */
> +#define NT_GNU_PROPERTY_TYPE_0  5       /* Program property */
> +
> +/* Values used in GNU .note.gnu.property notes
(NT_GNU_PROPERTY_TYPE_0).  */
> +
> +#define GNU_PROPERTY_STACK_SIZE                 1
> +#define GNU_PROPERTY_NO_COPY_ON_PROTECTED       2
> +
> +#define GNU_PROPERTY_LOPROC                     0xc0000000
> +#define GNU_PROPERTY_HIPROC                     0xdfffffff
> +#define GNU_PROPERTY_LOUSER                     0xe0000000
> +#define GNU_PROPERTY_HIUSER                     0xffffffff
> +
> +#define GNU_PROPERTY_X86_ISA_1_USED             0xc0000000
> +#define GNU_PROPERTY_X86_ISA_1_NEEDED           0xc0000001
> +
> +#define GNU_PROPERTY_X86_ISA_1_486              (1U << 0)
> +#define GNU_PROPERTY_X86_ISA_1_586              (1U << 1)
> +#define GNU_PROPERTY_X86_ISA_1_686              (1U << 2)
> +#define GNU_PROPERTY_X86_ISA_1_SSE              (1U << 3)
> +#define GNU_PROPERTY_X86_ISA_1_SSE2             (1U << 4)
> +#define GNU_PROPERTY_X86_ISA_1_SSE3             (1U << 5)
> +#define GNU_PROPERTY_X86_ISA_1_SSSE3            (1U << 6)
> +#define GNU_PROPERTY_X86_ISA_1_SSE4_1           (1U << 7)
> +#define GNU_PROPERTY_X86_ISA_1_SSE4_2           (1U << 8)
> +#define GNU_PROPERTY_X86_ISA_1_AVX              (1U << 9)
> +#define GNU_PROPERTY_X86_ISA_1_AVX2             (1U << 10)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512F          (1U << 11)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512CD         (1U << 12)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512ER         (1U << 13)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512PF         (1U << 14)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512VL         (1U << 15)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512DQ         (1U << 16)
> +#define GNU_PROPERTY_X86_ISA_1_AVX512BW         (1U << 17)
> +
> +#define GNU_PROPERTY_X86_FEATURE_1_AND          0xc0000002
> +#define GNU_PROPERTY_X86_FEATURE_1_IBT          (1U << 0)
> +#define GNU_PROPERTY_X86_FEATURE_1_SHSTK        (1U << 1)
> +
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_AND      0xc0000000
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI      (1u << 0)
> +#define GNU_PROPERTY_AARCH64_FEATURE_1_PAC      (1u << 1)
> +
>  /*
>   * Physical entry point into the kernel.
>   *
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems
  2019-06-06  5:45   ` Aleksandar Markovic
@ 2019-06-06  8:42     ` Markus Armbruster
  0 siblings, 0 replies; 16+ messages in thread
From: Markus Armbruster @ 2019-06-06  8:42 UTC (permalink / raw)
  To: Aleksandar Markovic
  Cc: peter.maydell, qemu-arm, Richard Henderson, qemu-devel, Dave.Martin

Aleksandar Markovic <aleksandar.m.mail@gmail.com> writes:

> On Jun 5, 2019 11:03 PM, "Richard Henderson" <richard.henderson@linaro.org>
> wrote:
>>
>> This is a collection of related
>
> Related to what?
>
>> defines for notes, copied
>> from glibc's <elf.h>.  We're not going to use all of these
>> right away, but it seemed foolish
>
> I don't think this an appropriate word for a commit message.

Calling an alternative you considered but rejected "foolish" feels
perfectly alright to me.

>> to cherry-pick only the
>> ones we need now.
>>
>> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signalsy
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signals Richard Henderson
@ 2019-06-06  9:53   ` Dave Martin
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Martin @ 2019-06-06  9:53 UTC (permalink / raw)
  To: Richard Henderson, g; +Cc: peter.maydell, qemu-arm, qemu-devel

On Wed, Jun 05, 2019 at 09:57:01PM +0100, Richard Henderson wrote:
> The value of btype for syscalls is CONSTRAINED UNPREDICTABLE,
> so we need to make sure that the value is 0 before clone,
> fork, or syscall return.
> 
> The kernel sets btype for the signal handler as if for a call.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/aarch64/cpu_loop.c |  7 +++++++
>  linux-user/aarch64/signal.c   | 10 ++++++++--
>  2 files changed, 15 insertions(+), 2 deletions(-)
> 
> diff --git a/linux-user/aarch64/cpu_loop.c b/linux-user/aarch64/cpu_loop.c
> index 2f2f63e3e8..1f68b13168 100644
> --- a/linux-user/aarch64/cpu_loop.c
> +++ b/linux-user/aarch64/cpu_loop.c
> @@ -86,6 +86,13 @@ void cpu_loop(CPUARMState *env)
>  
>          switch (trapnr) {
>          case EXCP_SWI:
> +            /*
> +             * The state of BTYPE on syscall entry is CONSTRAINED
> +             * UNPREDICTABLE.  The real kernel will need to tidy this up
> +             * as well.  Do this before syscalls so that the value is
> +             * correct on return from syscall (especially clone & fork).
> +             */
> +            env->btype = 0;

Note, after discussion with the architects, I think Linux won't bother
sanitising this field in my next spin of the patches.

If the SVC (or HVC or SMC) sits in a PROT_BTI page, then it won't
execute anyway unless BTYPE is already 0: otherwise, a branch target
exception would be taken instead.

If the insn isn't in a PROT_BTI page, then BTYPE could be nonzero when
the SVC (etc.) exception is taken, but it also won't matter in that case
what BTYPE is on return, since branch target exceptions are never
generated on insns in non-guarded pages.

For this to make a difference:

 a) the SVC be in a non-PROT_BTI page, just before a page boundary,
    where the next page is a PROT_BTI page, so that the exception return
    goes to the next page.

 b) the SVC must be an mprotect() call or similar that enabled PROT_BTI
    for the patch containing the SVC itself.

These are both silly things to do, and we probably don't care what
happens in such cases.

(Question: does qemu ever mprotect() the page containing PC?  I'd hope
not...)

>              ret = do_syscall(env,
>                               env->xregs[8],
>                               env->xregs[0],
> diff --git a/linux-user/aarch64/signal.c b/linux-user/aarch64/signal.c
> index f84a9cf28a..5605d404b3 100644
> --- a/linux-user/aarch64/signal.c
> +++ b/linux-user/aarch64/signal.c
> @@ -506,10 +506,16 @@ static void target_setup_frame(int usig, struct target_sigaction *ka,
>              + offsetof(struct target_rt_frame_record, tramp);
>      }
>      env->xregs[0] = usig;
> -    env->xregs[31] = frame_addr;
>      env->xregs[29] = frame_addr + fr_ofs;
> -    env->pc = ka->_sa_handler;
>      env->xregs[30] = return_addr;
> +    env->xregs[31] = frame_addr;
> +    env->pc = ka->_sa_handler;
> +
> +    /* Invoke the signal handler as if by indirect call.  */
> +    if (cpu_isar_feature(aa64_bti, arm_env_get_cpu(env))) {
> +        env->btype = 2;
> +    }
> +

Ack.  I had a simple test for this for native userspace, and it seems to
work as desired -- so I don't expect to change it.

Cheers
---Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes Richard Henderson
@ 2019-06-06 10:12   ` Dave Martin
  0 siblings, 0 replies; 16+ messages in thread
From: Dave Martin @ 2019-06-06 10:12 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-arm, qemu-devel

On Wed, Jun 05, 2019 at 09:57:05PM +0100, Richard Henderson wrote:
> For aarch64, this includes the GNU_PROPERTY_AARCH64_FEATURE_1_BTI bit,
> which indicates that the image should be mapped with guarded pages.

Heads-up: for arm64 I plan to move to making PT_GNU_PROPERTY
authoritiative for ELF on linux: if this is present, we use it to find
the note directly and ignore any other notes; if there is no
PT_GNU_PROPERTY entry then we assume there is no NT_GNU_PROPERTY_TYPE_0
note.

This is not quite decided yet, but to avoid fragmentation I'd prefer
qemu and Linux apply the same policy -- I'll keep you in the loop about
the decision.

I think you can reasonable upstream this patch in qemu and then
subsequently make it stricter without an ABI break.  Upstream GNU ld
generates this entry today.

> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/elfload.c | 83 +++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 75 insertions(+), 8 deletions(-)
> 
> diff --git a/linux-user/elfload.c b/linux-user/elfload.c
> index a57b7049dd..1a12c60a33 100644
> --- a/linux-user/elfload.c
> +++ b/linux-user/elfload.c
> @@ -2253,7 +2253,7 @@ static void load_elf_image(const char *image_name, int image_fd,
>      struct elfhdr *ehdr = (struct elfhdr *)bprm_buf;
>      struct elf_phdr *phdr;
>      abi_ulong load_addr, load_bias, loaddr, hiaddr, error;
> -    int i, retval;
> +    int i, retval, prot_exec = PROT_EXEC;
>      const char *errmsg;
>  
>      /* First of all, some simple consistency checks */
> @@ -2288,17 +2288,78 @@ static void load_elf_image(const char *image_name, int image_fd,
>      loaddr = -1, hiaddr = 0;
>      info->alignment = 0;
>      for (i = 0; i < ehdr->e_phnum; ++i) {
> -        if (phdr[i].p_type == PT_LOAD) {
> -            abi_ulong a = phdr[i].p_vaddr - phdr[i].p_offset;
> +        struct elf_phdr *eppnt = phdr + i;
> +
> +        if (eppnt->p_type == PT_LOAD) {
> +            abi_ulong a = eppnt->p_vaddr - eppnt->p_offset;
>              if (a < loaddr) {
>                  loaddr = a;
>              }
> -            a = phdr[i].p_vaddr + phdr[i].p_memsz;
> +            a = eppnt->p_vaddr + eppnt->p_memsz;
>              if (a > hiaddr) {
>                  hiaddr = a;
>              }
>              ++info->nsegs;
> -            info->alignment |= phdr[i].p_align;
> +            info->alignment |= eppnt->p_align;
> +        } else if (eppnt->p_type == PT_NOTE) {
> +#ifdef TARGET_AARCH64
> +            /*
> +             * Process NT_GNU_PROPERTY_TYPE_0.
> +             *
> +             * TODO: The only item that is AArch64 specific is the
> +             * GNU_PROPERTY_AARCH64_FEATURE_1_AND processing at the end.
> +             * If we were to ever process GNU_PROPERTY_X86_*, all of the
> +             * code through checking the gnu0 magic number is sharable.
> +             * But for now, since this *is* only used by AArch64, don't
> +             * process the note elsewhere.
> +             */
> +            const uint32_t gnu0_magic = const_le32('G' | 'N' << 8 | 'U' << 16);
> +            uint32_t note[7];
> +
> +            /*
> +             * The note contents are 7 words, but depending on LP64 vs ILP32
> +             * there may be an 8th padding word at the end.  Check for and
> +             * read the minimum size.  Further checks below will validate
> +             * that the sizes of everything involved are as we expect.
> +             */
> +            if (eppnt->p_filesz < sizeof(note)) {
> +                continue;
> +            }
> +            if (eppnt->p_offset + eppnt->p_filesz <= BPRM_BUF_SIZE) {
> +                memcpy(note, bprm_buf + eppnt->p_offset, sizeof(note));
> +            } else {
> +                retval = pread(image_fd, note, sizeof(note), eppnt->p_offset);
> +                if (retval != sizeof(note)) {
> +                    goto exit_perror;
> +                }
> +            }

Can we police that the segment alignment matches the ELF class (i.e., 8
for 64-bit, 4 for 32-bit)?

hjl specifies this, but it's controversial and sometimes missed --
there's a bug right now in GNU ld where --force-bti generates a note
with alignment 1.

Due to the high chance of screwing this up, it'd be good to police it
wherever appropriate.

> +#ifdef BSWAP_NEEDED
> +            for (i = 0; i < ARRAY_SIZE(note); ++i) {
> +                bswap32s(note + i);
> +            }
> +#endif
> +            /*
> +             * Check that this is a NT_GNU_PROPERTY_TYPE_0 note.
> +             * Again, descsz includes padding.  Full size validation
> +             * awaits checking the final payload.
> +             */
> +            if (note[0] != 4 ||                       /* namesz */
> +                note[1] < 12 ||                       /* descsz */
> +                note[2] != NT_GNU_PROPERTY_TYPE_0 ||  /* type */
> +                note[3] != gnu0_magic) {              /* name */
> +                continue;
> +            }
> +            /*
> +             * Check for the BTI feature.  If present, this indicates
> +             * that all the executable pages of the binary should be
> +             * mapped with PROT_BTI, so that branch targets are enforced.
> +             */
> +            if (note[4] == GNU_PROPERTY_AARCH64_FEATURE_1_AND &&
> +                note[5] == 4 &&
> +                (note[6] & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) {
> +                prot_exec |= TARGET_PROT_BTI;
> +            }
> +#endif /* TARGET_AARCH64 */
>          }
>      }
>  
> @@ -2358,9 +2419,15 @@ static void load_elf_image(const char *image_name, int image_fd,
>              abi_ulong vaddr, vaddr_po, vaddr_ps, vaddr_ef, vaddr_em, vaddr_len;
>              int elf_prot = 0;
>  
> -            if (eppnt->p_flags & PF_R) elf_prot =  PROT_READ;
> -            if (eppnt->p_flags & PF_W) elf_prot |= PROT_WRITE;
> -            if (eppnt->p_flags & PF_X) elf_prot |= PROT_EXEC;
> +            if (eppnt->p_flags & PF_R) {
> +                elf_prot |= PROT_READ;
> +            }
> +            if (eppnt->p_flags & PF_W) {
> +                elf_prot |= PROT_WRITE;
> +            }
> +            if (eppnt->p_flags & PF_X) {
> +                elf_prot |= prot_exec;
> +            }

Ack.

There are interesting subtleties with segment alignments here: the prot
flags on each segment "bleed" to an adjacent boundary.  I'm not sure
that behaviour is well specified, but we at least get away with it due
to conventions regarding the ordering of segments.

I still have to think about whether this works right for PROT_BTI, which
is a prohibition rather than a permission, suggesting that the bleed
rules might need to be inverted for it.

My current policy is that because and ELF is all-BTI if any page is BTI,
we probably get away with it, without having to do anything special.

[...]

Cheers
---Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test Richard Henderson
@ 2019-06-06 10:21   ` Dave Martin
  2019-06-06 14:24     ` Richard Henderson
  0 siblings, 1 reply; 16+ messages in thread
From: Dave Martin @ 2019-06-06 10:21 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-arm, qemu-devel

On Wed, Jun 05, 2019 at 09:57:06PM +0100, Richard Henderson wrote:
> This will build with older toolchains, without the upstream support
> for -mbranch-protection.  Such a toolchain will produce a warning
> in such cases,
> 
> ld: warning: /tmp/ccyZt0kq.o: unsupported GNU_PROPERTY_TYPE (5) \
> type: 0xc0000000
> 
> but the still places the note at the correct location in the binary
> for processing by the runtime loader.
> 
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  tests/tcg/aarch64/bti-1.c         | 77 +++++++++++++++++++++++++++++++
>  tests/tcg/aarch64/bti-crt.inc.c   | 69 +++++++++++++++++++++++++++
>  tests/tcg/aarch64/Makefile.target |  3 ++
>  3 files changed, 149 insertions(+)
>  create mode 100644 tests/tcg/aarch64/bti-1.c
>  create mode 100644 tests/tcg/aarch64/bti-crt.inc.c
> 
> diff --git a/tests/tcg/aarch64/bti-1.c b/tests/tcg/aarch64/bti-1.c
> new file mode 100644
> index 0000000000..2aee57ea7a
> --- /dev/null
> +++ b/tests/tcg/aarch64/bti-1.c
> @@ -0,0 +1,77 @@
> +/*
> + * Branch target identification, basic notskip cases.
> + */
> +
> +#include "bti-crt.inc.c"
> +
> +/*
> + * Work around lack of -mbranch-protection=standard in older toolchains.
> + * The signal handler is invoked by the kernel with PSTATE.BTYPE=2, which
> + * means that the handler must begin with a marker like BTI_C.
> + */
> +asm("skip2_sigill1:\n\
> +	hint	#34\n\
> +	b	skip2_sigill2\n\
> +.type skip2_sigill1,%function\n\
> +.size skip2_sigill1,8");
> +
> +extern void skip2_sigill1(int sig, siginfo_t *info, ucontext_t *uc)
> +    __attribute__((visibility("hidden")));
> +
> +static void __attribute__((used))
> +skip2_sigill2(int sig, siginfo_t *info, ucontext_t *uc)
> +{
> +    uc->uc_mcontext.pc += 8;
> +    uc->uc_mcontext.pstate = 1;
> +}
> +
> +#define NOP       "nop"
> +#define BTI_N     "hint #32"
> +#define BTI_C     "hint #34"
> +#define BTI_J     "hint #36"
> +#define BTI_JC    "hint #38"
> +
> +#define BTYPE_1(DEST) \
> +    asm("mov %0,#1; adr x16, 1f; br x16; 1: " DEST "; mov %0,#0" \
> +        : "=r"(skipped) : : "x16")
> +
> +#define BTYPE_2(DEST) \
> +    asm("mov %0,#1; adr x16, 1f; blr x16; 1: " DEST "; mov %0,#0" \
> +        : "=r"(skipped) : : "x16", "x30")
> +
> +#define BTYPE_3(DEST) \
> +    asm("mov %0,#1; adr x15, 1f; br x15; 1: " DEST "; mov %0,#0" \
> +        : "=r"(skipped) : : "x15")
> +
> +#define TEST(WHICH, DEST, EXPECT) \
> +    do { WHICH(DEST); fail += skipped ^ EXPECT; } while (0)
> +
> +
> +int main()
> +{
> +    int fail = 0;
> +    int skipped;
> +
> +    /* Signal-like with SA_SIGINFO.  */
> +    signal_info(SIGILL, skip2_sigill1);
> +
> +    TEST(BTYPE_1, NOP, 1);
> +    TEST(BTYPE_1, BTI_N, 1);
> +    TEST(BTYPE_1, BTI_C, 0);
> +    TEST(BTYPE_1, BTI_J, 0);
> +    TEST(BTYPE_1, BTI_JC, 0);
> +
> +    TEST(BTYPE_2, NOP, 1);
> +    TEST(BTYPE_2, BTI_N, 1);
> +    TEST(BTYPE_2, BTI_C, 0);
> +    TEST(BTYPE_2, BTI_J, 1);
> +    TEST(BTYPE_2, BTI_JC, 0);
> +
> +    TEST(BTYPE_3, NOP, 1);
> +    TEST(BTYPE_3, BTI_N, 1);
> +    TEST(BTYPE_3, BTI_C, 1);
> +    TEST(BTYPE_3, BTI_J, 0);
> +    TEST(BTYPE_3, BTI_JC, 0);
> +
> +    return fail;
> +}
> diff --git a/tests/tcg/aarch64/bti-crt.inc.c b/tests/tcg/aarch64/bti-crt.inc.c
> new file mode 100644
> index 0000000000..bb363853de
> --- /dev/null
> +++ b/tests/tcg/aarch64/bti-crt.inc.c
> @@ -0,0 +1,69 @@
> +/*
> + * Minimal user-environment for testing BTI.
> + *
> + * Normal libc is not built with BTI support enabled, and so could
> + * generate a BTI TRAP before ever reaching main.
> + */
> +
> +#include <stdlib.h>
> +#include <signal.h>
> +#include <ucontext.h>
> +#include <asm/unistd.h>
> +
> +int main(void);
> +
> +void _start(void)
> +{
> +    exit(main());
> +}
> +
> +void exit(int ret)
> +{
> +    register int x0 __asm__("x0") = ret;
> +    register int x8 __asm__("x8") = __NR_exit;
> +
> +    asm volatile("svc #0" : : "r"(x0), "r"(x8));
> +    __builtin_unreachable();
> +}
> +
> +/*
> + * Irritatingly, the user API struct sigaction does not match the
> + * kernel API struct sigaction.  So for simplicity, isolate the
> + * kernel ABI here, and make this act like signal.
> + */
> +void signal_info(int sig, void (*fn)(int, siginfo_t *, ucontext_t *))
> +{
> +    struct kernel_sigaction {
> +        void (*handler)(int, siginfo_t *, ucontext_t *);
> +        unsigned long flags;
> +        unsigned long restorer;
> +        unsigned long mask;
> +    } sa = { fn, SA_SIGINFO, 0, 0 };
> +
> +    register int x0 __asm__("x0") = sig;
> +    register void *x1 __asm__("x1") = &sa;
> +    register void *x2 __asm__("x2") = 0;
> +    register int x3 __asm__("x3") = sizeof(unsigned long);
> +    register int x8 __asm__("x8") = __NR_rt_sigaction;
> +
> +    asm volatile("svc #0"
> +                 : : "r"(x0), "r"(x1), "r"(x2), "r"(x3), "r"(x8) : "memory");
> +}
> +
> +/*
> + * Create the PT_NOTE that will enable BTI in the page tables.
> + * This will be created by the compiler with -mbranch-protection=standard,
> + * but as of 2019-03-29, this is has not been committed to gcc mainline.
> + * This will probably be in GCC10.

FYI, GCC9 has it.

> + */
> +asm(".section .note.gnu.property,\"a\"\n\
> +	.align	3\n\
> +	.long	4\n\
> +        .long	16\n\
> +        .long	5\n\
> +        .string	\"GNU\"\n\
> +	.long	0xc0000000\n\
> +	.long	4\n\
> +	.long	1\n\
> +        .align  3\n\
> +	.previous");

Note, this won't be enough to generate the PT_GNU_PROPERTY entry in the
program header table using older tools.

This may be work-round-able with a linker script, but I haven't looked
into it.

> diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
> index 2bb914975b..21da3bc37f 100644
> --- a/tests/tcg/aarch64/Makefile.target
> +++ b/tests/tcg/aarch64/Makefile.target
> @@ -18,4 +18,7 @@ run-fcvt: fcvt
>  AARCH64_TESTS += pauth-1
>  run-pauth-%: QEMU += -cpu max
>  
> +AARCH64_TESTS += bti-1
> +bti-1: LDFLAGS += -nostartfiles -nodefaultlibs -nostdlib
> +

Doesn't -nostdlib imply -nodefaultlibs and -nostartfiles?

Cheers
---Dave


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value
  2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value Richard Henderson
@ 2019-06-06 11:24   ` Aleksandar Markovic
  0 siblings, 0 replies; 16+ messages in thread
From: Aleksandar Markovic @ 2019-06-06 11:24 UTC (permalink / raw)
  To: Richard Henderson; +Cc: peter.maydell, qemu-arm, qemu-devel, Dave.Martin

On Jun 5, 2019 11:13 PM, "Richard Henderson" <richard.henderson@linaro.org>
wrote:
>
> The kernel will return -EINVAL for bits set in the prot argument
> that are unknown or invalid.  Previously we were simply cropping
> out the bits that we care about.
>
> Introduce validate_prot_to_pageflags to perform this check in a
> single place between the two syscalls.  Differentiate between
> the target and host versions of prot.  Compute the qemu internal
> page_flags value at the same time.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  linux-user/mmap.c | 106 ++++++++++++++++++++++++++++++++--------------
>  1 file changed, 75 insertions(+), 31 deletions(-)
>
> diff --git a/linux-user/mmap.c b/linux-user/mmap.c
> index af41339d57..3117f57fd8 100644
> --- a/linux-user/mmap.c
> +++ b/linux-user/mmap.c
> @@ -61,11 +61,38 @@ void mmap_fork_end(int child)
>          pthread_mutex_unlock(&mmap_mutex);
>  }
>
> +/*
> + * Validate target prot bitmask.
> + * Return the prot bitmask for the host in *HOST_PROT.
> + * Return 0 if the target prot bitmask is invalid, otherwise
> + * the internal qemu page_flags (which will include PAGE_VALID).
> + */
> +static int validate_prot_to_pageflags(int *host_prot, int prot)
> +{
> +    int valid = PROT_READ | PROT_WRITE | PROT_EXEC | TARGET_PROT_SEM;
> +    int page_flags = (prot & PAGE_BITS) | PAGE_VALID;
> +
> +    /*
> +     * For the host, we need not pass anything except read/write/exec.
> +     * While PROT_SEM is allowed by all hosts, it is also ignored, so
> +     * don't bother transforming guest bit to host bit.

I don't think that making assumptions based on an undocumented behavior is
the best practice.

Your “Let's don't bother” about such easy to implement things may create a
lot of bothering in future.

Regards,
Aleksandar

>  Any other
> +     * target-specific prot bits will not be understood by the host
> +     * and will need to be encoded into page_flags for qemu emulation.
> +     *
> +     * TODO: We do not actually have to map guest pages as executable,
> +     * since they will not be directly executed by the host.  We only
> +     * need to remember exec within page_flags.
> +     */
> +    *host_prot = prot & (PROT_READ | PROT_WRITE | PROT_EXEC);
> +
> +    return prot & ~valid ? 0 : page_flags;
> +}
> +
>  /* NOTE: all the constants are the HOST ones, but addresses are target.
*/
> -int target_mprotect(abi_ulong start, abi_ulong len, int prot)
> +int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
>  {
>      abi_ulong end, host_start, host_end, addr;
> -    int prot1, ret;
> +    int prot1, ret, page_flags, host_prot;
>
>  #ifdef DEBUG_MMAP
>      printf("mprotect: start=0x" TARGET_ABI_FMT_lx
> @@ -75,56 +102,65 @@ int target_mprotect(abi_ulong start, abi_ulong len,
int prot)
>             prot & PROT_EXEC ? 'x' : '-');
>  #endif
>
> -    if ((start & ~TARGET_PAGE_MASK) != 0)
> +    if ((start & ~TARGET_PAGE_MASK) != 0) {
>          return -TARGET_EINVAL;
> +    }
> +    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
> +    if (!page_flags) {
> +        return -TARGET_EINVAL;
> +    }
>      len = TARGET_PAGE_ALIGN(len);
>      end = start + len;
>      if (!guest_range_valid(start, len)) {
>          return -TARGET_ENOMEM;
>      }
> -    prot &= PROT_READ | PROT_WRITE | PROT_EXEC;
> -    if (len == 0)
> +    if (len == 0) {
>          return 0;
> +    }
>
>      mmap_lock();
>      host_start = start & qemu_host_page_mask;
>      host_end = HOST_PAGE_ALIGN(end);
>      if (start > host_start) {
>          /* handle host page containing start */
> -        prot1 = prot;
> -        for(addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
> +        prot1 = host_prot;
> +        for (addr = host_start; addr < start; addr += TARGET_PAGE_SIZE) {
>              prot1 |= page_get_flags(addr);
>          }
>          if (host_end == host_start + qemu_host_page_size) {
> -            for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
> +            for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
>                  prot1 |= page_get_flags(addr);
>              }
>              end = host_end;
>          }
> -        ret = mprotect(g2h(host_start), qemu_host_page_size, prot1 &
PAGE_BITS);
> -        if (ret != 0)
> +        ret = mprotect(g2h(host_start), qemu_host_page_size,
> +                       prot1 & PAGE_BITS);
> +        if (ret != 0) {
>              goto error;
> +        }
>          host_start += qemu_host_page_size;
>      }
>      if (end < host_end) {
> -        prot1 = prot;
> -        for(addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
> +        prot1 = host_prot;
> +        for (addr = end; addr < host_end; addr += TARGET_PAGE_SIZE) {
>              prot1 |= page_get_flags(addr);
>          }
> -        ret = mprotect(g2h(host_end - qemu_host_page_size),
qemu_host_page_size,
> -                       prot1 & PAGE_BITS);
> -        if (ret != 0)
> +        ret = mprotect(g2h(host_end - qemu_host_page_size),
> +                       qemu_host_page_size, prot1 & PAGE_BITS);
> +        if (ret != 0) {
>              goto error;
> +        }
>          host_end -= qemu_host_page_size;
>      }
>
>      /* handle the pages in the middle */
>      if (host_start < host_end) {
> -        ret = mprotect(g2h(host_start), host_end - host_start, prot);
> -        if (ret != 0)
> +        ret = mprotect(g2h(host_start), host_end - host_start,
host_prot);
> +        if (ret != 0) {
>              goto error;
> +        }
>      }
> -    page_set_flags(start, start + len, prot | PAGE_VALID);
> +    page_set_flags(start, start + len, page_flags);
>      mmap_unlock();
>      return 0;
>  error:
> @@ -364,10 +400,11 @@ abi_ulong mmap_find_vma(abi_ulong start, abi_ulong
size, abi_ulong align)
>  }
>
>  /* NOTE: all the constants are the HOST ones */
> -abi_long target_mmap(abi_ulong start, abi_ulong len, int prot,
> +abi_long target_mmap(abi_ulong start, abi_ulong len, int target_prot,
>                       int flags, int fd, abi_ulong offset)
>  {
>      abi_ulong ret, end, real_start, real_end, retaddr, host_offset,
host_len;
> +    int page_flags, host_prot;
>
>      mmap_lock();
>  #ifdef DEBUG_MMAP
> @@ -402,6 +439,12 @@ abi_long target_mmap(abi_ulong start, abi_ulong len,
int prot,
>          goto fail;
>      }
>
> +    page_flags = validate_prot_to_pageflags(&host_prot, target_prot);
> +    if (!page_flags) {
> +        errno = EINVAL;
> +        goto fail;
> +    }
> +
>      /* Also check for overflows... */
>      len = TARGET_PAGE_ALIGN(len);
>      if (!len) {
> @@ -467,14 +510,15 @@ abi_long target_mmap(abi_ulong start, abi_ulong
len, int prot,
>          /* Note: we prefer to control the mapping address. It is
>             especially important if qemu_host_page_size >
>             qemu_real_host_page_size */
> -        p = mmap(g2h(start), host_len, prot,
> +        p = mmap(g2h(start), host_len, host_prot,
>                   flags | MAP_FIXED | MAP_ANONYMOUS, -1, 0);
> -        if (p == MAP_FAILED)
> +        if (p == MAP_FAILED) {
>              goto fail;
> +        }
>          /* update start so that it points to the file position at
'offset' */
>          host_start = (unsigned long)p;
>          if (!(flags & MAP_ANONYMOUS)) {
> -            p = mmap(g2h(start), len, prot,
> +            p = mmap(g2h(start), len, host_prot,
>                       flags | MAP_FIXED, fd, host_offset);
>              if (p == MAP_FAILED) {
>                  munmap(g2h(start), host_len);
> @@ -508,19 +552,19 @@ abi_long target_mmap(abi_ulong start, abi_ulong
len, int prot,
>              /* msync() won't work here, so we return an error if write is
>                 possible while it is a shared mapping */
>              if ((flags & MAP_TYPE) == MAP_SHARED &&
> -                (prot & PROT_WRITE)) {
> +                (host_prot & PROT_WRITE)) {
>                  errno = EINVAL;
>                  goto fail;
>              }
> -            retaddr = target_mmap(start, len, prot | PROT_WRITE,
> +            retaddr = target_mmap(start, len, target_prot | PROT_WRITE,
>                                    MAP_FIXED | MAP_PRIVATE |
MAP_ANONYMOUS,
>                                    -1, 0);
>              if (retaddr == -1)
>                  goto fail;
>              if (pread(fd, g2h(start), len, offset) == -1)
>                  goto fail;
> -            if (!(prot & PROT_WRITE)) {
> -                ret = target_mprotect(start, len, prot);
> +            if (!(host_prot & PROT_WRITE)) {
> +                ret = target_mprotect(start, len, target_prot);
>                  assert(ret == 0);
>              }
>              goto the_end;
> @@ -531,13 +575,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong
len, int prot,
>              if (real_end == real_start + qemu_host_page_size) {
>                  /* one single host page */
>                  ret = mmap_frag(real_start, start, end,
> -                                prot, flags, fd, offset);
> +                                host_prot, flags, fd, offset);
>                  if (ret == -1)
>                      goto fail;
>                  goto the_end1;
>              }
>              ret = mmap_frag(real_start, start, real_start +
qemu_host_page_size,
> -                            prot, flags, fd, offset);
> +                            host_prot, flags, fd, offset);
>              if (ret == -1)
>                  goto fail;
>              real_start += qemu_host_page_size;
> @@ -546,7 +590,7 @@ abi_long target_mmap(abi_ulong start, abi_ulong len,
int prot,
>          if (end < real_end) {
>              ret = mmap_frag(real_end - qemu_host_page_size,
>                              real_end - qemu_host_page_size, end,
> -                            prot, flags, fd,
> +                            host_prot, flags, fd,
>                              offset + real_end - qemu_host_page_size -
start);
>              if (ret == -1)
>                  goto fail;
> @@ -562,13 +606,13 @@ abi_long target_mmap(abi_ulong start, abi_ulong
len, int prot,
>              else
>                  offset1 = offset + real_start - start;
>              p = mmap(g2h(real_start), real_end - real_start,
> -                     prot, flags, fd, offset1);
> +                     host_prot, flags, fd, offset1);
>              if (p == MAP_FAILED)
>                  goto fail;
>          }
>      }
>   the_end1:
> -    page_set_flags(start, start + len, prot | PAGE_VALID);
> +    page_set_flags(start, start + len, page_flags);
>   the_end:
>  #ifdef DEBUG_MMAP
>      printf("ret=0x" TARGET_ABI_FMT_lx "\n", start);
> --
> 2.17.1
>
>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test
  2019-06-06 10:21   ` Dave Martin
@ 2019-06-06 14:24     ` Richard Henderson
  0 siblings, 0 replies; 16+ messages in thread
From: Richard Henderson @ 2019-06-06 14:24 UTC (permalink / raw)
  To: Dave Martin; +Cc: peter.maydell, qemu-arm, qemu-devel

On 6/6/19 5:21 AM, Dave Martin wrote:
>> +/*
>> + * Create the PT_NOTE that will enable BTI in the page tables.
>> + * This will be created by the compiler with -mbranch-protection=standard,
>> + * but as of 2019-03-29, this is has not been committed to gcc mainline.
>> + * This will probably be in GCC10.
> 
> FYI, GCC9 has it.

Thanks.

>> + */
>> +asm(".section .note.gnu.property,\"a\"\n\
>> +	.align	3\n\
>> +	.long	4\n\
>> +        .long	16\n\
>> +        .long	5\n\
>> +        .string	\"GNU\"\n\
>> +	.long	0xc0000000\n\
>> +	.long	4\n\
>> +	.long	1\n\
>> +        .align  3\n\
>> +	.previous");
> 
> Note, this won't be enough to generate the PT_GNU_PROPERTY entry in the
> program header table using older tools.
> 
> This may be work-round-able with a linker script, but I haven't looked
> into it.

Yes, a linker script can create such an entry.

>> +AARCH64_TESTS += bti-1
>> +bti-1: LDFLAGS += -nostartfiles -nodefaultlibs -nostdlib
>> +
> 
> Doesn't -nostdlib imply -nodefaultlibs and -nostartfiles?

I don't believe so, but I'll double-check.


r~


^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2019-06-06 14:25 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-05 20:57 [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI Richard Henderson
2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signals Richard Henderson
2019-06-06  9:53   ` [Qemu-devel] [PATCH v6 1/6] linux-user/aarch64: Reset btype for syscalls and signalsy Dave Martin
2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 2/6] linux-user: Validate mmap/mprotect prot value Richard Henderson
2019-06-06 11:24   ` Aleksandar Markovic
2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 3/6] linux-user: Set PAGE_TARGET_1 for TARGET_PROT_BTI Richard Henderson
2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 4/6] include/elf: Add defines related to notes for GNU systems Richard Henderson
2019-06-05 21:17   ` Aleksandar Markovic
2019-06-06  5:45   ` Aleksandar Markovic
2019-06-06  8:42     ` Markus Armbruster
2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 5/6] linux-user: Parse NT_GNU_PROPERTY_TYPE_0 notes Richard Henderson
2019-06-06 10:12   ` Dave Martin
2019-06-05 20:57 ` [Qemu-devel] [PATCH v6 6/6] tests/tcg/aarch64: Add bti smoke test Richard Henderson
2019-06-06 10:21   ` Dave Martin
2019-06-06 14:24     ` Richard Henderson
2019-06-05 22:15 ` [Qemu-devel] [PATCH v6 0/6] linux-user/aarch64: Support PROT_BTI no-reply

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.