* [PATCH 0/8] arm64: ARMv8.5-A: Branch Target Identification support
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
This patch implements support for ARMv8.5-A Branch Target Identification
(BTI), which is a control flow integrity protection feature introduced
as part of the ARMv8.5-A extensions [1].
The series is based on v5.2-rc1.
Patch 1 is from Yu-Cheng Yu of Intel, providing generic support
for parsing the ELF NT_GNU_PROPERTY_TYPE_0 note. It makes sense to
share this mechanism with x86 rather than reinventing it.
Various things need nailing down before this can be upstreamable:
* Not tested with hugepages yet. (If anyone has any suggestions about
how best to do that, please shout!)
* The NT_GNU_PROPERTY_TYPE_0 ELF note parsing support is not upstream
yet and may be subject to further change.
Todo:
* Add BTI protection in the vDSO, so that user code can no longer
jump to random locations in there. Lack of this protection doesn't
break anything, however.
Tested on the ARM Fast Model.
Notes:
* GCC 9 can compile backwards-compatible BTI-enabled code with
-mbranch-protection=bti or -mbranch-protection=standard.
* Binutils trunk supports the new ELF note, but this isn't in a release
yet.
Creation of a BTI-enabled binary requires _everything_ linked in to
be BTI-enabled. For now ld --force-bti can be used to override this,
but some things may break until the required C library support is in
place.
There is no straightforward way to mark a .s file as BTI-enabled:
scraping the output from gcc -S works as a quick hack for now.
readelf -n can be used to examing the program properties in an ELF
file.
* Runtime mmap() and mprotect() can be used to enable BTI on a
page-by-page basis using the new PROT_BTI_GUARDED, but the code in
the affected pages still needs to be written or compiled to contain
the appopriate BTI landing pads.
Dave Martin (7):
mm: Reserve asm-generic prot flag 0x10 for arch use
arm64: docs: cpu-feature-registers: Document ID_AA64PFR1_EL1
arm64: Basic Branch Target Identification support
elf: Parse program properties before destroying the old process
elf: Allow arch to tweak initial mmap prot flags
arm64: elf: Enable BTI at exec based on ELF program properties
arm64: BTI: Decode BYTPE bits when printing PSTATE
Yu-cheng Yu (1):
binfmt_elf: Extract .note.gnu.property from an ELF file
Documentation/arm64/cpu-feature-registers.txt | 18 +-
Documentation/arm64/elf_hwcaps.txt | 4 +
arch/arm64/Kconfig | 26 ++
arch/arm64/include/asm/cpucaps.h | 3 +-
arch/arm64/include/asm/cpufeature.h | 6 +
arch/arm64/include/asm/elf.h | 28 ++
arch/arm64/include/asm/esr.h | 2 +-
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/asm/mman.h | 33 +++
arch/arm64/include/asm/pgtable-hwdef.h | 1 +
arch/arm64/include/asm/pgtable.h | 2 +-
arch/arm64/include/asm/ptrace.h | 3 +
arch/arm64/include/asm/sysreg.h | 2 +
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/mman.h | 9 +
arch/arm64/include/uapi/asm/ptrace.h | 1 +
arch/arm64/kernel/cpufeature.c | 17 ++
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/kernel/entry.S | 11 +
arch/arm64/kernel/process.c | 64 ++++-
arch/arm64/kernel/ptrace.c | 2 +-
arch/arm64/kernel/signal.c | 5 +
arch/arm64/kernel/syscall.c | 1 +
arch/arm64/kernel/traps.c | 7 +
fs/Kconfig.binfmt | 7 +
fs/Makefile | 1 +
fs/binfmt_elf.c | 31 ++-
fs/gnu_property.c | 363 ++++++++++++++++++++++++++
include/linux/elf.h | 32 +++
include/linux/mm.h | 3 +
include/uapi/asm-generic/mman-common.h | 1 +
include/uapi/linux/elf.h | 14 +
32 files changed, 684 insertions(+), 16 deletions(-)
create mode 100644 arch/arm64/include/asm/mman.h
create mode 100644 arch/arm64/include/uapi/asm/mman.h
create mode 100644 fs/gnu_property.c
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 1/8] binfmt_elf: Extract .note.gnu.property from an ELF file
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
From: Yu-cheng Yu <yu-cheng.yu@intel.com>
An ELF file's .note.gnu.property indicates features the executable file
can support. For example, the property GNU_PROPERTY_X86_FEATURE_1_AND
indicates the file supports GNU_PROPERTY_X86_FEATURE_1_IBT and/or
GNU_PROPERTY_X86_FEATURE_1_SHSTK.
This patch was part of the Control-flow Enforcement series; the original
patch is here: https://lkml.org/lkml/2018/11/20/205. Dave Martin responded
that ARM recently introduced new features to NT_GNU_PROPERTY_TYPE_0 with
properties closely modelled on GNU_PROPERTY_X86_FEATURE_1_AND, and it is
logical to split out the generic part. Here it is.
With this patch, if an arch needs to setup features from ELF properties,
it needs CONFIG_ARCH_USE_GNU_PROPERTY to be set, and a specific
arch_setup_property().
For example, for X86_64:
int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
{
int r;
uint32_t property;
r = get_gnu_property(ehdr, phdr, f, GNU_PROPERTY_X86_FEATURE_1_AND,
&property);
...
}
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com> [rebase to v5.2-rc1]
---
Notes from Dave:
* For arm64 at least, I'm hoping we can rely on the PT_GNU_PROPERTY
program header to find the program properties. If this works for
x86 too, I think the code could be simplified considerably.
Ideally we would always use that: having to exhaustively search
all the notes in the image instead doesn't feel like the right design
IMHO.
This needs further discussion, and I make no attempt to address it
for now.
---
fs/Kconfig.binfmt | 4 +
fs/Makefile | 1 +
fs/binfmt_elf.c | 13 ++
fs/gnu_property.c | 363 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/elf.h | 12 ++
include/uapi/linux/elf.h | 8 ++
6 files changed, 401 insertions(+)
create mode 100644 fs/gnu_property.c
diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
index b795f8d..175a1f5 100644
--- a/fs/Kconfig.binfmt
+++ b/fs/Kconfig.binfmt
@@ -35,6 +35,10 @@ config COMPAT_BINFMT_ELF
config ARCH_BINFMT_ELF_STATE
bool
+config ARCH_USE_GNU_PROPERTY
+ bool
+ depends on 64BIT
+
config BINFMT_ELF_FDPIC
bool "Kernel support for FDPIC ELF binaries"
default y if !BINFMT_ELF
diff --git a/fs/Makefile b/fs/Makefile
index c9aea23..b69f18c 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_BINFMT_ELF) += binfmt_elf.o
obj-$(CONFIG_COMPAT_BINFMT_ELF) += compat_binfmt_elf.o
obj-$(CONFIG_BINFMT_ELF_FDPIC) += binfmt_elf_fdpic.o
obj-$(CONFIG_BINFMT_FLAT) += binfmt_flat.o
+obj-$(CONFIG_ARCH_USE_GNU_PROPERTY) += gnu_property.o
obj-$(CONFIG_FS_MBCACHE) += mbcache.o
obj-$(CONFIG_FS_POSIX_ACL) += posix_acl.o
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa9e99a..18015fc 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1080,6 +1080,19 @@ static int load_elf_binary(struct linux_binprm *bprm)
}
if (interpreter) {
+ retval = arch_setup_property(&loc->interp_elf_ex,
+ interp_elf_phdata,
+ interpreter, true);
+ } else {
+ retval = arch_setup_property(&loc->elf_ex,
+ elf_phdata,
+ bprm->file, false);
+ }
+
+ if (retval < 0)
+ goto out_free_dentry;
+
+ if (interpreter) {
unsigned long interp_map_addr = 0;
elf_entry = load_elf_interp(&loc->interp_elf_ex,
diff --git a/fs/gnu_property.c b/fs/gnu_property.c
new file mode 100644
index 0000000..656ea39
--- /dev/null
+++ b/fs/gnu_property.c
@@ -0,0 +1,363 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Extract an ELF file's .note.gnu.property.
+ *
+ * The path from the ELF header to the note section is the following:
+ * elfhdr->elf_phdr->elf_note->property[].
+ */
+
+#include <uapi/linux/elf-em.h>
+#include <linux/processor.h>
+#include <linux/binfmts.h>
+#include <linux/elf.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/string.h>
+#include <linux/compat.h>
+
+/*
+ * The .note.gnu.property layout:
+ *
+ * struct elf_note {
+ * u32 n_namesz; --> sizeof(n_name[]); always (4)
+ * u32 n_ndescsz;--> sizeof(property[])
+ * u32 n_type; --> always NT_GNU_PROPERTY_TYPE_0
+ * };
+ * char n_name[4]; --> always 'GNU\0'
+ *
+ * struct {
+ * struct gnu_property {
+ * u32 pr_type;
+ * u32 pr_datasz;
+ * };
+ * u8 pr_data[pr_datasz];
+ * }[];
+ */
+
+#define BUF_SIZE (PAGE_SIZE / 4)
+
+struct gnu_property {
+ u32 pr_type;
+ u32 pr_datasz;
+};
+
+typedef bool (test_item_fn)(void *buf, u32 *arg, u32 type);
+typedef void *(next_item_fn)(void *buf, u32 *arg, u32 type);
+
+static inline bool test_note_type(void *buf, u32 *align, u32 note_type)
+{
+ struct elf_note *n = buf;
+
+ return ((n->n_type == note_type) && (n->n_namesz == 4) &&
+ (memcmp(n + 1, "GNU", 4) == 0));
+}
+
+static inline void *next_note(void *buf, u32 *align, u32 note_type)
+{
+ struct elf_note *n = buf;
+ u64 size;
+
+ if (check_add_overflow((u64)sizeof(*n), (u64)n->n_namesz, &size))
+ return NULL;
+
+ size = round_up(size, *align);
+
+ if (check_add_overflow(size, (u64)n->n_descsz, &size))
+ return NULL;
+
+ size = round_up(size, *align);
+
+ if (buf + size < buf)
+ return NULL;
+ else
+ return (buf + size);
+}
+
+static inline bool test_property(void *buf, u32 *max_type, u32 pr_type)
+{
+ struct gnu_property *pr = buf;
+
+ /*
+ * Property types must be in ascending order.
+ * Keep track of the max when testing each.
+ */
+ if (pr->pr_type > *max_type)
+ *max_type = pr->pr_type;
+
+ return (pr->pr_type == pr_type);
+}
+
+static inline void *next_property(void *buf, u32 *max_type, u32 pr_type)
+{
+ struct gnu_property *pr = buf;
+
+ if ((buf + sizeof(*pr) + pr->pr_datasz < buf) ||
+ (pr->pr_type > pr_type) ||
+ (pr->pr_type > *max_type))
+ return NULL;
+ else
+ return (buf + sizeof(*pr) + pr->pr_datasz);
+}
+
+/*
+ * Scan 'buf' for a pattern; return true if found.
+ * *pos is the distance from the beginning of buf to where
+ * the searched item or the next item is located.
+ */
+static int scan(u8 *buf, u32 buf_size, int item_size, test_item_fn test_item,
+ next_item_fn next_item, u32 *arg, u32 type, u32 *pos)
+{
+ int found = 0;
+ u8 *p, *max;
+
+ max = buf + buf_size;
+ if (max < buf)
+ return 0;
+
+ p = buf;
+
+ while ((p + item_size < max) && (p + item_size > buf)) {
+ if (test_item(p, arg, type)) {
+ found = 1;
+ break;
+ }
+
+ p = next_item(p, arg, type);
+ }
+
+ *pos = (p + item_size <= buf) ? 0 : (u32)(p - buf);
+ return found;
+}
+
+/*
+ * Search an NT_GNU_PROPERTY_TYPE_0 for the property of 'pr_type'.
+ */
+static int find_property(struct file *file, unsigned long desc_size,
+ loff_t file_offset, u8 *buf,
+ u32 pr_type, u32 *property)
+{
+ u32 buf_pos;
+ unsigned long read_size;
+ unsigned long done;
+ int found = 0;
+ int ret = 0;
+ u32 last_pr = 0;
+
+ *property = 0;
+ buf_pos = 0;
+
+ for (done = 0; done < desc_size; done += buf_pos) {
+ read_size = desc_size - done;
+ if (read_size > BUF_SIZE)
+ read_size = BUF_SIZE;
+
+ ret = kernel_read(file, buf, read_size, &file_offset);
+
+ if (ret != read_size)
+ return (ret < 0) ? ret : -EIO;
+
+ ret = 0;
+ found = scan(buf, read_size, sizeof(struct gnu_property),
+ test_property, next_property,
+ &last_pr, pr_type, &buf_pos);
+
+ if ((!buf_pos) || found)
+ break;
+
+ file_offset += buf_pos - read_size;
+ }
+
+ if (found) {
+ struct gnu_property *pr =
+ (struct gnu_property *)(buf + buf_pos);
+
+ if (pr->pr_datasz == 4) {
+ u32 *max = (u32 *)(buf + read_size);
+ u32 *data = (u32 *)((u8 *)pr + sizeof(*pr));
+
+ if (data + 1 <= max) {
+ *property = *data;
+ } else {
+ file_offset += buf_pos - read_size;
+ file_offset += sizeof(*pr);
+ ret = kernel_read(file, property, 4,
+ &file_offset);
+ }
+ }
+ }
+
+ return ret;
+}
+
+/*
+ * Search a PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
+ */
+static int find_note_type_0(struct file *file, loff_t file_offset,
+ unsigned long note_size, u32 align,
+ u32 pr_type, u32 *property)
+{
+ u8 *buf;
+ u32 buf_pos;
+ unsigned long read_size;
+ unsigned long done;
+ int found = 0;
+ int ret = 0;
+
+ buf = kmalloc(BUF_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ *property = 0;
+ buf_pos = 0;
+
+ for (done = 0; done < note_size; done += buf_pos) {
+ read_size = note_size - done;
+ if (read_size > BUF_SIZE)
+ read_size = BUF_SIZE;
+
+ ret = kernel_read(file, buf, read_size, &file_offset);
+
+ if (ret != read_size) {
+ ret = (ret < 0) ? ret : -EIO;
+ kfree(buf);
+ return ret;
+ }
+
+ /*
+ * item_size = sizeof(struct elf_note) + elf_note.n_namesz.
+ * n_namesz is 4 for the note type we look for.
+ */
+ ret = scan(buf, read_size, sizeof(struct elf_note) + 4,
+ test_note_type, next_note,
+ &align, NT_GNU_PROPERTY_TYPE_0, &buf_pos);
+
+ file_offset += buf_pos - read_size;
+
+ if (ret && !found) {
+ struct elf_note *n =
+ (struct elf_note *)(buf + buf_pos);
+ u64 start = round_up(sizeof(*n) + n->n_namesz, align);
+ u64 total = 0;
+
+ if (check_add_overflow(start, (u64)n->n_descsz, &total)) {
+ ret = -EINVAL;
+ break;
+ }
+ total = round_up(total, align);
+
+ ret = find_property(file, n->n_descsz,
+ file_offset + start,
+ buf, pr_type, property);
+ found++;
+ file_offset += total;
+ buf_pos += total;
+ } else if (!buf_pos || ret) {
+ ret = 0;
+ *property = 0;
+ break;
+ }
+ }
+
+ kfree(buf);
+ return ret;
+}
+
+/*
+ * Look at an ELF file's PT_NOTE segments, then NT_GNU_PROPERTY_TYPE_0, then
+ * the property of pr_type.
+ *
+ * Input:
+ * file: the file to search;
+ * phdr: the file's elf header;
+ * phnum: number of entries in phdr;
+ * pr_type: the property type.
+ *
+ * Output:
+ * The property found.
+ *
+ * Return:
+ * Zero or error.
+ */
+static int scan_segments_64(struct file *file, struct elf64_phdr *phdr,
+ int phnum, u32 pr_type, u32 *property)
+{
+ int i;
+ int err = 0;
+
+ for (i = 0; i < phnum; i++, phdr++) {
+ if ((phdr->p_type != PT_NOTE) || (phdr->p_align != 8))
+ continue;
+
+ /*
+ * Search the PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
+ */
+ err = find_note_type_0(file, phdr->p_offset, phdr->p_filesz,
+ phdr->p_align, pr_type, property);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_COMPAT
+static int scan_segments_32(struct file *file, struct elf32_phdr *phdr,
+ int phnum, u32 pr_type, u32 *property)
+{
+ int i;
+ int err = 0;
+
+ for (i = 0; i < phnum; i++, phdr++) {
+ if ((phdr->p_type != PT_NOTE) || (phdr->p_align != 4))
+ continue;
+
+ /*
+ * Search the PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
+ */
+ err = find_note_type_0(file, phdr->p_offset, phdr->p_filesz,
+ phdr->p_align, pr_type, property);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+#endif
+
+int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
+ u32 pr_type, u32 *property)
+{
+ struct elf64_hdr *ehdr64 = ehdr_p;
+ int err = 0;
+
+ *property = 0;
+
+ if (ehdr64->e_ident[EI_CLASS] == ELFCLASS64) {
+ struct elf64_phdr *phdr64 = phdr_p;
+
+ err = scan_segments_64(f, phdr64, ehdr64->e_phnum,
+ pr_type, property);
+ if (err < 0)
+ goto out;
+ } else {
+#ifdef CONFIG_COMPAT
+ struct elf32_hdr *ehdr32 = ehdr_p;
+
+ if (ehdr32->e_ident[EI_CLASS] == ELFCLASS32) {
+ struct elf32_phdr *phdr32 = phdr_p;
+
+ err = scan_segments_32(f, phdr32, ehdr32->e_phnum,
+ pr_type, property);
+ if (err < 0)
+ goto out;
+ }
+#else
+ WARN_ONCE(1, "Exec of 32-bit app, but CONFIG_COMPAT is not enabled.\n");
+ return -ENOTSUPP;
+#endif
+ }
+
+out:
+ return err;
+}
diff --git a/include/linux/elf.h b/include/linux/elf.h
index e3649b3..c15febe 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -56,4 +56,16 @@ static inline int elf_coredump_extra_notes_write(struct coredump_params *cprm) {
extern int elf_coredump_extra_notes_size(void);
extern int elf_coredump_extra_notes_write(struct coredump_params *cprm);
#endif
+
+#ifdef CONFIG_ARCH_USE_GNU_PROPERTY
+extern int arch_setup_property(void *ehdr, void *phdr, struct file *f,
+ bool interp);
+extern int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
+ u32 pr_type, u32 *feature);
+#else
+static inline int arch_setup_property(void *ehdr, void *phdr, struct file *f,
+ bool interp) { return 0; }
+static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
+ u32 pr_type, u32 *feature) { return 0; }
+#endif
#endif /* _LINUX_ELF_H */
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 34c02e4..7b7603a 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -372,6 +372,7 @@ typedef struct elf64_shdr {
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
#define NT_TASKSTRUCT 4
+#define NT_GNU_PROPERTY_TYPE_0 5
#define NT_AUXV 6
/*
* Note to userspace developers: size of NT_SIGINFO note may increase
@@ -443,4 +444,11 @@ typedef struct elf64_note {
Elf64_Word n_type; /* Content type */
} Elf64_Nhdr;
+/* .note.gnu.property types */
+#define GNU_PROPERTY_X86_FEATURE_1_AND (0xc0000002)
+
+/* Bits of GNU_PROPERTY_X86_FEATURE_1_AND */
+#define GNU_PROPERTY_X86_FEATURE_1_IBT (0x00000001)
+#define GNU_PROPERTY_X86_FEATURE_1_SHSTK (0x00000002)
+
#endif /* _UAPI_LINUX_ELF_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 1/8] binfmt_elf: Extract .note.gnu.property from an ELF file
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
From: Yu-cheng Yu <yu-cheng.yu@intel.com>
An ELF file's .note.gnu.property indicates features the executable file
can support. For example, the property GNU_PROPERTY_X86_FEATURE_1_AND
indicates the file supports GNU_PROPERTY_X86_FEATURE_1_IBT and/or
GNU_PROPERTY_X86_FEATURE_1_SHSTK.
This patch was part of the Control-flow Enforcement series; the original
patch is here: https://lkml.org/lkml/2018/11/20/205. Dave Martin responded
that ARM recently introduced new features to NT_GNU_PROPERTY_TYPE_0 with
properties closely modelled on GNU_PROPERTY_X86_FEATURE_1_AND, and it is
logical to split out the generic part. Here it is.
With this patch, if an arch needs to setup features from ELF properties,
it needs CONFIG_ARCH_USE_GNU_PROPERTY to be set, and a specific
arch_setup_property().
For example, for X86_64:
int arch_setup_property(void *ehdr, void *phdr, struct file *f, bool inter)
{
int r;
uint32_t property;
r = get_gnu_property(ehdr, phdr, f, GNU_PROPERTY_X86_FEATURE_1_AND,
&property);
...
}
Signed-off-by: H.J. Lu <hjl.tools@gmail.com>
Signed-off-by: Yu-cheng Yu <yu-cheng.yu@intel.com>
Signed-off-by: Dave Martin <Dave.Martin@arm.com> [rebase to v5.2-rc1]
---
Notes from Dave:
* For arm64 at least, I'm hoping we can rely on the PT_GNU_PROPERTY
program header to find the program properties. If this works for
x86 too, I think the code could be simplified considerably.
Ideally we would always use that: having to exhaustively search
all the notes in the image instead doesn't feel like the right design
IMHO.
This needs further discussion, and I make no attempt to address it
for now.
---
fs/Kconfig.binfmt | 4 +
fs/Makefile | 1 +
fs/binfmt_elf.c | 13 ++
fs/gnu_property.c | 363 +++++++++++++++++++++++++++++++++++++++++++++++
include/linux/elf.h | 12 ++
include/uapi/linux/elf.h | 8 ++
6 files changed, 401 insertions(+)
create mode 100644 fs/gnu_property.c
diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
index b795f8d..175a1f5 100644
--- a/fs/Kconfig.binfmt
+++ b/fs/Kconfig.binfmt
@@ -35,6 +35,10 @@ config COMPAT_BINFMT_ELF
config ARCH_BINFMT_ELF_STATE
bool
+config ARCH_USE_GNU_PROPERTY
+ bool
+ depends on 64BIT
+
config BINFMT_ELF_FDPIC
bool "Kernel support for FDPIC ELF binaries"
default y if !BINFMT_ELF
diff --git a/fs/Makefile b/fs/Makefile
index c9aea23..b69f18c 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -44,6 +44,7 @@ obj-$(CONFIG_BINFMT_ELF) += binfmt_elf.o
obj-$(CONFIG_COMPAT_BINFMT_ELF) += compat_binfmt_elf.o
obj-$(CONFIG_BINFMT_ELF_FDPIC) += binfmt_elf_fdpic.o
obj-$(CONFIG_BINFMT_FLAT) += binfmt_flat.o
+obj-$(CONFIG_ARCH_USE_GNU_PROPERTY) += gnu_property.o
obj-$(CONFIG_FS_MBCACHE) += mbcache.o
obj-$(CONFIG_FS_POSIX_ACL) += posix_acl.o
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index fa9e99a..18015fc 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1080,6 +1080,19 @@ static int load_elf_binary(struct linux_binprm *bprm)
}
if (interpreter) {
+ retval = arch_setup_property(&loc->interp_elf_ex,
+ interp_elf_phdata,
+ interpreter, true);
+ } else {
+ retval = arch_setup_property(&loc->elf_ex,
+ elf_phdata,
+ bprm->file, false);
+ }
+
+ if (retval < 0)
+ goto out_free_dentry;
+
+ if (interpreter) {
unsigned long interp_map_addr = 0;
elf_entry = load_elf_interp(&loc->interp_elf_ex,
diff --git a/fs/gnu_property.c b/fs/gnu_property.c
new file mode 100644
index 0000000..656ea39
--- /dev/null
+++ b/fs/gnu_property.c
@@ -0,0 +1,363 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Extract an ELF file's .note.gnu.property.
+ *
+ * The path from the ELF header to the note section is the following:
+ * elfhdr->elf_phdr->elf_note->property[].
+ */
+
+#include <uapi/linux/elf-em.h>
+#include <linux/processor.h>
+#include <linux/binfmts.h>
+#include <linux/elf.h>
+#include <linux/slab.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/string.h>
+#include <linux/compat.h>
+
+/*
+ * The .note.gnu.property layout:
+ *
+ * struct elf_note {
+ * u32 n_namesz; --> sizeof(n_name[]); always (4)
+ * u32 n_ndescsz;--> sizeof(property[])
+ * u32 n_type; --> always NT_GNU_PROPERTY_TYPE_0
+ * };
+ * char n_name[4]; --> always 'GNU\0'
+ *
+ * struct {
+ * struct gnu_property {
+ * u32 pr_type;
+ * u32 pr_datasz;
+ * };
+ * u8 pr_data[pr_datasz];
+ * }[];
+ */
+
+#define BUF_SIZE (PAGE_SIZE / 4)
+
+struct gnu_property {
+ u32 pr_type;
+ u32 pr_datasz;
+};
+
+typedef bool (test_item_fn)(void *buf, u32 *arg, u32 type);
+typedef void *(next_item_fn)(void *buf, u32 *arg, u32 type);
+
+static inline bool test_note_type(void *buf, u32 *align, u32 note_type)
+{
+ struct elf_note *n = buf;
+
+ return ((n->n_type == note_type) && (n->n_namesz == 4) &&
+ (memcmp(n + 1, "GNU", 4) == 0));
+}
+
+static inline void *next_note(void *buf, u32 *align, u32 note_type)
+{
+ struct elf_note *n = buf;
+ u64 size;
+
+ if (check_add_overflow((u64)sizeof(*n), (u64)n->n_namesz, &size))
+ return NULL;
+
+ size = round_up(size, *align);
+
+ if (check_add_overflow(size, (u64)n->n_descsz, &size))
+ return NULL;
+
+ size = round_up(size, *align);
+
+ if (buf + size < buf)
+ return NULL;
+ else
+ return (buf + size);
+}
+
+static inline bool test_property(void *buf, u32 *max_type, u32 pr_type)
+{
+ struct gnu_property *pr = buf;
+
+ /*
+ * Property types must be in ascending order.
+ * Keep track of the max when testing each.
+ */
+ if (pr->pr_type > *max_type)
+ *max_type = pr->pr_type;
+
+ return (pr->pr_type == pr_type);
+}
+
+static inline void *next_property(void *buf, u32 *max_type, u32 pr_type)
+{
+ struct gnu_property *pr = buf;
+
+ if ((buf + sizeof(*pr) + pr->pr_datasz < buf) ||
+ (pr->pr_type > pr_type) ||
+ (pr->pr_type > *max_type))
+ return NULL;
+ else
+ return (buf + sizeof(*pr) + pr->pr_datasz);
+}
+
+/*
+ * Scan 'buf' for a pattern; return true if found.
+ * *pos is the distance from the beginning of buf to where
+ * the searched item or the next item is located.
+ */
+static int scan(u8 *buf, u32 buf_size, int item_size, test_item_fn test_item,
+ next_item_fn next_item, u32 *arg, u32 type, u32 *pos)
+{
+ int found = 0;
+ u8 *p, *max;
+
+ max = buf + buf_size;
+ if (max < buf)
+ return 0;
+
+ p = buf;
+
+ while ((p + item_size < max) && (p + item_size > buf)) {
+ if (test_item(p, arg, type)) {
+ found = 1;
+ break;
+ }
+
+ p = next_item(p, arg, type);
+ }
+
+ *pos = (p + item_size <= buf) ? 0 : (u32)(p - buf);
+ return found;
+}
+
+/*
+ * Search an NT_GNU_PROPERTY_TYPE_0 for the property of 'pr_type'.
+ */
+static int find_property(struct file *file, unsigned long desc_size,
+ loff_t file_offset, u8 *buf,
+ u32 pr_type, u32 *property)
+{
+ u32 buf_pos;
+ unsigned long read_size;
+ unsigned long done;
+ int found = 0;
+ int ret = 0;
+ u32 last_pr = 0;
+
+ *property = 0;
+ buf_pos = 0;
+
+ for (done = 0; done < desc_size; done += buf_pos) {
+ read_size = desc_size - done;
+ if (read_size > BUF_SIZE)
+ read_size = BUF_SIZE;
+
+ ret = kernel_read(file, buf, read_size, &file_offset);
+
+ if (ret != read_size)
+ return (ret < 0) ? ret : -EIO;
+
+ ret = 0;
+ found = scan(buf, read_size, sizeof(struct gnu_property),
+ test_property, next_property,
+ &last_pr, pr_type, &buf_pos);
+
+ if ((!buf_pos) || found)
+ break;
+
+ file_offset += buf_pos - read_size;
+ }
+
+ if (found) {
+ struct gnu_property *pr =
+ (struct gnu_property *)(buf + buf_pos);
+
+ if (pr->pr_datasz == 4) {
+ u32 *max = (u32 *)(buf + read_size);
+ u32 *data = (u32 *)((u8 *)pr + sizeof(*pr));
+
+ if (data + 1 <= max) {
+ *property = *data;
+ } else {
+ file_offset += buf_pos - read_size;
+ file_offset += sizeof(*pr);
+ ret = kernel_read(file, property, 4,
+ &file_offset);
+ }
+ }
+ }
+
+ return ret;
+}
+
+/*
+ * Search a PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
+ */
+static int find_note_type_0(struct file *file, loff_t file_offset,
+ unsigned long note_size, u32 align,
+ u32 pr_type, u32 *property)
+{
+ u8 *buf;
+ u32 buf_pos;
+ unsigned long read_size;
+ unsigned long done;
+ int found = 0;
+ int ret = 0;
+
+ buf = kmalloc(BUF_SIZE, GFP_KERNEL);
+ if (!buf)
+ return -ENOMEM;
+
+ *property = 0;
+ buf_pos = 0;
+
+ for (done = 0; done < note_size; done += buf_pos) {
+ read_size = note_size - done;
+ if (read_size > BUF_SIZE)
+ read_size = BUF_SIZE;
+
+ ret = kernel_read(file, buf, read_size, &file_offset);
+
+ if (ret != read_size) {
+ ret = (ret < 0) ? ret : -EIO;
+ kfree(buf);
+ return ret;
+ }
+
+ /*
+ * item_size = sizeof(struct elf_note) + elf_note.n_namesz.
+ * n_namesz is 4 for the note type we look for.
+ */
+ ret = scan(buf, read_size, sizeof(struct elf_note) + 4,
+ test_note_type, next_note,
+ &align, NT_GNU_PROPERTY_TYPE_0, &buf_pos);
+
+ file_offset += buf_pos - read_size;
+
+ if (ret && !found) {
+ struct elf_note *n =
+ (struct elf_note *)(buf + buf_pos);
+ u64 start = round_up(sizeof(*n) + n->n_namesz, align);
+ u64 total = 0;
+
+ if (check_add_overflow(start, (u64)n->n_descsz, &total)) {
+ ret = -EINVAL;
+ break;
+ }
+ total = round_up(total, align);
+
+ ret = find_property(file, n->n_descsz,
+ file_offset + start,
+ buf, pr_type, property);
+ found++;
+ file_offset += total;
+ buf_pos += total;
+ } else if (!buf_pos || ret) {
+ ret = 0;
+ *property = 0;
+ break;
+ }
+ }
+
+ kfree(buf);
+ return ret;
+}
+
+/*
+ * Look at an ELF file's PT_NOTE segments, then NT_GNU_PROPERTY_TYPE_0, then
+ * the property of pr_type.
+ *
+ * Input:
+ * file: the file to search;
+ * phdr: the file's elf header;
+ * phnum: number of entries in phdr;
+ * pr_type: the property type.
+ *
+ * Output:
+ * The property found.
+ *
+ * Return:
+ * Zero or error.
+ */
+static int scan_segments_64(struct file *file, struct elf64_phdr *phdr,
+ int phnum, u32 pr_type, u32 *property)
+{
+ int i;
+ int err = 0;
+
+ for (i = 0; i < phnum; i++, phdr++) {
+ if ((phdr->p_type != PT_NOTE) || (phdr->p_align != 8))
+ continue;
+
+ /*
+ * Search the PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
+ */
+ err = find_note_type_0(file, phdr->p_offset, phdr->p_filesz,
+ phdr->p_align, pr_type, property);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+
+#ifdef CONFIG_COMPAT
+static int scan_segments_32(struct file *file, struct elf32_phdr *phdr,
+ int phnum, u32 pr_type, u32 *property)
+{
+ int i;
+ int err = 0;
+
+ for (i = 0; i < phnum; i++, phdr++) {
+ if ((phdr->p_type != PT_NOTE) || (phdr->p_align != 4))
+ continue;
+
+ /*
+ * Search the PT_NOTE segment for NT_GNU_PROPERTY_TYPE_0.
+ */
+ err = find_note_type_0(file, phdr->p_offset, phdr->p_filesz,
+ phdr->p_align, pr_type, property);
+ if (err)
+ return err;
+ }
+
+ return 0;
+}
+#endif
+
+int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
+ u32 pr_type, u32 *property)
+{
+ struct elf64_hdr *ehdr64 = ehdr_p;
+ int err = 0;
+
+ *property = 0;
+
+ if (ehdr64->e_ident[EI_CLASS] == ELFCLASS64) {
+ struct elf64_phdr *phdr64 = phdr_p;
+
+ err = scan_segments_64(f, phdr64, ehdr64->e_phnum,
+ pr_type, property);
+ if (err < 0)
+ goto out;
+ } else {
+#ifdef CONFIG_COMPAT
+ struct elf32_hdr *ehdr32 = ehdr_p;
+
+ if (ehdr32->e_ident[EI_CLASS] == ELFCLASS32) {
+ struct elf32_phdr *phdr32 = phdr_p;
+
+ err = scan_segments_32(f, phdr32, ehdr32->e_phnum,
+ pr_type, property);
+ if (err < 0)
+ goto out;
+ }
+#else
+ WARN_ONCE(1, "Exec of 32-bit app, but CONFIG_COMPAT is not enabled.\n");
+ return -ENOTSUPP;
+#endif
+ }
+
+out:
+ return err;
+}
diff --git a/include/linux/elf.h b/include/linux/elf.h
index e3649b3..c15febe 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -56,4 +56,16 @@ static inline int elf_coredump_extra_notes_write(struct coredump_params *cprm) {
extern int elf_coredump_extra_notes_size(void);
extern int elf_coredump_extra_notes_write(struct coredump_params *cprm);
#endif
+
+#ifdef CONFIG_ARCH_USE_GNU_PROPERTY
+extern int arch_setup_property(void *ehdr, void *phdr, struct file *f,
+ bool interp);
+extern int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
+ u32 pr_type, u32 *feature);
+#else
+static inline int arch_setup_property(void *ehdr, void *phdr, struct file *f,
+ bool interp) { return 0; }
+static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
+ u32 pr_type, u32 *feature) { return 0; }
+#endif
#endif /* _LINUX_ELF_H */
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 34c02e4..7b7603a 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -372,6 +372,7 @@ typedef struct elf64_shdr {
#define NT_PRFPREG 2
#define NT_PRPSINFO 3
#define NT_TASKSTRUCT 4
+#define NT_GNU_PROPERTY_TYPE_0 5
#define NT_AUXV 6
/*
* Note to userspace developers: size of NT_SIGINFO note may increase
@@ -443,4 +444,11 @@ typedef struct elf64_note {
Elf64_Word n_type; /* Content type */
} Elf64_Nhdr;
+/* .note.gnu.property types */
+#define GNU_PROPERTY_X86_FEATURE_1_AND (0xc0000002)
+
+/* Bits of GNU_PROPERTY_X86_FEATURE_1_AND */
+#define GNU_PROPERTY_X86_FEATURE_1_IBT (0x00000001)
+#define GNU_PROPERTY_X86_FEATURE_1_SHSTK (0x00000002)
+
#endif /* _UAPI_LINUX_ELF_H */
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/8] mm: Reserve asm-generic prot flag 0x10 for arch use
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
The asm-generic mman definitions are used by a few architectures
that also define an arch-specific PROT flag with value 0x10. This
currently applies to sparc and powerpc, and arm64 will soon join
in.
To help future maintainers, document the use of this flag in the
asm-generic header too.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
include/uapi/asm-generic/mman-common.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index abd238d..ad3c6e5 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -11,6 +11,7 @@
#define PROT_WRITE 0x2 /* page can be written */
#define PROT_EXEC 0x4 /* page can be executed */
#define PROT_SEM 0x8 /* page may be used for atomic ops */
+ /* 0x10 reserved for arch-specific use */
#define PROT_NONE 0x0 /* page can not be accessed */
#define PROT_GROWSDOWN 0x01000000 /* mprotect flag: extend change to start of growsdown vma */
#define PROT_GROWSUP 0x02000000 /* mprotect flag: extend change to end of growsup vma */
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 2/8] mm: Reserve asm-generic prot flag 0x10 for arch use
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
The asm-generic mman definitions are used by a few architectures
that also define an arch-specific PROT flag with value 0x10. This
currently applies to sparc and powerpc, and arm64 will soon join
in.
To help future maintainers, document the use of this flag in the
asm-generic header too.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
include/uapi/asm-generic/mman-common.h | 1 +
1 file changed, 1 insertion(+)
diff --git a/include/uapi/asm-generic/mman-common.h b/include/uapi/asm-generic/mman-common.h
index abd238d..ad3c6e5 100644
--- a/include/uapi/asm-generic/mman-common.h
+++ b/include/uapi/asm-generic/mman-common.h
@@ -11,6 +11,7 @@
#define PROT_WRITE 0x2 /* page can be written */
#define PROT_EXEC 0x4 /* page can be executed */
#define PROT_SEM 0x8 /* page may be used for atomic ops */
+ /* 0x10 reserved for arch-specific use */
#define PROT_NONE 0x0 /* page can not be accessed */
#define PROT_GROWSDOWN 0x01000000 /* mprotect flag: extend change to start of growsdown vma */
#define PROT_GROWSUP 0x02000000 /* mprotect flag: extend change to end of growsup vma */
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 3/8] arm64: docs: cpu-feature-registers: Document ID_AA64PFR1_EL1
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
Commit d71be2b6c0e1 ("arm64: cpufeature: Detect SSBS and advertise
to userspace") exposes ID_AA64PFR1_EL1 to userspace, but didn't
update the documentation to match.
Add it.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
Documentation/arm64/cpu-feature-registers.txt | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
index 684a0da..54d2bfa 100644
--- a/Documentation/arm64/cpu-feature-registers.txt
+++ b/Documentation/arm64/cpu-feature-registers.txt
@@ -160,7 +160,15 @@ infrastructure:
x--------------------------------------------------x
- 3) MIDR_EL1 - Main ID Register
+ 3) ID_AA64PFR1_EL1 - Processor Feature Register 1
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | SSBS | [7-4] | y |
+ x--------------------------------------------------x
+
+
+ 4) MIDR_EL1 - Main ID Register
x--------------------------------------------------x
| Name | bits | visible |
|--------------------------------------------------|
@@ -179,7 +187,7 @@ infrastructure:
as available on the CPU where it is fetched and is not a system
wide safe value.
- 4) ID_AA64ISAR1_EL1 - Instruction set attribute register 1
+ 5) ID_AA64ISAR1_EL1 - Instruction set attribute register 1
x--------------------------------------------------x
| Name | bits | visible |
@@ -201,7 +209,7 @@ infrastructure:
| DPB | [3-0] | y |
x--------------------------------------------------x
- 5) ID_AA64MMFR2_EL1 - Memory model feature register 2
+ 6) ID_AA64MMFR2_EL1 - Memory model feature register 2
x--------------------------------------------------x
| Name | bits | visible |
@@ -209,7 +217,7 @@ infrastructure:
| AT | [35-32] | y |
x--------------------------------------------------x
- 6) ID_AA64ZFR0_EL1 - SVE feature ID register 0
+ 7) ID_AA64ZFR0_EL1 - SVE feature ID register 0
x--------------------------------------------------x
| Name | bits | visible |
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 3/8] arm64: docs: cpu-feature-registers: Document ID_AA64PFR1_EL1
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
Commit d71be2b6c0e1 ("arm64: cpufeature: Detect SSBS and advertise
to userspace") exposes ID_AA64PFR1_EL1 to userspace, but didn't
update the documentation to match.
Add it.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
Documentation/arm64/cpu-feature-registers.txt | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
index 684a0da..54d2bfa 100644
--- a/Documentation/arm64/cpu-feature-registers.txt
+++ b/Documentation/arm64/cpu-feature-registers.txt
@@ -160,7 +160,15 @@ infrastructure:
x--------------------------------------------------x
- 3) MIDR_EL1 - Main ID Register
+ 3) ID_AA64PFR1_EL1 - Processor Feature Register 1
+ x--------------------------------------------------x
+ | Name | bits | visible |
+ |--------------------------------------------------|
+ | SSBS | [7-4] | y |
+ x--------------------------------------------------x
+
+
+ 4) MIDR_EL1 - Main ID Register
x--------------------------------------------------x
| Name | bits | visible |
|--------------------------------------------------|
@@ -179,7 +187,7 @@ infrastructure:
as available on the CPU where it is fetched and is not a system
wide safe value.
- 4) ID_AA64ISAR1_EL1 - Instruction set attribute register 1
+ 5) ID_AA64ISAR1_EL1 - Instruction set attribute register 1
x--------------------------------------------------x
| Name | bits | visible |
@@ -201,7 +209,7 @@ infrastructure:
| DPB | [3-0] | y |
x--------------------------------------------------x
- 5) ID_AA64MMFR2_EL1 - Memory model feature register 2
+ 6) ID_AA64MMFR2_EL1 - Memory model feature register 2
x--------------------------------------------------x
| Name | bits | visible |
@@ -209,7 +217,7 @@ infrastructure:
| AT | [35-32] | y |
x--------------------------------------------------x
- 6) ID_AA64ZFR0_EL1 - SVE feature ID register 0
+ 7) ID_AA64ZFR0_EL1 - SVE feature ID register 0
x--------------------------------------------------x
| Name | bits | visible |
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
This patch adds the bare minimum required to expose the ARMv8.5
Branch Target Identification feature to userspace.
By itself, this does _not_ automatically enable BTI for any initial
executable pages mapped by execve(). This will come later, but for
now it should be possible to enable BTI manually on those pages by
using mprotect() from within the target process.
Other arches already using the generic mman.h are already using
0x10 for arch-specific prot flags, so we use that for
PROT_BTI_GUARDED here.
For consistency, signal handler entry points in BTI guarded pages
are required to be annotated as such, just like any other function.
This blocks a relatively minor attack vector, but comforming
userspace will have the annotations anyway, so we may as well
enforce them.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
Notes:
* An #ifdef CONFIG_ARM64_BTI controls the cpufeature field visibility
for userspace. It's probably best not to report BTI as present if
the required kernel support is unavailable.
* It's not clear whether anything extra needs to be done for hugepages.
---
Documentation/arm64/cpu-feature-registers.txt | 2 ++
Documentation/arm64/elf_hwcaps.txt | 4 ++++
arch/arm64/Kconfig | 23 +++++++++++++++++++
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/cpufeature.h | 6 +++++
arch/arm64/include/asm/esr.h | 2 +-
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/asm/mman.h | 33 +++++++++++++++++++++++++++
arch/arm64/include/asm/pgtable-hwdef.h | 1 +
arch/arm64/include/asm/pgtable.h | 2 +-
arch/arm64/include/asm/ptrace.h | 1 +
arch/arm64/include/asm/sysreg.h | 2 ++
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/mman.h | 9 ++++++++
arch/arm64/include/uapi/asm/ptrace.h | 1 +
arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/kernel/entry.S | 11 +++++++++
arch/arm64/kernel/ptrace.c | 2 +-
arch/arm64/kernel/signal.c | 5 ++++
arch/arm64/kernel/syscall.c | 1 +
arch/arm64/kernel/traps.c | 7 ++++++
include/linux/mm.h | 3 +++
23 files changed, 134 insertions(+), 4 deletions(-)
create mode 100644 arch/arm64/include/asm/mman.h
create mode 100644 arch/arm64/include/uapi/asm/mman.h
diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
index 54d2bfa..602eb6e 100644
--- a/Documentation/arm64/cpu-feature-registers.txt
+++ b/Documentation/arm64/cpu-feature-registers.txt
@@ -165,6 +165,8 @@ infrastructure:
| Name | bits | visible |
|--------------------------------------------------|
| SSBS | [7-4] | y |
+ |--------------------------------------------------|
+ | BT | [3-0] | y |
x--------------------------------------------------x
diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
index b73a251..7a872d3 100644
--- a/Documentation/arm64/elf_hwcaps.txt
+++ b/Documentation/arm64/elf_hwcaps.txt
@@ -223,6 +223,10 @@ HWCAP_PACG
ID_AA64ISAR1_EL1.GPI == 0b0001, as described by
Documentation/arm64/pointer-authentication.txt.
+HWCAP2_BTI
+
+ Functionality implied by ID_AA64PFR0_EL1.BT == 0b0001.
+
4. Unused AT_HWCAP bits
-----------------------
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4780eb7..0f6765e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1361,6 +1361,29 @@ config ARM64_PTR_AUTH
endmenu
+menu "ARMv8.5 architectural features"
+
+config ARM64_BTI
+ bool "Branch Target Identification support"
+ default y
+ help
+ Branch Target Identification (part of the ARMv8.5 Extensions)
+ provides a mechanism to limit the set of locations to which computed
+ branch instructions such as BR or BLR can jump.
+
+ This is intended to provide complementary protection to other control
+ flow integrity protection mechanisms, such as the Pointer
+ authentication mechanism provided as part of the ARMv8.2 Extensions.
+
+ To make use of BTI on CPUs that support it, say Y.
+
+ Userspace binaries must also be specifically compiled to make use of
+ this mechanism. If you say N here or the hardware does not support
+ BTI, such binaries can still run, but you get no additional
+ enforcement of branch destinations.
+
+endmenu
+
config ARM64_SVE
bool "ARM Scalable Vector Extension support"
default y
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index defdc67..ccb44ae 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -62,7 +62,8 @@
#define ARM64_HAS_GENERIC_AUTH_IMP_DEF 41
#define ARM64_HAS_IRQ_PRIO_MASKING 42
#define ARM64_HAS_DCPODP 43
+#define ARM64_BTI 44
-#define ARM64_NCAPS 44
+#define ARM64_NCAPS 45
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index bc895c8..308cc54 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -617,6 +617,12 @@ static inline bool system_uses_irq_prio_masking(void)
cpus_have_const_cap(ARM64_HAS_IRQ_PRIO_MASKING);
}
+static inline bool system_supports_bti(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_BTI) &&
+ cpus_have_const_cap(ARM64_BTI);
+}
+
#define ARM64_SSBD_UNKNOWN -1
#define ARM64_SSBD_FORCE_DISABLE 0
#define ARM64_SSBD_KERNEL 1
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 0e27fe9..0dd43a5 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -33,7 +33,7 @@
#define ESR_ELx_EC_PAC (0x09) /* EL2 and above */
/* Unallocated EC: 0x0A - 0x0B */
#define ESR_ELx_EC_CP14_64 (0x0C)
-/* Unallocated EC: 0x0d */
+#define ESR_ELx_EC_BTI (0x0D)
#define ESR_ELx_EC_ILL (0x0E)
/* Unallocated EC: 0x0F - 0x10 */
#define ESR_ELx_EC_SVC32 (0x11)
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index b4bfb66..c6b918f 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -95,6 +95,7 @@
#define KERNEL_HWCAP_SVEBITPERM __khwcap2_feature(SVEBITPERM)
#define KERNEL_HWCAP_SVESHA3 __khwcap2_feature(SVESHA3)
#define KERNEL_HWCAP_SVESM4 __khwcap2_feature(SVESM4)
+#define KERNEL_HWCAP_BTI __khwcap2_feature(BTI)
/*
* This yields a mask that user programs can use to figure out what
diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h
new file mode 100644
index 0000000..a1f67aa
--- /dev/null
+++ b/arch/arm64/include/asm/mman.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_MMAN_H__
+#define __ASM_MMAN_H__
+
+#include <uapi/asm/mman.h>
+
+#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
+static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
+{
+ if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
+ return VM_ARM64_GP;
+
+ return 0;
+}
+
+#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
+static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
+{
+ return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
+}
+
+#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr)
+static inline int arm64_validate_prot(unsigned long prot, unsigned long addr)
+{
+ unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM;
+
+ if (system_supports_bti())
+ supported |= PROT_BTI_GUARDED;
+
+ return (prot & ~supported) == 0;
+}
+
+#endif /* ! __ASM_MMAN_H__ */
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index a69259c..6e8af3a 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -162,6 +162,7 @@
#define PTE_SHARED (_AT(pteval_t, 3) << 8) /* SH[1:0], inner shareable */
#define PTE_AF (_AT(pteval_t, 1) << 10) /* Access Flag */
#define PTE_NG (_AT(pteval_t, 1) << 11) /* nG */
+#define PTE_GP (_AT(pteval_t, 1) << 50) /* BTI guarded */
#define PTE_DBM (_AT(pteval_t, 1) << 51) /* Dirty Bit Management */
#define PTE_CONT (_AT(pteval_t, 1) << 52) /* Contiguous range */
#define PTE_PXN (_AT(pteval_t, 1) << 53) /* Privileged XN */
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 2c41b04..eefd9a44 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -640,7 +640,7 @@ static inline phys_addr_t pgd_page_paddr(pgd_t pgd)
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY |
- PTE_PROT_NONE | PTE_VALID | PTE_WRITE;
+ PTE_PROT_NONE | PTE_VALID | PTE_WRITE | PTE_GP;
/* preserve the hardware dirty information */
if (pte_hw_dirty(pte))
pte = pte_mkdirty(pte);
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index b2de329..b868ef11 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -41,6 +41,7 @@
/* Additional SPSR bits not exposed in the UABI */
#define PSR_IL_BIT (1 << 20)
+#define PSR_BTYPE_CALL (2 << 10)
/* AArch32-specific ptrace requests */
#define COMPAT_PTRACE_GETREGS 12
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 902d75b..2b8d364 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -604,10 +604,12 @@
/* id_aa64pfr1 */
#define ID_AA64PFR1_SSBS_SHIFT 4
+#define ID_AA64PFR1_BT_SHIFT 0
#define ID_AA64PFR1_SSBS_PSTATE_NI 0
#define ID_AA64PFR1_SSBS_PSTATE_ONLY 1
#define ID_AA64PFR1_SSBS_PSTATE_INSNS 2
+#define ID_AA64PFR1_BT_BTI 0x1
/* id_aa64zfr0 */
#define ID_AA64ZFR0_SM4_SHIFT 40
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index 1a772b1..ea5bdda 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -63,5 +63,6 @@
#define HWCAP2_SVEBITPERM (1 << 4)
#define HWCAP2_SVESHA3 (1 << 5)
#define HWCAP2_SVESM4 (1 << 6)
+#define HWCAP2_BTI (1 << 7)
#endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
new file mode 100644
index 0000000..4776b43
--- /dev/null
+++ b/arch/arm64/include/uapi/asm/mman.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI__ASM_MMAN_H
+#define _UAPI__ASM_MMAN_H
+
+#include <asm-generic/mman.h>
+
+#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
+
+#endif /* ! _UAPI__ASM_MMAN_H */
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index d78623a..e2361a1 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -46,6 +46,7 @@
#define PSR_I_BIT 0x00000080
#define PSR_A_BIT 0x00000100
#define PSR_D_BIT 0x00000200
+#define PSR_BTYPE_MASK 0x00000c00
#define PSR_SSBS_BIT 0x00001000
#define PSR_PAN_BIT 0x00400000
#define PSR_UAO_BIT 0x00800000
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index ca27e08..0a6dbb3 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -182,6 +182,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_SSBS_SHIFT, 4, ID_AA64PFR1_SSBS_PSTATE_NI),
+ ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_BTI),
+ FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_BT_SHIFT, 4, 0),
ARM64_FTR_END,
};
@@ -1558,6 +1560,18 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.min_field_value = 1,
},
#endif
+#ifdef CONFIG_ARM64_BTI
+ {
+ .desc = "Branch Target Identification",
+ .capability = ARM64_BTI,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .matches = has_cpuid_feature,
+ .sys_reg = SYS_ID_AA64PFR1_EL1,
+ .field_pos = ID_AA64PFR1_BT_SHIFT,
+ .min_field_value = ID_AA64PFR1_BT_BTI,
+ .sign = FTR_UNSIGNED,
+ },
+#endif
{},
};
@@ -1651,6 +1665,9 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
HWCAP_CAP(SYS_ID_AA64ZFR0_EL1, ID_AA64ZFR0_SM4_SHIFT, FTR_UNSIGNED, ID_AA64ZFR0_SM4, CAP_HWCAP, KERNEL_HWCAP_SVESM4),
#endif
HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_SSBS_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_SSBS_PSTATE_INSNS, CAP_HWCAP, KERNEL_HWCAP_SSBS),
+#ifdef CONFIG_ARM64_BTI
+ HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_BT_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_BT_BTI, CAP_HWCAP, KERNEL_HWCAP_BTI),
+#endif
#ifdef CONFIG_ARM64_PTR_AUTH
HWCAP_MULTI_CAP(ptr_auth_hwcap_addr_matches, CAP_HWCAP, KERNEL_HWCAP_PACA),
HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, KERNEL_HWCAP_PACG),
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index f6f7936..0fd3899 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -92,6 +92,7 @@ static const char *const hwcap_str[] = {
"svebitperm",
"svesha3",
"svesm4",
+ "bti",
NULL
};
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 1a7811b..da3694e 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -707,6 +707,8 @@ el0_sync:
b.eq el0_sp_pc
cmp x24, #ESR_ELx_EC_UNKNOWN // unknown exception in EL0
b.eq el0_undef
+ cmp x24, #ESR_ELx_EC_BTI // branch target exception
+ b.eq el0_bti
cmp x24, #ESR_ELx_EC_BREAKPT_LOW // debug exception in EL0
b.ge el0_dbg
b el0_inv
@@ -851,6 +853,15 @@ el0_undef:
mov x0, sp
bl do_undefinstr
b ret_to_user
+el0_bti:
+ /*
+ * Branch target exception
+ */
+ enable_daif
+ ct_user_exit
+ mov x0, sp
+ bl do_bti
+ b ret_to_user
el0_sys:
/*
* System instructions, for trapped cache maintenance instructions
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b82e0a9..3717b06 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
*/
#define SPSR_EL1_AARCH64_RES0_BITS \
(GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
- GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
+ GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
#define SPSR_EL1_AARCH32_RES0_BITS \
(GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index a9b0485..2e540f5 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
regs->regs[29] = (unsigned long)&user->next_frame->fp;
regs->pc = (unsigned long)ka->sa.sa_handler;
+ if (system_supports_bti()) {
+ regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
+ regs->pstate |= PSR_BTYPE_CALL;
+ }
+
if (ka->sa.sa_flags & SA_RESTORER)
sigtramp = ka->sa.sa_restorer;
else
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 5610ac0..85b456b 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
unsigned long flags = current_thread_info()->flags;
regs->orig_x0 = regs->regs[0];
+ regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
regs->syscallno = scno;
local_daif_restore(DAIF_PROCCTX);
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index ade3204..079ce1a 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -426,6 +426,12 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
}
+asmlinkage void __exception do_bti(struct pt_regs *regs)
+{
+ BUG_ON(!user_mode(regs));
+ force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
+}
+
#define __user_cache_maint(insn, address, res) \
if (address >= user_addr_max()) { \
res = -EFAULT; \
@@ -756,6 +762,7 @@ static const char *esr_class_str[] = {
[ESR_ELx_EC_FP_ASIMD] = "ASIMD",
[ESR_ELx_EC_CP10_ID] = "CP10 MRC/VMRS",
[ESR_ELx_EC_CP14_64] = "CP14 MCRR/MRRC",
+ [ESR_ELx_EC_BTI] = "BTI",
[ESR_ELx_EC_ILL] = "PSTATE.IL",
[ESR_ELx_EC_SVC32] = "SVC (AArch32)",
[ESR_ELx_EC_HVC32] = "HVC (AArch32)",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0e8834a..639dc54 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -318,6 +318,9 @@ extern unsigned int kobjsize(const void *objp);
#elif defined(CONFIG_SPARC64)
# define VM_SPARC_ADI VM_ARCH_1 /* Uses ADI tag for access control */
# define VM_ARCH_CLEAR VM_SPARC_ADI
+#elif defined(CONFIG_ARM64)
+# define VM_ARM64_GP VM_ARCH_1 /* BTI guarded page */
+# define VM_ARCH_CLEAR VM_ARM64_GP
#elif !defined(CONFIG_MMU)
# define VM_MAPPED_COPY VM_ARCH_1 /* T if mapped copy of data (nommu mmap) */
#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
This patch adds the bare minimum required to expose the ARMv8.5
Branch Target Identification feature to userspace.
By itself, this does _not_ automatically enable BTI for any initial
executable pages mapped by execve(). This will come later, but for
now it should be possible to enable BTI manually on those pages by
using mprotect() from within the target process.
Other arches already using the generic mman.h are already using
0x10 for arch-specific prot flags, so we use that for
PROT_BTI_GUARDED here.
For consistency, signal handler entry points in BTI guarded pages
are required to be annotated as such, just like any other function.
This blocks a relatively minor attack vector, but comforming
userspace will have the annotations anyway, so we may as well
enforce them.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
Notes:
* An #ifdef CONFIG_ARM64_BTI controls the cpufeature field visibility
for userspace. It's probably best not to report BTI as present if
the required kernel support is unavailable.
* It's not clear whether anything extra needs to be done for hugepages.
---
Documentation/arm64/cpu-feature-registers.txt | 2 ++
Documentation/arm64/elf_hwcaps.txt | 4 ++++
arch/arm64/Kconfig | 23 +++++++++++++++++++
arch/arm64/include/asm/cpucaps.h | 3 ++-
arch/arm64/include/asm/cpufeature.h | 6 +++++
arch/arm64/include/asm/esr.h | 2 +-
arch/arm64/include/asm/hwcap.h | 1 +
arch/arm64/include/asm/mman.h | 33 +++++++++++++++++++++++++++
arch/arm64/include/asm/pgtable-hwdef.h | 1 +
arch/arm64/include/asm/pgtable.h | 2 +-
arch/arm64/include/asm/ptrace.h | 1 +
arch/arm64/include/asm/sysreg.h | 2 ++
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/include/uapi/asm/mman.h | 9 ++++++++
arch/arm64/include/uapi/asm/ptrace.h | 1 +
arch/arm64/kernel/cpufeature.c | 17 ++++++++++++++
arch/arm64/kernel/cpuinfo.c | 1 +
arch/arm64/kernel/entry.S | 11 +++++++++
arch/arm64/kernel/ptrace.c | 2 +-
arch/arm64/kernel/signal.c | 5 ++++
arch/arm64/kernel/syscall.c | 1 +
arch/arm64/kernel/traps.c | 7 ++++++
include/linux/mm.h | 3 +++
23 files changed, 134 insertions(+), 4 deletions(-)
create mode 100644 arch/arm64/include/asm/mman.h
create mode 100644 arch/arm64/include/uapi/asm/mman.h
diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
index 54d2bfa..602eb6e 100644
--- a/Documentation/arm64/cpu-feature-registers.txt
+++ b/Documentation/arm64/cpu-feature-registers.txt
@@ -165,6 +165,8 @@ infrastructure:
| Name | bits | visible |
|--------------------------------------------------|
| SSBS | [7-4] | y |
+ |--------------------------------------------------|
+ | BT | [3-0] | y |
x--------------------------------------------------x
diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
index b73a251..7a872d3 100644
--- a/Documentation/arm64/elf_hwcaps.txt
+++ b/Documentation/arm64/elf_hwcaps.txt
@@ -223,6 +223,10 @@ HWCAP_PACG
ID_AA64ISAR1_EL1.GPI == 0b0001, as described by
Documentation/arm64/pointer-authentication.txt.
+HWCAP2_BTI
+
+ Functionality implied by ID_AA64PFR0_EL1.BT == 0b0001.
+
4. Unused AT_HWCAP bits
-----------------------
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 4780eb7..0f6765e 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1361,6 +1361,29 @@ config ARM64_PTR_AUTH
endmenu
+menu "ARMv8.5 architectural features"
+
+config ARM64_BTI
+ bool "Branch Target Identification support"
+ default y
+ help
+ Branch Target Identification (part of the ARMv8.5 Extensions)
+ provides a mechanism to limit the set of locations to which computed
+ branch instructions such as BR or BLR can jump.
+
+ This is intended to provide complementary protection to other control
+ flow integrity protection mechanisms, such as the Pointer
+ authentication mechanism provided as part of the ARMv8.2 Extensions.
+
+ To make use of BTI on CPUs that support it, say Y.
+
+ Userspace binaries must also be specifically compiled to make use of
+ this mechanism. If you say N here or the hardware does not support
+ BTI, such binaries can still run, but you get no additional
+ enforcement of branch destinations.
+
+endmenu
+
config ARM64_SVE
bool "ARM Scalable Vector Extension support"
default y
diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index defdc67..ccb44ae 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -62,7 +62,8 @@
#define ARM64_HAS_GENERIC_AUTH_IMP_DEF 41
#define ARM64_HAS_IRQ_PRIO_MASKING 42
#define ARM64_HAS_DCPODP 43
+#define ARM64_BTI 44
-#define ARM64_NCAPS 44
+#define ARM64_NCAPS 45
#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index bc895c8..308cc54 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -617,6 +617,12 @@ static inline bool system_uses_irq_prio_masking(void)
cpus_have_const_cap(ARM64_HAS_IRQ_PRIO_MASKING);
}
+static inline bool system_supports_bti(void)
+{
+ return IS_ENABLED(CONFIG_ARM64_BTI) &&
+ cpus_have_const_cap(ARM64_BTI);
+}
+
#define ARM64_SSBD_UNKNOWN -1
#define ARM64_SSBD_FORCE_DISABLE 0
#define ARM64_SSBD_KERNEL 1
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index 0e27fe9..0dd43a5 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -33,7 +33,7 @@
#define ESR_ELx_EC_PAC (0x09) /* EL2 and above */
/* Unallocated EC: 0x0A - 0x0B */
#define ESR_ELx_EC_CP14_64 (0x0C)
-/* Unallocated EC: 0x0d */
+#define ESR_ELx_EC_BTI (0x0D)
#define ESR_ELx_EC_ILL (0x0E)
/* Unallocated EC: 0x0F - 0x10 */
#define ESR_ELx_EC_SVC32 (0x11)
diff --git a/arch/arm64/include/asm/hwcap.h b/arch/arm64/include/asm/hwcap.h
index b4bfb66..c6b918f 100644
--- a/arch/arm64/include/asm/hwcap.h
+++ b/arch/arm64/include/asm/hwcap.h
@@ -95,6 +95,7 @@
#define KERNEL_HWCAP_SVEBITPERM __khwcap2_feature(SVEBITPERM)
#define KERNEL_HWCAP_SVESHA3 __khwcap2_feature(SVESHA3)
#define KERNEL_HWCAP_SVESM4 __khwcap2_feature(SVESM4)
+#define KERNEL_HWCAP_BTI __khwcap2_feature(BTI)
/*
* This yields a mask that user programs can use to figure out what
diff --git a/arch/arm64/include/asm/mman.h b/arch/arm64/include/asm/mman.h
new file mode 100644
index 0000000..a1f67aa
--- /dev/null
+++ b/arch/arm64/include/asm/mman.h
@@ -0,0 +1,33 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_MMAN_H__
+#define __ASM_MMAN_H__
+
+#include <uapi/asm/mman.h>
+
+#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
+static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
+{
+ if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
+ return VM_ARM64_GP;
+
+ return 0;
+}
+
+#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
+static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
+{
+ return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
+}
+
+#define arch_validate_prot(prot, addr) arm64_validate_prot(prot, addr)
+static inline int arm64_validate_prot(unsigned long prot, unsigned long addr)
+{
+ unsigned long supported = PROT_READ | PROT_WRITE | PROT_EXEC | PROT_SEM;
+
+ if (system_supports_bti())
+ supported |= PROT_BTI_GUARDED;
+
+ return (prot & ~supported) == 0;
+}
+
+#endif /* ! __ASM_MMAN_H__ */
diff --git a/arch/arm64/include/asm/pgtable-hwdef.h b/arch/arm64/include/asm/pgtable-hwdef.h
index a69259c..6e8af3a 100644
--- a/arch/arm64/include/asm/pgtable-hwdef.h
+++ b/arch/arm64/include/asm/pgtable-hwdef.h
@@ -162,6 +162,7 @@
#define PTE_SHARED (_AT(pteval_t, 3) << 8) /* SH[1:0], inner shareable */
#define PTE_AF (_AT(pteval_t, 1) << 10) /* Access Flag */
#define PTE_NG (_AT(pteval_t, 1) << 11) /* nG */
+#define PTE_GP (_AT(pteval_t, 1) << 50) /* BTI guarded */
#define PTE_DBM (_AT(pteval_t, 1) << 51) /* Dirty Bit Management */
#define PTE_CONT (_AT(pteval_t, 1) << 52) /* Contiguous range */
#define PTE_PXN (_AT(pteval_t, 1) << 53) /* Privileged XN */
diff --git a/arch/arm64/include/asm/pgtable.h b/arch/arm64/include/asm/pgtable.h
index 2c41b04..eefd9a44 100644
--- a/arch/arm64/include/asm/pgtable.h
+++ b/arch/arm64/include/asm/pgtable.h
@@ -640,7 +640,7 @@ static inline phys_addr_t pgd_page_paddr(pgd_t pgd)
static inline pte_t pte_modify(pte_t pte, pgprot_t newprot)
{
const pteval_t mask = PTE_USER | PTE_PXN | PTE_UXN | PTE_RDONLY |
- PTE_PROT_NONE | PTE_VALID | PTE_WRITE;
+ PTE_PROT_NONE | PTE_VALID | PTE_WRITE | PTE_GP;
/* preserve the hardware dirty information */
if (pte_hw_dirty(pte))
pte = pte_mkdirty(pte);
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index b2de329..b868ef11 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -41,6 +41,7 @@
/* Additional SPSR bits not exposed in the UABI */
#define PSR_IL_BIT (1 << 20)
+#define PSR_BTYPE_CALL (2 << 10)
/* AArch32-specific ptrace requests */
#define COMPAT_PTRACE_GETREGS 12
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 902d75b..2b8d364 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -604,10 +604,12 @@
/* id_aa64pfr1 */
#define ID_AA64PFR1_SSBS_SHIFT 4
+#define ID_AA64PFR1_BT_SHIFT 0
#define ID_AA64PFR1_SSBS_PSTATE_NI 0
#define ID_AA64PFR1_SSBS_PSTATE_ONLY 1
#define ID_AA64PFR1_SSBS_PSTATE_INSNS 2
+#define ID_AA64PFR1_BT_BTI 0x1
/* id_aa64zfr0 */
#define ID_AA64ZFR0_SM4_SHIFT 40
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index 1a772b1..ea5bdda 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -63,5 +63,6 @@
#define HWCAP2_SVEBITPERM (1 << 4)
#define HWCAP2_SVESHA3 (1 << 5)
#define HWCAP2_SVESM4 (1 << 6)
+#define HWCAP2_BTI (1 << 7)
#endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
new file mode 100644
index 0000000..4776b43
--- /dev/null
+++ b/arch/arm64/include/uapi/asm/mman.h
@@ -0,0 +1,9 @@
+/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
+#ifndef _UAPI__ASM_MMAN_H
+#define _UAPI__ASM_MMAN_H
+
+#include <asm-generic/mman.h>
+
+#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
+
+#endif /* ! _UAPI__ASM_MMAN_H */
diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index d78623a..e2361a1 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -46,6 +46,7 @@
#define PSR_I_BIT 0x00000080
#define PSR_A_BIT 0x00000100
#define PSR_D_BIT 0x00000200
+#define PSR_BTYPE_MASK 0x00000c00
#define PSR_SSBS_BIT 0x00001000
#define PSR_PAN_BIT 0x00400000
#define PSR_UAO_BIT 0x00800000
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index ca27e08..0a6dbb3 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -182,6 +182,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
static const struct arm64_ftr_bits ftr_id_aa64pfr1[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_SSBS_SHIFT, 4, ID_AA64PFR1_SSBS_PSTATE_NI),
+ ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_BTI),
+ FTR_STRICT, FTR_LOWER_SAFE, ID_AA64PFR1_BT_SHIFT, 4, 0),
ARM64_FTR_END,
};
@@ -1558,6 +1560,18 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.min_field_value = 1,
},
#endif
+#ifdef CONFIG_ARM64_BTI
+ {
+ .desc = "Branch Target Identification",
+ .capability = ARM64_BTI,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .matches = has_cpuid_feature,
+ .sys_reg = SYS_ID_AA64PFR1_EL1,
+ .field_pos = ID_AA64PFR1_BT_SHIFT,
+ .min_field_value = ID_AA64PFR1_BT_BTI,
+ .sign = FTR_UNSIGNED,
+ },
+#endif
{},
};
@@ -1651,6 +1665,9 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
HWCAP_CAP(SYS_ID_AA64ZFR0_EL1, ID_AA64ZFR0_SM4_SHIFT, FTR_UNSIGNED, ID_AA64ZFR0_SM4, CAP_HWCAP, KERNEL_HWCAP_SVESM4),
#endif
HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_SSBS_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_SSBS_PSTATE_INSNS, CAP_HWCAP, KERNEL_HWCAP_SSBS),
+#ifdef CONFIG_ARM64_BTI
+ HWCAP_CAP(SYS_ID_AA64PFR1_EL1, ID_AA64PFR1_BT_SHIFT, FTR_UNSIGNED, ID_AA64PFR1_BT_BTI, CAP_HWCAP, KERNEL_HWCAP_BTI),
+#endif
#ifdef CONFIG_ARM64_PTR_AUTH
HWCAP_MULTI_CAP(ptr_auth_hwcap_addr_matches, CAP_HWCAP, KERNEL_HWCAP_PACA),
HWCAP_MULTI_CAP(ptr_auth_hwcap_gen_matches, CAP_HWCAP, KERNEL_HWCAP_PACG),
diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index f6f7936..0fd3899 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -92,6 +92,7 @@ static const char *const hwcap_str[] = {
"svebitperm",
"svesha3",
"svesm4",
+ "bti",
NULL
};
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 1a7811b..da3694e 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -707,6 +707,8 @@ el0_sync:
b.eq el0_sp_pc
cmp x24, #ESR_ELx_EC_UNKNOWN // unknown exception in EL0
b.eq el0_undef
+ cmp x24, #ESR_ELx_EC_BTI // branch target exception
+ b.eq el0_bti
cmp x24, #ESR_ELx_EC_BREAKPT_LOW // debug exception in EL0
b.ge el0_dbg
b el0_inv
@@ -851,6 +853,15 @@ el0_undef:
mov x0, sp
bl do_undefinstr
b ret_to_user
+el0_bti:
+ /*
+ * Branch target exception
+ */
+ enable_daif
+ ct_user_exit
+ mov x0, sp
+ bl do_bti
+ b ret_to_user
el0_sys:
/*
* System instructions, for trapped cache maintenance instructions
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index b82e0a9..3717b06 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
*/
#define SPSR_EL1_AARCH64_RES0_BITS \
(GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
- GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
+ GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
#define SPSR_EL1_AARCH32_RES0_BITS \
(GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
diff --git a/arch/arm64/kernel/signal.c b/arch/arm64/kernel/signal.c
index a9b0485..2e540f5 100644
--- a/arch/arm64/kernel/signal.c
+++ b/arch/arm64/kernel/signal.c
@@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
regs->regs[29] = (unsigned long)&user->next_frame->fp;
regs->pc = (unsigned long)ka->sa.sa_handler;
+ if (system_supports_bti()) {
+ regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
+ regs->pstate |= PSR_BTYPE_CALL;
+ }
+
if (ka->sa.sa_flags & SA_RESTORER)
sigtramp = ka->sa.sa_restorer;
else
diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
index 5610ac0..85b456b 100644
--- a/arch/arm64/kernel/syscall.c
+++ b/arch/arm64/kernel/syscall.c
@@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
unsigned long flags = current_thread_info()->flags;
regs->orig_x0 = regs->regs[0];
+ regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
regs->syscallno = scno;
local_daif_restore(DAIF_PROCCTX);
diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c
index ade3204..079ce1a 100644
--- a/arch/arm64/kernel/traps.c
+++ b/arch/arm64/kernel/traps.c
@@ -426,6 +426,12 @@ asmlinkage void __exception do_undefinstr(struct pt_regs *regs)
force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
}
+asmlinkage void __exception do_bti(struct pt_regs *regs)
+{
+ BUG_ON(!user_mode(regs));
+ force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc);
+}
+
#define __user_cache_maint(insn, address, res) \
if (address >= user_addr_max()) { \
res = -EFAULT; \
@@ -756,6 +762,7 @@ static const char *esr_class_str[] = {
[ESR_ELx_EC_FP_ASIMD] = "ASIMD",
[ESR_ELx_EC_CP10_ID] = "CP10 MRC/VMRS",
[ESR_ELx_EC_CP14_64] = "CP14 MCRR/MRRC",
+ [ESR_ELx_EC_BTI] = "BTI",
[ESR_ELx_EC_ILL] = "PSTATE.IL",
[ESR_ELx_EC_SVC32] = "SVC (AArch32)",
[ESR_ELx_EC_HVC32] = "HVC (AArch32)",
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 0e8834a..639dc54 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -318,6 +318,9 @@ extern unsigned int kobjsize(const void *objp);
#elif defined(CONFIG_SPARC64)
# define VM_SPARC_ADI VM_ARCH_1 /* Uses ADI tag for access control */
# define VM_ARCH_CLEAR VM_SPARC_ADI
+#elif defined(CONFIG_ARM64)
+# define VM_ARM64_GP VM_ARCH_1 /* BTI guarded page */
+# define VM_ARCH_CLEAR VM_ARM64_GP
#elif !defined(CONFIG_MMU)
# define VM_MAPPED_COPY VM_ARCH_1 /* T if mapped copy of data (nommu mmap) */
#endif
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 13:02 ` Mark Rutland
0 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 13:02 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arm-kernel, linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu,
Arnd Bergmann, Richard Henderson, Andrew Jones, Will Deacon,
Catalin Marinas, Kristina Martšenko, Szabolcs Nagy,
Sudakshina Das, Paul Elliott
Hi Dave,
This generally looks good, but I have a few comments below.
On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> +{
> + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> + return VM_ARM64_GP;
> +
> + return 0;
> +}
> +
> +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> +{
> + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> +}
While the architectural name for the PTE bit is GP, it might make more
sense to call the vm flag VM_ARM64_BTI, since people are more likely to
recognise BTI than GP as a mnemonic.
Not a big deal either way, though.
[...]
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index b2de329..b868ef11 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -41,6 +41,7 @@
>
> /* Additional SPSR bits not exposed in the UABI */
> #define PSR_IL_BIT (1 << 20)
> +#define PSR_BTYPE_CALL (2 << 10)
I thought BTYPE was a 2-bit field, so isn't there at leat one other
value to have a mnemonic for?
Is it an enumeration or a bitmask?
> #endif /* _UAPI__ASM_HWCAP_H */
> diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> new file mode 100644
> index 0000000..4776b43
> --- /dev/null
> +++ b/arch/arm64/include/uapi/asm/mman.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI__ASM_MMAN_H
> +#define _UAPI__ASM_MMAN_H
> +
> +#include <asm-generic/mman.h>
> +
> +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
From prior discussions, I thought this would be PROT_BTI, without the
_GUARDED suffix. Do we really need that?
AFAICT, all other PROT_* definitions only have a single underscore, and
the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
powerpc.
[...]
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index b82e0a9..3717b06 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
> */
> #define SPSR_EL1_AARCH64_RES0_BITS \
> (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
> - GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
> + GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
> #define SPSR_EL1_AARCH32_RES0_BITS \
> (GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
Phew; I was worried this would be missed!
[...]
> @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> regs->regs[29] = (unsigned long)&user->next_frame->fp;
> regs->pc = (unsigned long)ka->sa.sa_handler;
>
> + if (system_supports_bti()) {
> + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
Nit: that can be:
regs->pstate &= ~PSR_BTYPE_MASK;
> diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> index 5610ac0..85b456b 100644
> --- a/arch/arm64/kernel/syscall.c
> +++ b/arch/arm64/kernel/syscall.c
> @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> unsigned long flags = current_thread_info()->flags;
>
> regs->orig_x0 = regs->regs[0];
> + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
Likewise:
regs->pstate &= ~PSR_BTYPE_MASK;
... though I don't understand why that would matter to syscalls, nor how
those bits could ever be set given we had to execute an SVC to get here.
What am I missing?
Thanks,
Mark.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 13:02 ` Mark Rutland
0 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 13:02 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
Hi Dave,
This generally looks good, but I have a few comments below.
On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> +{
> + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> + return VM_ARM64_GP;
> +
> + return 0;
> +}
> +
> +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> +{
> + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> +}
While the architectural name for the PTE bit is GP, it might make more
sense to call the vm flag VM_ARM64_BTI, since people are more likely to
recognise BTI than GP as a mnemonic.
Not a big deal either way, though.
[...]
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index b2de329..b868ef11 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -41,6 +41,7 @@
>
> /* Additional SPSR bits not exposed in the UABI */
> #define PSR_IL_BIT (1 << 20)
> +#define PSR_BTYPE_CALL (2 << 10)
I thought BTYPE was a 2-bit field, so isn't there at leat one other
value to have a mnemonic for?
Is it an enumeration or a bitmask?
> #endif /* _UAPI__ASM_HWCAP_H */
> diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> new file mode 100644
> index 0000000..4776b43
> --- /dev/null
> +++ b/arch/arm64/include/uapi/asm/mman.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI__ASM_MMAN_H
> +#define _UAPI__ASM_MMAN_H
> +
> +#include <asm-generic/mman.h>
> +
> +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
From prior discussions, I thought this would be PROT_BTI, without the
_GUARDED suffix. Do we really need that?
AFAICT, all other PROT_* definitions only have a single underscore, and
the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
powerpc.
[...]
> diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> index b82e0a9..3717b06 100644
> --- a/arch/arm64/kernel/ptrace.c
> +++ b/arch/arm64/kernel/ptrace.c
> @@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
> */
> #define SPSR_EL1_AARCH64_RES0_BITS \
> (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
> - GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
> + GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
> #define SPSR_EL1_AARCH32_RES0_BITS \
> (GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
Phew; I was worried this would be missed!
[...]
> @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> regs->regs[29] = (unsigned long)&user->next_frame->fp;
> regs->pc = (unsigned long)ka->sa.sa_handler;
>
> + if (system_supports_bti()) {
> + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
Nit: that can be:
regs->pstate &= ~PSR_BTYPE_MASK;
> diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> index 5610ac0..85b456b 100644
> --- a/arch/arm64/kernel/syscall.c
> +++ b/arch/arm64/kernel/syscall.c
> @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> unsigned long flags = current_thread_info()->flags;
>
> regs->orig_x0 = regs->regs[0];
> + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
Likewise:
regs->pstate &= ~PSR_BTYPE_MASK;
... though I don't understand why that would matter to syscalls, nor how
those bits could ever be set given we had to execute an SVC to get here.
What am I missing?
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 13:02 ` Mark Rutland
0 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 13:02 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arm-kernel, linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu,
Arnd Bergmann, Richard Henderson, Andrew Jones, Will Deacon,
Catalin Marinas, Kristina Martšenko, Szabolcs Nagy,
Sudakshina Das, Paul Elliott
Hi Dave,
This generally looks good, but I have a few comments below.
On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> +{
> + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> + return VM_ARM64_GP;
> +
> + return 0;
> +}
> +
> +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> +{
> + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> +}
While the architectural name for the PTE bit is GP, it might make more
sense to call the vm flag VM_ARM64_BTI, since people are more likely to
recognise BTI than GP as a mnemonic.
Not a big deal either way, though.
[...]
> diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> index b2de329..b868ef11 100644
> --- a/arch/arm64/include/asm/ptrace.h
> +++ b/arch/arm64/include/asm/ptrace.h
> @@ -41,6 +41,7 @@
>
> /* Additional SPSR bits not exposed in the UABI */
> #define PSR_IL_BIT (1 << 20)
> +#define PSR_BTYPE_CALL (2 << 10)
I thought BTYPE was a 2-bit field, so isn't there at leat one other
value to have a mnemonic for?
Is it an enumeration or a bitmask?
> #endif /* _UAPI__ASM_HWCAP_H */
> diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> new file mode 100644
> index 0000000..4776b43
> --- /dev/null
> +++ b/arch/arm64/include/uapi/asm/mman.h
> @@ -0,0 +1,9 @@
> +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> +#ifndef _UAPI__ASM_MMAN_H
> +#define _UAPI__ASM_MMAN_H
> +
> +#include <asm-generic/mman.h>
> +
> +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 13:02 ` Mark Rutland
@ 2019-05-24 14:53 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 14:53 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> Hi Dave,
>
> This generally looks good, but I have a few comments below.
>
> On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > +{
> > + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> > + return VM_ARM64_GP;
> > +
> > + return 0;
> > +}
> > +
> > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > +{
> > + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> > +}
>
> While the architectural name for the PTE bit is GP, it might make more
> sense to call the vm flag VM_ARM64_BTI, since people are more likely to
> recognise BTI than GP as a mnemonic.
>
> Not a big deal either way, though.
I'm happy to change it. It's a kernel internal flag used in
approximately zero places. So whatever name is most intuitive for
kernel maintainers is fine. Nobody else needs to look at it.
> > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > index b2de329..b868ef11 100644
> > --- a/arch/arm64/include/asm/ptrace.h
> > +++ b/arch/arm64/include/asm/ptrace.h
> > @@ -41,6 +41,7 @@
> >
> > /* Additional SPSR bits not exposed in the UABI */
> > #define PSR_IL_BIT (1 << 20)
> > +#define PSR_BTYPE_CALL (2 << 10)
>
> I thought BTYPE was a 2-bit field, so isn't there at leat one other
> value to have a mnemonic for?
>
> Is it an enumeration or a bitmask?
It's a 2-bit enumeration, and for now this is the only value that the
kernel uses: this determines the types of BTI landing pad permitted at
signal handler entry points in BTI guarded pages.
Possibly it would be clearer to write it
#define PSR_BTYPE_CALL (0b10 << 10)
but we don't write other ptrace.h constants this way. In UAPI headers
we should avoid GCC-isms, but here it's OK since we already rely on this
syntax internally.
I can change it if you prefer, though my preference is to leave it.
> > #endif /* _UAPI__ASM_HWCAP_H */
> > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > new file mode 100644
> > index 0000000..4776b43
> > --- /dev/null
> > +++ b/arch/arm64/include/uapi/asm/mman.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +#ifndef _UAPI__ASM_MMAN_H
> > +#define _UAPI__ASM_MMAN_H
> > +
> > +#include <asm-generic/mman.h>
> > +
> > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
>
> From prior discussions, I thought this would be PROT_BTI, without the
> _GUARDED suffix. Do we really need that?
>
> AFAICT, all other PROT_* definitions only have a single underscore, and
> the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> powerpc.
No strong opinon. I was trying to make the name less obscure, but I'm
equally happy with PROT_BTI if people prefer that.
> > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > index b82e0a9..3717b06 100644
> > --- a/arch/arm64/kernel/ptrace.c
> > +++ b/arch/arm64/kernel/ptrace.c
> > @@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
> > */
> > #define SPSR_EL1_AARCH64_RES0_BITS \
> > (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
> > - GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
> > + GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
> > #define SPSR_EL1_AARCH32_RES0_BITS \
> > (GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
>
> Phew; I was worried this would be missed!
It was. I had fun debugging that one :)
> > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > regs->pc = (unsigned long)ka->sa.sa_handler;
> >
> > + if (system_supports_bti()) {
> > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
>
> Nit: that can be:
>
> regs->pstate &= ~PSR_BTYPE_MASK;
x & ~y is sensitive to the type of y and can clobber high bits, so I
prefer not to write it. GCC generates the same code either way.
However, this will also trip us up elsewhere when the time comes, so
maybe it's a waste of time working around it here.
If you feel strongly, I'm happy to change it.
> > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > index 5610ac0..85b456b 100644
> > --- a/arch/arm64/kernel/syscall.c
> > +++ b/arch/arm64/kernel/syscall.c
> > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > unsigned long flags = current_thread_info()->flags;
> >
> > regs->orig_x0 = regs->regs[0];
> > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
>
> Likewise:
>
> regs->pstate &= ~PSR_BTYPE_MASK;
>
> ... though I don't understand why that would matter to syscalls, nor how
> those bits could ever be set given we had to execute an SVC to get here.
>
> What am I missing?
The behaviour is counterintuivite here. The architecture guarantees to
preserve BTYPE for traps, faults and asynchronous exceptions, but for a
synchronous execption from normal architectural execution of an
exception-generating instruction (SVC/HVC/SMC) the architecture leaves
it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
I suppose precisely because there's only one way to reach the SVC
handler, software knows for certain whether zero SPSR.BTYPE in that
case. So hardware doesn't need to do it.
This may also simplify some trapping/emulation scenarios, but I need to
think about that.
Hmmm, I need to go and look at the HVC entry points and SMC emulation in
KVM though -- similar issues likely apply. I didn't look.
Same for the bootwrapper, and anything else with exception vectors.
[...]
Cheers
---Dave
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 14:53 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 14:53 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martšenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> Hi Dave,
>
> This generally looks good, but I have a few comments below.
>
> On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > +{
> > + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> > + return VM_ARM64_GP;
> > +
> > + return 0;
> > +}
> > +
> > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > +{
> > + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> > +}
>
> While the architectural name for the PTE bit is GP, it might make more
> sense to call the vm flag VM_ARM64_BTI, since people are more likely to
> recognise BTI than GP as a mnemonic.
>
> Not a big deal either way, though.
I'm happy to change it. It's a kernel internal flag used in
approximately zero places. So whatever name is most intuitive for
kernel maintainers is fine. Nobody else needs to look at it.
> > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > index b2de329..b868ef11 100644
> > --- a/arch/arm64/include/asm/ptrace.h
> > +++ b/arch/arm64/include/asm/ptrace.h
> > @@ -41,6 +41,7 @@
> >
> > /* Additional SPSR bits not exposed in the UABI */
> > #define PSR_IL_BIT (1 << 20)
> > +#define PSR_BTYPE_CALL (2 << 10)
>
> I thought BTYPE was a 2-bit field, so isn't there at leat one other
> value to have a mnemonic for?
>
> Is it an enumeration or a bitmask?
It's a 2-bit enumeration, and for now this is the only value that the
kernel uses: this determines the types of BTI landing pad permitted at
signal handler entry points in BTI guarded pages.
Possibly it would be clearer to write it
#define PSR_BTYPE_CALL (0b10 << 10)
but we don't write other ptrace.h constants this way. In UAPI headers
we should avoid GCC-isms, but here it's OK since we already rely on this
syntax internally.
I can change it if you prefer, though my preference is to leave it.
> > #endif /* _UAPI__ASM_HWCAP_H */
> > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > new file mode 100644
> > index 0000000..4776b43
> > --- /dev/null
> > +++ b/arch/arm64/include/uapi/asm/mman.h
> > @@ -0,0 +1,9 @@
> > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > +#ifndef _UAPI__ASM_MMAN_H
> > +#define _UAPI__ASM_MMAN_H
> > +
> > +#include <asm-generic/mman.h>
> > +
> > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
>
> From prior discussions, I thought this would be PROT_BTI, without the
> _GUARDED suffix. Do we really need that?
>
> AFAICT, all other PROT_* definitions only have a single underscore, and
> the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> powerpc.
No strong opinon. I was trying to make the name less obscure, but I'm
equally happy with PROT_BTI if people prefer that.
> > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > index b82e0a9..3717b06 100644
> > --- a/arch/arm64/kernel/ptrace.c
> > +++ b/arch/arm64/kernel/ptrace.c
> > @@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
> > */
> > #define SPSR_EL1_AARCH64_RES0_BITS \
> > (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
> > - GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
> > + GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
> > #define SPSR_EL1_AARCH32_RES0_BITS \
> > (GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
>
> Phew; I was worried this would be missed!
It was. I had fun debugging that one :)
> > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > regs->pc = (unsigned long)ka->sa.sa_handler;
> >
> > + if (system_supports_bti()) {
> > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
>
> Nit: that can be:
>
> regs->pstate &= ~PSR_BTYPE_MASK;
x & ~y is sensitive to the type of y and can clobber high bits, so I
prefer not to write it. GCC generates the same code either way.
However, this will also trip us up elsewhere when the time comes, so
maybe it's a waste of time working around it here.
If you feel strongly, I'm happy to change it.
> > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > index 5610ac0..85b456b 100644
> > --- a/arch/arm64/kernel/syscall.c
> > +++ b/arch/arm64/kernel/syscall.c
> > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > unsigned long flags = current_thread_info()->flags;
> >
> > regs->orig_x0 = regs->regs[0];
> > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
>
> Likewise:
>
> regs->pstate &= ~PSR_BTYPE_MASK;
>
> ... though I don't understand why that would matter to syscalls, nor how
> those bits could ever be set given we had to execute an SVC to get here.
>
> What am I missing?
The behaviour is counterintuivite here. The architecture guarantees to
preserve BTYPE for traps, faults and asynchronous exceptions, but for a
synchronous execption from normal architectural execution of an
exception-generating instruction (SVC/HVC/SMC) the architecture leaves
it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
I suppose precisely because there's only one way to reach the SVC
handler, software knows for certain whether zero SPSR.BTYPE in that
case. So hardware doesn't need to do it.
This may also simplify some trapping/emulation scenarios, but I need to
think about that.
Hmmm, I need to go and look at the HVC entry points and SMC emulation in
KVM though -- similar issues likely apply. I didn't look.
Same for the bootwrapper, and anything else with exception vectors.
[...]
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 14:53 ` Dave Martin
@ 2019-05-24 15:38 ` Mark Rutland
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 15:38 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > > +{
> > > + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> > > + return VM_ARM64_GP;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > > +{
> > > + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> > > +}
> >
> > While the architectural name for the PTE bit is GP, it might make more
> > sense to call the vm flag VM_ARM64_BTI, since people are more likely to
> > recognise BTI than GP as a mnemonic.
> >
> > Not a big deal either way, though.
>
> I'm happy to change it. It's a kernel internal flag used in
> approximately zero places. So whatever name is most intuitive for
> kernel maintainers is fine. Nobody else needs to look at it.
Sure thing; I just know that I'm going to remember what BTI is much more
easily than I'll remember what GP is.
> > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > index b2de329..b868ef11 100644
> > > --- a/arch/arm64/include/asm/ptrace.h
> > > +++ b/arch/arm64/include/asm/ptrace.h
> > > @@ -41,6 +41,7 @@
> > >
> > > /* Additional SPSR bits not exposed in the UABI */
> > > #define PSR_IL_BIT (1 << 20)
> > > +#define PSR_BTYPE_CALL (2 << 10)
> >
> > I thought BTYPE was a 2-bit field, so isn't there at leat one other
> > value to have a mnemonic for?
> >
> > Is it an enumeration or a bitmask?
>
> It's a 2-bit enumeration, and for now this is the only value that the
> kernel uses: this determines the types of BTI landing pad permitted at
> signal handler entry points in BTI guarded pages.
>
> Possibly it would be clearer to write it
>
> #define PSR_BTYPE_CALL (0b10 << 10)
>
> but we don't write other ptrace.h constants this way. In UAPI headers
> we should avoid GCC-isms, but here it's OK since we already rely on this
> syntax internally.
>
> I can change it if you prefer, though my preference is to leave it.
I have no issue with the (2 << 10) form, but could we add mnemonics for
the other values now, even if we're not using them at this instant?
> > > #endif /* _UAPI__ASM_HWCAP_H */
> > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > new file mode 100644
> > > index 0000000..4776b43
> > > --- /dev/null
> > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI__ASM_MMAN_H
> > > +#define _UAPI__ASM_MMAN_H
> > > +
> > > +#include <asm-generic/mman.h>
> > > +
> > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> >
> > From prior discussions, I thought this would be PROT_BTI, without the
> > _GUARDED suffix. Do we really need that?
> >
> > AFAICT, all other PROT_* definitions only have a single underscore, and
> > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > powerpc.
>
> No strong opinon. I was trying to make the name less obscure, but I'm
> equally happy with PROT_BTI if people prefer that.
My personal opinion is that PROT_BTI is preferable, but I'll let others
chime in.
> > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > > index b82e0a9..3717b06 100644
> > > --- a/arch/arm64/kernel/ptrace.c
> > > +++ b/arch/arm64/kernel/ptrace.c
> > > @@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
> > > */
> > > #define SPSR_EL1_AARCH64_RES0_BITS \
> > > (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
> > > - GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
> > > + GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
> > > #define SPSR_EL1_AARCH32_RES0_BITS \
> > > (GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
> >
> > Phew; I was worried this would be missed!
>
> It was. I had fun debugging that one :)
>
> > > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > > regs->pc = (unsigned long)ka->sa.sa_handler;
> > >
> > > + if (system_supports_bti()) {
> > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> >
> > Nit: that can be:
> >
> > regs->pstate &= ~PSR_BTYPE_MASK;
>
> x & ~y is sensitive to the type of y and can clobber high bits, so I
> prefer not to write it. GCC generates the same code either way.
Ah, I thought this might befor type promotion.
> However, this will also trip us up elsewhere when the time comes, so
> maybe it's a waste of time working around it here.
>
> If you feel strongly, I'm happy to change it.
I'd rather we followed the same pattern as elsewhere, as having this
special case is confusing, and we'd still have the same bug elsewhere.
My concern here is consistency, so if you want to fix up all instances
to preserve the upper 32 bits of regs->pstate, I'd be happy. :)
I also think there are nicer/clearer ways to fix the type promotion
issue, like using UL in the field definitions, using explicit casts, or
adding helpers to set/clear bits with appropriate promotion.
> > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > index 5610ac0..85b456b 100644
> > > --- a/arch/arm64/kernel/syscall.c
> > > +++ b/arch/arm64/kernel/syscall.c
> > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > unsigned long flags = current_thread_info()->flags;
> > >
> > > regs->orig_x0 = regs->regs[0];
> > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> >
> > Likewise:
> >
> > regs->pstate &= ~PSR_BTYPE_MASK;
> >
> > ... though I don't understand why that would matter to syscalls, nor how
> > those bits could ever be set given we had to execute an SVC to get here.
> >
> > What am I missing?
>
> The behaviour is counterintuivite here. The architecture guarantees to
> preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> synchronous execption from normal architectural execution of an
> exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
I'm still missing something here. IIUC were BTYPE was non-zero, we
should take the BTI trap before executing the SVC/HVC/SMC, right?
Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
which would logically violate the BTI protection.
If the assumption is that software can fix that case up, and the ??C
exception is prioritized above the BTI exception, then I think that we
should check whether it was permitted rather than silently fixing it up.
> I suppose precisely because there's only one way to reach the SVC
> handler, software knows for certain whether zero SPSR.BTYPE in that
> case. So hardware doesn't need to do it.
As above, I thought BTYPE had to be zero in order for it to be possible
to execute the SVC/HVC/SMC, but there might be caveats.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 15:38 ` Mark Rutland
0 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 15:38 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martšenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > > +{
> > > + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> > > + return VM_ARM64_GP;
> > > +
> > > + return 0;
> > > +}
> > > +
> > > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > > +{
> > > + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> > > +}
> >
> > While the architectural name for the PTE bit is GP, it might make more
> > sense to call the vm flag VM_ARM64_BTI, since people are more likely to
> > recognise BTI than GP as a mnemonic.
> >
> > Not a big deal either way, though.
>
> I'm happy to change it. It's a kernel internal flag used in
> approximately zero places. So whatever name is most intuitive for
> kernel maintainers is fine. Nobody else needs to look at it.
Sure thing; I just know that I'm going to remember what BTI is much more
easily than I'll remember what GP is.
> > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > index b2de329..b868ef11 100644
> > > --- a/arch/arm64/include/asm/ptrace.h
> > > +++ b/arch/arm64/include/asm/ptrace.h
> > > @@ -41,6 +41,7 @@
> > >
> > > /* Additional SPSR bits not exposed in the UABI */
> > > #define PSR_IL_BIT (1 << 20)
> > > +#define PSR_BTYPE_CALL (2 << 10)
> >
> > I thought BTYPE was a 2-bit field, so isn't there at leat one other
> > value to have a mnemonic for?
> >
> > Is it an enumeration or a bitmask?
>
> It's a 2-bit enumeration, and for now this is the only value that the
> kernel uses: this determines the types of BTI landing pad permitted at
> signal handler entry points in BTI guarded pages.
>
> Possibly it would be clearer to write it
>
> #define PSR_BTYPE_CALL (0b10 << 10)
>
> but we don't write other ptrace.h constants this way. In UAPI headers
> we should avoid GCC-isms, but here it's OK since we already rely on this
> syntax internally.
>
> I can change it if you prefer, though my preference is to leave it.
I have no issue with the (2 << 10) form, but could we add mnemonics for
the other values now, even if we're not using them at this instant?
> > > #endif /* _UAPI__ASM_HWCAP_H */
> > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > new file mode 100644
> > > index 0000000..4776b43
> > > --- /dev/null
> > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI__ASM_MMAN_H
> > > +#define _UAPI__ASM_MMAN_H
> > > +
> > > +#include <asm-generic/mman.h>
> > > +
> > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> >
> > From prior discussions, I thought this would be PROT_BTI, without the
> > _GUARDED suffix. Do we really need that?
> >
> > AFAICT, all other PROT_* definitions only have a single underscore, and
> > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > powerpc.
>
> No strong opinon. I was trying to make the name less obscure, but I'm
> equally happy with PROT_BTI if people prefer that.
My personal opinion is that PROT_BTI is preferable, but I'll let others
chime in.
> > > diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
> > > index b82e0a9..3717b06 100644
> > > --- a/arch/arm64/kernel/ptrace.c
> > > +++ b/arch/arm64/kernel/ptrace.c
> > > @@ -1860,7 +1860,7 @@ void syscall_trace_exit(struct pt_regs *regs)
> > > */
> > > #define SPSR_EL1_AARCH64_RES0_BITS \
> > > (GENMASK_ULL(63, 32) | GENMASK_ULL(27, 25) | GENMASK_ULL(23, 22) | \
> > > - GENMASK_ULL(20, 13) | GENMASK_ULL(11, 10) | GENMASK_ULL(5, 5))
> > > + GENMASK_ULL(20, 13) | GENMASK_ULL(5, 5))
> > > #define SPSR_EL1_AARCH32_RES0_BITS \
> > > (GENMASK_ULL(63, 32) | GENMASK_ULL(22, 22) | GENMASK_ULL(20, 20))
> >
> > Phew; I was worried this would be missed!
>
> It was. I had fun debugging that one :)
>
> > > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > > regs->pc = (unsigned long)ka->sa.sa_handler;
> > >
> > > + if (system_supports_bti()) {
> > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> >
> > Nit: that can be:
> >
> > regs->pstate &= ~PSR_BTYPE_MASK;
>
> x & ~y is sensitive to the type of y and can clobber high bits, so I
> prefer not to write it. GCC generates the same code either way.
Ah, I thought this might befor type promotion.
> However, this will also trip us up elsewhere when the time comes, so
> maybe it's a waste of time working around it here.
>
> If you feel strongly, I'm happy to change it.
I'd rather we followed the same pattern as elsewhere, as having this
special case is confusing, and we'd still have the same bug elsewhere.
My concern here is consistency, so if you want to fix up all instances
to preserve the upper 32 bits of regs->pstate, I'd be happy. :)
I also think there are nicer/clearer ways to fix the type promotion
issue, like using UL in the field definitions, using explicit casts, or
adding helpers to set/clear bits with appropriate promotion.
> > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > index 5610ac0..85b456b 100644
> > > --- a/arch/arm64/kernel/syscall.c
> > > +++ b/arch/arm64/kernel/syscall.c
> > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > unsigned long flags = current_thread_info()->flags;
> > >
> > > regs->orig_x0 = regs->regs[0];
> > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> >
> > Likewise:
> >
> > regs->pstate &= ~PSR_BTYPE_MASK;
> >
> > ... though I don't understand why that would matter to syscalls, nor how
> > those bits could ever be set given we had to execute an SVC to get here.
> >
> > What am I missing?
>
> The behaviour is counterintuivite here. The architecture guarantees to
> preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> synchronous execption from normal architectural execution of an
> exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
I'm still missing something here. IIUC were BTYPE was non-zero, we
should take the BTI trap before executing the SVC/HVC/SMC, right?
Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
which would logically violate the BTI protection.
If the assumption is that software can fix that case up, and the ??C
exception is prioritized above the BTI exception, then I think that we
should check whether it was permitted rather than silently fixing it up.
> I suppose precisely because there's only one way to reach the SVC
> handler, software knows for certain whether zero SPSR.BTYPE in that
> case. So hardware doesn't need to do it.
As above, I thought BTYPE had to be zero in order for it to be possible
to execute the SVC/HVC/SMC, but there might be caveats.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 15:38 ` Mark Rutland
@ 2019-05-24 16:12 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 16:12 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martšenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > > > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > > > +{
> > > > + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> > > > + return VM_ARM64_GP;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > > > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > > > +{
> > > > + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> > > > +}
> > >
> > > While the architectural name for the PTE bit is GP, it might make more
> > > sense to call the vm flag VM_ARM64_BTI, since people are more likely to
> > > recognise BTI than GP as a mnemonic.
> > >
> > > Not a big deal either way, though.
> >
> > I'm happy to change it. It's a kernel internal flag used in
> > approximately zero places. So whatever name is most intuitive for
> > kernel maintainers is fine. Nobody else needs to look at it.
>
> Sure thing; I just know that I'm going to remember what BTI is much more
> easily than I'll remember what GP is.
>
> > > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > > index b2de329..b868ef11 100644
> > > > --- a/arch/arm64/include/asm/ptrace.h
> > > > +++ b/arch/arm64/include/asm/ptrace.h
> > > > @@ -41,6 +41,7 @@
> > > >
> > > > /* Additional SPSR bits not exposed in the UABI */
> > > > #define PSR_IL_BIT (1 << 20)
> > > > +#define PSR_BTYPE_CALL (2 << 10)
> > >
> > > I thought BTYPE was a 2-bit field, so isn't there at leat one other
> > > value to have a mnemonic for?
> > >
> > > Is it an enumeration or a bitmask?
> >
> > It's a 2-bit enumeration, and for now this is the only value that the
> > kernel uses: this determines the types of BTI landing pad permitted at
> > signal handler entry points in BTI guarded pages.
> >
> > Possibly it would be clearer to write it
> >
> > #define PSR_BTYPE_CALL (0b10 << 10)
> >
> > but we don't write other ptrace.h constants this way. In UAPI headers
> > we should avoid GCC-isms, but here it's OK since we already rely on this
> > syntax internally.
> >
> > I can change it if you prefer, though my preference is to leave it.
>
> I have no issue with the (2 << 10) form, but could we add mnemonics for
> the other values now, even if we're not using them at this instant?
Can do. How about.
PSR_BTYPE_NONE (0 << 10)
PSR_BTYPE_JC (1 << 10)
PSR_BTYPE_C (2 << 10)
PSR_BTYPE_J (3 << 10)
That matches the way I decode PSTATE for splats.
The architecture does not define mnemonics so these are my invention,
but anyway this is just for the kernel.
>
> > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > > new file mode 100644
> > > > index 0000000..4776b43
> > > > --- /dev/null
> > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > @@ -0,0 +1,9 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > +#define _UAPI__ASM_MMAN_H
> > > > +
> > > > +#include <asm-generic/mman.h>
> > > > +
> > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> > >
> > > From prior discussions, I thought this would be PROT_BTI, without the
> > > _GUARDED suffix. Do we really need that?
> > >
> > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > powerpc.
> >
> > No strong opinon. I was trying to make the name less obscure, but I'm
> > equally happy with PROT_BTI if people prefer that.
>
> My personal opinion is that PROT_BTI is preferable, but I'll let others
> chime in.
[...]
If nobody objects, I'll change it as you propose, and change the vma
flag to match.
> > > > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > > > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > > > regs->pc = (unsigned long)ka->sa.sa_handler;
> > > >
> > > > + if (system_supports_bti()) {
> > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > >
> > > Nit: that can be:
> > >
> > > regs->pstate &= ~PSR_BTYPE_MASK;
> >
> > x & ~y is sensitive to the type of y and can clobber high bits, so I
> > prefer not to write it. GCC generates the same code either way.
>
> Ah, I thought this might befor type promotion.
>
> > However, this will also trip us up elsewhere when the time comes, so
> > maybe it's a waste of time working around it here.
> >
> > If you feel strongly, I'm happy to change it.
>
> I'd rather we followed the same pattern as elsewhere, as having this
> special case is confusing, and we'd still have the same bug elsewhere.
>
> My concern here is consistency, so if you want to fix up all instances
> to preserve the upper 32 bits of regs->pstate, I'd be happy. :)
>
> I also think there are nicer/clearer ways to fix the type promotion
> issue, like using UL in the field definitions, using explicit casts, or
> adding helpers to set/clear bits with appropriate promotion.
Sure, I change it to be more consistent.
Wrapping this idiom up in a clear_bits() wrapper might be an idea,
but in advance of that it makes little sense to do it just in this one
place.
I don't really like annotating header #defines with arbitrary types
until it's really necessary, so I'll leave it for now.
> > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > index 5610ac0..85b456b 100644
> > > > --- a/arch/arm64/kernel/syscall.c
> > > > +++ b/arch/arm64/kernel/syscall.c
> > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > unsigned long flags = current_thread_info()->flags;
> > > >
> > > > regs->orig_x0 = regs->regs[0];
> > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > >
> > > Likewise:
> > >
> > > regs->pstate &= ~PSR_BTYPE_MASK;
> > >
> > > ... though I don't understand why that would matter to syscalls, nor how
> > > those bits could ever be set given we had to execute an SVC to get here.
> > >
> > > What am I missing?
> >
> > The behaviour is counterintuivite here. The architecture guarantees to
> > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > synchronous execption from normal architectural execution of an
> > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
>
> I'm still missing something here. IIUC were BTYPE was non-zero, we
> should take the BTI trap before executing the SVC/HVC/SMC, right?
>
> Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> which would logically violate the BTI protection.
Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
have legitimately branched to the SVC insn directly and BTYPE would
be nonzero, but no trap would occur.
We should still logically zero BTYPE across SVC in that case, because
the SVC may itself branch: a signal could be delivered on return and
we want the prevailing BTYPE then to be 0 for capture in the signal
frame. Doing this zeroing in signal delivery because if the signal
is not delivered in SVE then a nonzero BTYPE might be live, and we
must then restore it properly on sigreturn.
As you observe, this scenario should be impossible if the SVC insn
is in a guarded page, unless userspace does something foolhardy like
catching the SIGILL and fudging BTYPE or the return address.
> If the assumption is that software can fix that case up, and the ??C
> exception is prioritized above the BTI exception, then I think that we
> should check whether it was permitted rather than silently fixing it up.
I don't think there is any silent fixup here: if SVC is executed
immediately after a branch then the BTI exception should preempt the
architectural execution of the SVC: if the SVC is architecturally
executed at all, any preceding branch must have been compliant.
> > I suppose precisely because there's only one way to reach the SVC
> > handler, software knows for certain whether zero SPSR.BTYPE in that
> > case. So hardware doesn't need to do it.
>
> As above, I thought BTYPE had to be zero in order for it to be possible
> to execute the SVC/HVC/SMC, but there might be caveats.
Modulo the GP bit in the page table (as described in more detail above).
There may be caveats I've missed though -- I need to take another look.
Feel free to continue digging :)
(Now I come to think of it I also need to look at rseq etc., which is
another magic kernel-mediated branch mechanism.)
Cheers
---Dave
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 16:12 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 16:12 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Will Deacon, Richard Henderson, Andrew Jones,
Kristina Martšenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > +#define arch_calc_vm_prot_bits(prot, pkey) arm64_calc_vm_prot_bits(prot)
> > > > +static inline unsigned long arm64_calc_vm_prot_bits(unsigned long prot)
> > > > +{
> > > > + if (system_supports_bti() && (prot & PROT_BTI_GUARDED))
> > > > + return VM_ARM64_GP;
> > > > +
> > > > + return 0;
> > > > +}
> > > > +
> > > > +#define arch_vm_get_page_prot(vm_flags) arm64_vm_get_page_prot(vm_flags)
> > > > +static inline pgprot_t arm64_vm_get_page_prot(unsigned long vm_flags)
> > > > +{
> > > > + return (vm_flags & VM_ARM64_GP) ? __pgprot(PTE_GP) : __pgprot(0);
> > > > +}
> > >
> > > While the architectural name for the PTE bit is GP, it might make more
> > > sense to call the vm flag VM_ARM64_BTI, since people are more likely to
> > > recognise BTI than GP as a mnemonic.
> > >
> > > Not a big deal either way, though.
> >
> > I'm happy to change it. It's a kernel internal flag used in
> > approximately zero places. So whatever name is most intuitive for
> > kernel maintainers is fine. Nobody else needs to look at it.
>
> Sure thing; I just know that I'm going to remember what BTI is much more
> easily than I'll remember what GP is.
>
> > > > diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
> > > > index b2de329..b868ef11 100644
> > > > --- a/arch/arm64/include/asm/ptrace.h
> > > > +++ b/arch/arm64/include/asm/ptrace.h
> > > > @@ -41,6 +41,7 @@
> > > >
> > > > /* Additional SPSR bits not exposed in the UABI */
> > > > #define PSR_IL_BIT (1 << 20)
> > > > +#define PSR_BTYPE_CALL (2 << 10)
> > >
> > > I thought BTYPE was a 2-bit field, so isn't there at leat one other
> > > value to have a mnemonic for?
> > >
> > > Is it an enumeration or a bitmask?
> >
> > It's a 2-bit enumeration, and for now this is the only value that the
> > kernel uses: this determines the types of BTI landing pad permitted at
> > signal handler entry points in BTI guarded pages.
> >
> > Possibly it would be clearer to write it
> >
> > #define PSR_BTYPE_CALL (0b10 << 10)
> >
> > but we don't write other ptrace.h constants this way. In UAPI headers
> > we should avoid GCC-isms, but here it's OK since we already rely on this
> > syntax internally.
> >
> > I can change it if you prefer, though my preference is to leave it.
>
> I have no issue with the (2 << 10) form, but could we add mnemonics for
> the other values now, even if we're not using them at this instant?
Can do. How about.
PSR_BTYPE_NONE (0 << 10)
PSR_BTYPE_JC (1 << 10)
PSR_BTYPE_C (2 << 10)
PSR_BTYPE_J (3 << 10)
That matches the way I decode PSTATE for splats.
The architecture does not define mnemonics so these are my invention,
but anyway this is just for the kernel.
>
> > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > > new file mode 100644
> > > > index 0000000..4776b43
> > > > --- /dev/null
> > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > @@ -0,0 +1,9 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > +#define _UAPI__ASM_MMAN_H
> > > > +
> > > > +#include <asm-generic/mman.h>
> > > > +
> > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> > >
> > > From prior discussions, I thought this would be PROT_BTI, without the
> > > _GUARDED suffix. Do we really need that?
> > >
> > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > powerpc.
> >
> > No strong opinon. I was trying to make the name less obscure, but I'm
> > equally happy with PROT_BTI if people prefer that.
>
> My personal opinion is that PROT_BTI is preferable, but I'll let others
> chime in.
[...]
If nobody objects, I'll change it as you propose, and change the vma
flag to match.
> > > > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > > > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > > > regs->pc = (unsigned long)ka->sa.sa_handler;
> > > >
> > > > + if (system_supports_bti()) {
> > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > >
> > > Nit: that can be:
> > >
> > > regs->pstate &= ~PSR_BTYPE_MASK;
> >
> > x & ~y is sensitive to the type of y and can clobber high bits, so I
> > prefer not to write it. GCC generates the same code either way.
>
> Ah, I thought this might befor type promotion.
>
> > However, this will also trip us up elsewhere when the time comes, so
> > maybe it's a waste of time working around it here.
> >
> > If you feel strongly, I'm happy to change it.
>
> I'd rather we followed the same pattern as elsewhere, as having this
> special case is confusing, and we'd still have the same bug elsewhere.
>
> My concern here is consistency, so if you want to fix up all instances
> to preserve the upper 32 bits of regs->pstate, I'd be happy. :)
>
> I also think there are nicer/clearer ways to fix the type promotion
> issue, like using UL in the field definitions, using explicit casts, or
> adding helpers to set/clear bits with appropriate promotion.
Sure, I change it to be more consistent.
Wrapping this idiom up in a clear_bits() wrapper might be an idea,
but in advance of that it makes little sense to do it just in this one
place.
I don't really like annotating header #defines with arbitrary types
until it's really necessary, so I'll leave it for now.
> > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > index 5610ac0..85b456b 100644
> > > > --- a/arch/arm64/kernel/syscall.c
> > > > +++ b/arch/arm64/kernel/syscall.c
> > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > unsigned long flags = current_thread_info()->flags;
> > > >
> > > > regs->orig_x0 = regs->regs[0];
> > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > >
> > > Likewise:
> > >
> > > regs->pstate &= ~PSR_BTYPE_MASK;
> > >
> > > ... though I don't understand why that would matter to syscalls, nor how
> > > those bits could ever be set given we had to execute an SVC to get here.
> > >
> > > What am I missing?
> >
> > The behaviour is counterintuivite here. The architecture guarantees to
> > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > synchronous execption from normal architectural execution of an
> > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
>
> I'm still missing something here. IIUC were BTYPE was non-zero, we
> should take the BTI trap before executing the SVC/HVC/SMC, right?
>
> Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> which would logically violate the BTI protection.
Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
have legitimately branched to the SVC insn directly and BTYPE would
be nonzero, but no trap would occur.
We should still logically zero BTYPE across SVC in that case, because
the SVC may itself branch: a signal could be delivered on return and
we want the prevailing BTYPE then to be 0 for capture in the signal
frame. Doing this zeroing in signal delivery because if the signal
is not delivered in SVE then a nonzero BTYPE might be live, and we
must then restore it properly on sigreturn.
As you observe, this scenario should be impossible if the SVC insn
is in a guarded page, unless userspace does something foolhardy like
catching the SIGILL and fudging BTYPE or the return address.
> If the assumption is that software can fix that case up, and the ??C
> exception is prioritized above the BTI exception, then I think that we
> should check whether it was permitted rather than silently fixing it up.
I don't think there is any silent fixup here: if SVC is executed
immediately after a branch then the BTI exception should preempt the
architectural execution of the SVC: if the SVC is architecturally
executed at all, any preceding branch must have been compliant.
> > I suppose precisely because there's only one way to reach the SVC
> > handler, software knows for certain whether zero SPSR.BTYPE in that
> > case. So hardware doesn't need to do it.
>
> As above, I thought BTYPE had to be zero in order for it to be possible
> to execute the SVC/HVC/SMC, but there might be caveats.
Modulo the GP bit in the page table (as described in more detail above).
There may be caveats I've missed though -- I need to take another look.
Feel free to continue digging :)
(Now I come to think of it I also need to look at rseq etc., which is
another magic kernel-mediated branch mechanism.)
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 16:12 ` Dave Martin
@ 2019-05-24 17:19 ` Mark Rutland
-1 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 17:19 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martšenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 05:12:40PM +0100, Dave Martin wrote:
> On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > > /* Additional SPSR bits not exposed in the UABI */
> > > > > #define PSR_IL_BIT (1 << 20)
> > > > > +#define PSR_BTYPE_CALL (2 << 10)
> > > >
> > > > I thought BTYPE was a 2-bit field, so isn't there at leat one other
> > > > value to have a mnemonic for?
> > > >
> > > > Is it an enumeration or a bitmask?
> > >
> > > It's a 2-bit enumeration, and for now this is the only value that the
> > > kernel uses: this determines the types of BTI landing pad permitted at
> > > signal handler entry points in BTI guarded pages.
> > >
> > > Possibly it would be clearer to write it
> > >
> > > #define PSR_BTYPE_CALL (0b10 << 10)
> > >
> > > but we don't write other ptrace.h constants this way. In UAPI headers
> > > we should avoid GCC-isms, but here it's OK since we already rely on this
> > > syntax internally.
> > >
> > > I can change it if you prefer, though my preference is to leave it.
> >
> > I have no issue with the (2 << 10) form, but could we add mnemonics for
> > the other values now, even if we're not using them at this instant?
>
> Can do. How about.
>
> PSR_BTYPE_NONE (0 << 10)
> PSR_BTYPE_JC (1 << 10)
> PSR_BTYPE_C (2 << 10)
> PSR_BTYPE_J (3 << 10)
>
> That matches the way I decode PSTATE for splats.
>
> The architecture does not define mnemonics so these are my invention,
> but anyway this is just for the kernel.
That looks good to me!
[...]
> > > > > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > > > > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > > > > regs->pc = (unsigned long)ka->sa.sa_handler;
> > > > >
> > > > > + if (system_supports_bti()) {
> > > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > >
> > > > Nit: that can be:
> > > >
> > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > >
> > > x & ~y is sensitive to the type of y and can clobber high bits, so I
> > > prefer not to write it. GCC generates the same code either way.
> >
> > Ah, I thought this might befor type promotion.
> >
> > > However, this will also trip us up elsewhere when the time comes, so
> > > maybe it's a waste of time working around it here.
> > >
> > > If you feel strongly, I'm happy to change it.
> >
> > I'd rather we followed the same pattern as elsewhere, as having this
> > special case is confusing, and we'd still have the same bug elsewhere.
> >
> > My concern here is consistency, so if you want to fix up all instances
> > to preserve the upper 32 bits of regs->pstate, I'd be happy. :)
> >
> > I also think there are nicer/clearer ways to fix the type promotion
> > issue, like using UL in the field definitions, using explicit casts, or
> > adding helpers to set/clear bits with appropriate promotion.
>
> Sure, I change it to be more consistent.
>
> Wrapping this idiom up in a clear_bits() wrapper might be an idea,
> but in advance of that it makes little sense to do it just in this one
> place.
>
> I don't really like annotating header #defines with arbitrary types
> until it's really necessary, so I'll leave it for now.
Sure, that's fine by me.
[...]
> > > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > > index 5610ac0..85b456b 100644
> > > > > --- a/arch/arm64/kernel/syscall.c
> > > > > +++ b/arch/arm64/kernel/syscall.c
> > > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > > unsigned long flags = current_thread_info()->flags;
> > > > >
> > > > > regs->orig_x0 = regs->regs[0];
> > > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > >
> > > > Likewise:
> > > >
> > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > > >
> > > > ... though I don't understand why that would matter to syscalls, nor how
> > > > those bits could ever be set given we had to execute an SVC to get here.
> > > >
> > > > What am I missing?
> > >
> > > The behaviour is counterintuivite here. The architecture guarantees to
> > > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > > synchronous execption from normal architectural execution of an
> > > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
> >
> > I'm still missing something here. IIUC were BTYPE was non-zero, we
> > should take the BTI trap before executing the SVC/HVC/SMC, right?
> >
> > Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> > which would logically violate the BTI protection.
>
> Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
> have legitimately branched to the SVC insn directly and BTYPE would
> be nonzero, but no trap would occur.
I agree that would be the case immediately before we execute the SVC,
but I think there's a subtlety here w.r.t. what exactly happens as an
SVC is executed.
My understanding was that even for unguarded pages, the execution of any
(non branch/eret) instruction would zero PSTATE.BTYPE.
For SVC it's not clear to me whether generating the SVC exception is
considered to be an effect of completing the execution of an SVC,
whether it's considered as preempting the execution of the SVC, or
whether that's IMPLEMENTATION DEFINED.
Consequently it's not clear to me whether or not executing an SVC clears
PSTATE.BTYPE before the act of taking the exception samples PSTATE. I
would hope that it does, as this would be in keeping with the way the
ELR is updated.
I think that we should try to clarify that before we commit ourselves to
the most painful interpretation here. Especially as similar would apply
to HVC and SMC, and I strongly suspect firmware in general is unlikely
to fix up the PSTATE.BTYPE of a caller.
> We should still logically zero BTYPE across SVC in that case, because
> the SVC may itself branch: a signal could be delivered on return and
> we want the prevailing BTYPE then to be 0 for capture in the signal
> frame. Doing this zeroing in signal delivery because if the signal
> is not delivered in SVE then a nonzero BTYPE might be live, and we
> must then restore it properly on sigreturn.
I'm not sure I follow this.
If we deliver a signal, the kernel generates a pristine PSTATE for the
signal handler, and the interrupted context doesn't matter.
Saving/restoring the state of the interrupted context is identical to
returning without delivering the signal, and we'd have a problem
regardless.
> As you observe, this scenario should be impossible if the SVC insn
> is in a guarded page, unless userspace does something foolhardy like
> catching the SIGILL and fudging BTYPE or the return address.
I think userspace gets to pick up the pieces in this case. Much like
signal delivery, it would need to generate a sensible PSTATE itself.
[...]
> (Now I come to think of it I also need to look at rseq etc., which is
> another magic kernel-mediated branch mechanism.)
Fun. ;)
Thanks,
Mark.
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-24 17:19 ` Mark Rutland
0 siblings, 0 replies; 42+ messages in thread
From: Mark Rutland @ 2019-05-24 17:19 UTC (permalink / raw)
To: Dave Martin
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Will Deacon, Richard Henderson, Andrew Jones,
Kristina Martšenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 05:12:40PM +0100, Dave Martin wrote:
> On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > > /* Additional SPSR bits not exposed in the UABI */
> > > > > #define PSR_IL_BIT (1 << 20)
> > > > > +#define PSR_BTYPE_CALL (2 << 10)
> > > >
> > > > I thought BTYPE was a 2-bit field, so isn't there at leat one other
> > > > value to have a mnemonic for?
> > > >
> > > > Is it an enumeration or a bitmask?
> > >
> > > It's a 2-bit enumeration, and for now this is the only value that the
> > > kernel uses: this determines the types of BTI landing pad permitted at
> > > signal handler entry points in BTI guarded pages.
> > >
> > > Possibly it would be clearer to write it
> > >
> > > #define PSR_BTYPE_CALL (0b10 << 10)
> > >
> > > but we don't write other ptrace.h constants this way. In UAPI headers
> > > we should avoid GCC-isms, but here it's OK since we already rely on this
> > > syntax internally.
> > >
> > > I can change it if you prefer, though my preference is to leave it.
> >
> > I have no issue with the (2 << 10) form, but could we add mnemonics for
> > the other values now, even if we're not using them at this instant?
>
> Can do. How about.
>
> PSR_BTYPE_NONE (0 << 10)
> PSR_BTYPE_JC (1 << 10)
> PSR_BTYPE_C (2 << 10)
> PSR_BTYPE_J (3 << 10)
>
> That matches the way I decode PSTATE for splats.
>
> The architecture does not define mnemonics so these are my invention,
> but anyway this is just for the kernel.
That looks good to me!
[...]
> > > > > @@ -741,6 +741,11 @@ static void setup_return(struct pt_regs *regs, struct k_sigaction *ka,
> > > > > regs->regs[29] = (unsigned long)&user->next_frame->fp;
> > > > > regs->pc = (unsigned long)ka->sa.sa_handler;
> > > > >
> > > > > + if (system_supports_bti()) {
> > > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > >
> > > > Nit: that can be:
> > > >
> > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > >
> > > x & ~y is sensitive to the type of y and can clobber high bits, so I
> > > prefer not to write it. GCC generates the same code either way.
> >
> > Ah, I thought this might befor type promotion.
> >
> > > However, this will also trip us up elsewhere when the time comes, so
> > > maybe it's a waste of time working around it here.
> > >
> > > If you feel strongly, I'm happy to change it.
> >
> > I'd rather we followed the same pattern as elsewhere, as having this
> > special case is confusing, and we'd still have the same bug elsewhere.
> >
> > My concern here is consistency, so if you want to fix up all instances
> > to preserve the upper 32 bits of regs->pstate, I'd be happy. :)
> >
> > I also think there are nicer/clearer ways to fix the type promotion
> > issue, like using UL in the field definitions, using explicit casts, or
> > adding helpers to set/clear bits with appropriate promotion.
>
> Sure, I change it to be more consistent.
>
> Wrapping this idiom up in a clear_bits() wrapper might be an idea,
> but in advance of that it makes little sense to do it just in this one
> place.
>
> I don't really like annotating header #defines with arbitrary types
> until it's really necessary, so I'll leave it for now.
Sure, that's fine by me.
[...]
> > > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > > index 5610ac0..85b456b 100644
> > > > > --- a/arch/arm64/kernel/syscall.c
> > > > > +++ b/arch/arm64/kernel/syscall.c
> > > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > > unsigned long flags = current_thread_info()->flags;
> > > > >
> > > > > regs->orig_x0 = regs->regs[0];
> > > > > + regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > >
> > > > Likewise:
> > > >
> > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > > >
> > > > ... though I don't understand why that would matter to syscalls, nor how
> > > > those bits could ever be set given we had to execute an SVC to get here.
> > > >
> > > > What am I missing?
> > >
> > > The behaviour is counterintuivite here. The architecture guarantees to
> > > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > > synchronous execption from normal architectural execution of an
> > > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
> >
> > I'm still missing something here. IIUC were BTYPE was non-zero, we
> > should take the BTI trap before executing the SVC/HVC/SMC, right?
> >
> > Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> > which would logically violate the BTI protection.
>
> Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
> have legitimately branched to the SVC insn directly and BTYPE would
> be nonzero, but no trap would occur.
I agree that would be the case immediately before we execute the SVC,
but I think there's a subtlety here w.r.t. what exactly happens as an
SVC is executed.
My understanding was that even for unguarded pages, the execution of any
(non branch/eret) instruction would zero PSTATE.BTYPE.
For SVC it's not clear to me whether generating the SVC exception is
considered to be an effect of completing the execution of an SVC,
whether it's considered as preempting the execution of the SVC, or
whether that's IMPLEMENTATION DEFINED.
Consequently it's not clear to me whether or not executing an SVC clears
PSTATE.BTYPE before the act of taking the exception samples PSTATE. I
would hope that it does, as this would be in keeping with the way the
ELR is updated.
I think that we should try to clarify that before we commit ourselves to
the most painful interpretation here. Especially as similar would apply
to HVC and SMC, and I strongly suspect firmware in general is unlikely
to fix up the PSTATE.BTYPE of a caller.
> We should still logically zero BTYPE across SVC in that case, because
> the SVC may itself branch: a signal could be delivered on return and
> we want the prevailing BTYPE then to be 0 for capture in the signal
> frame. Doing this zeroing in signal delivery because if the signal
> is not delivered in SVE then a nonzero BTYPE might be live, and we
> must then restore it properly on sigreturn.
I'm not sure I follow this.
If we deliver a signal, the kernel generates a pristine PSTATE for the
signal handler, and the interrupted context doesn't matter.
Saving/restoring the state of the interrupted context is identical to
returning without delivering the signal, and we'd have a problem
regardless.
> As you observe, this scenario should be impossible if the SVC insn
> is in a guarded page, unless userspace does something foolhardy like
> catching the SIGILL and fudging BTYPE or the return address.
I think userspace gets to pick up the pieces in this case. Much like
signal delivery, it would need to generate a sensible PSTATE itself.
[...]
> (Now I come to think of it I also need to look at rseq etc., which is
> another magic kernel-mediated branch mechanism.)
Fun. ;)
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 17:19 ` Mark Rutland
(?)
@ 2019-05-28 10:52 ` Dave P Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave P Martin @ 2019-05-28 10:52 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martsenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 06:19:10PM +0100, Mark Rutland wrote:
> On Fri, May 24, 2019 at 05:12:40PM +0100, Dave Martin wrote:
> > On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
[...]
> > > > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > > > index 5610ac0..85b456b 100644
> > > > > > --- a/arch/arm64/kernel/syscall.c
> > > > > > +++ b/arch/arm64/kernel/syscall.c
> > > > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > > > unsigned long flags = current_thread_info()->flags;
> > > > > >
> > > > > > regs->orig_x0 = regs->regs[0];
> > > > > > +regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > > >
> > > > > Likewise:
> > > > >
> > > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > > > >
> > > > > ... though I don't understand why that would matter to syscalls, nor how
> > > > > those bits could ever be set given we had to execute an SVC to get here.
> > > > >
> > > > > What am I missing?
> > > >
> > > > The behaviour is counterintuivite here. The architecture guarantees to
> > > > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > > > synchronous execption from normal architectural execution of an
> > > > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > > > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
> > >
> > > I'm still missing something here. IIUC were BTYPE was non-zero, we
> > > should take the BTI trap before executing the SVC/HVC/SMC, right?
> > >
> > > Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> > > which would logically violate the BTI protection.
> >
> > Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
> > have legitimately branched to the SVC insn directly and BTYPE would
> > be nonzero, but no trap would occur.
>
> I agree that would be the case immediately before we execute the SVC,
> but I think there's a subtlety here w.r.t. what exactly happens as an
> SVC is executed.
>
> My understanding was that even for unguarded pages, the execution of any
> (non branch/eret) instruction would zero PSTATE.BTYPE.
>
> For SVC it's not clear to me whether generating the SVC exception is
> considered to be an effect of completing the execution of an SVC,
> whether it's considered as preempting the execution of the SVC, or
> whether that's IMPLEMENTATION DEFINED.
>
> Consequently it's not clear to me whether or not executing an SVC clears
> PSTATE.BTYPE before the act of taking the exception samples PSTATE. I
> would hope that it does, as this would be in keeping with the way the
> ELR is updated.
OTOH, the wording calls this case out quite explicitly. It seems odd to
do that if the more general wording applies.
I'll take another look and request clarficiation.
> I think that we should try to clarify that before we commit ourselves to
> the most painful interpretation here. Especially as similar would apply
> to HVC and SMC, and I strongly suspect firmware in general is unlikely
> to fix up the PSTATE.BTYPE of a caller.
>
> > We should still logically zero BTYPE across SVC in that case, because
> > the SVC may itself branch: a signal could be delivered on return and
> > we want the prevailing BTYPE then to be 0 for capture in the signal
> > frame. Doing this zeroing in signal delivery because if the signal
> > is not delivered in SVE then a nonzero BTYPE might be live, and we
> > must then restore it properly on sigreturn.
>
> I'm not sure I follow this.
>
> If we deliver a signal, the kernel generates a pristine PSTATE for the
> signal handler, and the interrupted context doesn't matter.
>
> Saving/restoring the state of the interrupted context is identical to
> returning without delivering the signal, and we'd have a problem
> regardless.
My test looks garbled... since the point I was making was tangential, I
don't elaborate it for now.
> > As you observe, this scenario should be impossible if the SVC insn
> > is in a guarded page, unless userspace does something foolhardy like
> > catching the SIGILL and fudging BTYPE or the return address.
>
> I think userspace gets to pick up the pieces in this case. Much like
> signal delivery, it would need to generate a sensible PSTATE itself.
Agreed, there is no way to hide this kind of thing from userspace code
that messes with the signal frame -- so we shouldn't try.
> [...]
>
> > (Now I come to think of it I also need to look at rseq etc., which is
> > another magic kernel-mediated branch mechanism.)
Meh.
Cheers
---Dave
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-28 10:52 ` Dave P Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave P Martin @ 2019-05-28 10:52 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Will Deacon, Richard Henderson, Andrew Jones,
Kristina Martsenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 06:19:10PM +0100, Mark Rutland wrote:
> On Fri, May 24, 2019 at 05:12:40PM +0100, Dave Martin wrote:
> > On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
[...]
> > > > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > > > index 5610ac0..85b456b 100644
> > > > > > --- a/arch/arm64/kernel/syscall.c
> > > > > > +++ b/arch/arm64/kernel/syscall.c
> > > > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > > > unsigned long flags = current_thread_info()->flags;
> > > > > >
> > > > > > regs->orig_x0 = regs->regs[0];
> > > > > > +regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > > >
> > > > > Likewise:
> > > > >
> > > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > > > >
> > > > > ... though I don't understand why that would matter to syscalls, nor how
> > > > > those bits could ever be set given we had to execute an SVC to get here.
> > > > >
> > > > > What am I missing?
> > > >
> > > > The behaviour is counterintuivite here. The architecture guarantees to
> > > > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > > > synchronous execption from normal architectural execution of an
> > > > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > > > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
> > >
> > > I'm still missing something here. IIUC were BTYPE was non-zero, we
> > > should take the BTI trap before executing the SVC/HVC/SMC, right?
> > >
> > > Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> > > which would logically violate the BTI protection.
> >
> > Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
> > have legitimately branched to the SVC insn directly and BTYPE would
> > be nonzero, but no trap would occur.
>
> I agree that would be the case immediately before we execute the SVC,
> but I think there's a subtlety here w.r.t. what exactly happens as an
> SVC is executed.
>
> My understanding was that even for unguarded pages, the execution of any
> (non branch/eret) instruction would zero PSTATE.BTYPE.
>
> For SVC it's not clear to me whether generating the SVC exception is
> considered to be an effect of completing the execution of an SVC,
> whether it's considered as preempting the execution of the SVC, or
> whether that's IMPLEMENTATION DEFINED.
>
> Consequently it's not clear to me whether or not executing an SVC clears
> PSTATE.BTYPE before the act of taking the exception samples PSTATE. I
> would hope that it does, as this would be in keeping with the way the
> ELR is updated.
OTOH, the wording calls this case out quite explicitly. It seems odd to
do that if the more general wording applies.
I'll take another look and request clarficiation.
> I think that we should try to clarify that before we commit ourselves to
> the most painful interpretation here. Especially as similar would apply
> to HVC and SMC, and I strongly suspect firmware in general is unlikely
> to fix up the PSTATE.BTYPE of a caller.
>
> > We should still logically zero BTYPE across SVC in that case, because
> > the SVC may itself branch: a signal could be delivered on return and
> > we want the prevailing BTYPE then to be 0 for capture in the signal
> > frame. Doing this zeroing in signal delivery because if the signal
> > is not delivered in SVE then a nonzero BTYPE might be live, and we
> > must then restore it properly on sigreturn.
>
> I'm not sure I follow this.
>
> If we deliver a signal, the kernel generates a pristine PSTATE for the
> signal handler, and the interrupted context doesn't matter.
>
> Saving/restoring the state of the interrupted context is identical to
> returning without delivering the signal, and we'd have a problem
> regardless.
My test looks garbled... since the point I was making was tangential, I
don't elaborate it for now.
> > As you observe, this scenario should be impossible if the SVC insn
> > is in a guarded page, unless userspace does something foolhardy like
> > catching the SIGILL and fudging BTYPE or the return address.
>
> I think userspace gets to pick up the pieces in this case. Much like
> signal delivery, it would need to generate a sensible PSTATE itself.
Agreed, there is no way to hide this kind of thing from userspace code
that messes with the signal frame -- so we shouldn't try.
> [...]
>
> > (Now I come to think of it I also need to look at rseq etc., which is
> > another magic kernel-mediated branch mechanism.)
Meh.
Cheers
---Dave
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-05-28 10:52 ` Dave P Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave P Martin @ 2019-05-28 10:52 UTC (permalink / raw)
To: Mark Rutland
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Paul Elliott, Arnd Bergmann,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martsenko, linux-kernel, Catalin Marinas,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 06:19:10PM +0100, Mark Rutland wrote:
> On Fri, May 24, 2019 at 05:12:40PM +0100, Dave Martin wrote:
> > On Fri, May 24, 2019 at 04:38:48PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave Martin wrote:
> > > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
[...]
> > > > > > diff --git a/arch/arm64/kernel/syscall.c b/arch/arm64/kernel/syscall.c
> > > > > > index 5610ac0..85b456b 100644
> > > > > > --- a/arch/arm64/kernel/syscall.c
> > > > > > +++ b/arch/arm64/kernel/syscall.c
> > > > > > @@ -66,6 +66,7 @@ static void el0_svc_common(struct pt_regs *regs, int scno, int sc_nr,
> > > > > > unsigned long flags = current_thread_info()->flags;
> > > > > >
> > > > > > regs->orig_x0 = regs->regs[0];
> > > > > > +regs->pstate &= ~(regs->pstate & PSR_BTYPE_MASK);
> > > > >
> > > > > Likewise:
> > > > >
> > > > > regs->pstate &= ~PSR_BTYPE_MASK;
> > > > >
> > > > > ... though I don't understand why that would matter to syscalls, nor how
> > > > > those bits could ever be set given we had to execute an SVC to get here.
> > > > >
> > > > > What am I missing?
> > > >
> > > > The behaviour is counterintuivite here. The architecture guarantees to
> > > > preserve BTYPE for traps, faults and asynchronous exceptions, but for a
> > > > synchronous execption from normal architectural execution of an
> > > > exception-generating instruction (SVC/HVC/SMC) the architecture leaves
> > > > it IMP DEF whether BTYPE is preserved or zeroed in SPSR.
> > >
> > > I'm still missing something here. IIUC were BTYPE was non-zero, we
> > > should take the BTI trap before executing the SVC/HVC/SMC, right?
> > >
> > > Otherwise, it would be possible to erroneously branch to an SVC/HVC/SMC,
> > > which would logically violate the BTI protection.
> >
> > Only if the SVC (etc.) is in a BTI guarded page. Otherwise, we could
> > have legitimately branched to the SVC insn directly and BTYPE would
> > be nonzero, but no trap would occur.
>
> I agree that would be the case immediately before we execute the SVC,
> but I think there's a subtlety here w.r.t. what exactly happens as an
> SVC is executed.
>
> My understanding was that even for unguarded pages, the execution of any
> (non branch/eret) instruction would zero PSTATE.BTYPE.
>
> For SVC it's not clear to me whether generating the SVC exception is
> considered to be an effect of completing the execution of an SVC,
> whether it's considered as preempting the execution of the SVC, or
> whether that's IMPLEMENTATION DEFINED.
>
> Consequently it's not clear to me whether or not executing an SVC clears
> PSTATE.BTYPE before the act of taking the exception samples PSTATE. I
> would hope that it does, as this would be in keeping with the way the
> ELR is updated.
OTOH, the wording calls this case out quite explicitly. It seems odd to
do that if the more general wording applies.
I'll take another look and request clarficiation.
> I think that we should try to clarify that before we commit ourselves to
> the most painful interpretation here. Especially as similar would apply
> to HVC and SMC, and I strongly suspect firmware in general is unlikely
> to fix up the PSTATE.BTYPE of a caller.
>
> > We should still logically zero BTYPE across SVC in that case, because
> > the SVC may itself branch: a signal could be delivered on return and
> > we want the prevailing BTYPE then to be 0 for capture in the signal
> > frame. Doing this zeroing in signal delivery because if the signal
> > is not delivered in SVE then a nonzero BTYPE might be live, and we
> > must then restore it properly on sigreturn.
>
> I'm not sure I follow this.
>
> If we deliver a signal, the kernel generates a pristine PSTATE for the
> signal handler, and the interrupted context doesn't matter.
>
> Saving/restoring the state of the interrupted context is identical to
> returning without delivering the signal, and we'd have a problem
> regardless.
My test looks garbled... since the point I was making was tangential, I
don't elaborate it for now.
> > As you observe, this scenario should be impossible if the SVC insn
> > is in a guarded page, unless userspace does something foolhardy like
> > catching the SIGILL and fudging BTYPE or the return address.
>
> I think userspace gets to pick up the pieces in this case. Much like
> signal delivery, it would need to generate a sensible PSTATE itself.
Agreed, there is no way to hide this kind of thing from userspace code
that messes with the signal frame -- so we shouldn't try.
> [...]
>
> > (Now I come to think of it I also need to look at rseq etc., which is
> > another magic kernel-mediated branch mechanism.)
Meh.
Cheers
---Dave
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-05-24 14:53 ` Dave Martin
@ 2019-06-06 17:11 ` Catalin Marinas
-1 siblings, 0 replies; 42+ messages in thread
From: Catalin Marinas @ 2019-06-06 17:11 UTC (permalink / raw)
To: Dave Martin
Cc: Mark Rutland, linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones,
Paul Elliott, Arnd Bergmann, Szabolcs Nagy, Will Deacon,
Richard Henderson, linux-kernel, Kristina Martšenko,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > #endif /* _UAPI__ASM_HWCAP_H */
> > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > new file mode 100644
> > > index 0000000..4776b43
> > > --- /dev/null
> > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI__ASM_MMAN_H
> > > +#define _UAPI__ASM_MMAN_H
> > > +
> > > +#include <asm-generic/mman.h>
> > > +
> > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> >
> > From prior discussions, I thought this would be PROT_BTI, without the
> > _GUARDED suffix. Do we really need that?
> >
> > AFAICT, all other PROT_* definitions only have a single underscore, and
> > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > powerpc.
>
> No strong opinon. I was trying to make the name less obscure, but I'm
> equally happy with PROT_BTI if people prefer that.
I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
(and a VM_ARM64_MTE in the high VMA flag bits).
--
Catalin
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-06-06 17:11 ` Catalin Marinas
0 siblings, 0 replies; 42+ messages in thread
From: Catalin Marinas @ 2019-06-06 17:11 UTC (permalink / raw)
To: Dave Martin
Cc: Mark Rutland, linux-arch, Yu-cheng Yu, Arnd Bergmann,
Paul Elliott, H.J. Lu, Szabolcs Nagy, Richard Henderson,
Will Deacon, Andrew Jones, Kristina Martšenko, linux-kernel,
Sudakshina Das, linux-arm-kernel
On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > #endif /* _UAPI__ASM_HWCAP_H */
> > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > new file mode 100644
> > > index 0000000..4776b43
> > > --- /dev/null
> > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > @@ -0,0 +1,9 @@
> > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > +#ifndef _UAPI__ASM_MMAN_H
> > > +#define _UAPI__ASM_MMAN_H
> > > +
> > > +#include <asm-generic/mman.h>
> > > +
> > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> >
> > From prior discussions, I thought this would be PROT_BTI, without the
> > _GUARDED suffix. Do we really need that?
> >
> > AFAICT, all other PROT_* definitions only have a single underscore, and
> > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > powerpc.
>
> No strong opinon. I was trying to make the name less obscure, but I'm
> equally happy with PROT_BTI if people prefer that.
I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
(and a VM_ARM64_MTE in the high VMA flag bits).
--
Catalin
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-06-06 17:11 ` Catalin Marinas
@ 2019-06-06 17:23 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-06-06 17:23 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, linux-arch, Yu-cheng Yu, Arnd Bergmann,
Paul Elliott, H.J. Lu, Szabolcs Nagy, Richard Henderson,
Will Deacon, Andrew Jones, Kristina Martšenko, linux-kernel,
Sudakshina Das, linux-arm-kernel
On Thu, Jun 06, 2019 at 06:11:56PM +0100, Catalin Marinas wrote:
> On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > > new file mode 100644
> > > > index 0000000..4776b43
> > > > --- /dev/null
> > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > @@ -0,0 +1,9 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > +#define _UAPI__ASM_MMAN_H
> > > > +
> > > > +#include <asm-generic/mman.h>
> > > > +
> > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> > >
> > > From prior discussions, I thought this would be PROT_BTI, without the
> > > _GUARDED suffix. Do we really need that?
> > >
> > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > powerpc.
> >
> > No strong opinon. I was trying to make the name less obscure, but I'm
> > equally happy with PROT_BTI if people prefer that.
>
> I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
> (and a VM_ARM64_MTE in the high VMA flag bits).
Ack.
Some things need attention, so I need to respin this series anyway.
skip_faulting_instruction() and kprobes/uprobes may need looking at,
plus I want to simply the ELF parsing (at least to skip some cost for
arm64).
Cheers
---Dave
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-06-06 17:23 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-06-06 17:23 UTC (permalink / raw)
To: Catalin Marinas
Cc: Mark Rutland, linux-arch, Yu-cheng Yu, Arnd Bergmann,
Paul Elliott, H.J. Lu, Szabolcs Nagy, Will Deacon,
Richard Henderson, Andrew Jones, Kristina Martšenko,
linux-kernel, Sudakshina Das, linux-arm-kernel
On Thu, Jun 06, 2019 at 06:11:56PM +0100, Catalin Marinas wrote:
> On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > diff --git a/arch/arm64/include/uapi/asm/mman.h b/arch/arm64/include/uapi/asm/mman.h
> > > > new file mode 100644
> > > > index 0000000..4776b43
> > > > --- /dev/null
> > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > @@ -0,0 +1,9 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > +#define _UAPI__ASM_MMAN_H
> > > > +
> > > > +#include <asm-generic/mman.h>
> > > > +
> > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded page */
> > >
> > > From prior discussions, I thought this would be PROT_BTI, without the
> > > _GUARDED suffix. Do we really need that?
> > >
> > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > powerpc.
> >
> > No strong opinon. I was trying to make the name less obscure, but I'm
> > equally happy with PROT_BTI if people prefer that.
>
> I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
> (and a VM_ARM64_MTE in the high VMA flag bits).
Ack.
Some things need attention, so I need to respin this series anyway.
skip_faulting_instruction() and kprobes/uprobes may need looking at,
plus I want to simply the ELF parsing (at least to skip some cost for
arm64).
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-06-06 17:23 ` Dave Martin
@ 2019-06-06 17:34 ` Yu-cheng Yu
-1 siblings, 0 replies; 42+ messages in thread
From: Yu-cheng Yu @ 2019-06-06 17:34 UTC (permalink / raw)
To: Dave Martin, Catalin Marinas
Cc: Mark Rutland, linux-arch, Arnd Bergmann, Paul Elliott, H.J. Lu,
Szabolcs Nagy, Richard Henderson, Will Deacon, Andrew Jones,
Kristina Martšenko, linux-kernel, Sudakshina Das,
linux-arm-kernel
On Thu, 2019-06-06 at 18:23 +0100, Dave Martin wrote:
> On Thu, Jun 06, 2019 at 06:11:56PM +0100, Catalin Marinas wrote:
> > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > > diff --git a/arch/arm64/include/uapi/asm/mman.h
> > > > > b/arch/arm64/include/uapi/asm/mman.h
> > > > > new file mode 100644
> > > > > index 0000000..4776b43
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > > +#define _UAPI__ASM_MMAN_H
> > > > > +
> > > > > +#include <asm-generic/mman.h>
> > > > > +
> > > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded
> > > > > page */
> > > >
> > > > From prior discussions, I thought this would be PROT_BTI, without the
> > > > _GUARDED suffix. Do we really need that?
> > > >
> > > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > > powerpc.
> > >
> > > No strong opinon. I was trying to make the name less obscure, but I'm
> > > equally happy with PROT_BTI if people prefer that.
> >
> > I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
> > (and a VM_ARM64_MTE in the high VMA flag bits).
>
> Ack.
>
> Some things need attention, so I need to respin this series anyway.
>
> skip_faulting_instruction() and kprobes/uprobes may need looking at,
> plus I want to simply the ELF parsing (at least to skip some cost for
> arm64).
Can we add a case in the 'consistency checks for the interpreter' (right above
where you add arch_parse_property()) for PT_NOTE? That way you can still use
part of the same parser.
Yu-cheng
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-06-06 17:34 ` Yu-cheng Yu
0 siblings, 0 replies; 42+ messages in thread
From: Yu-cheng Yu @ 2019-06-06 17:34 UTC (permalink / raw)
To: Dave Martin, Catalin Marinas
Cc: Mark Rutland, linux-arch, Andrew Jones, Arnd Bergmann,
Paul Elliott, H.J. Lu, Szabolcs Nagy, Will Deacon,
Richard Henderson, linux-kernel, Kristina Martšenko,
Sudakshina Das, linux-arm-kernel
On Thu, 2019-06-06 at 18:23 +0100, Dave Martin wrote:
> On Thu, Jun 06, 2019 at 06:11:56PM +0100, Catalin Marinas wrote:
> > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > > diff --git a/arch/arm64/include/uapi/asm/mman.h
> > > > > b/arch/arm64/include/uapi/asm/mman.h
> > > > > new file mode 100644
> > > > > index 0000000..4776b43
> > > > > --- /dev/null
> > > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > > @@ -0,0 +1,9 @@
> > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > > +#define _UAPI__ASM_MMAN_H
> > > > > +
> > > > > +#include <asm-generic/mman.h>
> > > > > +
> > > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded
> > > > > page */
> > > >
> > > > From prior discussions, I thought this would be PROT_BTI, without the
> > > > _GUARDED suffix. Do we really need that?
> > > >
> > > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > > powerpc.
> > >
> > > No strong opinon. I was trying to make the name less obscure, but I'm
> > > equally happy with PROT_BTI if people prefer that.
> >
> > I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
> > (and a VM_ARM64_MTE in the high VMA flag bits).
>
> Ack.
>
> Some things need attention, so I need to respin this series anyway.
>
> skip_faulting_instruction() and kprobes/uprobes may need looking at,
> plus I want to simply the ELF parsing (at least to skip some cost for
> arm64).
Can we add a case in the 'consistency checks for the interpreter' (right above
where you add arch_parse_property()) for PT_NOTE? That way you can still use
part of the same parser.
Yu-cheng
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
2019-06-06 17:34 ` Yu-cheng Yu
@ 2019-06-06 17:56 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-06-06 17:56 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Catalin Marinas, Mark Rutland, linux-arch, Andrew Jones,
Arnd Bergmann, Paul Elliott, H.J. Lu, Szabolcs Nagy, Will Deacon,
Richard Henderson, linux-kernel, Kristina Martšenko,
Sudakshina Das, linux-arm-kernel
On Thu, Jun 06, 2019 at 10:34:22AM -0700, Yu-cheng Yu wrote:
> On Thu, 2019-06-06 at 18:23 +0100, Dave Martin wrote:
> > On Thu, Jun 06, 2019 at 06:11:56PM +0100, Catalin Marinas wrote:
> > > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> > > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > > > diff --git a/arch/arm64/include/uapi/asm/mman.h
> > > > > > b/arch/arm64/include/uapi/asm/mman.h
> > > > > > new file mode 100644
> > > > > > index 0000000..4776b43
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > > > @@ -0,0 +1,9 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > > > +#define _UAPI__ASM_MMAN_H
> > > > > > +
> > > > > > +#include <asm-generic/mman.h>
> > > > > > +
> > > > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded
> > > > > > page */
> > > > >
> > > > > From prior discussions, I thought this would be PROT_BTI, without the
> > > > > _GUARDED suffix. Do we really need that?
> > > > >
> > > > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > > > powerpc.
> > > >
> > > > No strong opinon. I was trying to make the name less obscure, but I'm
> > > > equally happy with PROT_BTI if people prefer that.
> > >
> > > I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
> > > (and a VM_ARM64_MTE in the high VMA flag bits).
> >
> > Ack.
> >
> > Some things need attention, so I need to respin this series anyway.
> >
> > skip_faulting_instruction() and kprobes/uprobes may need looking at,
> > plus I want to simply the ELF parsing (at least to skip some cost for
> > arm64).
>
> Can we add a case in the 'consistency checks for the interpreter' (right above
> where you add arch_parse_property()) for PT_NOTE? That way you can still use
> part of the same parser.
I think for arm64 that we can skip searching all the notes by checking
for a PT_GNU_PROPERTY entry; once that's found, the actual
NT_GNU_PROPERTY_TYPE_0 parsing should be common. If there's no
PT_GNU_PROPERTY entry, we can immediately give up.
For x86, would it makes sense to use PT_GNU_PROPERTY if it's there,
and fall back to scanning all the notes otherwise? Ideally we
wouldn't need the fallback, but if there are binaries in the wild with
NT_GNU_PROPERTY_TYPE_0 that lack a PT_GNU_PROPERTY entry, we may be
stuck with that.
Thoughts?
Cheers
---Dave
^ permalink raw reply [flat|nested] 42+ messages in thread
* Re: [PATCH 4/8] arm64: Basic Branch Target Identification support
@ 2019-06-06 17:56 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-06-06 17:56 UTC (permalink / raw)
To: Yu-cheng Yu
Cc: Mark Rutland, linux-arch, Andrew Jones, Arnd Bergmann,
Paul Elliott, H.J. Lu, Szabolcs Nagy, Catalin Marinas,
Will Deacon, linux-kernel, Kristina Martšenko,
Richard Henderson, Sudakshina Das, linux-arm-kernel
On Thu, Jun 06, 2019 at 10:34:22AM -0700, Yu-cheng Yu wrote:
> On Thu, 2019-06-06 at 18:23 +0100, Dave Martin wrote:
> > On Thu, Jun 06, 2019 at 06:11:56PM +0100, Catalin Marinas wrote:
> > > On Fri, May 24, 2019 at 03:53:06PM +0100, Dave P Martin wrote:
> > > > On Fri, May 24, 2019 at 02:02:17PM +0100, Mark Rutland wrote:
> > > > > On Fri, May 24, 2019 at 11:25:29AM +0100, Dave Martin wrote:
> > > > > > #endif /* _UAPI__ASM_HWCAP_H */
> > > > > > diff --git a/arch/arm64/include/uapi/asm/mman.h
> > > > > > b/arch/arm64/include/uapi/asm/mman.h
> > > > > > new file mode 100644
> > > > > > index 0000000..4776b43
> > > > > > --- /dev/null
> > > > > > +++ b/arch/arm64/include/uapi/asm/mman.h
> > > > > > @@ -0,0 +1,9 @@
> > > > > > +/* SPDX-License-Identifier: GPL-2.0 WITH Linux-syscall-note */
> > > > > > +#ifndef _UAPI__ASM_MMAN_H
> > > > > > +#define _UAPI__ASM_MMAN_H
> > > > > > +
> > > > > > +#include <asm-generic/mman.h>
> > > > > > +
> > > > > > +#define PROT_BTI_GUARDED 0x10 /* BTI guarded
> > > > > > page */
> > > > >
> > > > > From prior discussions, I thought this would be PROT_BTI, without the
> > > > > _GUARDED suffix. Do we really need that?
> > > > >
> > > > > AFAICT, all other PROT_* definitions only have a single underscore, and
> > > > > the existing arch-specific flags are PROT_ADI on sparc, and PROT_SAO on
> > > > > powerpc.
> > > >
> > > > No strong opinon. I was trying to make the name less obscure, but I'm
> > > > equally happy with PROT_BTI if people prefer that.
> > >
> > > I prefer PROT_BTI as well. We are going to add a PROT_MTE at some point
> > > (and a VM_ARM64_MTE in the high VMA flag bits).
> >
> > Ack.
> >
> > Some things need attention, so I need to respin this series anyway.
> >
> > skip_faulting_instruction() and kprobes/uprobes may need looking at,
> > plus I want to simply the ELF parsing (at least to skip some cost for
> > arm64).
>
> Can we add a case in the 'consistency checks for the interpreter' (right above
> where you add arch_parse_property()) for PT_NOTE? That way you can still use
> part of the same parser.
I think for arm64 that we can skip searching all the notes by checking
for a PT_GNU_PROPERTY entry; once that's found, the actual
NT_GNU_PROPERTY_TYPE_0 parsing should be common. If there's no
PT_GNU_PROPERTY entry, we can immediately give up.
For x86, would it makes sense to use PT_GNU_PROPERTY if it's there,
and fall back to scanning all the notes otherwise? Ideally we
wouldn't need the fallback, but if there are binaries in the wild with
NT_GNU_PROPERTY_TYPE_0 that lack a PT_GNU_PROPERTY entry, we may be
stuck with that.
Thoughts?
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread
* [PATCH 5/8] elf: Parse program properties before destroying the old process
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
Currently we try to read program properties from
NT_GNU_PROPERTY_TYPE_0 ELF notes. However, we do this too late to
either report failures cleanly or influence certain aspects of
process setup such as the default mmap permissions for the new
executable's pages (which will matter for arm64 for example).
So, split parsing of the notes from use: rename
arch_setup_property() to arch_parse_property() to make the intent
clear, and hoist it before flush_old_exec() so that we can still
bail out gracefully if needed.
Also propagate arch_state into the call so that the arch backend
has somewhere to stash information for later use.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
fs/binfmt_elf.c | 26 +++++++++++++-------------
include/linux/elf.h | 15 +++++++++++----
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 18015fc..32c9c13 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -851,6 +851,19 @@ static int load_elf_binary(struct linux_binprm *bprm)
}
}
+ if (interpreter) {
+ retval = arch_parse_property(&loc->interp_elf_ex,
+ interp_elf_phdata,
+ interpreter, true, &arch_state);
+ } else {
+ retval = arch_parse_property(&loc->elf_ex,
+ elf_phdata,
+ bprm->file, false, &arch_state);
+ }
+
+ if (retval < 0)
+ goto out_free_dentry;
+
/*
* Allow arch code to reject the ELF at this point, whilst it's
* still possible to return an error to the code that invoked
@@ -1080,19 +1093,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
}
if (interpreter) {
- retval = arch_setup_property(&loc->interp_elf_ex,
- interp_elf_phdata,
- interpreter, true);
- } else {
- retval = arch_setup_property(&loc->elf_ex,
- elf_phdata,
- bprm->file, false);
- }
-
- if (retval < 0)
- goto out_free_dentry;
-
- if (interpreter) {
unsigned long interp_map_addr = 0;
elf_entry = load_elf_interp(&loc->interp_elf_ex,
diff --git a/include/linux/elf.h b/include/linux/elf.h
index c15febe..cfcf154 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -57,14 +57,21 @@ extern int elf_coredump_extra_notes_size(void);
extern int elf_coredump_extra_notes_write(struct coredump_params *cprm);
#endif
+struct arch_elf_state;
+
#ifdef CONFIG_ARCH_USE_GNU_PROPERTY
-extern int arch_setup_property(void *ehdr, void *phdr, struct file *f,
- bool interp);
+extern int arch_parse_property(void *ehdr, void *phdr, struct file *f,
+ bool interp, struct arch_elf_state *arch_state);
extern int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
u32 pr_type, u32 *feature);
#else
-static inline int arch_setup_property(void *ehdr, void *phdr, struct file *f,
- bool interp) { return 0; }
+static inline int arch_parse_property(void *ehdr, void *phdr, struct file *f,
+ bool interp,
+ struct arch_elf_state *arch_state)
+{
+ return 0;
+}
+
static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
u32 pr_type, u32 *feature) { return 0; }
#endif
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 5/8] elf: Parse program properties before destroying the old process
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
Currently we try to read program properties from
NT_GNU_PROPERTY_TYPE_0 ELF notes. However, we do this too late to
either report failures cleanly or influence certain aspects of
process setup such as the default mmap permissions for the new
executable's pages (which will matter for arm64 for example).
So, split parsing of the notes from use: rename
arch_setup_property() to arch_parse_property() to make the intent
clear, and hoist it before flush_old_exec() so that we can still
bail out gracefully if needed.
Also propagate arch_state into the call so that the arch backend
has somewhere to stash information for later use.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
fs/binfmt_elf.c | 26 +++++++++++++-------------
include/linux/elf.h | 15 +++++++++++----
2 files changed, 24 insertions(+), 17 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 18015fc..32c9c13 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -851,6 +851,19 @@ static int load_elf_binary(struct linux_binprm *bprm)
}
}
+ if (interpreter) {
+ retval = arch_parse_property(&loc->interp_elf_ex,
+ interp_elf_phdata,
+ interpreter, true, &arch_state);
+ } else {
+ retval = arch_parse_property(&loc->elf_ex,
+ elf_phdata,
+ bprm->file, false, &arch_state);
+ }
+
+ if (retval < 0)
+ goto out_free_dentry;
+
/*
* Allow arch code to reject the ELF at this point, whilst it's
* still possible to return an error to the code that invoked
@@ -1080,19 +1093,6 @@ static int load_elf_binary(struct linux_binprm *bprm)
}
if (interpreter) {
- retval = arch_setup_property(&loc->interp_elf_ex,
- interp_elf_phdata,
- interpreter, true);
- } else {
- retval = arch_setup_property(&loc->elf_ex,
- elf_phdata,
- bprm->file, false);
- }
-
- if (retval < 0)
- goto out_free_dentry;
-
- if (interpreter) {
unsigned long interp_map_addr = 0;
elf_entry = load_elf_interp(&loc->interp_elf_ex,
diff --git a/include/linux/elf.h b/include/linux/elf.h
index c15febe..cfcf154 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -57,14 +57,21 @@ extern int elf_coredump_extra_notes_size(void);
extern int elf_coredump_extra_notes_write(struct coredump_params *cprm);
#endif
+struct arch_elf_state;
+
#ifdef CONFIG_ARCH_USE_GNU_PROPERTY
-extern int arch_setup_property(void *ehdr, void *phdr, struct file *f,
- bool interp);
+extern int arch_parse_property(void *ehdr, void *phdr, struct file *f,
+ bool interp, struct arch_elf_state *arch_state);
extern int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
u32 pr_type, u32 *feature);
#else
-static inline int arch_setup_property(void *ehdr, void *phdr, struct file *f,
- bool interp) { return 0; }
+static inline int arch_parse_property(void *ehdr, void *phdr, struct file *f,
+ bool interp,
+ struct arch_elf_state *arch_state)
+{
+ return 0;
+}
+
static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
u32 pr_type, u32 *feature) { return 0; }
#endif
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 6/8] elf: Allow arch to tweak initial mmap prot flags
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
An arch may want to tweak the mmap prot flags for an ELF
executable's initial mappings. For example, arm64 is going to need
to add PROT_BTI_GUARDED for executable pages in an ELF process
whose executable is marked as using Branch Target Identification
(an ARMv8.5-A control flow integrity feature).
So that this can be done in a generic way, add a hook
arch_elf_adjust_prot() to modify the prot flags as desired: arches
can select CONFIG_HAVE_ELF_PROT and implement their own backend
where necessary.
By default, leave the prot flags unchanged.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
fs/Kconfig.binfmt | 3 +++
fs/binfmt_elf.c | 18 ++++++++++++------
include/linux/elf.h | 13 +++++++++++++
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
index 175a1f5..cd3d315 100644
--- a/fs/Kconfig.binfmt
+++ b/fs/Kconfig.binfmt
@@ -35,6 +35,9 @@ config COMPAT_BINFMT_ELF
config ARCH_BINFMT_ELF_STATE
bool
+config ARCH_HAVE_ELF_PROT
+ bool
+
config ARCH_USE_GNU_PROPERTY
bool
depends on 64BIT
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 32c9c13..3d88dcc 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -524,7 +524,8 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp,
#endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */
-static inline int make_prot(u32 p_flags)
+static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state,
+ bool has_interp, bool is_interp)
{
int prot = 0;
@@ -534,7 +535,8 @@ static inline int make_prot(u32 p_flags)
prot |= PROT_WRITE;
if (p_flags & PF_X)
prot |= PROT_EXEC;
- return prot;
+
+ return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp);
}
/* This is much more generalized than the library routine read function,
@@ -544,7 +546,8 @@ static inline int make_prot(u32 p_flags)
static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
struct file *interpreter, unsigned long *interp_map_addr,
- unsigned long no_base, struct elf_phdr *interp_elf_phdata)
+ unsigned long no_base, struct elf_phdr *interp_elf_phdata,
+ struct arch_elf_state *arch_state)
{
struct elf_phdr *eppnt;
unsigned long load_addr = 0;
@@ -576,7 +579,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) {
if (eppnt->p_type == PT_LOAD) {
int elf_type = MAP_PRIVATE | MAP_DENYWRITE;
- int elf_prot = make_prot(eppnt->p_flags);
+ int elf_prot = make_prot(eppnt->p_flags, arch_state,
+ true, true);
unsigned long vaddr = 0;
unsigned long k, map_addr;
@@ -952,7 +956,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
elf_fixed = MAP_FIXED;
}
- elf_prot = make_prot(elf_ppnt->p_flags);
+ elf_prot = make_prot(elf_ppnt->p_flags, &arch_state,
+ !!interpreter, false);
elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;
@@ -1098,7 +1103,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
elf_entry = load_elf_interp(&loc->interp_elf_ex,
interpreter,
&interp_map_addr,
- load_bias, interp_elf_phdata);
+ load_bias, interp_elf_phdata,
+ &arch_state);
if (!IS_ERR((void *)elf_entry)) {
/*
* load_elf_interp() returns relocation
diff --git a/include/linux/elf.h b/include/linux/elf.h
index cfcf154..2057187 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -75,4 +75,17 @@ static inline int arch_parse_property(void *ehdr, void *phdr, struct file *f,
static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
u32 pr_type, u32 *feature) { return 0; }
#endif
+
+#ifdef CONFIG_ARCH_HAVE_ELF_PROT
+int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
+ bool has_interp, bool is_interp);
+#else
+static inline int arch_elf_adjust_prot(int prot,
+ const struct arch_elf_state *state,
+ bool has_interp, bool is_interp)
+{
+ return prot;
+}
+#endif
+
#endif /* _LINUX_ELF_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 6/8] elf: Allow arch to tweak initial mmap prot flags
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
An arch may want to tweak the mmap prot flags for an ELF
executable's initial mappings. For example, arm64 is going to need
to add PROT_BTI_GUARDED for executable pages in an ELF process
whose executable is marked as using Branch Target Identification
(an ARMv8.5-A control flow integrity feature).
So that this can be done in a generic way, add a hook
arch_elf_adjust_prot() to modify the prot flags as desired: arches
can select CONFIG_HAVE_ELF_PROT and implement their own backend
where necessary.
By default, leave the prot flags unchanged.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
fs/Kconfig.binfmt | 3 +++
fs/binfmt_elf.c | 18 ++++++++++++------
include/linux/elf.h | 13 +++++++++++++
3 files changed, 28 insertions(+), 6 deletions(-)
diff --git a/fs/Kconfig.binfmt b/fs/Kconfig.binfmt
index 175a1f5..cd3d315 100644
--- a/fs/Kconfig.binfmt
+++ b/fs/Kconfig.binfmt
@@ -35,6 +35,9 @@ config COMPAT_BINFMT_ELF
config ARCH_BINFMT_ELF_STATE
bool
+config ARCH_HAVE_ELF_PROT
+ bool
+
config ARCH_USE_GNU_PROPERTY
bool
depends on 64BIT
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 32c9c13..3d88dcc 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -524,7 +524,8 @@ static inline int arch_check_elf(struct elfhdr *ehdr, bool has_interp,
#endif /* !CONFIG_ARCH_BINFMT_ELF_STATE */
-static inline int make_prot(u32 p_flags)
+static inline int make_prot(u32 p_flags, struct arch_elf_state *arch_state,
+ bool has_interp, bool is_interp)
{
int prot = 0;
@@ -534,7 +535,8 @@ static inline int make_prot(u32 p_flags)
prot |= PROT_WRITE;
if (p_flags & PF_X)
prot |= PROT_EXEC;
- return prot;
+
+ return arch_elf_adjust_prot(prot, arch_state, has_interp, is_interp);
}
/* This is much more generalized than the library routine read function,
@@ -544,7 +546,8 @@ static inline int make_prot(u32 p_flags)
static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
struct file *interpreter, unsigned long *interp_map_addr,
- unsigned long no_base, struct elf_phdr *interp_elf_phdata)
+ unsigned long no_base, struct elf_phdr *interp_elf_phdata,
+ struct arch_elf_state *arch_state)
{
struct elf_phdr *eppnt;
unsigned long load_addr = 0;
@@ -576,7 +579,8 @@ static unsigned long load_elf_interp(struct elfhdr *interp_elf_ex,
for (i = 0; i < interp_elf_ex->e_phnum; i++, eppnt++) {
if (eppnt->p_type == PT_LOAD) {
int elf_type = MAP_PRIVATE | MAP_DENYWRITE;
- int elf_prot = make_prot(eppnt->p_flags);
+ int elf_prot = make_prot(eppnt->p_flags, arch_state,
+ true, true);
unsigned long vaddr = 0;
unsigned long k, map_addr;
@@ -952,7 +956,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
elf_fixed = MAP_FIXED;
}
- elf_prot = make_prot(elf_ppnt->p_flags);
+ elf_prot = make_prot(elf_ppnt->p_flags, &arch_state,
+ !!interpreter, false);
elf_flags = MAP_PRIVATE | MAP_DENYWRITE | MAP_EXECUTABLE;
@@ -1098,7 +1103,8 @@ static int load_elf_binary(struct linux_binprm *bprm)
elf_entry = load_elf_interp(&loc->interp_elf_ex,
interpreter,
&interp_map_addr,
- load_bias, interp_elf_phdata);
+ load_bias, interp_elf_phdata,
+ &arch_state);
if (!IS_ERR((void *)elf_entry)) {
/*
* load_elf_interp() returns relocation
diff --git a/include/linux/elf.h b/include/linux/elf.h
index cfcf154..2057187 100644
--- a/include/linux/elf.h
+++ b/include/linux/elf.h
@@ -75,4 +75,17 @@ static inline int arch_parse_property(void *ehdr, void *phdr, struct file *f,
static inline int get_gnu_property(void *ehdr_p, void *phdr_p, struct file *f,
u32 pr_type, u32 *feature) { return 0; }
#endif
+
+#ifdef CONFIG_ARCH_HAVE_ELF_PROT
+int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
+ bool has_interp, bool is_interp);
+#else
+static inline int arch_elf_adjust_prot(int prot,
+ const struct arch_elf_state *state,
+ bool has_interp, bool is_interp)
+{
+ return prot;
+}
+#endif
+
#endif /* _LINUX_ELF_H */
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 7/8] arm64: elf: Enable BTI at exec based on ELF program properties
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
For BTI protection to be as comprehensive as possible, it is
desirable to have BTI enabled from process startup. If this is not
done, the process must use mprotect() to enable BTI for each of its
executable mappings, but this is painful to do in the libc startup
code. It's simpler and more sound to have the kernel do it
instead.
To this end, detect BTI support in the executable (or ELF
interpreter, as appropriate), via the
NT_GNU_PROGRAM_PROPERTY_TYPE_0 note, and tweak the initial prot
flags for the process' executable pages to include PROT_BTI_GUARDED
as appropriate.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/Kconfig | 3 +++
arch/arm64/include/asm/elf.h | 28 ++++++++++++++++++++++
arch/arm64/kernel/process.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/elf.h | 8 ++++++-
4 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0f6765e..f8af7f2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -8,6 +8,7 @@ config ARM64
select ACPI_MCFG if (ACPI && PCI)
select ACPI_SPCR_TABLE if ACPI
select ACPI_PPTT if ACPI
+ select ARCH_BINFMT_ELF_STATE
select ARCH_CLOCKSOURCE_DATA
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEVMEM_IS_ALLOWED
@@ -33,6 +34,7 @@ config ARM64
select ARCH_HAS_SYSCALL_WRAPPER
select ARCH_HAS_TEARDOWN_DMA_OPS if IOMMU_SUPPORT
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
+ select ARCH_HAVE_ELF_PROT
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_INLINE_READ_LOCK if !PREEMPT
select ARCH_INLINE_READ_LOCK_BH if !PREEMPT
@@ -62,6 +64,7 @@ config ARM64
select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE if !PREEMPT
select ARCH_KEEP_MEMBLOCK
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_USE_GNU_PROPERTY if BINFMT_ELF
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_SUPPORTS_MEMORY_FAILURE
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 355d120..bba45fa 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -126,6 +126,7 @@
#ifndef __ASSEMBLY__
#include <linux/bug.h>
+#include <linux/fs.h>
#include <asm/processor.h> /* for signal_minsigstksz, used by ARCH_DLINFO */
typedef unsigned long elf_greg_t;
@@ -221,6 +222,33 @@ extern int aarch32_setup_additional_pages(struct linux_binprm *bprm,
#endif /* CONFIG_COMPAT */
+struct arch_elf_state {
+ int flags;
+};
+
+#define ARM64_ELF_BTI (1 << 0)
+
+#define INIT_ARCH_ELF_STATE { \
+ .flags = 0, \
+}
+
+int arch_parse_property(void *ehdr, void *phdr, struct file *f, bool interp,
+ struct arch_elf_state *state);
+
+static inline int arch_elf_pt_proc(void *ehdr, void *phdr,
+ struct file *f, bool is_interp,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
+
+static inline int arch_check_elf(void *ehdr, bool has_interp,
+ void *interp_ehdr,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
+
#endif /* !__ASSEMBLY__ */
#endif
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 3767fb2..104b0d8 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -22,12 +22,14 @@
#include <linux/compat.h>
#include <linux/efi.h>
+#include <linux/elf.h>
#include <linux/export.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
#include <linux/sched/task.h>
#include <linux/sched/task_stack.h>
#include <linux/kernel.h>
+#include <linux/mman.h>
#include <linux/mm.h>
#include <linux/stddef.h>
#include <linux/unistd.h>
@@ -552,3 +554,56 @@ void arch_setup_new_exec(void)
ptrauth_thread_init_user(current);
}
+
+#ifdef CONFIG_BINFMT_ELF
+int arch_parse_property(void *ehdr, void *phdr, struct file *f, bool interp,
+ struct arch_elf_state *state)
+{
+ union any_elf_hdr {
+ struct elf32_hdr hdr32;
+ struct elf64_hdr hdr64;
+ };
+ const union any_elf_hdr *e = ehdr;
+
+ int ret;
+ u32 val;
+
+ /* Currently we have GNU program properties only for native: */
+ if (e->hdr64.e_machine != EM_AARCH64) {
+ WARN_ON(!IS_ENABLED(CONFIG_COMPAT) ||
+ e->hdr64.e_machine != EM_ARM);
+
+ return 0;
+ }
+
+ ret = get_gnu_property(ehdr, phdr, f,
+ GNU_PROPERTY_AARCH64_FEATURE_1_AND,
+ &val);
+ if (ret)
+ return ret;
+
+ if (val & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) {
+ if (!system_supports_bti())
+ return -EINVAL;
+
+ state->flags |= ARM64_ELF_BTI;
+ }
+
+ return 0;
+}
+
+int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
+ bool has_interp, bool is_interp)
+{
+ if (is_interp != has_interp)
+ return prot;
+
+ if (!(state->flags & ARM64_ELF_BTI))
+ return prot;
+
+ if (prot & PROT_EXEC)
+ prot |= PROT_BTI_GUARDED;
+
+ return prot;
+}
+#endif
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 7b7603a..1c8e455 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -444,11 +444,17 @@ typedef struct elf64_note {
Elf64_Word n_type; /* Content type */
} Elf64_Nhdr;
-/* .note.gnu.property types */
+/* .note.gnu.property types for x86: */
#define GNU_PROPERTY_X86_FEATURE_1_AND (0xc0000002)
/* Bits of GNU_PROPERTY_X86_FEATURE_1_AND */
#define GNU_PROPERTY_X86_FEATURE_1_IBT (0x00000001)
#define GNU_PROPERTY_X86_FEATURE_1_SHSTK (0x00000002)
+/* .note.gnu.property types for EM_AARCH64: */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc0000000
+
+/* Bits for GNU_PROPERTY_AARCH64_FEATURE_1_BTI */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0)
+
#endif /* _UAPI_LINUX_ELF_H */
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 7/8] arm64: elf: Enable BTI at exec based on ELF program properties
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
For BTI protection to be as comprehensive as possible, it is
desirable to have BTI enabled from process startup. If this is not
done, the process must use mprotect() to enable BTI for each of its
executable mappings, but this is painful to do in the libc startup
code. It's simpler and more sound to have the kernel do it
instead.
To this end, detect BTI support in the executable (or ELF
interpreter, as appropriate), via the
NT_GNU_PROGRAM_PROPERTY_TYPE_0 note, and tweak the initial prot
flags for the process' executable pages to include PROT_BTI_GUARDED
as appropriate.
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/Kconfig | 3 +++
arch/arm64/include/asm/elf.h | 28 ++++++++++++++++++++++
arch/arm64/kernel/process.c | 55 ++++++++++++++++++++++++++++++++++++++++++++
include/uapi/linux/elf.h | 8 ++++++-
4 files changed, 93 insertions(+), 1 deletion(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 0f6765e..f8af7f2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -8,6 +8,7 @@ config ARM64
select ACPI_MCFG if (ACPI && PCI)
select ACPI_SPCR_TABLE if ACPI
select ACPI_PPTT if ACPI
+ select ARCH_BINFMT_ELF_STATE
select ARCH_CLOCKSOURCE_DATA
select ARCH_HAS_DEBUG_VIRTUAL
select ARCH_HAS_DEVMEM_IS_ALLOWED
@@ -33,6 +34,7 @@ config ARM64
select ARCH_HAS_SYSCALL_WRAPPER
select ARCH_HAS_TEARDOWN_DMA_OPS if IOMMU_SUPPORT
select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
+ select ARCH_HAVE_ELF_PROT
select ARCH_HAVE_NMI_SAFE_CMPXCHG
select ARCH_INLINE_READ_LOCK if !PREEMPT
select ARCH_INLINE_READ_LOCK_BH if !PREEMPT
@@ -62,6 +64,7 @@ config ARM64
select ARCH_INLINE_SPIN_UNLOCK_IRQRESTORE if !PREEMPT
select ARCH_KEEP_MEMBLOCK
select ARCH_USE_CMPXCHG_LOCKREF
+ select ARCH_USE_GNU_PROPERTY if BINFMT_ELF
select ARCH_USE_QUEUED_RWLOCKS
select ARCH_USE_QUEUED_SPINLOCKS
select ARCH_SUPPORTS_MEMORY_FAILURE
diff --git a/arch/arm64/include/asm/elf.h b/arch/arm64/include/asm/elf.h
index 355d120..bba45fa 100644
--- a/arch/arm64/include/asm/elf.h
+++ b/arch/arm64/include/asm/elf.h
@@ -126,6 +126,7 @@
#ifndef __ASSEMBLY__
#include <linux/bug.h>
+#include <linux/fs.h>
#include <asm/processor.h> /* for signal_minsigstksz, used by ARCH_DLINFO */
typedef unsigned long elf_greg_t;
@@ -221,6 +222,33 @@ extern int aarch32_setup_additional_pages(struct linux_binprm *bprm,
#endif /* CONFIG_COMPAT */
+struct arch_elf_state {
+ int flags;
+};
+
+#define ARM64_ELF_BTI (1 << 0)
+
+#define INIT_ARCH_ELF_STATE { \
+ .flags = 0, \
+}
+
+int arch_parse_property(void *ehdr, void *phdr, struct file *f, bool interp,
+ struct arch_elf_state *state);
+
+static inline int arch_elf_pt_proc(void *ehdr, void *phdr,
+ struct file *f, bool is_interp,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
+
+static inline int arch_check_elf(void *ehdr, bool has_interp,
+ void *interp_ehdr,
+ struct arch_elf_state *state)
+{
+ return 0;
+}
+
#endif /* !__ASSEMBLY__ */
#endif
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 3767fb2..104b0d8 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -22,12 +22,14 @@
#include <linux/compat.h>
#include <linux/efi.h>
+#include <linux/elf.h>
#include <linux/export.h>
#include <linux/sched.h>
#include <linux/sched/debug.h>
#include <linux/sched/task.h>
#include <linux/sched/task_stack.h>
#include <linux/kernel.h>
+#include <linux/mman.h>
#include <linux/mm.h>
#include <linux/stddef.h>
#include <linux/unistd.h>
@@ -552,3 +554,56 @@ void arch_setup_new_exec(void)
ptrauth_thread_init_user(current);
}
+
+#ifdef CONFIG_BINFMT_ELF
+int arch_parse_property(void *ehdr, void *phdr, struct file *f, bool interp,
+ struct arch_elf_state *state)
+{
+ union any_elf_hdr {
+ struct elf32_hdr hdr32;
+ struct elf64_hdr hdr64;
+ };
+ const union any_elf_hdr *e = ehdr;
+
+ int ret;
+ u32 val;
+
+ /* Currently we have GNU program properties only for native: */
+ if (e->hdr64.e_machine != EM_AARCH64) {
+ WARN_ON(!IS_ENABLED(CONFIG_COMPAT) ||
+ e->hdr64.e_machine != EM_ARM);
+
+ return 0;
+ }
+
+ ret = get_gnu_property(ehdr, phdr, f,
+ GNU_PROPERTY_AARCH64_FEATURE_1_AND,
+ &val);
+ if (ret)
+ return ret;
+
+ if (val & GNU_PROPERTY_AARCH64_FEATURE_1_BTI) {
+ if (!system_supports_bti())
+ return -EINVAL;
+
+ state->flags |= ARM64_ELF_BTI;
+ }
+
+ return 0;
+}
+
+int arch_elf_adjust_prot(int prot, const struct arch_elf_state *state,
+ bool has_interp, bool is_interp)
+{
+ if (is_interp != has_interp)
+ return prot;
+
+ if (!(state->flags & ARM64_ELF_BTI))
+ return prot;
+
+ if (prot & PROT_EXEC)
+ prot |= PROT_BTI_GUARDED;
+
+ return prot;
+}
+#endif
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index 7b7603a..1c8e455 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -444,11 +444,17 @@ typedef struct elf64_note {
Elf64_Word n_type; /* Content type */
} Elf64_Nhdr;
-/* .note.gnu.property types */
+/* .note.gnu.property types for x86: */
#define GNU_PROPERTY_X86_FEATURE_1_AND (0xc0000002)
/* Bits of GNU_PROPERTY_X86_FEATURE_1_AND */
#define GNU_PROPERTY_X86_FEATURE_1_IBT (0x00000001)
#define GNU_PROPERTY_X86_FEATURE_1_SHSTK (0x00000002)
+/* .note.gnu.property types for EM_AARCH64: */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_AND 0xc0000000
+
+/* Bits for GNU_PROPERTY_AARCH64_FEATURE_1_BTI */
+#define GNU_PROPERTY_AARCH64_FEATURE_1_BTI (1U << 0)
+
#endif /* _UAPI_LINUX_ELF_H */
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* [PATCH 8/8] arm64: BTI: Decode BYTPE bits when printing PSTATE
2019-05-24 10:25 ` Dave Martin
@ 2019-05-24 10:25 ` Dave Martin
-1 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-kernel, linux-arch, Yu-cheng Yu, H.J. Lu, Arnd Bergmann,
Richard Henderson, Andrew Jones, Will Deacon, Catalin Marinas,
Kristina Martšenko, Szabolcs Nagy, Sudakshina Das,
Paul Elliott
The current code to print PSTATE symbolically when generating
backtraces etc., does not include the BYTPE field used by Branch
Target Identification.
So, decode BYTPE and print it too.
In the interests of human-readability, print the classes of BTI
matched. The symbolic motation, BYTPE (PSTATE[11:10]) and
permitted classes of subsequent instruction are:
-- (BTYPE=0b00): any insn
jc (BTYPE=0b01): BTI jc, BTI j, BTI c, PACIxSP
-c (BYTPE=0b10): BTI jc, BTI c, PACIxSP
j- (BTYPE=0b11): BTI jc, BTI j
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/include/asm/ptrace.h | 4 +++-
arch/arm64/kernel/process.c | 9 +++++++--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index b868ef11..f91e51c 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -40,8 +40,10 @@
#define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
/* Additional SPSR bits not exposed in the UABI */
+#define PSR_BTYPE_SHIFT 10
+
#define PSR_IL_BIT (1 << 20)
-#define PSR_BTYPE_CALL (2 << 10)
+#define PSR_BTYPE_CALL (2 << PSR_BTYPE_SHIFT)
/* AArch32-specific ptrace requests */
#define COMPAT_PTRACE_GETREGS 12
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 104b0d8..dde5c40 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -236,7 +236,11 @@ static void print_pstate(struct pt_regs *regs)
pstate & PSR_AA32_I_BIT ? 'I' : 'i',
pstate & PSR_AA32_F_BIT ? 'F' : 'f');
} else {
- printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
+ static const char *const btypes[] = { "--", "jc", "-c", "j-" };
+ const char *btype_str = btypes[(pstate & PSR_BTYPE_MASK) >>
+ PSR_BTYPE_SHIFT];
+
+ printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO BTYPE=%s)\n",
pstate,
pstate & PSR_N_BIT ? 'N' : 'n',
pstate & PSR_Z_BIT ? 'Z' : 'z',
@@ -247,7 +251,8 @@ static void print_pstate(struct pt_regs *regs)
pstate & PSR_I_BIT ? 'I' : 'i',
pstate & PSR_F_BIT ? 'F' : 'f',
pstate & PSR_PAN_BIT ? '+' : '-',
- pstate & PSR_UAO_BIT ? '+' : '-');
+ pstate & PSR_UAO_BIT ? '+' : '-',
+ btype_str);
}
}
--
2.1.4
^ permalink raw reply related [flat|nested] 42+ messages in thread* [PATCH 8/8] arm64: BTI: Decode BYTPE bits when printing PSTATE
@ 2019-05-24 10:25 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-05-24 10:25 UTC (permalink / raw)
To: linux-arm-kernel
Cc: linux-arch, H.J. Lu, Yu-cheng Yu, Andrew Jones, Paul Elliott,
Arnd Bergmann, Szabolcs Nagy, Will Deacon, Richard Henderson,
linux-kernel, Kristina Martšenko, Catalin Marinas,
Sudakshina Das
The current code to print PSTATE symbolically when generating
backtraces etc., does not include the BYTPE field used by Branch
Target Identification.
So, decode BYTPE and print it too.
In the interests of human-readability, print the classes of BTI
matched. The symbolic motation, BYTPE (PSTATE[11:10]) and
permitted classes of subsequent instruction are:
-- (BTYPE=0b00): any insn
jc (BTYPE=0b01): BTI jc, BTI j, BTI c, PACIxSP
-c (BYTPE=0b10): BTI jc, BTI c, PACIxSP
j- (BTYPE=0b11): BTI jc, BTI j
Signed-off-by: Dave Martin <Dave.Martin@arm.com>
---
arch/arm64/include/asm/ptrace.h | 4 +++-
arch/arm64/kernel/process.c | 9 +++++++--
2 files changed, 10 insertions(+), 3 deletions(-)
diff --git a/arch/arm64/include/asm/ptrace.h b/arch/arm64/include/asm/ptrace.h
index b868ef11..f91e51c 100644
--- a/arch/arm64/include/asm/ptrace.h
+++ b/arch/arm64/include/asm/ptrace.h
@@ -40,8 +40,10 @@
#define GIC_PRIO_IRQOFF (GIC_PRIO_IRQON & ~0x80)
/* Additional SPSR bits not exposed in the UABI */
+#define PSR_BTYPE_SHIFT 10
+
#define PSR_IL_BIT (1 << 20)
-#define PSR_BTYPE_CALL (2 << 10)
+#define PSR_BTYPE_CALL (2 << PSR_BTYPE_SHIFT)
/* AArch32-specific ptrace requests */
#define COMPAT_PTRACE_GETREGS 12
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 104b0d8..dde5c40 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -236,7 +236,11 @@ static void print_pstate(struct pt_regs *regs)
pstate & PSR_AA32_I_BIT ? 'I' : 'i',
pstate & PSR_AA32_F_BIT ? 'F' : 'f');
} else {
- printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO)\n",
+ static const char *const btypes[] = { "--", "jc", "-c", "j-" };
+ const char *btype_str = btypes[(pstate & PSR_BTYPE_MASK) >>
+ PSR_BTYPE_SHIFT];
+
+ printk("pstate: %08llx (%c%c%c%c %c%c%c%c %cPAN %cUAO BTYPE=%s)\n",
pstate,
pstate & PSR_N_BIT ? 'N' : 'n',
pstate & PSR_Z_BIT ? 'Z' : 'z',
@@ -247,7 +251,8 @@ static void print_pstate(struct pt_regs *regs)
pstate & PSR_I_BIT ? 'I' : 'i',
pstate & PSR_F_BIT ? 'F' : 'f',
pstate & PSR_PAN_BIT ? '+' : '-',
- pstate & PSR_UAO_BIT ? '+' : '-');
+ pstate & PSR_UAO_BIT ? '+' : '-',
+ btype_str);
}
}
--
2.1.4
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 42+ messages in thread
* Re: [PATCH 0/8] arm64: ARMv8.5-A: Branch Target Identification support
2019-05-24 10:25 ` Dave Martin
` (8 preceding siblings ...)
(?)
@ 2019-06-05 21:12 ` Richard Henderson
2019-06-06 9:34 ` Dave Martin
-1 siblings, 1 reply; 42+ messages in thread
From: Richard Henderson @ 2019-06-05 21:12 UTC (permalink / raw)
To: Dave Martin, linux-arm-kernel
Cc: Paul Elliott, Arnd Bergmann, Szabolcs Nagy, Catalin Marinas,
Will Deacon, Kristina Martšenko, Sudakshina Das
On 5/24/19 5:25 AM, Dave Martin wrote:
> Various things need nailing down before this can be upstreamable:
>
> * Not tested with hugepages yet. (If anyone has any suggestions about
> how best to do that, please shout!)
The qemu code_gen_buffer is generally between 32MB and 2GB, contiguous. It
usually gets AnonHugePage'd. I assume there's no difference in where the
hugepages come from on your side?
It won't be too difficult to modify qemu to make it's own code generation
BTI-safe, and is on the schedule to do eventually anyway.
r~
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread* Re: [PATCH 0/8] arm64: ARMv8.5-A: Branch Target Identification support
2019-06-05 21:12 ` [PATCH 0/8] arm64: ARMv8.5-A: Branch Target Identification support Richard Henderson
@ 2019-06-06 9:34 ` Dave Martin
0 siblings, 0 replies; 42+ messages in thread
From: Dave Martin @ 2019-06-06 9:34 UTC (permalink / raw)
To: Richard Henderson
Cc: Paul Elliott, Arnd Bergmann, Szabolcs Nagy, Catalin Marinas,
Will Deacon, Kristina Martšenko, Sudakshina Das,
linux-arm-kernel
On Wed, Jun 05, 2019 at 04:12:03PM -0500, Richard Henderson wrote:
> On 5/24/19 5:25 AM, Dave Martin wrote:
> > Various things need nailing down before this can be upstreamable:
> >
> > * Not tested with hugepages yet. (If anyone has any suggestions about
> > how best to do that, please shout!)
>
> The qemu code_gen_buffer is generally between 32MB and 2GB, contiguous. It
> usually gets AnonHugePage'd. I assume there's no difference in where the
> hugepages come from on your side?
There are some internal differences between hugetlbfs and anonymous huge
pages, but I'm not very familiar with the details.
> It won't be too difficult to modify qemu to make it's own code generation
> BTI-safe, and is on the schedule to do eventually anyway.
For userspace, it really shouldn't make a difference where the pages
come from. With hugetlbfs, probably mprotect() only works at huge page
granularity; for anonymous huge pages, mprotect() should work at page
granularity, with the kernel shattering huge pages as required.
I guess I can come up with a handcrafted test or two.
Cheers
---Dave
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 42+ messages in thread