LinuxLists.cc - [PATCH 00/17] ARMv8.3 pointer authentication support

[permalink] [raw]

Subject: [PATCH v5 01/17] arm64: add pointer authentication register bits

From: Mark Rutland <[email protected]>

The ARMv8.3 pointer authentication extension adds:

* New fields in ID_AA64ISAR1 to report the presence of pointer
authentication functionality.

* New control bits in SCTLR_ELx to enable this functionality.

* New system registers to hold the keys necessary for this
functionality.

* A new ESR_ELx.EC code used when the new instructions are affected by
configurable traps

This patch adds the relevant definitions to <asm/sysreg.h> and
<asm/esr.h> for these, to be used by subsequent patches.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Suzuki K Poulose <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/esr.h | 3 ++-
arch/arm64/include/asm/sysreg.h | 30 ++++++++++++++++++++++++++++++
2 files changed, 32 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
index ce70c3ffb993..022785162281 100644
--- a/arch/arm64/include/asm/esr.h
+++ b/arch/arm64/include/asm/esr.h
@@ -30,7 +30,8 @@
#define ESR_ELx_EC_CP14_LS (0x06)
#define ESR_ELx_EC_FP_ASIMD (0x07)
#define ESR_ELx_EC_CP10_ID (0x08)
-/* Unallocated EC: 0x09 - 0x0B */
+#define ESR_ELx_EC_PAC (0x09)
+/* Unallocated EC: 0x0A - 0x0B */
#define ESR_ELx_EC_CP14_64 (0x0C)
/* Unallocated EC: 0x0d */
#define ESR_ELx_EC_ILL (0x0E)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index c1470931b897..343b7a3c59e0 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -171,6 +171,19 @@
#define SYS_TTBR1_EL1 sys_reg(3, 0, 2, 0, 1)
#define SYS_TCR_EL1 sys_reg(3, 0, 2, 0, 2)

+#define SYS_APIAKEYLO_EL1 sys_reg(3, 0, 2, 1, 0)
+#define SYS_APIAKEYHI_EL1 sys_reg(3, 0, 2, 1, 1)
+#define SYS_APIBKEYLO_EL1 sys_reg(3, 0, 2, 1, 2)
+#define SYS_APIBKEYHI_EL1 sys_reg(3, 0, 2, 1, 3)
+
+#define SYS_APDAKEYLO_EL1 sys_reg(3, 0, 2, 2, 0)
+#define SYS_APDAKEYHI_EL1 sys_reg(3, 0, 2, 2, 1)
+#define SYS_APDBKEYLO_EL1 sys_reg(3, 0, 2, 2, 2)
+#define SYS_APDBKEYHI_EL1 sys_reg(3, 0, 2, 2, 3)
+
+#define SYS_APGAKEYLO_EL1 sys_reg(3, 0, 2, 3, 0)
+#define SYS_APGAKEYHI_EL1 sys_reg(3, 0, 2, 3, 1)
+
#define SYS_ICC_PMR_EL1 sys_reg(3, 0, 4, 6, 0)

#define SYS_AFSR0_EL1 sys_reg(3, 0, 5, 1, 0)
@@ -419,9 +432,13 @@
#define SYS_ICH_LR15_EL2 __SYS__LR8_EL2(7)

/* Common SCTLR_ELx flags. */
+#define SCTLR_ELx_ENIA (1 << 31)
+#define SCTLR_ELx_ENIB (1 << 30)
+#define SCTLR_ELx_ENDA (1 << 27)
#define SCTLR_ELx_EE (1 << 25)
#define SCTLR_ELx_IESB (1 << 21)
#define SCTLR_ELx_WXN (1 << 19)
+#define SCTLR_ELx_ENDB (1 << 13)
#define SCTLR_ELx_I (1 << 12)
#define SCTLR_ELx_SA (1 << 3)
#define SCTLR_ELx_C (1 << 2)
@@ -515,11 +532,24 @@
#define ID_AA64ISAR0_AES_SHIFT 4

/* id_aa64isar1 */
+#define ID_AA64ISAR1_GPI_SHIFT 28
+#define ID_AA64ISAR1_GPA_SHIFT 24
#define ID_AA64ISAR1_LRCPC_SHIFT 20
#define ID_AA64ISAR1_FCMA_SHIFT 16
#define ID_AA64ISAR1_JSCVT_SHIFT 12
+#define ID_AA64ISAR1_API_SHIFT 8
+#define ID_AA64ISAR1_APA_SHIFT 4
#define ID_AA64ISAR1_DPB_SHIFT 0

+#define ID_AA64ISAR1_APA_NI 0x0
+#define ID_AA64ISAR1_APA_ARCHITECTED 0x1
+#define ID_AA64ISAR1_API_NI 0x0
+#define ID_AA64ISAR1_API_IMP_DEF 0x1
+#define ID_AA64ISAR1_GPA_NI 0x0
+#define ID_AA64ISAR1_GPA_ARCHITECTED 0x1
+#define ID_AA64ISAR1_GPI_NI 0x0
+#define ID_AA64ISAR1_GPI_IMP_DEF 0x1
+
/* id_aa64pfr0 */
#define ID_AA64PFR0_CSV3_SHIFT 60
#define ID_AA64PFR0_CSV2_SHIFT 56
--
2.11.0

2018-10-05 08:50:42

[permalink] [raw]

Subject: [PATCH v5 04/17] arm64: Don't trap host pointer auth use to EL2

From: Mark Rutland <[email protected]>

To allow EL0 (and/or EL1) to use pointer authentication functionality,
we must ensure that pointer authentication instructions and accesses to
pointer authentication keys are not trapped to EL2.

This patch ensures that HCR_EL2 is configured appropriately when the
kernel is booted at EL2. For non-VHE kernels we set HCR_EL2.{API,APK},
ensuring that EL1 can access keys and permit EL0 use of instructions.
For VHE kernels host EL0 (TGE && E2H) is unaffected by these settings,
and it doesn't matter how we configure HCR_EL2.{API,APK}, so we don't
bother setting them.

This does not enable support for KVM guests, since KVM manages HCR_EL2
itself when running VMs.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
Acked-by: Christoffer Dall <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: [email protected]
---
arch/arm64/include/asm/kvm_arm.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index f885f4e96002..1405bb24acac 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -24,6 +24,8 @@

/* Hyp Configuration Register (HCR) bits */
#define HCR_FWB (UL(1) << 46)
+#define HCR_API (UL(1) << 41)
+#define HCR_APK (UL(1) << 40)
#define HCR_TEA (UL(1) << 37)
#define HCR_TERR (UL(1) << 36)
#define HCR_TLOR (UL(1) << 35)
@@ -87,7 +89,7 @@
HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
HCR_FMO | HCR_IMO)
#define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
-#define HCR_HOST_NVHE_FLAGS (HCR_RW)
+#define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK)
#define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)

/* TCR_EL2 Registers bits */
--
2.11.0

2018-10-05 08:50:46

[permalink] [raw]

Subject: [PATCH v5 03/17] arm64/kvm: hide ptrauth from guests

From: Mark Rutland <[email protected]>

In subsequent patches we're going to expose ptrauth to the host kernel
and userspace, but things are a bit trickier for guest kernels. For the
time being, let's hide ptrauth from KVM guests.

Regardless of how well-behaved the guest kernel is, guest userspace
could attempt to use ptrauth instructions, triggering a trap to EL2,
resulting in noise from kvm_handle_unknown_ec(). So let's write up a
handler for the PAC trap, which silently injects an UNDEF into the
guest, as if the feature were really missing.

Signed-off-by: Mark Rutland <[email protected]>
[kristina: fix comment]
Signed-off-by: Kristina Martsenko <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: [email protected]
---
arch/arm64/kvm/handle_exit.c | 18 ++++++++++++++++++
arch/arm64/kvm/sys_regs.c | 8 ++++++++
2 files changed, 26 insertions(+)

diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index e5e741bfffe1..53759b3c165d 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -173,6 +173,23 @@ static int handle_sve(struct kvm_vcpu *vcpu, struct kvm_run *run)
return 1;
}

+/*
+ * Guest usage of a ptrauth instruction (which the guest EL1 did not turn into
+ * a NOP).
+ */
+static int kvm_handle_ptrauth(struct kvm_vcpu *vcpu, struct kvm_run *run)
+{
+ /*
+ * We don't currently support ptrauth in a guest, and we mask the ID
+ * registers to prevent well-behaved guests from trying to make use of
+ * it.
+ *
+ * Inject an UNDEF, as if the feature really isn't present.
+ */
+ kvm_inject_undefined(vcpu);
+ return 1;
+}
+
static exit_handle_fn arm_exit_handlers[] = {
[0 ... ESR_ELx_EC_MAX] = kvm_handle_unknown_ec,
[ESR_ELx_EC_WFx] = kvm_handle_wfx,
@@ -195,6 +212,7 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
[ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
[ESR_ELx_EC_FP_ASIMD] = handle_no_fpsimd,
+ [ESR_ELx_EC_PAC] = kvm_handle_ptrauth,
};

static exit_handle_fn kvm_get_exit_handler(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 22fbbdbece3c..1ca592d38c3c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1040,6 +1040,14 @@ static u64 read_id_reg(struct sys_reg_desc const *r, bool raz)
kvm_debug("SVE unsupported for guests, suppressing\n");

val &= ~(0xfUL << ID_AA64PFR0_SVE_SHIFT);
+ } else if (id == SYS_ID_AA64ISAR1_EL1) {
+ const u64 ptrauth_mask = (0xfUL << ID_AA64ISAR1_APA_SHIFT) |
+ (0xfUL << ID_AA64ISAR1_API_SHIFT) |
+ (0xfUL << ID_AA64ISAR1_GPA_SHIFT) |
+ (0xfUL << ID_AA64ISAR1_GPI_SHIFT);
+ if (val & ptrauth_mask)
+ kvm_debug("ptrauth unsupported for guests, suppressing\n");
+ val &= ~ptrauth_mask;
} else if (id == SYS_ID_AA64MMFR1_EL1) {
if (val & (0xfUL << ID_AA64MMFR1_LOR_SHIFT))
kvm_debug("LORegions unsupported for guests, suppressing\n");
--
2.11.0

2018-10-05 08:50:46

[permalink] [raw]

Subject: [PATCH v5 05/17] arm64/cpufeature: detect pointer authentication

From: Mark Rutland <[email protected]>

So that we can dynamically handle the presence of pointer authentication
functionality, wire up probing code in cpufeature.c.

From ARMv8.3 onwards, ID_AA64ISAR1 is no longer entirely RES0, and now
has four fields describing the presence of pointer authentication
functionality:

* APA - address authentication present, using an architected algorithm
* API - address authentication present, using an IMP DEF algorithm
* GPA - generic authentication present, using an architected algorithm
* GPI - generic authentication present, using an IMP DEF algorithm

For the moment we only care about address authentication, so we only
need to check APA and API. It is assumed that if all CPUs support an IMP
DEF algorithm, the same algorithm is used across all CPUs.

Note that when we implement KVM support, we will also need to ensure
that CPUs have uniform support for GPA and GPI.

Signed-off-by: Mark Rutland <[email protected]>
[kristina: update cpucap numbers]
Signed-off-by: Kristina Martsenko <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Suzuki K Poulose <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/cpucaps.h | 5 ++++-
arch/arm64/kernel/cpufeature.c | 47 ++++++++++++++++++++++++++++++++++++++++
2 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h
index ae1f70450fb2..276d4c95aa3c 100644
--- a/arch/arm64/include/asm/cpucaps.h
+++ b/arch/arm64/include/asm/cpucaps.h
@@ -51,7 +51,10 @@
#define ARM64_SSBD 30
#define ARM64_MISMATCHED_CACHE_TYPE 31
#define ARM64_HAS_STAGE2_FWB 32
+#define ARM64_HAS_ADDRESS_AUTH_ARCH 33
+#define ARM64_HAS_ADDRESS_AUTH_IMP_DEF 34
+#define ARM64_HAS_ADDRESS_AUTH 35

-#define ARM64_NCAPS 33
+#define ARM64_NCAPS 36

#endif /* __ASM_CPUCAPS_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index e238b7932096..0dd171c7d71e 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -142,6 +142,10 @@ static const struct arm64_ftr_bits ftr_id_aa64isar1[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_LRCPC_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_FCMA_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_JSCVT_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+ FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_API_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE_IF_IS_ENABLED(CONFIG_ARM64_PTR_AUTH),
+ FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_APA_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE, ID_AA64ISAR1_DPB_SHIFT, 4, 0),
ARM64_FTR_END,
};
@@ -1035,6 +1039,22 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
WARN_ON(val & (7 << 27 | 7 << 21));
}

+#ifdef CONFIG_ARM64_PTR_AUTH
+static bool has_address_auth(const struct arm64_cpu_capabilities *entry,
+ int __unused)
+{
+ u64 isar1 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
+ bool api, apa;
+
+ apa = cpuid_feature_extract_unsigned_field(isar1,
+ ID_AA64ISAR1_APA_SHIFT) > 0;
+ api = cpuid_feature_extract_unsigned_field(isar1,
+ ID_AA64ISAR1_API_SHIFT) > 0;
+
+ return apa || api;
+}
+#endif /* CONFIG_ARM64_PTR_AUTH */
+
static const struct arm64_cpu_capabilities arm64_features[] = {
{
.desc = "GIC system register CPU interface",
@@ -1222,6 +1242,33 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.cpu_enable = cpu_enable_hw_dbm,
},
#endif
+#ifdef CONFIG_ARM64_PTR_AUTH
+ {
+ .desc = "Address authentication (architected algorithm)",
+ .capability = ARM64_HAS_ADDRESS_AUTH_ARCH,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .sys_reg = SYS_ID_AA64ISAR1_EL1,
+ .sign = FTR_UNSIGNED,
+ .field_pos = ID_AA64ISAR1_APA_SHIFT,
+ .min_field_value = ID_AA64ISAR1_APA_ARCHITECTED,
+ .matches = has_cpuid_feature,
+ },
+ {
+ .desc = "Address authentication (IMP DEF algorithm)",
+ .capability = ARM64_HAS_ADDRESS_AUTH_IMP_DEF,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .sys_reg = SYS_ID_AA64ISAR1_EL1,
+ .sign = FTR_UNSIGNED,
+ .field_pos = ID_AA64ISAR1_API_SHIFT,
+ .min_field_value = ID_AA64ISAR1_API_IMP_DEF,
+ .matches = has_cpuid_feature,
+ },
+ {
+ .capability = ARM64_HAS_ADDRESS_AUTH,
+ .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .matches = has_address_auth,
+ },
+#endif /* CONFIG_ARM64_PTR_AUTH */
{},
};

--
2.11.0

2018-10-05 08:50:52

[permalink] [raw]

Subject: [PATCH v5 06/17] asm-generic: mm_hooks: allow hooks to be overridden individually

From: Mark Rutland <[email protected]>

Currently, an architecture must either implement all of the mm hooks
itself, or use all of those provided by the asm-generic implementation.
When an architecture only needs to override a single hook, it must copy
the stub implementations from the asm-generic version.

To avoid this repetition, allow each hook to be overridden indiviually,
by placing each under an #ifndef block. As architectures providing their
own hooks can't include this file today, this shouldn't adversely affect
any existing hooks.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
Acked-by: Arnd Bergmann <[email protected]>
Cc: [email protected]
---
include/asm-generic/mm_hooks.h | 11 +++++++++++
1 file changed, 11 insertions(+)

diff --git a/include/asm-generic/mm_hooks.h b/include/asm-generic/mm_hooks.h
index 8ac4e68a12f0..2b3ee15d3702 100644
--- a/include/asm-generic/mm_hooks.h
+++ b/include/asm-generic/mm_hooks.h
@@ -7,31 +7,42 @@
#ifndef _ASM_GENERIC_MM_HOOKS_H
#define _ASM_GENERIC_MM_HOOKS_H

+#ifndef arch_dup_mmap
static inline int arch_dup_mmap(struct mm_struct *oldmm,
struct mm_struct *mm)
{
return 0;
}
+#endif

+#ifndef arch_exit_mmap
static inline void arch_exit_mmap(struct mm_struct *mm)
{
}
+#endif

+#ifndef arch_unmap
static inline void arch_unmap(struct mm_struct *mm,
struct vm_area_struct *vma,
unsigned long start, unsigned long end)
{
}
+#endif

+#ifndef arch_bprm_mm_init
static inline void arch_bprm_mm_init(struct mm_struct *mm,
struct vm_area_struct *vma)
{
}
+#endif

+#ifndef arch_vma_access_permitted
static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
bool write, bool execute, bool foreign)
{
/* by default, allow everything */
return true;
}
+#endif
+
#endif /* _ASM_GENERIC_MM_HOOKS_H */
--
2.11.0

2018-10-05 08:50:58

[permalink] [raw]

Subject: [PATCH v5 07/17] arm64: add basic pointer authentication support

From: Mark Rutland <[email protected]>

This patch adds basic support for pointer authentication, allowing
userspace to make use of APIAKey. The kernel maintains an APIAKey value
for each process (shared by all threads within), which is initialised to
a random value at exec() time.

To describe that address authentication instructions are available, the
ID_AA64ISAR0.{APA,API} fields are exposed to userspace. A new hwcap,
APIA, is added to describe that the kernel manages APIAKey.

Instructions using other keys (APIBKey, APDAKey, APDBKey) are disabled,
and will behave as NOPs. These may be made use of in future patches.

No support is added for the generic key (APGAKey), though this cannot be
trapped or made to behave as a NOP. Its presence is not advertised with
a hwcap.

Signed-off-by: Mark Rutland <[email protected]>
[kristina: init keys in arch_bprm_mm_init; add AA64ISAR1.API HWCAP_CAP; use sysreg_clear_set]
Signed-off-by: Kristina Martsenko <[email protected]>
Tested-by: Adam Wallis <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Ramana Radhakrishnan <[email protected]>
Cc: Suzuki K Poulose <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/mmu.h | 5 +++
arch/arm64/include/asm/mmu_context.h | 16 ++++++++-
arch/arm64/include/asm/pointer_auth.h | 63 +++++++++++++++++++++++++++++++++++
arch/arm64/include/uapi/asm/hwcap.h | 1 +
arch/arm64/kernel/cpufeature.c | 10 ++++++
arch/arm64/kernel/cpuinfo.c | 1 +
6 files changed, 95 insertions(+), 1 deletion(-)
create mode 100644 arch/arm64/include/asm/pointer_auth.h

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index dd320df0d026..f6480ea7b0d5 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -25,10 +25,15 @@

#ifndef __ASSEMBLY__

+#include <asm/pointer_auth.h>
+
typedef struct {
atomic64_t id;
void *vdso;
unsigned long flags;
+#ifdef CONFIG_ARM64_PTR_AUTH
+ struct ptrauth_keys ptrauth_keys;
+#endif
} mm_context_t;

/*
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 39ec0b8a689e..983f80925566 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -29,7 +29,6 @@
#include <asm/cacheflush.h>
#include <asm/cpufeature.h>
#include <asm/proc-fns.h>
-#include <asm-generic/mm_hooks.h>
#include <asm/cputype.h>
#include <asm/pgtable.h>
#include <asm/sysreg.h>
@@ -216,6 +215,8 @@ static inline void __switch_mm(struct mm_struct *next)
return;
}

+ mm_ctx_ptrauth_switch(&next->context);
+
check_and_switch_context(next, cpu);
}

@@ -241,6 +242,19 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
void verify_cpu_asid_bits(void);
void post_ttbr_update_workaround(void);

+static inline void arch_bprm_mm_init(struct mm_struct *mm,
+ struct vm_area_struct *vma)
+{
+ mm_ctx_ptrauth_init(&mm->context);
+}
+#define arch_bprm_mm_init arch_bprm_mm_init
+
+/*
+ * We need to override arch_bprm_mm_init before including the generic hooks,
+ * which are otherwise sufficient for us.
+ */
+#include <asm-generic/mm_hooks.h>
+
#endif /* !__ASSEMBLY__ */

#endif /* !__ASM_MMU_CONTEXT_H */
diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
new file mode 100644
index 000000000000..2aefedc31d9e
--- /dev/null
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -0,0 +1,63 @@
+// SPDX-License-Identifier: GPL-2.0
+#ifndef __ASM_POINTER_AUTH_H
+#define __ASM_POINTER_AUTH_H
+
+#include <linux/random.h>
+
+#include <asm/cpufeature.h>
+#include <asm/sysreg.h>
+
+#ifdef CONFIG_ARM64_PTR_AUTH
+/*
+ * Each key is a 128-bit quantity which is split across a pair of 64-bit
+ * registers (Lo and Hi).
+ */
+struct ptrauth_key {
+ unsigned long lo, hi;
+};
+
+/*
+ * We give each process its own instruction A key (APIAKey), which is shared by
+ * all threads. This is inherited upon fork(), and reinitialised upon exec*().
+ * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
+ * instructions behaving as NOPs.
+ */
+struct ptrauth_keys {
+ struct ptrauth_key apia;
+};
+
+static inline void ptrauth_keys_init(struct ptrauth_keys *keys)
+{
+ if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
+ return;
+
+ get_random_bytes(keys, sizeof(*keys));
+}
+
+#define __ptrauth_key_install(k, v) \
+do { \
+ struct ptrauth_key __pki_v = (v); \
+ write_sysreg_s(__pki_v.lo, SYS_ ## k ## KEYLO_EL1); \
+ write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \
+} while (0)
+
+static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
+{
+ if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
+ return;
+
+ __ptrauth_key_install(APIA, keys->apia);
+}
+
+#define mm_ctx_ptrauth_init(ctx) \
+ ptrauth_keys_init(&(ctx)->ptrauth_keys)
+
+#define mm_ctx_ptrauth_switch(ctx) \
+ ptrauth_keys_switch(&(ctx)->ptrauth_keys)
+
+#else /* CONFIG_ARM64_PTR_AUTH */
+#define mm_ctx_ptrauth_init(ctx)
+#define mm_ctx_ptrauth_switch(ctx)
+#endif /* CONFIG_ARM64_PTR_AUTH */
+
+#endif /* __ASM_POINTER_AUTH_H */
diff --git a/arch/arm64/include/uapi/asm/hwcap.h b/arch/arm64/include/uapi/asm/hwcap.h
index 17c65c8f33cb..01f02ac500ae 100644
--- a/arch/arm64/include/uapi/asm/hwcap.h
+++ b/arch/arm64/include/uapi/asm/hwcap.h
@@ -48,5 +48,6 @@
#define HWCAP_USCAT (1 << 25)
#define HWCAP_ILRCPC (1 << 26)
#define HWCAP_FLAGM (1 << 27)
+#define HWCAP_APIA (1 << 28)

#endif /* _UAPI__ASM_HWCAP_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 0dd171c7d71e..3157685aa56a 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1040,6 +1040,11 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
}

#ifdef CONFIG_ARM64_PTR_AUTH
+static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap)
+{
+ sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA);
+}
+
static bool has_address_auth(const struct arm64_cpu_capabilities *entry,
int __unused)
{
@@ -1267,6 +1272,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
.capability = ARM64_HAS_ADDRESS_AUTH,
.type = ARM64_CPUCAP_SYSTEM_FEATURE,
.matches = has_address_auth,
+ .cpu_enable = cpu_enable_address_auth,
},
#endif /* CONFIG_ARM64_PTR_AUTH */
{},
@@ -1314,6 +1320,10 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
#ifdef CONFIG_ARM64_SVE
HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, HWCAP_SVE),
#endif
+#ifdef CONFIG_ARM64_PTR_AUTH
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_APA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA),
+ HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_API_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA),
+#endif
{},
};

diff --git a/arch/arm64/kernel/cpuinfo.c b/arch/arm64/kernel/cpuinfo.c
index e9ab7b3ed317..608411e3aaff 100644
--- a/arch/arm64/kernel/cpuinfo.c
+++ b/arch/arm64/kernel/cpuinfo.c
@@ -81,6 +81,7 @@ static const char *const hwcap_str[] = {
"uscat",
"ilrcpc",
"flagm",
+ "apia",
NULL
};

--
2.11.0

2018-10-05 08:51:09

[permalink] [raw]

Subject: [PATCH v5 09/17] arm64: perf: strip PAC when unwinding userspace

From: Mark Rutland <[email protected]>

When the kernel is unwinding userspace callchains, we can't expect that
the userspace consumer of these callchains has the data necessary to
strip the PAC from the stored LR.

This patch has the kernel strip the PAC from user stackframes when the
in-kernel unwinder is used. This only affects the LR value, and not the
FP.

This only affects the in-kernel unwinder. When userspace performs
unwinding, it is up to userspace to strip PACs as necessary (which can
be determined from DWARF information).

Signed-off-by: Mark Rutland <[email protected]>
[kristina: add pointer_auth.h #include]
Signed-off-by: Kristina Martsenko <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Ramana Radhakrishnan <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/pointer_auth.h | 7 +++++++
arch/arm64/kernel/perf_callchain.c | 6 +++++-
2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index 15486079e9ec..f5a4b075be65 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -57,6 +57,12 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
*/
#define ptrauth_pac_mask() GENMASK(54, VA_BITS)

+/* Only valid for EL0 TTBR0 instruction pointers */
+static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
+{
+ return ptr & ~ptrauth_pac_mask();
+}
+
#define mm_ctx_ptrauth_init(ctx) \
ptrauth_keys_init(&(ctx)->ptrauth_keys)

@@ -64,6 +70,7 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
ptrauth_keys_switch(&(ctx)->ptrauth_keys)

#else /* CONFIG_ARM64_PTR_AUTH */
+#define ptrauth_strip_insn_pac(lr) (lr)
#define mm_ctx_ptrauth_init(ctx)
#define mm_ctx_ptrauth_switch(ctx)
#endif /* CONFIG_ARM64_PTR_AUTH */
diff --git a/arch/arm64/kernel/perf_callchain.c b/arch/arm64/kernel/perf_callchain.c
index bcafd7dcfe8b..94754f07f67a 100644
--- a/arch/arm64/kernel/perf_callchain.c
+++ b/arch/arm64/kernel/perf_callchain.c
@@ -18,6 +18,7 @@
#include <linux/perf_event.h>
#include <linux/uaccess.h>

+#include <asm/pointer_auth.h>
#include <asm/stacktrace.h>

struct frame_tail {
@@ -35,6 +36,7 @@ user_backtrace(struct frame_tail __user *tail,
{
struct frame_tail buftail;
unsigned long err;
+ unsigned long lr;

/* Also check accessibility of one struct frame_tail beyond */
if (!access_ok(VERIFY_READ, tail, sizeof(buftail)))
@@ -47,7 +49,9 @@ user_backtrace(struct frame_tail __user *tail,
if (err)
return NULL;

- perf_callchain_store(entry, buftail.lr);
+ lr = ptrauth_strip_insn_pac(buftail.lr);
+
+ perf_callchain_store(entry, lr);

/*
* Frame pointers should strictly progress back up the stack
--
2.11.0

2018-10-05 08:51:26

[permalink] [raw]

Subject: [PATCH v5 11/17] arm64: docs: document pointer authentication

From: Mark Rutland <[email protected]>

Now that we've added code to support pointer authentication, add some
documentation so that people can figure out if/how to use it.

Signed-off-by: Mark Rutland <[email protected]>
[kristina: update cpu-feature-registers.txt]
Signed-off-by: Kristina Martsenko <[email protected]>
Cc: Andrew Jones <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Ramana Radhakrishnan <[email protected]>
Cc: Will Deacon <[email protected]>
---
Documentation/arm64/booting.txt | 8 +++
Documentation/arm64/cpu-feature-registers.txt | 4 ++
Documentation/arm64/elf_hwcaps.txt | 5 ++
Documentation/arm64/pointer-authentication.txt | 84 ++++++++++++++++++++++++++
4 files changed, 101 insertions(+)
create mode 100644 Documentation/arm64/pointer-authentication.txt

diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt
index 8d0df62c3fe0..8df9f4658d6f 100644
--- a/Documentation/arm64/booting.txt
+++ b/Documentation/arm64/booting.txt
@@ -205,6 +205,14 @@ Before jumping into the kernel, the following conditions must be met:
ICC_SRE_EL2.SRE (bit 0) must be initialised to 0b0.
- The DT or ACPI tables must describe a GICv2 interrupt controller.

+ For CPUs with pointer authentication functionality:
+ - If EL3 is present:
+ SCR_EL3.APK (bit 16) must be initialised to 0b1
+ SCR_EL3.API (bit 17) must be initialised to 0b1
+ - If the kernel is entered at EL1:
+ HCR_EL2.APK (bit 40) must be initialised to 0b1
+ HCR_EL2.API (bit 41) must be initialised to 0b1
+
The requirements described above for CPU mode, caches, MMUs, architected
timers, coherency and system registers apply to all CPUs. All CPUs must
enter the kernel in the same exception level.
diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
index 7964f03846b1..b165677ffab9 100644
--- a/Documentation/arm64/cpu-feature-registers.txt
+++ b/Documentation/arm64/cpu-feature-registers.txt
@@ -190,6 +190,10 @@ infrastructure:
|--------------------------------------------------|
| JSCVT | [15-12] | y |
|--------------------------------------------------|
+ | API | [11-8] | y |
+ |--------------------------------------------------|
+ | APA | [7-4] | y |
+ |--------------------------------------------------|
| DPB | [3-0] | y |
x--------------------------------------------------x

diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
index d6aff2c5e9e2..95509a7b0ffe 100644
--- a/Documentation/arm64/elf_hwcaps.txt
+++ b/Documentation/arm64/elf_hwcaps.txt
@@ -178,3 +178,8 @@ HWCAP_ILRCPC
HWCAP_FLAGM

Functionality implied by ID_AA64ISAR0_EL1.TS == 0b0001.
+
+HWCAP_APIA
+
+ EL0 AddPac and Auth functionality using APIAKey_EL1 is enabled, as
+ described by Documentation/arm64/pointer-authentication.txt.
diff --git a/Documentation/arm64/pointer-authentication.txt b/Documentation/arm64/pointer-authentication.txt
new file mode 100644
index 000000000000..8a9cb5713770
--- /dev/null
+++ b/Documentation/arm64/pointer-authentication.txt
@@ -0,0 +1,84 @@
+Pointer authentication in AArch64 Linux
+=======================================
+
+Author: Mark Rutland <[email protected]>
+Date: 2017-07-19
+
+This document briefly describes the provision of pointer authentication
+functionality in AArch64 Linux.
+
+
+Architecture overview
+---------------------
+
+The ARMv8.3 Pointer Authentication extension adds primitives that can be
+used to mitigate certain classes of attack where an attacker can corrupt
+the contents of some memory (e.g. the stack).
+
+The extension uses a Pointer Authentication Code (PAC) to determine
+whether pointers have been modified unexpectedly. A PAC is derived from
+a pointer, another value (such as the stack pointer), and a secret key
+held in system registers.
+
+The extension adds instructions to insert a valid PAC into a pointer,
+and to verify/remove the PAC from a pointer. The PAC occupies a number
+of high-order bits of the pointer, which varies dependent on the
+configured virtual address size and whether pointer tagging is in use.
+
+A subset of these instructions have been allocated from the HINT
+encoding space. In the absence of the extension (or when disabled),
+these instructions behave as NOPs. Applications and libraries using
+these instructions operate correctly regardless of the presence of the
+extension.
+
+
+Basic support
+-------------
+
+When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
+present, the kernel will assign a random APIAKey value to each process
+at exec*() time. This key is shared by all threads within the process,
+and the key is preserved across fork(). Presence of functionality using
+APIAKey is advertised via HWCAP_APIA.
+
+Recent versions of GCC can compile code with APIAKey-based return
+address protection when passed the -msign-return-address option. This
+uses instructions in the HINT space, and such code can run on systems
+without the pointer authentication extension.
+
+The remaining instruction and data keys (APIBKey, APDAKey, APDBKey) are
+reserved for future use, and instructions using these keys must not be
+used by software until a purpose and scope for their use has been
+decided. To enable future software using these keys to function on
+contemporary kernels, where possible, instructions using these keys are
+made to behave as NOPs.
+
+The generic key (APGAKey) is currently unsupported. Instructions using
+the generic key must not be used by software.
+
+
+Debugging
+---------
+
+When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
+present, the kernel will expose the position of TTBR0 PAC bits in the
+NT_ARM_PAC_MASK regset (struct user_pac_mask), which userspace can
+acqure via PTRACE_GETREGSET.
+
+Separate masks are exposed for data pointers and instruction pointers,
+as the set of PAC bits can vary between the two. Debuggers should not
+expect that HWCAP_APIA implies the presence (or non-presence) of this
+regset -- in future the kernel may support the use of APIBKey, APDAKey,
+and/or APBAKey, even in the absence of APIAKey.
+
+Note that the masks apply to TTBR0 addresses, and are not valid to apply
+to TTBR1 addresses (e.g. kernel pointers).
+
+
+Virtualization
+--------------
+
+Pointer authentication is not currently supported in KVM guests. KVM
+will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
+the feature will result in an UNDEFINED exception being injected into
+the guest.
--
2.11.0

2018-10-05 08:51:32

[permalink] [raw]

Subject: [RFC 12/17] arm64: move ptrauth keys to thread_info

From: Mark Rutland <[email protected]>

To use pointer authentication in the kernel, we'll need to switch keys
in the entry assembly. This patch moves the pointer auth keys into
thread_info to make this possible.

There should be no functional change as a result of this patch.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
---
arch/arm64/include/asm/mmu.h | 5 -----
arch/arm64/include/asm/mmu_context.h | 13 -------------
arch/arm64/include/asm/pointer_auth.h | 13 +++++++------
arch/arm64/include/asm/thread_info.h | 4 ++++
arch/arm64/kernel/process.c | 4 ++++
5 files changed, 15 insertions(+), 24 deletions(-)

diff --git a/arch/arm64/include/asm/mmu.h b/arch/arm64/include/asm/mmu.h
index f6480ea7b0d5..dd320df0d026 100644
--- a/arch/arm64/include/asm/mmu.h
+++ b/arch/arm64/include/asm/mmu.h
@@ -25,15 +25,10 @@

#ifndef __ASSEMBLY__

-#include <asm/pointer_auth.h>
-
typedef struct {
atomic64_t id;
void *vdso;
unsigned long flags;
-#ifdef CONFIG_ARM64_PTR_AUTH
- struct ptrauth_keys ptrauth_keys;
-#endif
} mm_context_t;

/*
diff --git a/arch/arm64/include/asm/mmu_context.h b/arch/arm64/include/asm/mmu_context.h
index 983f80925566..387e810063c7 100644
--- a/arch/arm64/include/asm/mmu_context.h
+++ b/arch/arm64/include/asm/mmu_context.h
@@ -215,8 +215,6 @@ static inline void __switch_mm(struct mm_struct *next)
return;
}

- mm_ctx_ptrauth_switch(&next->context);
-
check_and_switch_context(next, cpu);
}

@@ -242,17 +240,6 @@ switch_mm(struct mm_struct *prev, struct mm_struct *next,
void verify_cpu_asid_bits(void);
void post_ttbr_update_workaround(void);

-static inline void arch_bprm_mm_init(struct mm_struct *mm,
- struct vm_area_struct *vma)
-{
- mm_ctx_ptrauth_init(&mm->context);
-}
-#define arch_bprm_mm_init arch_bprm_mm_init
-
-/*
- * We need to override arch_bprm_mm_init before including the generic hooks,
- * which are otherwise sufficient for us.
- */
#include <asm-generic/mm_hooks.h>

#endif /* !__ASSEMBLY__ */
diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index f5a4b075be65..cedb03bd175b 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -63,16 +63,17 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
return ptr & ~ptrauth_pac_mask();
}

-#define mm_ctx_ptrauth_init(ctx) \
- ptrauth_keys_init(&(ctx)->ptrauth_keys)
+#define ptrauth_task_init_user(tsk) \
+ ptrauth_keys_init(&(tsk)->thread_info.keys_user); \
+ ptrauth_keys_switch(&(tsk)->thread_info.keys_user)

-#define mm_ctx_ptrauth_switch(ctx) \
- ptrauth_keys_switch(&(ctx)->ptrauth_keys)
+#define ptrauth_task_switch(tsk) \
+ ptrauth_keys_switch(&(tsk)->thread_info.keys_user)

#else /* CONFIG_ARM64_PTR_AUTH */
#define ptrauth_strip_insn_pac(lr) (lr)
-#define mm_ctx_ptrauth_init(ctx)
-#define mm_ctx_ptrauth_switch(ctx)
+#define ptrauth_task_init_user(tsk)
+#define ptrauth_task_switch(tsk)
#endif /* CONFIG_ARM64_PTR_AUTH */

#endif /* __ASM_POINTER_AUTH_H */
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index cb2c10a8f0a8..ea9272fb52d4 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -28,6 +28,7 @@
struct task_struct;

#include <asm/memory.h>
+#include <asm/pointer_auth.h>
#include <asm/stack_pointer.h>
#include <asm/types.h>

@@ -43,6 +44,9 @@ struct thread_info {
u64 ttbr0; /* saved TTBR0_EL1 */
#endif
int preempt_count; /* 0 => preemptable, <0 => bug */
+#ifdef CONFIG_ARM64_PTR_AUTH
+ struct ptrauth_keys keys_user;
+#endif
};

#define thread_saved_pc(tsk) \
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 7f1628effe6d..fae52be66c92 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -57,6 +57,7 @@
#include <asm/fpsimd.h>
#include <asm/mmu_context.h>
#include <asm/processor.h>
+#include <asm/pointer_auth.h>
#include <asm/stacktrace.h>

#ifdef CONFIG_STACKPROTECTOR
@@ -425,6 +426,7 @@ __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
contextidr_thread_switch(next);
entry_task_switch(next);
uao_thread_switch(next);
+ ptrauth_task_switch(next);

/*
* Complete any pending TLB or cache maintenance on this CPU in case
@@ -492,6 +494,8 @@ unsigned long arch_randomize_brk(struct mm_struct *mm)
void arch_setup_new_exec(void)
{
current->mm->context.flags = is_compat_task() ? MMCF_AARCH32 : 0;
+
+ ptrauth_task_init_user(current);
}

#ifdef CONFIG_GCC_PLUGIN_STACKLEAK
--
2.11.0

2018-10-05 08:51:39

[permalink] [raw]

Subject: [RFC 14/17] arm64: unwind: strip PAC from kernel addresses

From: Mark Rutland <[email protected]>

When we enable pointer authentication in the kernel, LR values saved to
the stack will have a PAC which we must strip in order to retrieve the
real return address.

Strip PACs when unwinding the stack in order to account for this.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
---
arch/arm64/include/asm/pointer_auth.h | 10 +++++++---
arch/arm64/kernel/ptrace.c | 2 +-
arch/arm64/kernel/stacktrace.c | 3 +++
3 files changed, 11 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index 5e40533f4ea2..e60f225d9fa2 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -55,12 +55,16 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
* The EL0 pointer bits used by a pointer authentication code.
* This is dependent on TBI0 being enabled, or bits 63:56 would also apply.
*/
-#define ptrauth_pac_mask() GENMASK(54, VA_BITS)
+#define ptrauth_pac_mask_ttbr0() GENMASK(54, VA_BITS)
+
+#define ptrauth_pac_mask_ttbr1() (GENMASK(63, 56) | GENMASK(54, VA_BITS))

-/* Only valid for EL0 TTBR0 instruction pointers */
static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
{
- return ptr & ~ptrauth_pac_mask();
+ if (ptr & BIT_ULL(55))
+ return ptr | ptrauth_pac_mask_ttbr1();
+ else
+ return ptr & ~ptrauth_pac_mask_ttbr0();
}

#define ptrauth_task_init_user(tsk) \
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index cb8246f8c603..bf4d6d384e4f 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -970,7 +970,7 @@ static int pac_mask_get(struct task_struct *target,
* depending on TCR_EL1.TBID*, which we may make use of in future, so
* we expose separate masks.
*/
- unsigned long mask = ptrauth_pac_mask();
+ unsigned long mask = ptrauth_pac_mask_ttbr0();
struct user_pac_mask uregs = {
.data_mask = mask,
.insn_mask = mask,
diff --git a/arch/arm64/kernel/stacktrace.c b/arch/arm64/kernel/stacktrace.c
index 4989f7ea1e59..44f6a64a8006 100644
--- a/arch/arm64/kernel/stacktrace.c
+++ b/arch/arm64/kernel/stacktrace.c
@@ -24,6 +24,7 @@
#include <linux/stacktrace.h>

#include <asm/irq.h>
+#include <asm/pointer_auth.h>
#include <asm/stack_pointer.h>
#include <asm/stacktrace.h>

@@ -56,6 +57,8 @@ int notrace unwind_frame(struct task_struct *tsk, struct stackframe *frame)
frame->fp = READ_ONCE_NOCHECK(*(unsigned long *)(fp));
frame->pc = READ_ONCE_NOCHECK(*(unsigned long *)(fp + 8));

+ frame->pc = ptrauth_strip_insn_pac(frame->pc);
+
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
if (tsk->ret_stack &&
(frame->pc == (unsigned long)return_to_handler)) {
--
2.11.0

2018-10-05 08:51:46

[permalink] [raw]

Subject: [RFC 15/17] arm64: enable ptrauth earlier

When the kernel is compiled with pointer auth instructions, the boot CPU
needs to start using pointer auth very early, so change the cpucap to
account for this.

A function that enables pointer auth cannot return, so inline such
functions or compile them without pointer auth.

Do not use the cpu_enable callback, to avoid compiling the whole
callchain down to cpu_enable without pointer auth.

Note the change in behavior: if the boot CPU has pointer auth and a late
CPU does not, we panic. Until now we would have just disabled pointer
auth in this case.

Signed-off-by: Kristina Martsenko <[email protected]>
---
arch/arm64/include/asm/cpufeature.h | 9 +++++++++
arch/arm64/include/asm/pointer_auth.h | 18 ++++++++++++++++++
arch/arm64/kernel/cpufeature.c | 14 ++++----------
arch/arm64/kernel/smp.c | 7 ++++++-
4 files changed, 37 insertions(+), 11 deletions(-)

diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
index 1717ba1db35d..af4ca92a5fa9 100644
--- a/arch/arm64/include/asm/cpufeature.h
+++ b/arch/arm64/include/asm/cpufeature.h
@@ -292,6 +292,15 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
*/
#define ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE ARM64_CPUCAP_SCOPE_BOOT_CPU

+/*
+ * CPU feature used early in the boot based on the boot CPU. It is safe for a
+ * late CPU to have this feature even though the boot CPU hasn't enabled it,
+ * although the feature will not be used by Linux in this case. If the boot CPU
+ * has enabled this feature already, then every late CPU must have it.
+ */
+#define ARM64_CPUCAP_BOOT_CPU_FEATURE \
+ (ARM64_CPUCAP_SCOPE_BOOT_CPU | ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU)
+
struct arm64_cpu_capabilities {
const char *desc;
u16 capability;
diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index e60f225d9fa2..0634f06c3af2 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -11,6 +11,13 @@

#ifdef CONFIG_ARM64_PTR_AUTH
/*
+ * Compile the function without pointer authentication instructions. This
+ * allows pointer authentication to be enabled/disabled within the function
+ * (but leaves the function unprotected by pointer authentication).
+ */
+#define __no_ptrauth __attribute__((target("sign-return-address=none")))
+
+/*
* Each key is a 128-bit quantity which is split across a pair of 64-bit
* registers (Lo and Hi).
*/
@@ -51,6 +58,15 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
__ptrauth_key_install(APIA, keys->apia);
}

+static __always_inline void ptrauth_cpu_enable(void)
+{
+ if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
+ return;
+
+ sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA);
+ isb();
+}
+
/*
* The EL0 pointer bits used by a pointer authentication code.
* This is dependent on TBI0 being enabled, or bits 63:56 would also apply.
@@ -71,8 +87,10 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
ptrauth_keys_init(&(tsk)->thread_info.keys_user)

#else /* CONFIG_ARM64_PTR_AUTH */
+#define __no_ptrauth
#define ptrauth_strip_insn_pac(lr) (lr)
#define ptrauth_task_init_user(tsk)
+#define ptrauth_cpu_enable(tsk)
#endif /* CONFIG_ARM64_PTR_AUTH */

#endif /* __ASM_POINTER_AUTH_H */
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index 3157685aa56a..380ee01145e8 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -1040,15 +1040,10 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
}

#ifdef CONFIG_ARM64_PTR_AUTH
-static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap)
-{
- sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA);
-}
-
static bool has_address_auth(const struct arm64_cpu_capabilities *entry,
int __unused)
{
- u64 isar1 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
+ u64 isar1 = read_sysreg(id_aa64isar1_el1);
bool api, apa;

apa = cpuid_feature_extract_unsigned_field(isar1,
@@ -1251,7 +1246,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
{
.desc = "Address authentication (architected algorithm)",
.capability = ARM64_HAS_ADDRESS_AUTH_ARCH,
- .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
.sys_reg = SYS_ID_AA64ISAR1_EL1,
.sign = FTR_UNSIGNED,
.field_pos = ID_AA64ISAR1_APA_SHIFT,
@@ -1261,7 +1256,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
{
.desc = "Address authentication (IMP DEF algorithm)",
.capability = ARM64_HAS_ADDRESS_AUTH_IMP_DEF,
- .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
.sys_reg = SYS_ID_AA64ISAR1_EL1,
.sign = FTR_UNSIGNED,
.field_pos = ID_AA64ISAR1_API_SHIFT,
@@ -1270,9 +1265,8 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
},
{
.capability = ARM64_HAS_ADDRESS_AUTH,
- .type = ARM64_CPUCAP_SYSTEM_FEATURE,
+ .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
.matches = has_address_auth,
- .cpu_enable = cpu_enable_address_auth,
},
#endif /* CONFIG_ARM64_PTR_AUTH */
{},
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 25fcd22a4bb2..09690024dce8 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -53,6 +53,7 @@
#include <asm/numa.h>
#include <asm/pgtable.h>
#include <asm/pgalloc.h>
+#include <asm/pointer_auth.h>
#include <asm/processor.h>
#include <asm/smp_plat.h>
#include <asm/sections.h>
@@ -211,6 +212,8 @@ asmlinkage notrace void secondary_start_kernel(void)
*/
check_local_cpu_capabilities();

+ ptrauth_cpu_enable();
+
if (cpu_ops[cpu]->cpu_postboot)
cpu_ops[cpu]->cpu_postboot();

@@ -405,7 +408,7 @@ void __init smp_cpus_done(unsigned int max_cpus)
mark_linear_text_alias_ro();
}

-void __init smp_prepare_boot_cpu(void)
+void __init __no_ptrauth smp_prepare_boot_cpu(void)
{
set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
/*
@@ -414,6 +417,8 @@ void __init smp_prepare_boot_cpu(void)
*/
jump_label_init();
cpuinfo_store_boot_cpu();
+
+ ptrauth_cpu_enable();
}

static u64 __init of_get_cpu_mpidr(struct device_node *dn)
--
2.11.0

2018-10-05 08:51:57

[permalink] [raw]

Subject: [RFC 17/17] arm64: compile the kernel with ptrauth -msign-return-address

Compile all functions with two ptrauth instructions: paciasp in the
prologue to sign the return address, and autiasp in the epilogue to
authenticate the return address. This should help protect the kernel
against attacks using return-oriented programming.

CONFIG_ARM64_PTR_AUTH enables pointer auth for both userspace and the
kernel.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
---
arch/arm64/Makefile | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
index 106039d25e2f..dbcd43ea99d8 100644
--- a/arch/arm64/Makefile
+++ b/arch/arm64/Makefile
@@ -56,6 +56,10 @@ KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)

+ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
+KBUILD_CFLAGS += -msign-return-address=all
+endif
+
ifeq ($(CONFIG_CPU_BIG_ENDIAN), y)
KBUILD_CPPFLAGS += -mbig-endian
CHECKFLAGS += -D__AARCH64EB__
--
2.11.0

2018-10-05 08:52:17

[permalink] [raw]

Subject: [PATCH v5 08/17] arm64: expose user PAC bit positions via ptrace

From: Mark Rutland <[email protected]>

When pointer authentication is in use, data/instruction pointers have a
number of PAC bits inserted into them. The number and position of these
bits depends on the configured TCR_ELx.TxSZ and whether tagging is
enabled. ARMv8.3 allows tagging to differ for instruction and data
pointers.

For userspace debuggers to unwind the stack and/or to follow pointer
chains, they need to be able to remove the PAC bits before attempting to
use a pointer.

This patch adds a new structure with masks describing the location of
the PAC bits in userspace instruction and data pointers (i.e. those
addressable via TTBR0), which userspace can query via PTRACE_GETREGSET.
By clearing these bits from pointers (and replacing them with the value
of bit 55), userspace can acquire the PAC-less versions.

This new regset is exposed when the kernel is built with (user) pointer
authentication support, and the feature is enabled. Otherwise, it is
hidden.

Signed-off-by: Mark Rutland <[email protected]>
[kristina: cpus_have_cap -> cpus_have_const_cap]
Signed-off-by: Kristina Martsenko <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Ramana Radhakrishnan <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/include/asm/pointer_auth.h | 8 ++++++++
arch/arm64/include/uapi/asm/ptrace.h | 7 +++++++
arch/arm64/kernel/ptrace.c | 38 +++++++++++++++++++++++++++++++++++
include/uapi/linux/elf.h | 1 +
4 files changed, 54 insertions(+)

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index 2aefedc31d9e..15486079e9ec 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -2,9 +2,11 @@
#ifndef __ASM_POINTER_AUTH_H
#define __ASM_POINTER_AUTH_H

+#include <linux/bitops.h>
#include <linux/random.h>

#include <asm/cpufeature.h>
+#include <asm/memory.h>
#include <asm/sysreg.h>

#ifdef CONFIG_ARM64_PTR_AUTH
@@ -49,6 +51,12 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
__ptrauth_key_install(APIA, keys->apia);
}

+/*
+ * The EL0 pointer bits used by a pointer authentication code.
+ * This is dependent on TBI0 being enabled, or bits 63:56 would also apply.
+ */
+#define ptrauth_pac_mask() GENMASK(54, VA_BITS)
+
#define mm_ctx_ptrauth_init(ctx) \
ptrauth_keys_init(&(ctx)->ptrauth_keys)

diff --git a/arch/arm64/include/uapi/asm/ptrace.h b/arch/arm64/include/uapi/asm/ptrace.h
index 98c4ce55d9c3..4994d718771a 100644
--- a/arch/arm64/include/uapi/asm/ptrace.h
+++ b/arch/arm64/include/uapi/asm/ptrace.h
@@ -228,6 +228,13 @@ struct user_sve_header {
SVE_PT_SVE_OFFSET + SVE_PT_SVE_SIZE(vq, flags) \
: SVE_PT_FPSIMD_OFFSET + SVE_PT_FPSIMD_SIZE(vq, flags))

+/* pointer authentication masks (NT_ARM_PAC_MASK) */
+
+struct user_pac_mask {
+ __u64 data_mask;
+ __u64 insn_mask;
+};
+
#endif /* __ASSEMBLY__ */

#endif /* _UAPI__ASM_PTRACE_H */
diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c
index 6219486fa25f..cb8246f8c603 100644
--- a/arch/arm64/kernel/ptrace.c
+++ b/arch/arm64/kernel/ptrace.c
@@ -46,6 +46,7 @@
#include <asm/debug-monitors.h>
#include <asm/fpsimd.h>
#include <asm/pgtable.h>
+#include <asm/pointer_auth.h>
#include <asm/stacktrace.h>
#include <asm/syscall.h>
#include <asm/traps.h>
@@ -958,6 +959,30 @@ static int sve_set(struct task_struct *target,

#endif /* CONFIG_ARM64_SVE */

+#ifdef CONFIG_ARM64_PTR_AUTH
+static int pac_mask_get(struct task_struct *target,
+ const struct user_regset *regset,
+ unsigned int pos, unsigned int count,
+ void *kbuf, void __user *ubuf)
+{
+ /*
+ * The PAC bits can differ across data and instruction pointers
+ * depending on TCR_EL1.TBID*, which we may make use of in future, so
+ * we expose separate masks.
+ */
+ unsigned long mask = ptrauth_pac_mask();
+ struct user_pac_mask uregs = {
+ .data_mask = mask,
+ .insn_mask = mask,
+ };
+
+ if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
+ return -EINVAL;
+
+ return user_regset_copyout(&pos, &count, &kbuf, &ubuf, &uregs, 0, -1);
+}
+#endif /* CONFIG_ARM64_PTR_AUTH */
+
enum aarch64_regset {
REGSET_GPR,
REGSET_FPR,
@@ -970,6 +995,9 @@ enum aarch64_regset {
#ifdef CONFIG_ARM64_SVE
REGSET_SVE,
#endif
+#ifdef CONFIG_ARM64_PTR_AUTH
+ REGSET_PAC_MASK,
+#endif
};

static const struct user_regset aarch64_regsets[] = {
@@ -1039,6 +1067,16 @@ static const struct user_regset aarch64_regsets[] = {
.get_size = sve_get_size,
},
#endif
+#ifdef CONFIG_ARM64_PTR_AUTH
+ [REGSET_PAC_MASK] = {
+ .core_note_type = NT_ARM_PAC_MASK,
+ .n = sizeof(struct user_pac_mask) / sizeof(u64),
+ .size = sizeof(u64),
+ .align = sizeof(u64),
+ .get = pac_mask_get,
+ /* this cannot be set dynamically */
+ },
+#endif
};

static const struct user_regset_view user_aarch64_view = {
diff --git a/include/uapi/linux/elf.h b/include/uapi/linux/elf.h
index c5358e0ae7c5..3f23273d690c 100644
--- a/include/uapi/linux/elf.h
+++ b/include/uapi/linux/elf.h
@@ -420,6 +420,7 @@ typedef struct elf64_shdr {
#define NT_ARM_HW_WATCH 0x403 /* ARM hardware watchpoint registers */
#define NT_ARM_SYSTEM_CALL 0x404 /* ARM system call number */
#define NT_ARM_SVE 0x405 /* ARM Scalable Vector Extension registers */
+#define NT_ARM_PAC_MASK 0x406 /* ARM pointer authentication code masks */
#define NT_ARC_V2 0x600 /* ARCv2 accumulator/extra registers */
#define NT_VMCOREDD 0x700 /* Vmcore Device Dump Note */
#define NT_MIPS_DSP 0x800 /* MIPS DSP ASE registers */
--
2.11.0

2018-10-05 08:52:29

[permalink] [raw]

Subject: [PATCH v5 10/17] arm64: enable pointer authentication

From: Mark Rutland <[email protected]>

Now that all the necessary bits are in place for userspace, add the
necessary Kconfig logic to allow this to be enabled.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Will Deacon <[email protected]>
---
arch/arm64/Kconfig | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)

diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 1b1a0e95c751..8a6d44160fa8 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -1134,6 +1134,29 @@ config ARM64_RAS_EXTN

endmenu

+menu "ARMv8.3 architectural features"
+
+config ARM64_PTR_AUTH
+ bool "Enable support for pointer authentication"
+ default y
+ help
+ Pointer authentication (part of the ARMv8.3 Extensions) provides
+ instructions for signing and authenticating pointers against secret
+ keys, which can be used to mitigate Return Oriented Programming (ROP)
+ and other attacks.
+
+ This option enables these instructions at EL0 (i.e. for userspace).
+
+ Choosing this option will cause the kernel to initialise secret keys
+ for each process at exec() time, with these keys being
+ context-switched along with the process.
+
+ The feature is detected at runtime. If the feature is not present in
+ hardware it will not be advertised to userspace nor will it be
+ enabled.
+
+endmenu
+
config ARM64_SVE
bool "ARM Scalable Vector Extension support"
default y
--
2.11.0

2018-10-05 08:52:35

[permalink] [raw]

Subject: [PATCH v5 02/17] arm64/kvm: consistently handle host HCR_EL2 flags

From: Mark Rutland <[email protected]>

In KVM we define the configuration of HCR_EL2 for a VHE HOST in
HCR_HOST_VHE_FLAGS, but we don't have a similar definition for the
non-VHE host flags, and open-code HCR_RW. Further, in head.S we
open-code the flags for VHE and non-VHE configurations.

In future, we're going to want to configure more flags for the host, so
lets add a HCR_HOST_NVHE_FLAGS defintion, and consistently use both
HCR_HOST_VHE_FLAGS and HCR_HOST_NVHE_FLAGS in the kvm code and head.S.

We now use mov_q to generate the HCR_EL2 value, as we use when
configuring other registers in head.S.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
Reviewed-by: Christoffer Dall <[email protected]>
Cc: Catalin Marinas <[email protected]>
Cc: Marc Zyngier <[email protected]>
Cc: Will Deacon <[email protected]>
Cc: [email protected]
---
arch/arm64/include/asm/kvm_arm.h | 1 +
arch/arm64/kernel/head.S | 5 ++---
arch/arm64/kvm/hyp/switch.c | 2 +-
3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index aa45df752a16..f885f4e96002 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -87,6 +87,7 @@
HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
HCR_FMO | HCR_IMO)
#define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
+#define HCR_HOST_NVHE_FLAGS (HCR_RW)
#define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)

/* TCR_EL2 Registers bits */
diff --git a/arch/arm64/kernel/head.S b/arch/arm64/kernel/head.S
index b0853069702f..651a06b1980f 100644
--- a/arch/arm64/kernel/head.S
+++ b/arch/arm64/kernel/head.S
@@ -494,10 +494,9 @@ ENTRY(el2_setup)
#endif

/* Hyp configuration. */
- mov x0, #HCR_RW // 64-bit EL1
+ mov_q x0, HCR_HOST_NVHE_FLAGS
cbz x2, set_hcr
- orr x0, x0, #HCR_TGE // Enable Host Extensions
- orr x0, x0, #HCR_E2H
+ mov_q x0, HCR_HOST_VHE_FLAGS
set_hcr:
msr hcr_el2, x0
isb
diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c
index ca46153d7915..a1c32c1f2267 100644
--- a/arch/arm64/kvm/hyp/switch.c
+++ b/arch/arm64/kvm/hyp/switch.c
@@ -157,7 +157,7 @@ static void __hyp_text __deactivate_traps_nvhe(void)
mdcr_el2 |= MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT;

write_sysreg(mdcr_el2, mdcr_el2);
- write_sysreg(HCR_RW, hcr_el2);
+ write_sysreg(HCR_HOST_NVHE_FLAGS, hcr_el2);
write_sysreg(CPTR_EL2_DEFAULT, cptr_el2);
}

--
2.11.0

2018-10-05 08:52:50

[permalink] [raw]

Subject: [RFC 13/17] arm64: install user ptrauth keys at kernel exit time

From: Mark Rutland <[email protected]>

This will mean we do more work per EL0 exception return, but is a
stepping-stone to enable keys within the kernel.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
---
arch/arm64/include/asm/pointer_auth.h | 7 +------
arch/arm64/include/asm/ptrauth-asm.h | 26 ++++++++++++++++++++++++++
arch/arm64/kernel/asm-offsets.c | 7 +++++++
arch/arm64/kernel/entry.S | 9 +++++++--
arch/arm64/kernel/process.c | 1 -
5 files changed, 41 insertions(+), 9 deletions(-)
create mode 100644 arch/arm64/include/asm/ptrauth-asm.h

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index cedb03bd175b..5e40533f4ea2 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -64,16 +64,11 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
}

#define ptrauth_task_init_user(tsk) \
- ptrauth_keys_init(&(tsk)->thread_info.keys_user); \
- ptrauth_keys_switch(&(tsk)->thread_info.keys_user)
-
-#define ptrauth_task_switch(tsk) \
- ptrauth_keys_switch(&(tsk)->thread_info.keys_user)
+ ptrauth_keys_init(&(tsk)->thread_info.keys_user)

#else /* CONFIG_ARM64_PTR_AUTH */
#define ptrauth_strip_insn_pac(lr) (lr)
#define ptrauth_task_init_user(tsk)
-#define ptrauth_task_switch(tsk)
#endif /* CONFIG_ARM64_PTR_AUTH */

#endif /* __ASM_POINTER_AUTH_H */
diff --git a/arch/arm64/include/asm/ptrauth-asm.h b/arch/arm64/include/asm/ptrauth-asm.h
new file mode 100644
index 000000000000..f50bdfc4046c
--- /dev/null
+++ b/arch/arm64/include/asm/ptrauth-asm.h
@@ -0,0 +1,26 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef __ASM_PTRAUTH_ASM_H
+#define __ASM_PTRAUTH_ASM_H
+
+#include <asm/asm-offsets.h>
+#include <asm/sysreg.h>
+
+#ifdef CONFIG_ARM64_PTR_AUTH
+
+ .macro ptrauth_keys_install_user tsk, tmp
+alternative_if ARM64_HAS_ADDRESS_AUTH
+ ldr \tmp, [\tsk, #(TSK_TI_KEYS_USER + PTRAUTH_KEY_APIALO)]
+ msr_s SYS_APIAKEYLO_EL1, \tmp
+ ldr \tmp, [\tsk, #(TSK_TI_KEYS_USER + PTRAUTH_KEY_APIAHI)]
+ msr_s SYS_APIAKEYHI_EL1, \tmp
+alternative_else_nop_endif
+ .endm
+
+#else /* CONFIG_ARM64_PTR_AUTH */
+
+ .macro ptrauth_keys_install_user tsk, tmp
+ .endm
+
+#endif /* CONFIG_ARM64_PTR_AUTH */
+
+#endif /* __ASM_PTRAUTH_ASM_H */
diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index 323aeb5f2fe6..b6be0dd037fd 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -45,6 +45,9 @@ int main(void)
#ifdef CONFIG_ARM64_SW_TTBR0_PAN
DEFINE(TSK_TI_TTBR0, offsetof(struct task_struct, thread_info.ttbr0));
#endif
+#ifdef CONFIG_ARM64_PTR_AUTH
+ DEFINE(TSK_TI_KEYS_USER, offsetof(struct task_struct, thread_info.keys_user));
+#endif
DEFINE(TSK_STACK, offsetof(struct task_struct, stack));
BLANK();
DEFINE(THREAD_CPU_CONTEXT, offsetof(struct task_struct, thread.cpu_context));
@@ -169,5 +172,9 @@ int main(void)
DEFINE(SDEI_EVENT_INTREGS, offsetof(struct sdei_registered_event, interrupted_regs));
DEFINE(SDEI_EVENT_PRIORITY, offsetof(struct sdei_registered_event, priority));
#endif
+#ifdef CONFIG_ARM64_PTR_AUTH
+ DEFINE(PTRAUTH_KEY_APIALO, offsetof(struct ptrauth_keys, apia.lo));
+ DEFINE(PTRAUTH_KEY_APIAHI, offsetof(struct ptrauth_keys, apia.hi));
+#endif
return 0;
}
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 09dbea221a27..1e925f6d2978 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -23,8 +23,9 @@
#include <linux/linkage.h>

#include <asm/alternative.h>
-#include <asm/assembler.h>
#include <asm/asm-offsets.h>
+#include <asm/asm-uaccess.h>
+#include <asm/assembler.h>
#include <asm/cpufeature.h>
#include <asm/errno.h>
#include <asm/esr.h>
@@ -33,8 +34,8 @@
#include <asm/mmu.h>
#include <asm/processor.h>
#include <asm/ptrace.h>
+#include <asm/ptrauth-asm.h>
#include <asm/thread_info.h>
-#include <asm/asm-uaccess.h>
#include <asm/unistd.h>

/*
@@ -325,6 +326,10 @@ alternative_else_nop_endif
apply_ssbd 0, x0, x1
.endif

+ .if \el == 0
+ ptrauth_keys_install_user tsk, x0
+ .endif
+
msr elr_el1, x21 // set up the return data
msr spsr_el1, x22
ldp x0, x1, [sp, #16 * 0]
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index fae52be66c92..857ae05cd04c 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -426,7 +426,6 @@ __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
contextidr_thread_switch(next);
entry_task_switch(next);
uao_thread_switch(next);
- ptrauth_task_switch(next);

/*
* Complete any pending TLB or cache maintenance on this CPU in case
--
2.11.0

2018-10-05 08:53:09

[permalink] [raw]

Subject: [RFC 16/17] arm64: initialize and switch ptrauth kernel keys

Set up keys to use pointer auth in the kernel. Each task has its own
APIAKey, which is initialized during fork. The key is changed during
context switch and on kernel entry from EL0.

A function that changes the key cannot return, so inline such functions.

Signed-off-by: Mark Rutland <[email protected]>
Signed-off-by: Kristina Martsenko <[email protected]>
---
arch/arm64/include/asm/pointer_auth.h | 9 ++++++++-
arch/arm64/include/asm/ptrauth-asm.h | 13 +++++++++++++
arch/arm64/include/asm/thread_info.h | 1 +
arch/arm64/kernel/asm-offsets.c | 1 +
arch/arm64/kernel/entry.S | 4 ++++
arch/arm64/kernel/process.c | 3 +++
arch/arm64/kernel/smp.c | 3 +++
7 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
index 0634f06c3af2..e94ca7df8dab 100644
--- a/arch/arm64/include/asm/pointer_auth.h
+++ b/arch/arm64/include/asm/pointer_auth.h
@@ -50,12 +50,13 @@ do { \
write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \
} while (0)

-static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
+static __always_inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
{
if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
return;

__ptrauth_key_install(APIA, keys->apia);
+ isb();
}

static __always_inline void ptrauth_cpu_enable(void)
@@ -85,11 +86,17 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)

#define ptrauth_task_init_user(tsk) \
ptrauth_keys_init(&(tsk)->thread_info.keys_user)
+#define ptrauth_task_init_kernel(tsk) \
+ ptrauth_keys_init(&(tsk)->thread_info.keys_kernel)
+#define ptrauth_task_switch(tsk) \
+ ptrauth_keys_switch(&(tsk)->thread_info.keys_kernel)

#else /* CONFIG_ARM64_PTR_AUTH */
#define __no_ptrauth
#define ptrauth_strip_insn_pac(lr) (lr)
#define ptrauth_task_init_user(tsk)
+#define ptrauth_task_init_kernel(tsk)
+#define ptrauth_task_switch(tsk)
#define ptrauth_cpu_enable(tsk)
#endif /* CONFIG_ARM64_PTR_AUTH */

diff --git a/arch/arm64/include/asm/ptrauth-asm.h b/arch/arm64/include/asm/ptrauth-asm.h
index f50bdfc4046c..3ef1cc8903d5 100644
--- a/arch/arm64/include/asm/ptrauth-asm.h
+++ b/arch/arm64/include/asm/ptrauth-asm.h
@@ -16,11 +16,24 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
alternative_else_nop_endif
.endm

+ .macro ptrauth_keys_install_kernel tsk, tmp
+alternative_if ARM64_HAS_ADDRESS_AUTH
+ ldr \tmp, [\tsk, #(TSK_TI_KEYS_KERNEL + PTRAUTH_KEY_APIALO)]
+ msr_s SYS_APIAKEYLO_EL1, \tmp
+ ldr \tmp, [\tsk, #(TSK_TI_KEYS_KERNEL + PTRAUTH_KEY_APIAHI)]
+ msr_s SYS_APIAKEYHI_EL1, \tmp
+ isb
+alternative_else_nop_endif
+ .endm
+
#else /* CONFIG_ARM64_PTR_AUTH */

.macro ptrauth_keys_install_user tsk, tmp
.endm

+ .macro ptrauth_keys_install_kernel tsk, tmp
+ .endm
+
#endif /* CONFIG_ARM64_PTR_AUTH */

#endif /* __ASM_PTRAUTH_ASM_H */
diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
index ea9272fb52d4..e3ec5345addc 100644
--- a/arch/arm64/include/asm/thread_info.h
+++ b/arch/arm64/include/asm/thread_info.h
@@ -46,6 +46,7 @@ struct thread_info {
int preempt_count; /* 0 => preemptable, <0 => bug */
#ifdef CONFIG_ARM64_PTR_AUTH
struct ptrauth_keys keys_user;
+ struct ptrauth_keys keys_kernel;
#endif
};

diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
index b6be0dd037fd..6c61c9722b47 100644
--- a/arch/arm64/kernel/asm-offsets.c
+++ b/arch/arm64/kernel/asm-offsets.c
@@ -47,6 +47,7 @@ int main(void)
#endif
#ifdef CONFIG_ARM64_PTR_AUTH
DEFINE(TSK_TI_KEYS_USER, offsetof(struct task_struct, thread_info.keys_user));
+ DEFINE(TSK_TI_KEYS_KERNEL, offsetof(struct task_struct, thread_info.keys_kernel));
#endif
DEFINE(TSK_STACK, offsetof(struct task_struct, stack));
BLANK();
diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
index 1e925f6d2978..a4503da445f7 100644
--- a/arch/arm64/kernel/entry.S
+++ b/arch/arm64/kernel/entry.S
@@ -250,6 +250,10 @@ alternative_else_nop_endif
msr sp_el0, tsk
.endif

+ .if \el == 0
+ ptrauth_keys_install_kernel tsk, x20
+ .endif
+
/*
* Registers that may be useful after this macro is invoked:
*
diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
index 857ae05cd04c..a866996610de 100644
--- a/arch/arm64/kernel/process.c
+++ b/arch/arm64/kernel/process.c
@@ -330,6 +330,8 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
*/
fpsimd_flush_task_state(p);

+ ptrauth_task_init_kernel(p);
+
if (likely(!(p->flags & PF_KTHREAD))) {
*childregs = *current_pt_regs();
childregs->regs[0] = 0;
@@ -426,6 +428,7 @@ __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
contextidr_thread_switch(next);
entry_task_switch(next);
uao_thread_switch(next);
+ ptrauth_task_switch(next);

/*
* Complete any pending TLB or cache maintenance on this CPU in case
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 09690024dce8..d952dd62c780 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -212,6 +212,7 @@ asmlinkage notrace void secondary_start_kernel(void)
*/
check_local_cpu_capabilities();

+ ptrauth_task_switch(current);
ptrauth_cpu_enable();

if (cpu_ops[cpu]->cpu_postboot)
@@ -418,6 +419,8 @@ void __init __no_ptrauth smp_prepare_boot_cpu(void)
jump_label_init();
cpuinfo_store_boot_cpu();

+ ptrauth_task_init_kernel(current);
+ ptrauth_task_switch(current);
ptrauth_cpu_enable();
}

--
2.11.0

2018-10-05 09:01:39

[permalink] [raw]

Subject: Re: [RFC 17/17] arm64: compile the kernel with ptrauth -msign-return-address

On 05/10/2018 09:47, Kristina Martsenko wrote:
> Compile all functions with two ptrauth instructions: paciasp in the
> prologue to sign the return address, and autiasp in the epilogue to
> authenticate the return address. This should help protect the kernel
> against attacks using return-oriented programming.
>
> CONFIG_ARM64_PTR_AUTH enables pointer auth for both userspace and the
> kernel.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Signed-off-by: Kristina Martsenko <[email protected]>
> ---
> arch/arm64/Makefile | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 106039d25e2f..dbcd43ea99d8 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -56,6 +56,10 @@ KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
> KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
> KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
>
> +ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> +KBUILD_CFLAGS += -msign-return-address=all

Glad to see this being done and being proposed for mainline.

I can see why you would prefer this though have you guys experimented at
all with -msign-return-address=non-leaf as well ?

Orthogonally and just fair warning - the command lines for this are also
being revised to provide ROP and JOP protection using BTI from v8.5-a
during the GCC-9 timeframe but I suspect that's a different option.

regards
Ramana

Reviewed-by: Ramana Radhakrishnan <[email protected]>

2018-10-05 09:05:19

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On 05/10/2018 09:47, Kristina Martsenko wrote:
> From: Mark Rutland <[email protected]>
>
> Now that we've added code to support pointer authentication, add some
> documentation so that people can figure out if/how to use it.
>
> Signed-off-by: Mark Rutland <[email protected]>
> [kristina: update cpu-feature-registers.txt]
> Signed-off-by: Kristina Martsenko <[email protected]>
> Cc: Andrew Jones <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Ramana Radhakrishnan <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> Documentation/arm64/booting.txt | 8 +++
> Documentation/arm64/cpu-feature-registers.txt | 4 ++
> Documentation/arm64/elf_hwcaps.txt | 5 ++
> Documentation/arm64/pointer-authentication.txt | 84 ++++++++++++++++++++++++++
> 4 files changed, 101 insertions(+)
> create mode 100644 Documentation/arm64/pointer-authentication.txt
>
> diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt
> index 8d0df62c3fe0..8df9f4658d6f 100644
> --- a/Documentation/arm64/booting.txt
> +++ b/Documentation/arm64/booting.txt
> @@ -205,6 +205,14 @@ Before jumping into the kernel, the following conditions must be met:
> ICC_SRE_EL2.SRE (bit 0) must be initialised to 0b0.
> - The DT or ACPI tables must describe a GICv2 interrupt controller.
>
> + For CPUs with pointer authentication functionality:
> + - If EL3 is present:
> + SCR_EL3.APK (bit 16) must be initialised to 0b1
> + SCR_EL3.API (bit 17) must be initialised to 0b1
> + - If the kernel is entered at EL1:
> + HCR_EL2.APK (bit 40) must be initialised to 0b1
> + HCR_EL2.API (bit 41) must be initialised to 0b1
> +
> The requirements described above for CPU mode, caches, MMUs, architected
> timers, coherency and system registers apply to all CPUs. All CPUs must
> enter the kernel in the same exception level.
> diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
> index 7964f03846b1..b165677ffab9 100644
> --- a/Documentation/arm64/cpu-feature-registers.txt
> +++ b/Documentation/arm64/cpu-feature-registers.txt
> @@ -190,6 +190,10 @@ infrastructure:
> |--------------------------------------------------|
> | JSCVT | [15-12] | y |
> |--------------------------------------------------|
> + | API | [11-8] | y |
> + |--------------------------------------------------|
> + | APA | [7-4] | y |
> + |--------------------------------------------------|
> | DPB | [3-0] | y |
> x--------------------------------------------------x
>
> diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
> index d6aff2c5e9e2..95509a7b0ffe 100644
> --- a/Documentation/arm64/elf_hwcaps.txt
> +++ b/Documentation/arm64/elf_hwcaps.txt
> @@ -178,3 +178,8 @@ HWCAP_ILRCPC
> HWCAP_FLAGM
>
> Functionality implied by ID_AA64ISAR0_EL1.TS == 0b0001.
> +
> +HWCAP_APIA
> +
> + EL0 AddPac and Auth functionality using APIAKey_EL1 is enabled, as
> + described by Documentation/arm64/pointer-authentication.txt.
> diff --git a/Documentation/arm64/pointer-authentication.txt b/Documentation/arm64/pointer-authentication.txt
> new file mode 100644
> index 000000000000..8a9cb5713770
> --- /dev/null
> +++ b/Documentation/arm64/pointer-authentication.txt
> @@ -0,0 +1,84 @@
> +Pointer authentication in AArch64 Linux
> +=======================================
> +
> +Author: Mark Rutland <[email protected]>
> +Date: 2017-07-19
> +
> +This document briefly describes the provision of pointer authentication
> +functionality in AArch64 Linux.
> +
> +
> +Architecture overview
> +---------------------
> +
> +The ARMv8.3 Pointer Authentication extension adds primitives that can be
> +used to mitigate certain classes of attack where an attacker can corrupt
> +the contents of some memory (e.g. the stack).
> +
> +The extension uses a Pointer Authentication Code (PAC) to determine
> +whether pointers have been modified unexpectedly. A PAC is derived from
> +a pointer, another value (such as the stack pointer), and a secret key
> +held in system registers.
> +
> +The extension adds instructions to insert a valid PAC into a pointer,
> +and to verify/remove the PAC from a pointer. The PAC occupies a number
> +of high-order bits of the pointer, which varies dependent on the
> +configured virtual address size and whether pointer tagging is in use.

s/pointer tagging/top byte ignore unless that's the terminology in the
rest of the kernel documentation ?

> +
> +A subset of these instructions have been allocated from the HINT
> +encoding space. In the absence of the extension (or when disabled),
> +these instructions behave as NOPs. Applications and libraries using
> +these instructions operate correctly regardless of the presence of the
> +extension.
> +
> +
> +Basic support
> +-------------
> +
> +When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
> +present, the kernel will assign a random APIAKey value to each process
> +at exec*() time. This key is shared by all threads within the process,
> +and the key is preserved across fork(). Presence of functionality using
> +APIAKey is advertised via HWCAP_APIA.
> +
> +Recent versions of GCC can compile code with APIAKey-based return
> +address protection when passed the -msign-return-address option. This
> +uses instructions in the HINT space, and such code can run on systems
> +without the pointer authentication extension.

Just a clarification.

This uses instructions in the hint space for architecture levels less
than armv8.3-a by default. If folks use -march=armv8.3-a you will start
seeing the combined forms of retaa appear.

> +
> +The remaining instruction and data keys (APIBKey, APDAKey, APDBKey) are
> +reserved for future use, and instructions using these keys must not be
> +used by software until a purpose and scope for their use has been
> +decided. To enable future software using these keys to function on
> +contemporary kernels, where possible, instructions using these keys are
> +made to behave as NOPs.
> +
> +The generic key (APGAKey) is currently unsupported. Instructions using
> +the generic key must not be used by software.
> +
> +
> +Debugging
> +---------
> +
> +When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
> +present, the kernel will expose the position of TTBR0 PAC bits in the
> +NT_ARM_PAC_MASK regset (struct user_pac_mask), which userspace can
> +acqure via PTRACE_GETREGSET.
> +
> +Separate masks are exposed for data pointers and instruction pointers,
> +as the set of PAC bits can vary between the two. Debuggers should not
> +expect that HWCAP_APIA implies the presence (or non-presence) of this
> +regset -- in future the kernel may support the use of APIBKey, APDAKey,
> +and/or APBAKey, even in the absence of APIAKey.
> +
> +Note that the masks apply to TTBR0 addresses, and are not valid to apply
> +to TTBR1 addresses (e.g. kernel pointers).
> +
> +
> +Virtualization
> +--------------
> +
> +Pointer authentication is not currently supported in KVM guests. KVM
> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
> +the feature will result in an UNDEFINED exception being injected into
> +the guest.

However applications using instructions from the hint space will
continue to work albeit without any protection (as they would just be
nops) ?

regards,
Ramana

Reviewed-by: Ramana Radhakrishnan <[email protected]>

>

2018-10-06 12:51:30

by Amit Kachhap

[permalink] [raw]

Subject: Re: [RFC 15/17] arm64: enable ptrauth earlier

On 10/05/2018 02:17 PM, Kristina Martsenko wrote:
> When the kernel is compiled with pointer auth instructions, the boot CPU
> needs to start using pointer auth very early, so change the cpucap to
> account for this.
>
> A function that enables pointer auth cannot return, so inline such
> functions or compile them without pointer auth.
>
> Do not use the cpu_enable callback, to avoid compiling the whole
> callchain down to cpu_enable without pointer auth.
>
> Note the change in behavior: if the boot CPU has pointer auth and a late
> CPU does not, we panic. Until now we would have just disabled pointer
> auth in this case.
>
> Signed-off-by: Kristina Martsenko <[email protected]>
> ---
> arch/arm64/include/asm/cpufeature.h | 9 +++++++++
> arch/arm64/include/asm/pointer_auth.h | 18 ++++++++++++++++++
> arch/arm64/kernel/cpufeature.c | 14 ++++----------
> arch/arm64/kernel/smp.c | 7 ++++++-
> 4 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 1717ba1db35d..af4ca92a5fa9 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -292,6 +292,15 @@ extern struct arm64_ftr_reg arm64_ftr_reg_ctrel0;
> */
> #define ARM64_CPUCAP_STRICT_BOOT_CPU_FEATURE ARM64_CPUCAP_SCOPE_BOOT_CPU
>
> +/*
> + * CPU feature used early in the boot based on the boot CPU. It is safe for a
> + * late CPU to have this feature even though the boot CPU hasn't enabled it,
> + * although the feature will not be used by Linux in this case. If the boot CPU
> + * has enabled this feature already, then every late CPU must have it.
> + */
> +#define ARM64_CPUCAP_BOOT_CPU_FEATURE \
> + (ARM64_CPUCAP_SCOPE_BOOT_CPU | ARM64_CPUCAP_PERMITTED_FOR_LATE_CPU)
> +
> struct arm64_cpu_capabilities {
> const char *desc;
> u16 capability;
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index e60f225d9fa2..0634f06c3af2 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -11,6 +11,13 @@
>
> #ifdef CONFIG_ARM64_PTR_AUTH
> /*
> + * Compile the function without pointer authentication instructions. This
> + * allows pointer authentication to be enabled/disabled within the function
> + * (but leaves the function unprotected by pointer authentication).
> + */
> +#define __no_ptrauth __attribute__((target("sign-return-address=none")))
> +
> +/*
> * Each key is a 128-bit quantity which is split across a pair of 64-bit
> * registers (Lo and Hi).
> */
> @@ -51,6 +58,15 @@ static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
> __ptrauth_key_install(APIA, keys->apia);
> }
>
> +static __always_inline void ptrauth_cpu_enable(void)
> +{
> + if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
> + return;
> +
> + sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA);
> + isb();
> +}
> +
> /*
> * The EL0 pointer bits used by a pointer authentication code.
> * This is dependent on TBI0 being enabled, or bits 63:56 would also apply.
> @@ -71,8 +87,10 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
> ptrauth_keys_init(&(tsk)->thread_info.keys_user)
>
> #else /* CONFIG_ARM64_PTR_AUTH */
> +#define __no_ptrauth
> #define ptrauth_strip_insn_pac(lr) (lr)
> #define ptrauth_task_init_user(tsk)
> +#define ptrauth_cpu_enable(tsk)
> #endif /* CONFIG_ARM64_PTR_AUTH */
>
> #endif /* __ASM_POINTER_AUTH_H */
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 3157685aa56a..380ee01145e8 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1040,15 +1040,10 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
> }
>
> #ifdef CONFIG_ARM64_PTR_AUTH
> -static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap)
> -{
> - sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA);
> -}
> -
> static bool has_address_auth(const struct arm64_cpu_capabilities *entry,
> int __unused)
> {
> - u64 isar1 = read_sanitised_ftr_reg(SYS_ID_AA64ISAR1_EL1);
> + u64 isar1 = read_sysreg(id_aa64isar1_el1);
> bool api, apa;
>
> apa = cpuid_feature_extract_unsigned_field(isar1,
> @@ -1251,7 +1246,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> {
> .desc = "Address authentication (architected algorithm)",
> .capability = ARM64_HAS_ADDRESS_AUTH_ARCH,
> - .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> + .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> .sys_reg = SYS_ID_AA64ISAR1_EL1,
> .sign = FTR_UNSIGNED,
> .field_pos = ID_AA64ISAR1_APA_SHIFT,
> @@ -1261,7 +1256,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> {
> .desc = "Address authentication (IMP DEF algorithm)",
> .capability = ARM64_HAS_ADDRESS_AUTH_IMP_DEF,
> - .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> + .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> .sys_reg = SYS_ID_AA64ISAR1_EL1,
> .sign = FTR_UNSIGNED,
> .field_pos = ID_AA64ISAR1_API_SHIFT,
> @@ -1270,9 +1265,8 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> },
> {
> .capability = ARM64_HAS_ADDRESS_AUTH,
> - .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> + .type = ARM64_CPUCAP_BOOT_CPU_FEATURE,
> .matches = has_address_auth,
> - .cpu_enable = cpu_enable_address_auth,
> },
> #endif /* CONFIG_ARM64_PTR_AUTH */
> {},
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 25fcd22a4bb2..09690024dce8 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -53,6 +53,7 @@
> #include <asm/numa.h>
> #include <asm/pgtable.h>
> #include <asm/pgalloc.h>
> +#include <asm/pointer_auth.h>
> #include <asm/processor.h>
> #include <asm/smp_plat.h>
> #include <asm/sections.h>
> @@ -211,6 +212,8 @@ asmlinkage notrace void secondary_start_kernel(void)
This function secondary_start_kernel attribute can be set to
__no_ptrauth for better redability as below, although no functionality
is broken as this function does not return.
> */
> check_local_cpu_capabilities();
>
> + ptrauth_cpu_enable();
There are some function calls before so wondering if pointer
authentication and cpu capabilities check required by ptrauth can be
moved still up.
> +
> if (cpu_ops[cpu]->cpu_postboot)
> cpu_ops[cpu]->cpu_postboot();
>
> @@ -405,7 +408,7 @@ void __init smp_cpus_done(unsigned int max_cpus)
> mark_linear_text_alias_ro();
> }
>
> -void __init smp_prepare_boot_cpu(void)
> +void __init __no_ptrauth smp_prepare_boot_cpu(void)
> {
> set_my_cpu_offset(per_cpu_offset(smp_processor_id()));
> /*
> @@ -414,6 +417,8 @@ void __init smp_prepare_boot_cpu(void)
> */
> jump_label_init();
> cpuinfo_store_boot_cpu();
> +
> + ptrauth_cpu_enable();
> }
>
> static u64 __init of_get_cpu_mpidr(struct device_node *dn)
>

2018-10-06 12:57:21

by Amit Kachhap

[permalink] [raw]

Subject: Re: [RFC 16/17] arm64: initialize and switch ptrauth kernel keys

On 10/05/2018 02:17 PM, Kristina Martsenko wrote:
> Set up keys to use pointer auth in the kernel. Each task has its own
> APIAKey, which is initialized during fork. The key is changed during
> context switch and on kernel entry from EL0.
>
> A function that changes the key cannot return, so inline such functions.

For all the RFC patches in this series,
Reviewed-by: Amit Daniel Kachhap <[email protected]>

>
> Signed-off-by: Mark Rutland <[email protected]>
> Signed-off-by: Kristina Martsenko <[email protected]>
> ---
> arch/arm64/include/asm/pointer_auth.h | 9 ++++++++-
> arch/arm64/include/asm/ptrauth-asm.h | 13 +++++++++++++
> arch/arm64/include/asm/thread_info.h | 1 +
> arch/arm64/kernel/asm-offsets.c | 1 +
> arch/arm64/kernel/entry.S | 4 ++++
> arch/arm64/kernel/process.c | 3 +++
> arch/arm64/kernel/smp.c | 3 +++
> 7 files changed, 33 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> index 0634f06c3af2..e94ca7df8dab 100644
> --- a/arch/arm64/include/asm/pointer_auth.h
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -50,12 +50,13 @@ do { \
> write_sysreg_s(__pki_v.hi, SYS_ ## k ## KEYHI_EL1); \
> } while (0)
>
> -static inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
> +static __always_inline void ptrauth_keys_switch(struct ptrauth_keys *keys)
> {
> if (!cpus_have_const_cap(ARM64_HAS_ADDRESS_AUTH))
> return;
>
> __ptrauth_key_install(APIA, keys->apia);
> + isb();
> }
>
> static __always_inline void ptrauth_cpu_enable(void)
> @@ -85,11 +86,17 @@ static inline unsigned long ptrauth_strip_insn_pac(unsigned long ptr)
>
> #define ptrauth_task_init_user(tsk) \
> ptrauth_keys_init(&(tsk)->thread_info.keys_user)
> +#define ptrauth_task_init_kernel(tsk) \
> + ptrauth_keys_init(&(tsk)->thread_info.keys_kernel)
> +#define ptrauth_task_switch(tsk) \
> + ptrauth_keys_switch(&(tsk)->thread_info.keys_kernel)
>
> #else /* CONFIG_ARM64_PTR_AUTH */
> #define __no_ptrauth
> #define ptrauth_strip_insn_pac(lr) (lr)
> #define ptrauth_task_init_user(tsk)
> +#define ptrauth_task_init_kernel(tsk)
> +#define ptrauth_task_switch(tsk)
> #define ptrauth_cpu_enable(tsk)
> #endif /* CONFIG_ARM64_PTR_AUTH */
>
> diff --git a/arch/arm64/include/asm/ptrauth-asm.h b/arch/arm64/include/asm/ptrauth-asm.h
> index f50bdfc4046c..3ef1cc8903d5 100644
> --- a/arch/arm64/include/asm/ptrauth-asm.h
> +++ b/arch/arm64/include/asm/ptrauth-asm.h
> @@ -16,11 +16,24 @@ alternative_if ARM64_HAS_ADDRESS_AUTH
> alternative_else_nop_endif
> .endm
>
> + .macro ptrauth_keys_install_kernel tsk, tmp
> +alternative_if ARM64_HAS_ADDRESS_AUTH
> + ldr \tmp, [\tsk, #(TSK_TI_KEYS_KERNEL + PTRAUTH_KEY_APIALO)]
> + msr_s SYS_APIAKEYLO_EL1, \tmp
> + ldr \tmp, [\tsk, #(TSK_TI_KEYS_KERNEL + PTRAUTH_KEY_APIAHI)]
> + msr_s SYS_APIAKEYHI_EL1, \tmp
> + isb
> +alternative_else_nop_endif
> + .endm
> +
> #else /* CONFIG_ARM64_PTR_AUTH */
>
> .macro ptrauth_keys_install_user tsk, tmp
> .endm
>
> + .macro ptrauth_keys_install_kernel tsk, tmp
> + .endm
> +
> #endif /* CONFIG_ARM64_PTR_AUTH */
>
> #endif /* __ASM_PTRAUTH_ASM_H */
> diff --git a/arch/arm64/include/asm/thread_info.h b/arch/arm64/include/asm/thread_info.h
> index ea9272fb52d4..e3ec5345addc 100644
> --- a/arch/arm64/include/asm/thread_info.h
> +++ b/arch/arm64/include/asm/thread_info.h
> @@ -46,6 +46,7 @@ struct thread_info {
> int preempt_count; /* 0 => preemptable, <0 => bug */
> #ifdef CONFIG_ARM64_PTR_AUTH
> struct ptrauth_keys keys_user;
> + struct ptrauth_keys keys_kernel;
> #endif
> };
>
> diff --git a/arch/arm64/kernel/asm-offsets.c b/arch/arm64/kernel/asm-offsets.c
> index b6be0dd037fd..6c61c9722b47 100644
> --- a/arch/arm64/kernel/asm-offsets.c
> +++ b/arch/arm64/kernel/asm-offsets.c
> @@ -47,6 +47,7 @@ int main(void)
> #endif
> #ifdef CONFIG_ARM64_PTR_AUTH
> DEFINE(TSK_TI_KEYS_USER, offsetof(struct task_struct, thread_info.keys_user));
> + DEFINE(TSK_TI_KEYS_KERNEL, offsetof(struct task_struct, thread_info.keys_kernel));
> #endif
> DEFINE(TSK_STACK, offsetof(struct task_struct, stack));
> BLANK();
> diff --git a/arch/arm64/kernel/entry.S b/arch/arm64/kernel/entry.S
> index 1e925f6d2978..a4503da445f7 100644
> --- a/arch/arm64/kernel/entry.S
> +++ b/arch/arm64/kernel/entry.S
> @@ -250,6 +250,10 @@ alternative_else_nop_endif
> msr sp_el0, tsk
> .endif
>
> + .if \el == 0
> + ptrauth_keys_install_kernel tsk, x20

There is 1 function before "__uaccess_ttbr0_disable" for which
__always_inline attribute can be set instead of just inline.

> + .endif
> +
> /*
> * Registers that may be useful after this macro is invoked:
> *
> diff --git a/arch/arm64/kernel/process.c b/arch/arm64/kernel/process.c
> index 857ae05cd04c..a866996610de 100644
> --- a/arch/arm64/kernel/process.c
> +++ b/arch/arm64/kernel/process.c
> @@ -330,6 +330,8 @@ int copy_thread(unsigned long clone_flags, unsigned long stack_start,
> */
> fpsimd_flush_task_state(p);
>
> + ptrauth_task_init_kernel(p);
> +
> if (likely(!(p->flags & PF_KTHREAD))) {
> *childregs = *current_pt_regs();
> childregs->regs[0] = 0;
> @@ -426,6 +428,7 @@ __notrace_funcgraph struct task_struct *__switch_to(struct task_struct *prev,
> contextidr_thread_switch(next);
> entry_task_switch(next);
> uao_thread_switch(next);
> + ptrauth_task_switch(next);
>
> /*
> * Complete any pending TLB or cache maintenance on this CPU in case
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 09690024dce8..d952dd62c780 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -212,6 +212,7 @@ asmlinkage notrace void secondary_start_kernel(void)
> */
> check_local_cpu_capabilities();
>
> + ptrauth_task_switch(current);
> ptrauth_cpu_enable();
>
> if (cpu_ops[cpu]->cpu_postboot)
> @@ -418,6 +419,8 @@ void __init __no_ptrauth smp_prepare_boot_cpu(void)
> jump_label_init();
> cpuinfo_store_boot_cpu();
>
> + ptrauth_task_init_kernel(current);
> + ptrauth_task_switch(current);
> ptrauth_cpu_enable();
> }
>
>

2018-10-11 14:01:57

[permalink] [raw]

Subject: Re: [RFC 17/17] arm64: compile the kernel with ptrauth -msign-return-address

On 05/10/2018 10:01, Ramana Radhakrishnan wrote:
> On 05/10/2018 09:47, Kristina Martsenko wrote:
>> Compile all functions with two ptrauth instructions: paciasp in the
>> prologue to sign the return address, and autiasp in the epilogue to
>> authenticate the return address. This should help protect the kernel
>> against attacks using return-oriented programming.
>>
>> CONFIG_ARM64_PTR_AUTH enables pointer auth for both userspace and the
>> kernel.
>>
>> Signed-off-by: Mark Rutland <[email protected]>
>> Signed-off-by: Kristina Martsenko <[email protected]>
>> ---
>> ? arch/arm64/Makefile | 4 ++++
>> ? 1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>> index 106039d25e2f..dbcd43ea99d8 100644
>> --- a/arch/arm64/Makefile
>> +++ b/arch/arm64/Makefile
>> @@ -56,6 +56,10 @@ KBUILD_AFLAGS??? += $(lseinstr) $(brokengasinst)
>> ? KBUILD_CFLAGS??? += $(call cc-option,-mabi=lp64)
>> ? KBUILD_AFLAGS??? += $(call cc-option,-mabi=lp64)
>> ? +ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
>> +KBUILD_CFLAGS??? += -msign-return-address=all
>
> Glad to see this being done and being proposed for mainline.
>
> I can see why you would prefer this though have you guys experimented at
> all with -msign-return-address=non-leaf as well ?

I've tried non-leaf and it works too. I'd be fine with switching to it,
I'm not sure which would be better for the kernel.

What kind of experiments did you have in mind? If I understand
correctly, then compared to non-leaf, "all" additionally protects leaf
functions that write to the stack. I don't know how many of those there
are in the kernel (or will be in the future). I also don't know the
additional performance impact of "all", as I don't think we have any
v8.3 hardware to test on yet. There is a minor code size impact (0.36%
on the current kernel), but I'm not sure how much that matters.

> Orthogonally and just fair warning - the command lines for this are also
> being revised to provide ROP and JOP protection using BTI from v8.5-a
> during the GCC-9 timeframe but I suspect that's a different option.

Thanks. I expect it will be a separate Kconfig option to build the
kernel with BTI and pointer auth, yes.

> Reviewed-by: Ramana Radhakrishnan? <[email protected]>

Thanks!

Kristina

2018-10-11 14:25:27

by Vladimir Murzin

[permalink] [raw]

Subject: Re: [RFC 17/17] arm64: compile the kernel with ptrauth -msign-return-address

Hi Kristina,

On 05/10/18 09:47, Kristina Martsenko wrote:
> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
> index 106039d25e2f..dbcd43ea99d8 100644
> --- a/arch/arm64/Makefile
> +++ b/arch/arm64/Makefile
> @@ -56,6 +56,10 @@ KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
> KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
> KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
>
> +ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
> +KBUILD_CFLAGS += -msign-return-address=all
> +endif

Should not it be done via cc-option so old toolchains keep working [1]?

[1]
$ aarch64-linux-gnu-gcc --version
aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease)
Copyright (C) 2014 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ aarch64-linux-gnu-gcc -msign-return-address=all
aarch64-linux-gnu-gcc: error: unrecognized command line option '-msign-return-address=all'
...

Cheers
Vladimir

2018-10-11 16:31:05

[permalink] [raw]

Subject: Re: [PATCH v5 01/17] arm64: add pointer authentication register bits

On Fri, Oct 05, 2018 at 09:47:38AM +0100, Kristina Martsenko wrote:
> From: Mark Rutland <[email protected]>
>
> The ARMv8.3 pointer authentication extension adds:
>
> * New fields in ID_AA64ISAR1 to report the presence of pointer
> authentication functionality.
>
> * New control bits in SCTLR_ELx to enable this functionality.
>
> * New system registers to hold the keys necessary for this
> functionality.
>
> * A new ESR_ELx.EC code used when the new instructions are affected by
> configurable traps
>
> This patch adds the relevant definitions to <asm/sysreg.h> and
> <asm/esr.h> for these, to be used by subsequent patches.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Signed-off-by: Kristina Martsenko <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Marc Zyngier <[email protected]>
> Cc: Suzuki K Poulose <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> arch/arm64/include/asm/esr.h | 3 ++-
> arch/arm64/include/asm/sysreg.h | 30 ++++++++++++++++++++++++++++++
> 2 files changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h
> index ce70c3ffb993..022785162281 100644
> --- a/arch/arm64/include/asm/esr.h
> +++ b/arch/arm64/include/asm/esr.h
> @@ -30,7 +30,8 @@
> #define ESR_ELx_EC_CP14_LS (0x06)
> #define ESR_ELx_EC_FP_ASIMD (0x07)
> #define ESR_ELx_EC_CP10_ID (0x08)
> -/* Unallocated EC: 0x09 - 0x0B */
> +#define ESR_ELx_EC_PAC (0x09)

Really minor nit: but shouldn't this be ESR_EL2_EC_PAC, since this trap
can't occur at EL1 afaict?

Rest of the patch looks good:

Reviewed-by: Will Deacon <[email protected]>

Will

2018-10-11 19:29:14

by Suzuki K Poulose

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

Hi Kristina,

On 05/10/18 09:47, Kristina Martsenko wrote:
> From: Mark Rutland <[email protected]>
>
> This patch adds basic support for pointer authentication, allowing
> userspace to make use of APIAKey. The kernel maintains an APIAKey value
> for each process (shared by all threads within), which is initialised to
> a random value at exec() time.
>
> To describe that address authentication instructions are available, the
> ID_AA64ISAR0.{APA,API} fields are exposed to userspace. A new hwcap,
> APIA, is added to describe that the kernel manages APIAKey.
>
> Instructions using other keys (APIBKey, APDAKey, APDBKey) are disabled,
> and will behave as NOPs. These may be made use of in future patches.
>
> No support is added for the generic key (APGAKey), though this cannot be
> trapped or made to behave as a NOP. Its presence is not advertised with
> a hwcap.
>
> Signed-off-by: Mark Rutland <[email protected]>
> [kristina: init keys in arch_bprm_mm_init; add AA64ISAR1.API HWCAP_CAP; use sysreg_clear_set]
> Signed-off-by: Kristina Martsenko <[email protected]>
> Tested-by: Adam Wallis <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Ramana Radhakrishnan <[email protected]>
> Cc: Suzuki K Poulose <[email protected]>
> Cc: Will Deacon <[email protected]>

> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 0dd171c7d71e..3157685aa56a 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -1040,6 +1040,11 @@ static void cpu_has_fwb(const struct arm64_cpu_capabilities *__unused)
> }
>
> #ifdef CONFIG_ARM64_PTR_AUTH
> +static void cpu_enable_address_auth(struct arm64_cpu_capabilities const *cap)
> +{
> + sysreg_clear_set(sctlr_el1, 0, SCTLR_ELx_ENIA);
> +}
> +
> static bool has_address_auth(const struct arm64_cpu_capabilities *entry,
> int __unused)
> {
> @@ -1267,6 +1272,7 @@ static const struct arm64_cpu_capabilities arm64_features[] = {
> .capability = ARM64_HAS_ADDRESS_AUTH,
> .type = ARM64_CPUCAP_SYSTEM_FEATURE,
> .matches = has_address_auth,
> + .cpu_enable = cpu_enable_address_auth,
> },
> #endif /* CONFIG_ARM64_PTR_AUTH */
> {},
> @@ -1314,6 +1320,10 @@ static const struct arm64_cpu_capabilities arm64_elf_hwcaps[] = {
> #ifdef CONFIG_ARM64_SVE
> HWCAP_CAP(SYS_ID_AA64PFR0_EL1, ID_AA64PFR0_SVE_SHIFT, FTR_UNSIGNED, ID_AA64PFR0_SVE, CAP_HWCAP, HWCAP_SVE),
> #endif
> +#ifdef CONFIG_ARM64_PTR_AUTH
> + HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_APA_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA),
> + HWCAP_CAP(SYS_ID_AA64ISAR1_EL1, ID_AA64ISAR1_API_SHIFT, FTR_UNSIGNED, 1, CAP_HWCAP, HWCAP_APIA),

This is a bit problematic. If all the CPUs have just the IMPDEF
algorithm available, we could fail to match the first entry (APA) for a
late secondary CPU and thus thus fail the CPU from booting. I guess we
need a custom entry which reuses the has_address_auth() as the matches().

Rest looks fine to me.

Suzuki

2018-10-12 08:54:45

[permalink] [raw]

Subject: Re: [PATCH v5 01/17] arm64: add pointer authentication register bits

On Thu, Oct 11, 2018 at 05:28:14PM +0100, Will Deacon wrote:
> On Fri, Oct 05, 2018 at 09:47:38AM +0100, Kristina Martsenko wrote:

> > +#define ESR_ELx_EC_PAC (0x09)
>
> Really minor nit: but shouldn't this be ESR_EL2_EC_PAC, since this trap
> can't occur at EL1 afaict?

It can also be taken to EL3 dependent on SCR_EL3.API.

We use ESR_ELx_EC_<foo> for other exceptions that can't be taken to EL1
(e.g. ESR_ELx_EC_SMC{32,64}), so I think it would be more consistent to
leave this as ESR_ELx_EC_PAC rather than ESR_EL2_EC_PAC.

Thanks,
Mark.

2018-10-12 08:58:04

[permalink] [raw]

Subject: Re: [PATCH v5 01/17] arm64: add pointer authentication register bits

On Fri, Oct 12, 2018 at 09:53:54AM +0100, Mark Rutland wrote:
> On Thu, Oct 11, 2018 at 05:28:14PM +0100, Will Deacon wrote:
> > On Fri, Oct 05, 2018 at 09:47:38AM +0100, Kristina Martsenko wrote:
>
> > > +#define ESR_ELx_EC_PAC (0x09)
> >
> > Really minor nit: but shouldn't this be ESR_EL2_EC_PAC, since this trap
> > can't occur at EL1 afaict?
>
> It can also be taken to EL3 dependent on SCR_EL3.API.
>
> We use ESR_ELx_EC_<foo> for other exceptions that can't be taken to EL1
> (e.g. ESR_ELx_EC_SMC{32,64}), so I think it would be more consistent to
> leave this as ESR_ELx_EC_PAC rather than ESR_EL2_EC_PAC.

Fair enough, but if we grow a different EC for ESR_EL1 that uses encoding
0x09, this all falls apart. At the very list, maybe we should comment those
that are EL2 or higher with /* EL2 and above */ or just fix the misnomer and
drop the useless _ELx_ part of the names completely.

Will

2018-10-12 09:50:57

[permalink] [raw]

Subject: Re: [PATCH v5 01/17] arm64: add pointer authentication register bits

On Fri, Oct 12, 2018 at 09:56:05AM +0100, Will Deacon wrote:
> On Fri, Oct 12, 2018 at 09:53:54AM +0100, Mark Rutland wrote:
> > On Thu, Oct 11, 2018 at 05:28:14PM +0100, Will Deacon wrote:
> > > On Fri, Oct 05, 2018 at 09:47:38AM +0100, Kristina Martsenko wrote:
> >
> > > > +#define ESR_ELx_EC_PAC (0x09)
> > >
> > > Really minor nit: but shouldn't this be ESR_EL2_EC_PAC, since this trap
> > > can't occur at EL1 afaict?
> >
> > It can also be taken to EL3 dependent on SCR_EL3.API.
> >
> > We use ESR_ELx_EC_<foo> for other exceptions that can't be taken to EL1
> > (e.g. ESR_ELx_EC_SMC{32,64}), so I think it would be more consistent to
> > leave this as ESR_ELx_EC_PAC rather than ESR_EL2_EC_PAC.
>
> Fair enough, but if we grow a different EC for ESR_EL1 that uses encoding
> 0x09, this all falls apart.

We haven't had overlapping encodings so far, and if we did, we'd want to
apply some policy to all of these definitions, no?

> At the very list, maybe we should comment those that are EL2 or higher
> with /* EL2 and above */ or just fix the misnomer and drop the useless
> _ELx_ part of the names completely.

A comment sounds fine to me.

I'm not sure that s/_ELx// buys us any clarity, though; I don't think
that ESR_EC_PAC is clearly more constrained than ESR_ELx_EC_PAC.

Thanks,
Mark.

2018-10-15 22:37:31

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On Fri, Oct 5, 2018 at 1:47 AM, Kristina Martsenko
<[email protected]> wrote:
> From: Mark Rutland <[email protected]>
>
> Now that we've added code to support pointer authentication, add some
> documentation so that people can figure out if/how to use it.
>
> Signed-off-by: Mark Rutland <[email protected]>
> [kristina: update cpu-feature-registers.txt]
> Signed-off-by: Kristina Martsenko <[email protected]>
> Cc: Andrew Jones <[email protected]>
> Cc: Catalin Marinas <[email protected]>
> Cc: Ramana Radhakrishnan <[email protected]>
> Cc: Will Deacon <[email protected]>
> ---
> Documentation/arm64/booting.txt | 8 +++
> Documentation/arm64/cpu-feature-registers.txt | 4 ++
> Documentation/arm64/elf_hwcaps.txt | 5 ++
> Documentation/arm64/pointer-authentication.txt | 84 ++++++++++++++++++++++++++
> 4 files changed, 101 insertions(+)
> create mode 100644 Documentation/arm64/pointer-authentication.txt
>
> diff --git a/Documentation/arm64/booting.txt b/Documentation/arm64/booting.txt
> index 8d0df62c3fe0..8df9f4658d6f 100644
> --- a/Documentation/arm64/booting.txt
> +++ b/Documentation/arm64/booting.txt
> @@ -205,6 +205,14 @@ Before jumping into the kernel, the following conditions must be met:
> ICC_SRE_EL2.SRE (bit 0) must be initialised to 0b0.
> - The DT or ACPI tables must describe a GICv2 interrupt controller.
>
> + For CPUs with pointer authentication functionality:
> + - If EL3 is present:
> + SCR_EL3.APK (bit 16) must be initialised to 0b1
> + SCR_EL3.API (bit 17) must be initialised to 0b1
> + - If the kernel is entered at EL1:
> + HCR_EL2.APK (bit 40) must be initialised to 0b1
> + HCR_EL2.API (bit 41) must be initialised to 0b1
> +
> The requirements described above for CPU mode, caches, MMUs, architected
> timers, coherency and system registers apply to all CPUs. All CPUs must
> enter the kernel in the same exception level.
> diff --git a/Documentation/arm64/cpu-feature-registers.txt b/Documentation/arm64/cpu-feature-registers.txt
> index 7964f03846b1..b165677ffab9 100644
> --- a/Documentation/arm64/cpu-feature-registers.txt
> +++ b/Documentation/arm64/cpu-feature-registers.txt
> @@ -190,6 +190,10 @@ infrastructure:
> |--------------------------------------------------|
> | JSCVT | [15-12] | y |
> |--------------------------------------------------|
> + | API | [11-8] | y |
> + |--------------------------------------------------|
> + | APA | [7-4] | y |
> + |--------------------------------------------------|
> | DPB | [3-0] | y |
> x--------------------------------------------------x
>
> diff --git a/Documentation/arm64/elf_hwcaps.txt b/Documentation/arm64/elf_hwcaps.txt
> index d6aff2c5e9e2..95509a7b0ffe 100644
> --- a/Documentation/arm64/elf_hwcaps.txt
> +++ b/Documentation/arm64/elf_hwcaps.txt
> @@ -178,3 +178,8 @@ HWCAP_ILRCPC
> HWCAP_FLAGM
>
> Functionality implied by ID_AA64ISAR0_EL1.TS == 0b0001.
> +
> +HWCAP_APIA
> +
> + EL0 AddPac and Auth functionality using APIAKey_EL1 is enabled, as
> + described by Documentation/arm64/pointer-authentication.txt.
> diff --git a/Documentation/arm64/pointer-authentication.txt b/Documentation/arm64/pointer-authentication.txt
> new file mode 100644
> index 000000000000..8a9cb5713770
> --- /dev/null
> +++ b/Documentation/arm64/pointer-authentication.txt
> @@ -0,0 +1,84 @@
> +Pointer authentication in AArch64 Linux
> +=======================================
> +
> +Author: Mark Rutland <[email protected]>
> +Date: 2017-07-19
> +
> +This document briefly describes the provision of pointer authentication
> +functionality in AArch64 Linux.
> +
> +
> +Architecture overview
> +---------------------
> +
> +The ARMv8.3 Pointer Authentication extension adds primitives that can be
> +used to mitigate certain classes of attack where an attacker can corrupt
> +the contents of some memory (e.g. the stack).
> +
> +The extension uses a Pointer Authentication Code (PAC) to determine
> +whether pointers have been modified unexpectedly. A PAC is derived from
> +a pointer, another value (such as the stack pointer), and a secret key
> +held in system registers.
> +
> +The extension adds instructions to insert a valid PAC into a pointer,
> +and to verify/remove the PAC from a pointer. The PAC occupies a number
> +of high-order bits of the pointer, which varies dependent on the
> +configured virtual address size and whether pointer tagging is in use.
> +
> +A subset of these instructions have been allocated from the HINT
> +encoding space. In the absence of the extension (or when disabled),
> +these instructions behave as NOPs. Applications and libraries using
> +these instructions operate correctly regardless of the presence of the
> +extension.
> +
> +
> +Basic support
> +-------------
> +
> +When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
> +present, the kernel will assign a random APIAKey value to each process
> +at exec*() time. This key is shared by all threads within the process,
> +and the key is preserved across fork(). Presence of functionality using
> +APIAKey is advertised via HWCAP_APIA.

It might be useful to include documentation here on how many bits of
the address are being used for the PAC bits (I'm assuming it's 7?)

-Kees

--
Kees Cook
Pixel Security

2018-10-15 22:43:20

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

On Fri, Oct 5, 2018 at 1:47 AM, Kristina Martsenko
<[email protected]> wrote:
> This series adds support for the ARMv8.3 pointer authentication
> extension. The series contains Mark's original patches to enable pointer
> authentication for userspace [1], followed by early RFC patches using
> pointer authentication in the kernel.

It wasn't obvious to me where the PAC mismatch exceptions will be
caught. I'm mainly curious to compare the PAC exception handling to
the existing stack-protector panic(). Can you point me to which
routines manage that? (Perhaps I just missed it in the series...)

Thanks for the series! I'm quite excited for ARMv8.3 hardware. :)

-Kees

--
Kees Cook
Pixel Security

2018-10-15 22:49:14

[permalink] [raw]

Subject: Re: [RFC 17/17] arm64: compile the kernel with ptrauth -msign-return-address

On Thu, Oct 11, 2018 at 7:23 AM, Vladimir Murzin
<[email protected]> wrote:
> Hi Kristina,
>
> On 05/10/18 09:47, Kristina Martsenko wrote:
>> diff --git a/arch/arm64/Makefile b/arch/arm64/Makefile
>> index 106039d25e2f..dbcd43ea99d8 100644
>> --- a/arch/arm64/Makefile
>> +++ b/arch/arm64/Makefile
>> @@ -56,6 +56,10 @@ KBUILD_AFLAGS += $(lseinstr) $(brokengasinst)
>> KBUILD_CFLAGS += $(call cc-option,-mabi=lp64)
>> KBUILD_AFLAGS += $(call cc-option,-mabi=lp64)
>>
>> +ifeq ($(CONFIG_ARM64_PTR_AUTH),y)
>> +KBUILD_CFLAGS += -msign-return-address=all
>> +endif
>
> Should not it be done via cc-option so old toolchains keep working [1]?
>
> [1]
> $ aarch64-linux-gnu-gcc --version
> aarch64-linux-gnu-gcc (Linaro GCC 2014.11) 4.9.3 20141031 (prerelease)
> Copyright (C) 2014 Free Software Foundation, Inc.
> This is free software; see the source for copying conditions. There is NO
> warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
>
> $ aarch64-linux-gnu-gcc -msign-return-address=all
> aarch64-linux-gnu-gcc: error: unrecognized command line option '-msign-return-address=all'
> ...

I would like to see CONFIG_ARM64_PTR_AUTH testing for compiler support
via Kconfig (as stack-protector does). This would allow developers to
only see the option if it was available (i.e. no "downgrade" happens
if the compiler is missing support). Using cc-option runs the risk of
building a kernel with CONFIG_ARM64_PTR_AUTH set, but _not_ actually
using ptr auth.

-Kees

--
Kees Cook
Pixel Security

2018-10-16 16:15:47

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> On 05/10/2018 09:47, Kristina Martsenko wrote:
>> From: Mark Rutland <[email protected]>
>>
>> Now that we've added code to support pointer authentication, add some
>> documentation so that people can figure out if/how to use it.
>>
>> Signed-off-by: Mark Rutland <[email protected]>
>> [kristina: update cpu-feature-registers.txt]
>> Signed-off-by: Kristina Martsenko <[email protected]>
>> Cc: Andrew Jones <[email protected]>
>> Cc: Catalin Marinas <[email protected]>
>> Cc: Ramana Radhakrishnan <[email protected]>
>> Cc: Will Deacon <[email protected]>
>> ---
>> ? Documentation/arm64/booting.txt??????????????? |? 8 +++
>> ? Documentation/arm64/cpu-feature-registers.txt? |? 4 ++
>> ? Documentation/arm64/elf_hwcaps.txt???????????? |? 5 ++
>> ? Documentation/arm64/pointer-authentication.txt | 84
>> ++++++++++++++++++++++++++
>> ? 4 files changed, 101 insertions(+)
>> ? create mode 100644 Documentation/arm64/pointer-authentication.txt
>>
>> diff --git a/Documentation/arm64/booting.txt
>> b/Documentation/arm64/booting.txt
>> index 8d0df62c3fe0..8df9f4658d6f 100644
>> --- a/Documentation/arm64/booting.txt
>> +++ b/Documentation/arm64/booting.txt
>> @@ -205,6 +205,14 @@ Before jumping into the kernel, the following
>> conditions must be met:
>> ????? ICC_SRE_EL2.SRE (bit 0) must be initialised to 0b0.
>> ??? - The DT or ACPI tables must describe a GICv2 interrupt controller.
>> ? +? For CPUs with pointer authentication functionality:
>> +? - If EL3 is present:
>> +??? SCR_EL3.APK (bit 16) must be initialised to 0b1
>> +??? SCR_EL3.API (bit 17) must be initialised to 0b1
>> +? - If the kernel is entered at EL1:
>> +??? HCR_EL2.APK (bit 40) must be initialised to 0b1
>> +??? HCR_EL2.API (bit 41) must be initialised to 0b1
>> +
>> ? The requirements described above for CPU mode, caches, MMUs,
>> architected
>> ? timers, coherency and system registers apply to all CPUs.? All CPUs
>> must
>> ? enter the kernel in the same exception level.
>> diff --git a/Documentation/arm64/cpu-feature-registers.txt
>> b/Documentation/arm64/cpu-feature-registers.txt
>> index 7964f03846b1..b165677ffab9 100644
>> --- a/Documentation/arm64/cpu-feature-registers.txt
>> +++ b/Documentation/arm64/cpu-feature-registers.txt
>> @@ -190,6 +190,10 @@ infrastructure:
>> ?????? |--------------------------------------------------|
>> ?????? | JSCVT??????????????????????? | [15-12] |??? y??? |
>> ?????? |--------------------------------------------------|
>> +???? | API????????????????????????? | [11-8]? |??? y??? |
>> +???? |--------------------------------------------------|
>> +???? | APA????????????????????????? | [7-4]?? |??? y??? |
>> +???? |--------------------------------------------------|
>> ?????? | DPB????????????????????????? | [3-0]?? |??? y??? |
>> ?????? x--------------------------------------------------x
>> ? diff --git a/Documentation/arm64/elf_hwcaps.txt
>> b/Documentation/arm64/elf_hwcaps.txt
>> index d6aff2c5e9e2..95509a7b0ffe 100644
>> --- a/Documentation/arm64/elf_hwcaps.txt
>> +++ b/Documentation/arm64/elf_hwcaps.txt
>> @@ -178,3 +178,8 @@ HWCAP_ILRCPC
>> ? HWCAP_FLAGM
>> ? ????? Functionality implied by ID_AA64ISAR0_EL1.TS == 0b0001.
>> +
>> +HWCAP_APIA
>> +
>> +??? EL0 AddPac and Auth functionality using APIAKey_EL1 is enabled, as
>> +??? described by Documentation/arm64/pointer-authentication.txt.
>> diff --git a/Documentation/arm64/pointer-authentication.txt
>> b/Documentation/arm64/pointer-authentication.txt
>> new file mode 100644
>> index 000000000000..8a9cb5713770
>> --- /dev/null
>> +++ b/Documentation/arm64/pointer-authentication.txt
>> @@ -0,0 +1,84 @@
>> +Pointer authentication in AArch64 Linux
>> +=======================================
>> +
>> +Author: Mark Rutland <[email protected]>
>> +Date: 2017-07-19
>> +
>> +This document briefly describes the provision of pointer authentication
>> +functionality in AArch64 Linux.
>> +
>> +
>> +Architecture overview
>> +---------------------
>> +
>> +The ARMv8.3 Pointer Authentication extension adds primitives that can be
>> +used to mitigate certain classes of attack where an attacker can corrupt
>> +the contents of some memory (e.g. the stack).
>> +
>> +The extension uses a Pointer Authentication Code (PAC) to determine
>> +whether pointers have been modified unexpectedly. A PAC is derived from
>> +a pointer, another value (such as the stack pointer), and a secret key
>> +held in system registers.
>> +
>> +The extension adds instructions to insert a valid PAC into a pointer,
>> +and to verify/remove the PAC from a pointer. The PAC occupies a number
>> +of high-order bits of the pointer, which varies dependent on the
>> +configured virtual address size and whether pointer tagging is in use.
>
> s/pointer tagging/top byte ignore unless that's the terminology in the
> rest of the kernel documentation ?

The rest of the kernel documentation calls them "tagged pointers", and
doesn't use "top byte ignore", for example
Documentation/arm64/tagged-pointers.txt:
https://elixir.bootlin.com/linux/latest/source/Documentation/arm64/tagged-pointers.txt

>
>> +
>> +A subset of these instructions have been allocated from the HINT
>> +encoding space. In the absence of the extension (or when disabled),
>> +these instructions behave as NOPs. Applications and libraries using
>> +these instructions operate correctly regardless of the presence of the
>> +extension.
>> +
>> +
>> +Basic support
>> +-------------
>> +
>> +When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
>> +present, the kernel will assign a random APIAKey value to each process
>> +at exec*() time. This key is shared by all threads within the process,
>> +and the key is preserved across fork(). Presence of functionality using
>> +APIAKey is advertised via HWCAP_APIA.
>> +
>> +Recent versions of GCC can compile code with APIAKey-based return
>> +address protection when passed the -msign-return-address option. This
>> +uses instructions in the HINT space, and such code can run on systems
>> +without the pointer authentication extension.
>
> Just a clarification.
>
> This uses instructions in the hint space for architecture levels less
> than armv8.3-a by default. If folks use -march=armv8.3-a you will start
> seeing the combined forms of retaa appear.

I'll amend this to:

"This uses instructions in the HINT space (unless -march=armv8.3-a or
higher is also passed), and such code can run on systems without the
pointer authentication extension."

>
>> +
>> +The remaining instruction and data keys (APIBKey, APDAKey, APDBKey) are
>> +reserved for future use, and instructions using these keys must not be
>> +used by software until a purpose and scope for their use has been
>> +decided. To enable future software using these keys to function on
>> +contemporary kernels, where possible, instructions using these keys are
>> +made to behave as NOPs.
>> +
>> +The generic key (APGAKey) is currently unsupported. Instructions using
>> +the generic key must not be used by software.
>> +
>> +
>> +Debugging
>> +---------
>> +
>> +When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
>> +present, the kernel will expose the position of TTBR0 PAC bits in the
>> +NT_ARM_PAC_MASK regset (struct user_pac_mask), which userspace can
>> +acqure via PTRACE_GETREGSET.
>> +
>> +Separate masks are exposed for data pointers and instruction pointers,
>> +as the set of PAC bits can vary between the two. Debuggers should not
>> +expect that HWCAP_APIA implies the presence (or non-presence) of this
>> +regset -- in future the kernel may support the use of APIBKey, APDAKey,
>> +and/or APBAKey, even in the absence of APIAKey.
>> +
>> +Note that the masks apply to TTBR0 addresses, and are not valid to apply
>> +to TTBR1 addresses (e.g. kernel pointers).
>> +
>> +
>> +Virtualization
>> +--------------
>> +
>> +Pointer authentication is not currently supported in KVM guests. KVM
>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
>> +the feature will result in an UNDEFINED exception being injected into
>> +the guest.
>
> However applications using instructions from the hint space will
> continue to work albeit without any protection (as they would just be
> nops) ?

Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
will trap and KVM will inject an "Unknown reason" exception into the
guest (which will cause a Linux guest to send a SIGILL to the application).

In the latter case we could instead pretend the instruction was a NOP
and not inject an exception, but trapping twice per every function would
probably be terrible for performance. The guest shouldn't be setting
EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
not present (because KVM has hidden it).

The other special case is the XPACLRI instruction, which is also in the
HINT space. Currently it will trap and KVM will inject an exception into
the guest. We should probably change this to NOP instead, as that's what
applications will expect. Unfortunately there is no EnIA-like control to
make it NOP.

One option is for KVM to pretend the instruction was a NOP and return to
the guest. But if XPACLRI gets executed frequently, then the constant
trapping might hurt performance. I don't know how frequently it might
get used, as I don't know of any applications currently using it. From
what I understand, it may be used by userspace stack unwinders.

(Also worth noting - as far as I can tell there is no easy way for KVM
to know which pointer authentication instruction caused the trap, so we
may have to do something unusual like use "at s12e1r" to read guest
memory and check for XPACLRI.)

The other option is to turn off trapping entirely. However then on a
big.LITTLE system with mismatched pointer authentication support
instructions will work intermittently on some CPUs but not others.

Thoughts?

>
> Reviewed-by: Ramana Radhakrishnan? <[email protected]>

Thanks!

Kristina

2018-10-19 11:16:26

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote:
> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> new file mode 100644
> index 000000000000..2aefedc31d9e
> --- /dev/null
> +++ b/arch/arm64/include/asm/pointer_auth.h
> @@ -0,0 +1,63 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#ifndef __ASM_POINTER_AUTH_H
> +#define __ASM_POINTER_AUTH_H
> +
> +#include <linux/random.h>
> +
> +#include <asm/cpufeature.h>
> +#include <asm/sysreg.h>
> +
> +#ifdef CONFIG_ARM64_PTR_AUTH
> +/*
> + * Each key is a 128-bit quantity which is split across a pair of 64-bit
> + * registers (Lo and Hi).
> + */
> +struct ptrauth_key {
> + unsigned long lo, hi;
> +};
> +
> +/*
> + * We give each process its own instruction A key (APIAKey), which is shared by
> + * all threads. This is inherited upon fork(), and reinitialised upon exec*().
> + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
> + * instructions behaving as NOPs.
> + */

I don't remember the past discussions but I assume the tools guys are ok
with a single key shared by multiple threads. Ramana, could you ack this
part, FTR?

(and it would help if someone from the Android and Chrome camps can
confirm)

Thanks.

--
Catalin

2018-10-19 11:26:00

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

[+Cyrill Gorcunov for CRIU stuff]

On Fri, Oct 19, 2018 at 12:15:43PM +0100, Catalin Marinas wrote:
> On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote:
> > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> > new file mode 100644
> > index 000000000000..2aefedc31d9e
> > --- /dev/null
> > +++ b/arch/arm64/include/asm/pointer_auth.h
> > @@ -0,0 +1,63 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#ifndef __ASM_POINTER_AUTH_H
> > +#define __ASM_POINTER_AUTH_H
> > +
> > +#include <linux/random.h>
> > +
> > +#include <asm/cpufeature.h>
> > +#include <asm/sysreg.h>
> > +
> > +#ifdef CONFIG_ARM64_PTR_AUTH
> > +/*
> > + * Each key is a 128-bit quantity which is split across a pair of 64-bit
> > + * registers (Lo and Hi).
> > + */
> > +struct ptrauth_key {
> > + unsigned long lo, hi;
> > +};
> > +
> > +/*
> > + * We give each process its own instruction A key (APIAKey), which is shared by
> > + * all threads. This is inherited upon fork(), and reinitialised upon exec*().
> > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
> > + * instructions behaving as NOPs.
> > + */
>
> I don't remember the past discussions but I assume the tools guys are ok
> with a single key shared by multiple threads. Ramana, could you ack this
> part, FTR?
>
> (and it would help if someone from the Android and Chrome camps can
> confirm)

FWIW: I think we should be entertaining a prctl() interface to use a new
key on a per-thread basis. Obviously, this would need to be used with care
(e.g. you'd fork(); use the prctl() and then you'd better not return from
the calling function!).

Assuming we want this (Kees -- I was under the impression that everything in
Android would end up with the same key otherwise?), then the question is
do we want:

- prctl() get/set operations for the key, or
- prctl() set_random_key operation, or
- both of the above?

Part of the answer to that may lie in the requirements of CRIU, where I
strongly suspect they need explicit get/set operations, although these
could be gated on CONFIG_CHECKPOINT_RESTORE=y.

Will

2018-10-19 11:37:58

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> > On 05/10/2018 09:47, Kristina Martsenko wrote:
> >> +Virtualization
> >> +--------------
> >> +
> >> +Pointer authentication is not currently supported in KVM guests. KVM
> >> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
> >> +the feature will result in an UNDEFINED exception being injected into
> >> +the guest.
> >
> > However applications using instructions from the hint space will
> > continue to work albeit without any protection (as they would just be
> > nops) ?
>
> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
> will trap and KVM will inject an "Unknown reason" exception into the
> guest (which will cause a Linux guest to send a SIGILL to the application).

I think that part is fine. If KVM (a fairly recent version with CPUID
sanitisation) does not enable ptr auth, the CPUID should not advertise
this feature either so the guest kernel should not enable it. For the
above instructions in the HINT space, they will just be NOPs. If the
guest kernel enables the feature regardless of the CPUID information, it
deserves to get an "Unknown reason" exception.

> In the latter case we could instead pretend the instruction was a NOP
> and not inject an exception, but trapping twice per every function would
> probably be terrible for performance. The guest shouldn't be setting
> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
> not present (because KVM has hidden it).

I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
that the feature is present via CPUID.

> The other special case is the XPACLRI instruction, which is also in the
> HINT space. Currently it will trap and KVM will inject an exception into
> the guest. We should probably change this to NOP instead, as that's what
> applications will expect. Unfortunately there is no EnIA-like control to
> make it NOP.

Very good catch. Basically if EL2 doesn't know about ptr auth (older
distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
to have ptr auth enabled, being built for the specific HW). So a user
app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
(injected by the guest kernel following the injection of "Unknown
reason" exception by KVM).

Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
Could we restrict it to only being used at run-time if the corresponding
HWCAP is set? This means redefining this instruction as no longer in the
NOP space.

> One option is for KVM to pretend the instruction was a NOP and return to
> the guest. But if XPACLRI gets executed frequently, then the constant
> trapping might hurt performance. I don't know how frequently it might
> get used, as I don't know of any applications currently using it. From
> what I understand, it may be used by userspace stack unwinders.
>
> (Also worth noting - as far as I can tell there is no easy way for KVM
> to know which pointer authentication instruction caused the trap, so we
> may have to do something unusual like use "at s12e1r" to read guest
> memory and check for XPACLRI.)

Indeed, it's not an easy fix. As discussed (in the office), we can't
even guarantee that the guest stage 1 translation is stable and points
to the actual XPACLRI instruction.

> The other option is to turn off trapping entirely. However then on a
> big.LITTLE system with mismatched pointer authentication support
> instructions will work intermittently on some CPUs but not others.

That's another case but let's assume we never see such configurations ;).

--
Catalin

2018-10-19 11:39:07

[permalink] [raw]

Subject: Re: [RFC 12/17] arm64: move ptrauth keys to thread_info

On Fri, Oct 05, 2018 at 09:47:49AM +0100, Kristina Martsenko wrote:
> From: Mark Rutland <[email protected]>
>
> To use pointer authentication in the kernel, we'll need to switch keys
> in the entry assembly. This patch moves the pointer auth keys into
> thread_info to make this possible.
>
> There should be no functional change as a result of this patch.
>
> Signed-off-by: Mark Rutland <[email protected]>
> Signed-off-by: Kristina Martsenko <[email protected]>

Can we actually fold this into patch 7? It also leaves the door open to
allowing per-thread keys.

--
Catalin

2018-10-19 11:48:01

by Marc Zyngier

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On 19/10/18 12:35, Catalin Marinas wrote:
> On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
>> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
>>> On 05/10/2018 09:47, Kristina Martsenko wrote:

[...]

>> The other option is to turn off trapping entirely. However then on a
>> big.LITTLE system with mismatched pointer authentication support
>> instructions will work intermittently on some CPUs but not others.
>
> That's another case but let's assume we never see such configurations ;).

I'd like to put it on the record that I'm not willing to support such a
configuration. So my ask is that if we detect a system where only some
of the CPUs have pointer authentication support, we either:

1) prevent some of the CPUs from booting (that's harsh)
2) disable KVM (that's easy)

I'm perfectly happy with (2).

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2018-10-19 12:23:19

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On Fri, Oct 19, 2018 at 12:35:56PM +0100, Catalin Marinas wrote:
> On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
> > On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> > > On 05/10/2018 09:47, Kristina Martsenko wrote:
> > The other special case is the XPACLRI instruction, which is also in the
> > HINT space. Currently it will trap and KVM will inject an exception into
> > the guest. We should probably change this to NOP instead, as that's what
> > applications will expect. Unfortunately there is no EnIA-like control to
> > make it NOP.
>
> Very good catch. Basically if EL2 doesn't know about ptr auth (older
> distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> to have ptr auth enabled, being built for the specific HW). So a user
> app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> (injected by the guest kernel following the injection of "Unknown
> reason" exception by KVM).
>
> Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> Could we restrict it to only being used at run-time if the corresponding
> HWCAP is set? This means redefining this instruction as no longer in the
> NOP space.

My main worry is that this instruction is used when unwinding C++
exceptions, so I think we'll see it fairly often.

Effectively, the architecture means these instructions can result in a
SIGILL if they are used under an OS/hypervisor that doesn't know about
the feature (i.e. any mainline kernel release so far). I think that's a
massive problem for the current implementation in GCC. Worse, if
distributions are currently shipping binaries built with this, they
basically have a ticking bomb in their applications where things will
start crashing when they encounter CPUs that implement pointer
authentication.

Ramana: do you know whether people are building binaries with this stuff
enabled by default?

Will

2018-10-19 12:37:22

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

On Fri, Oct 05, 2018 at 09:47:37AM +0100, Kristina Martsenko wrote:
> 1) Key support
>
> This series enables the use of instructions using APIAKey, which is
> initialised and maintained per-process (shared by all threads). GCC
> currently only makes use of APIAKey.
>
> This series does not add support for APIBKey, APDAKey, APDBKey, nor
> APGAKey. HINT-space instructions using these keys will currently execute
> as NOPs. Support for these keys can be added as users appear.
>
> Note that while we expose the cpuid register (ID_AA64ISAR1_EL1) to
> userspace, it only contains one feature for address authentication
> (API/APA), so it cannot be used by userspace to tell which keys the
> kernel supports. For this the kernel exposes HWCAP bits, one per key
> (currently only APIAKey), which must be checked instead.

Given that the architecture doesn't provide an identification mechanism
for the case where only one of the keys is available, I would much prefer
that we expose both of the keys to userspace. Is the only downside of
that a possible exception entry overhead if the kernel wants to use pointer
authentication as well?

Having an initial implementation where the B key operations act as NOPs
isn't ideal if we want to support future users -- chances are they'll
be put off because deployed kernels don't give them whatever security
guarantees they require. It's a bit of a chicken-and-egg problem, so
unless we have good reasons to keep the B key hidden, I think we should
be exposing it from the start.

Will

2018-10-19 14:43:39

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On 19/10/2018 12:35, Catalin Marinas wrote:
> On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
>> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
>>> On 05/10/2018 09:47, Kristina Martsenko wrote:
>>>> +Virtualization
>>>> +--------------
>>>> +
>>>> +Pointer authentication is not currently supported in KVM guests. KVM
>>>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
>>>> +the feature will result in an UNDEFINED exception being injected into
>>>> +the guest.
>>>
>>> However applications using instructions from the hint space will
>>> continue to work albeit without any protection (as they would just be
>>> nops) ?
>>
>> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
>> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
>> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
>> will trap and KVM will inject an "Unknown reason" exception into the
>> guest (which will cause a Linux guest to send a SIGILL to the application).
>
> I think that part is fine. If KVM (a fairly recent version with CPUID
> sanitisation) does not enable ptr auth, the CPUID should not advertise
> this feature either so the guest kernel should not enable it. For the
> above instructions in the HINT space, they will just be NOPs. If the
> guest kernel enables the feature regardless of the CPUID information, it
> deserves to get an "Unknown reason" exception.
>
>> In the latter case we could instead pretend the instruction was a NOP
>> and not inject an exception, but trapping twice per every function would
>> probably be terrible for performance. The guest shouldn't be setting
>> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
>> not present (because KVM has hidden it).
>
> I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
> that the feature is present via CPUID.
>
>> The other special case is the XPACLRI instruction, which is also in the
>> HINT space. Currently it will trap and KVM will inject an exception into
>> the guest. We should probably change this to NOP instead, as that's what
>> applications will expect. Unfortunately there is no EnIA-like control to
>> make it NOP.
>
> Very good catch. Basically if EL2 doesn't know about ptr auth (older
> distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> to have ptr auth enabled, being built for the specific HW). So a user
> app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> (injected by the guest kernel following the injection of "Unknown
> reason" exception by KVM).
>
> Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> Could we restrict it to only being used at run-time if the corresponding
> HWCAP is set? This means redefining this instruction as no longer in the
> NOP space.

I think an alternative solution is to just disable trapping of pointer
auth instructions in KVM. This will mean that the instructions will
behave the same in the guest as they do in the host. HINT-space
instructions (including XPACLRI) will behave as NOPs (or perform their
function, if enabled by the guest), and will not trap.

A side effect of disabling trapping is that keys may effectively leak
from one guest to another, since one guest may set a key and another
guest may use an instruction that uses that key. But this can be fixed
by zeroing the keys every time we enter a guest. We can additionally
trap key accesses (which is separate from instruction trapping), to have
guests fail more reliably and avoid restoring host keys on guest exit.

Things still won't work well on big.LITTLE systems with mismatched
pointer auth support between CPUs, but as Marc pointed out in the other
email, we can just disable KVM on such systems when we detect a pointer
auth mismatch.

If we want current stable kernels to support guests that use HINT-space
pointer auth instructions, we'll need to backport the above changes to
stable kernels as well.

Even if we restricted userspace to only use XPACLRI if the HWCAP is set,
current stable kernels would still not be able to handle the HINT-space
PAC/AUT instructions that GCC generates, if the guest is pointer auth
aware. None of the stable kernels have the CPUID sanitisation patches,
so the guest would enable pointer auth, which would cause the PAC/AUT
instructions to trap.

>> One option is for KVM to pretend the instruction was a NOP and return to
>> the guest. But if XPACLRI gets executed frequently, then the constant
>> trapping might hurt performance. I don't know how frequently it might
>> get used, as I don't know of any applications currently using it. From
>> what I understand, it may be used by userspace stack unwinders.
>>
>> (Also worth noting - as far as I can tell there is no easy way for KVM
>> to know which pointer authentication instruction caused the trap, so we
>> may have to do something unusual like use "at s12e1r" to read guest
>> memory and check for XPACLRI.)
>
> Indeed, it's not an easy fix. As discussed (in the office), we can't
> even guarantee that the guest stage 1 translation is stable and points
> to the actual XPACLRI instruction.
>
>> The other option is to turn off trapping entirely. However then on a
>> big.LITTLE system with mismatched pointer authentication support
>> instructions will work intermittently on some CPUs but not others.
>
> That's another case but let's assume we never see such configurations ;).

Kristina

2018-10-19 15:11:17

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On Fri, Oct 19, 2018 at 03:42:23PM +0100, Kristina Martsenko wrote:
> On 19/10/2018 12:35, Catalin Marinas wrote:
> > On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
> >> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> >>> On 05/10/2018 09:47, Kristina Martsenko wrote:
> >>>> +Virtualization
> >>>> +--------------
> >>>> +
> >>>> +Pointer authentication is not currently supported in KVM guests. KVM
> >>>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
> >>>> +the feature will result in an UNDEFINED exception being injected into
> >>>> +the guest.
> >>>
> >>> However applications using instructions from the hint space will
> >>> continue to work albeit without any protection (as they would just be
> >>> nops) ?
> >>
> >> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
> >> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
> >> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
> >> will trap and KVM will inject an "Unknown reason" exception into the
> >> guest (which will cause a Linux guest to send a SIGILL to the application).
> >
> > I think that part is fine. If KVM (a fairly recent version with CPUID
> > sanitisation) does not enable ptr auth, the CPUID should not advertise
> > this feature either so the guest kernel should not enable it. For the
> > above instructions in the HINT space, they will just be NOPs. If the
> > guest kernel enables the feature regardless of the CPUID information, it
> > deserves to get an "Unknown reason" exception.
> >
> >> In the latter case we could instead pretend the instruction was a NOP
> >> and not inject an exception, but trapping twice per every function would
> >> probably be terrible for performance. The guest shouldn't be setting
> >> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
> >> not present (because KVM has hidden it).
> >
> > I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
> > that the feature is present via CPUID.
> >
> >> The other special case is the XPACLRI instruction, which is also in the
> >> HINT space. Currently it will trap and KVM will inject an exception into
> >> the guest. We should probably change this to NOP instead, as that's what
> >> applications will expect. Unfortunately there is no EnIA-like control to
> >> make it NOP.
> >
> > Very good catch. Basically if EL2 doesn't know about ptr auth (older
> > distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> > CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> > to have ptr auth enabled, being built for the specific HW). So a user
> > app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> > (injected by the guest kernel following the injection of "Unknown
> > reason" exception by KVM).
> >
> > Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> > Could we restrict it to only being used at run-time if the corresponding
> > HWCAP is set? This means redefining this instruction as no longer in the
> > NOP space.
>
> I think an alternative solution is to just disable trapping of pointer
> auth instructions in KVM. This will mean that the instructions will
> behave the same in the guest as they do in the host. HINT-space
> instructions (including XPACLRI) will behave as NOPs (or perform their
> function, if enabled by the guest), and will not trap.

OK, so this means disabling the trap (during early EL2 setup) but still
sanitizing the CPUID not to report the feature to EL1 unless fully
supported on all CPUs.

> A side effect of disabling trapping is that keys may effectively leak
> from one guest to another, since one guest may set a key and another
> guest may use an instruction that uses that key. But this can be fixed
> by zeroing the keys every time we enter a guest. We can additionally
> trap key accesses (which is separate from instruction trapping), to have
> guests fail more reliably and avoid restoring host keys on guest exit.

Actually, if the CPUID doesn't report the feature present, do we care
that a stupid guest writes some register and expects it to be preserved
and not leaked?

> Things still won't work well on big.LITTLE systems with mismatched
> pointer auth support between CPUs, but as Marc pointed out in the other
> email, we can just disable KVM on such systems when we detect a pointer
> auth mismatch.

If we ever see such system, we could follow the approach above - disable
trapping at EL2 if the feature is present but do not report it to EL1
via CPUID. A guest should not expect it to work properly.

> If we want current stable kernels to support guests that use HINT-space
> pointer auth instructions, we'll need to backport the above changes to
> stable kernels as well.

I agree.

> Even if we restricted userspace to only use XPACLRI if the HWCAP is set,
> current stable kernels would still not be able to handle the HINT-space
> PAC/AUT instructions that GCC generates, if the guest is pointer auth
> aware. None of the stable kernels have the CPUID sanitisation patches,
> so the guest would enable pointer auth, which would cause the PAC/AUT
> instructions to trap.

Ah, CPUID sanitisation would have to be backported as well. We can
probably get away with something simpler which does not sanitise
big.LITTLE (and I would not expect "enterprise" big.LITTLE machines) but
simply cap/mask the CPUID to what is known to the respective kernel
version. That shouldn't be too intrusive.

--
Catalin

2018-10-19 15:37:50

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <[email protected]> wrote:
> [+Cyrill Gorcunov for CRIU stuff]
>
> On Fri, Oct 19, 2018 at 12:15:43PM +0100, Catalin Marinas wrote:
>> On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote:
>> > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
>> > new file mode 100644
>> > index 000000000000..2aefedc31d9e
>> > --- /dev/null
>> > +++ b/arch/arm64/include/asm/pointer_auth.h
>> > @@ -0,0 +1,63 @@
>> > +// SPDX-License-Identifier: GPL-2.0
>> > +#ifndef __ASM_POINTER_AUTH_H
>> > +#define __ASM_POINTER_AUTH_H
>> > +
>> > +#include <linux/random.h>
>> > +
>> > +#include <asm/cpufeature.h>
>> > +#include <asm/sysreg.h>
>> > +
>> > +#ifdef CONFIG_ARM64_PTR_AUTH
>> > +/*
>> > + * Each key is a 128-bit quantity which is split across a pair of 64-bit
>> > + * registers (Lo and Hi).
>> > + */
>> > +struct ptrauth_key {
>> > + unsigned long lo, hi;
>> > +};
>> > +
>> > +/*
>> > + * We give each process its own instruction A key (APIAKey), which is shared by
>> > + * all threads. This is inherited upon fork(), and reinitialised upon exec*().
>> > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
>> > + * instructions behaving as NOPs.
>> > + */
>>
>> I don't remember the past discussions but I assume the tools guys are ok
>> with a single key shared by multiple threads. Ramana, could you ack this
>> part, FTR?
>>
>> (and it would help if someone from the Android and Chrome camps can
>> confirm)
>
> FWIW: I think we should be entertaining a prctl() interface to use a new
> key on a per-thread basis. Obviously, this would need to be used with care
> (e.g. you'd fork(); use the prctl() and then you'd better not return from
> the calling function!).
>
> Assuming we want this (Kees -- I was under the impression that everything in
> Android would end up with the same key otherwise?), then the question is
> do we want:
>
> - prctl() get/set operations for the key, or
> - prctl() set_random_key operation, or
> - both of the above?
>
> Part of the answer to that may lie in the requirements of CRIU, where I
> strongly suspect they need explicit get/set operations, although these
> could be gated on CONFIG_CHECKPOINT_RESTORE=y.

Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes.
No reason to allow explicit access to the key (and selected algo) if
we don't have to.

As for per-thread or not, having a "pick a new key now" prctl() sounds
good, but I'd like to have an eye toward having it just be "automatic"
on clone().

-Kees

--
Kees Cook
Pixel Security

2018-10-19 15:50:27

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote:
> On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <[email protected]> wrote:
> > FWIW: I think we should be entertaining a prctl() interface to use a new
> > key on a per-thread basis. Obviously, this would need to be used with care
> > (e.g. you'd fork(); use the prctl() and then you'd better not return from
> > the calling function!).
> >
> > Assuming we want this (Kees -- I was under the impression that everything in
> > Android would end up with the same key otherwise?), then the question is
> > do we want:
> >
> > - prctl() get/set operations for the key, or
> > - prctl() set_random_key operation, or
> > - both of the above?
> >
> > Part of the answer to that may lie in the requirements of CRIU, where I
> > strongly suspect they need explicit get/set operations, although these
> > could be gated on CONFIG_CHECKPOINT_RESTORE=y.
>
> Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes.
> No reason to allow explicit access to the key (and selected algo) if
> we don't have to.

Makes sense.

> As for per-thread or not, having a "pick a new key now" prctl() sounds
> good, but I'd like to have an eye toward having it just be "automatic"
> on clone().

I thought about that too, but we're out of clone() flags afaict and there's
no arch hook in there. We could add yet another clone syscall, but yuck (and
I reckon viro would kill us).

Or are you saying that we could infer the behaviour from the existing set
of flags?

Will

2018-10-19 15:56:16

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote:
> On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <[email protected]> wrote:
> > Assuming we want this (Kees -- I was under the impression that everything in
> > Android would end up with the same key otherwise?), then the question is
> > do we want:
> >
> > - prctl() get/set operations for the key, or
> > - prctl() set_random_key operation, or
> > - both of the above?
> >
> > Part of the answer to that may lie in the requirements of CRIU, where I
> > strongly suspect they need explicit get/set operations, although these
> > could be gated on CONFIG_CHECKPOINT_RESTORE=y.
>
> Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes.
> No reason to allow explicit access to the key (and selected algo) if
> we don't have to.

As a minor aside, the PAC algorithm (which can be IMPLEMENTATION
DEFINED) is fixed in HW, and cannot be selected dynamically.

Thus if a process is using pointer authentication, it would not be
possible for CRIU to migrate that process to a CPU with a different PAC
algorithm.

Thanks,
Mark.

2018-10-19 16:07:19

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 19, 2018 at 8:49 AM, Will Deacon <[email protected]> wrote:
> On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote:
>> On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <[email protected]> wrote:
>> > FWIW: I think we should be entertaining a prctl() interface to use a new
>> > key on a per-thread basis. Obviously, this would need to be used with care
>> > (e.g. you'd fork(); use the prctl() and then you'd better not return from
>> > the calling function!).
>> >
>> > Assuming we want this (Kees -- I was under the impression that everything in
>> > Android would end up with the same key otherwise?), then the question is
>> > do we want:
>> >
>> > - prctl() get/set operations for the key, or
>> > - prctl() set_random_key operation, or
>> > - both of the above?
>> >
>> > Part of the answer to that may lie in the requirements of CRIU, where I
>> > strongly suspect they need explicit get/set operations, although these
>> > could be gated on CONFIG_CHECKPOINT_RESTORE=y.
>>
>> Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes.
>> No reason to allow explicit access to the key (and selected algo) if
>> we don't have to.
>
> Makes sense.
>
>> As for per-thread or not, having a "pick a new key now" prctl() sounds
>> good, but I'd like to have an eye toward having it just be "automatic"
>> on clone().
>
> I thought about that too, but we're out of clone() flags afaict and there's
> no arch hook in there. We could add yet another clone syscall, but yuck (and
> I reckon viro would kill us).
>
> Or are you saying that we could infer the behaviour from the existing set
> of flags?

I mean if it's starting a new thread, it should get a new key
automatically, just like the ssp canary happens in dup_task_struct().

(Or did I miss some context for why that's not possible?)

-Kees

--
Kees Cook
Pixel Security

2018-10-19 16:17:35

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 19, 2018 at 09:05:57AM -0700, Kees Cook wrote:
> On Fri, Oct 19, 2018 at 8:49 AM, Will Deacon <[email protected]> wrote:
> > On Fri, Oct 19, 2018 at 08:36:45AM -0700, Kees Cook wrote:
> >> On Fri, Oct 19, 2018 at 4:24 AM, Will Deacon <[email protected]> wrote:
> >> > FWIW: I think we should be entertaining a prctl() interface to use a new
> >> > key on a per-thread basis. Obviously, this would need to be used with care
> >> > (e.g. you'd fork(); use the prctl() and then you'd better not return from
> >> > the calling function!).
> >> >
> >> > Assuming we want this (Kees -- I was under the impression that everything in
> >> > Android would end up with the same key otherwise?), then the question is
> >> > do we want:
> >> >
> >> > - prctl() get/set operations for the key, or
> >> > - prctl() set_random_key operation, or
> >> > - both of the above?
> >> >
> >> > Part of the answer to that may lie in the requirements of CRIU, where I
> >> > strongly suspect they need explicit get/set operations, although these
> >> > could be gated on CONFIG_CHECKPOINT_RESTORE=y.
> >>
> >> Oh CRIU. Yikes. I'd like the get/set to be gated by the CONFIG, yes.
> >> No reason to allow explicit access to the key (and selected algo) if
> >> we don't have to.
> >
> > Makes sense.
> >
> >> As for per-thread or not, having a "pick a new key now" prctl() sounds
> >> good, but I'd like to have an eye toward having it just be "automatic"
> >> on clone().
> >
> > I thought about that too, but we're out of clone() flags afaict and there's
> > no arch hook in there. We could add yet another clone syscall, but yuck (and
> > I reckon viro would kill us).
> >
> > Or are you saying that we could infer the behaviour from the existing set
> > of flags?
>
> I mean if it's starting a new thread, it should get a new key
> automatically, just like the ssp canary happens in dup_task_struct().
>
> (Or did I miss some context for why that's not possible?)

The problem with that is if the child thread (in userspace) returns from
the function that called fork(), then it will explode because the link
register will have been signed with the parent key.

Will

2018-10-19 16:50:10

by Cyrill Gorcunov

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Fri, Oct 19, 2018 at 12:24:04PM +0100, Will Deacon wrote:
>
> FWIW: I think we should be entertaining a prctl() interface to use a new
> key on a per-thread basis. Obviously, this would need to be used with care
> (e.g. you'd fork(); use the prctl() and then you'd better not return from
> the calling function!).
>
> Assuming we want this (Kees -- I was under the impression that everything in
> Android would end up with the same key otherwise?), then the question is
> do we want:
>
> - prctl() get/set operations for the key, or
> - prctl() set_random_key operation, or
> - both of the above?
>
> Part of the answer to that may lie in the requirements of CRIU, where I
> strongly suspect they need explicit get/set operations, although these
> could be gated on CONFIG_CHECKPOINT_RESTORE=y.

Indeed. Without get/set I think we won't be able to restore programs.

2018-10-19 17:46:34

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On Fri, Oct 19, 2018 at 04:10:29PM +0100, Catalin Marinas wrote:
> On Fri, Oct 19, 2018 at 03:42:23PM +0100, Kristina Martsenko wrote:
> > On 19/10/2018 12:35, Catalin Marinas wrote:
> > > On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
> > >> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
> > >>> On 05/10/2018 09:47, Kristina Martsenko wrote:
> > >>>> +Virtualization
> > >>>> +--------------
> > >>>> +
> > >>>> +Pointer authentication is not currently supported in KVM guests. KVM
> > >>>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
> > >>>> +the feature will result in an UNDEFINED exception being injected into
> > >>>> +the guest.
> > >>>
> > >>> However applications using instructions from the hint space will
> > >>> continue to work albeit without any protection (as they would just be
> > >>> nops) ?
> > >>
> > >> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
> > >> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
> > >> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
> > >> will trap and KVM will inject an "Unknown reason" exception into the
> > >> guest (which will cause a Linux guest to send a SIGILL to the application).
> > >
> > > I think that part is fine. If KVM (a fairly recent version with CPUID
> > > sanitisation) does not enable ptr auth, the CPUID should not advertise
> > > this feature either so the guest kernel should not enable it. For the
> > > above instructions in the HINT space, they will just be NOPs. If the
> > > guest kernel enables the feature regardless of the CPUID information, it
> > > deserves to get an "Unknown reason" exception.
> > >
> > >> In the latter case we could instead pretend the instruction was a NOP
> > >> and not inject an exception, but trapping twice per every function would
> > >> probably be terrible for performance. The guest shouldn't be setting
> > >> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
> > >> not present (because KVM has hidden it).
> > >
> > > I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
> > > that the feature is present via CPUID.
> > >
> > >> The other special case is the XPACLRI instruction, which is also in the
> > >> HINT space. Currently it will trap and KVM will inject an exception into
> > >> the guest. We should probably change this to NOP instead, as that's what
> > >> applications will expect. Unfortunately there is no EnIA-like control to
> > >> make it NOP.
> > >
> > > Very good catch. Basically if EL2 doesn't know about ptr auth (older
> > > distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> > > CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> > > to have ptr auth enabled, being built for the specific HW). So a user
> > > app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> > > (injected by the guest kernel following the injection of "Unknown
> > > reason" exception by KVM).
> > >
> > > Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> > > Could we restrict it to only being used at run-time if the corresponding
> > > HWCAP is set? This means redefining this instruction as no longer in the
> > > NOP space.
> >
> > I think an alternative solution is to just disable trapping of pointer
> > auth instructions in KVM. This will mean that the instructions will
> > behave the same in the guest as they do in the host. HINT-space
> > instructions (including XPACLRI) will behave as NOPs (or perform their
> > function, if enabled by the guest), and will not trap.
>
> OK, so this means disabling the trap (during early EL2 setup) but still
> sanitizing the CPUID not to report the feature to EL1 unless fully
> supported on all CPUs.

... which is perfectly sensible, but not actually my main concern here.
I'm worried about the possibility of distributions shipping *now* with
userspace that's built with these instructions. That stuff is going to
break if/when it encounters v8.3 hardware, and I don't think we can do
much about it other than alert them to the potential issue.

Will

2018-10-23 08:38:11

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On 19/10/2018 12:15, Catalin Marinas wrote:
> On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote:
>> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
>> new file mode 100644
>> index 000000000000..2aefedc31d9e
>> --- /dev/null
>> +++ b/arch/arm64/include/asm/pointer_auth.h
>> @@ -0,0 +1,63 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +#ifndef __ASM_POINTER_AUTH_H
>> +#define __ASM_POINTER_AUTH_H
>> +
>> +#include <linux/random.h>
>> +
>> +#include <asm/cpufeature.h>
>> +#include <asm/sysreg.h>
>> +
>> +#ifdef CONFIG_ARM64_PTR_AUTH
>> +/*
>> + * Each key is a 128-bit quantity which is split across a pair of 64-bit
>> + * registers (Lo and Hi).
>> + */
>> +struct ptrauth_key {
>> + unsigned long lo, hi;
>> +};
>> +
>> +/*
>> + * We give each process its own instruction A key (APIAKey), which is shared by
>> + * all threads. This is inherited upon fork(), and reinitialised upon exec*().
>> + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
>> + * instructions behaving as NOPs.
>> + */
>
> I don't remember the past discussions but I assume the tools guys are ok
> with a single key shared by multiple threads. Ramana, could you ack this
> part, FTR?

Sorry about the slow response, I've been traveling.

Ack and Will's response covers the reasons why pretty well. A prctl call
would be a good enhancement.

regards
Ramana

2018-10-23 08:40:47

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

On 19/10/2018 13:36, Will Deacon wrote:
> On Fri, Oct 05, 2018 at 09:47:37AM +0100, Kristina Martsenko wrote:
>> 1) Key support
>>
>> This series enables the use of instructions using APIAKey, which is
>> initialised and maintained per-process (shared by all threads). GCC
>> currently only makes use of APIAKey.
>>
>> This series does not add support for APIBKey, APDAKey, APDBKey, nor
>> APGAKey. HINT-space instructions using these keys will currently execute
>> as NOPs. Support for these keys can be added as users appear.
>>
>> Note that while we expose the cpuid register (ID_AA64ISAR1_EL1) to
>> userspace, it only contains one feature for address authentication
>> (API/APA), so it cannot be used by userspace to tell which keys the
>> kernel supports. For this the kernel exposes HWCAP bits, one per key
>> (currently only APIAKey), which must be checked instead.
>
> Given that the architecture doesn't provide an identification mechanism
> for the case where only one of the keys is available, I would much prefer
> that we expose both of the keys to userspace. Is the only downside of
> that a possible exception entry overhead if the kernel wants to use pointer
> authentication as well?
>
> Having an initial implementation where the B key operations act as NOPs
> isn't ideal if we want to support future users -- chances are they'll
> be put off because deployed kernels don't give them whatever security
> guarantees they require. It's a bit of a chicken-and-egg problem, so
> unless we have good reasons to keep the B key hidden, I think we should
> be exposing it from the start.

There are patches in flight to get B key signing support in for GCC 9 -
so exposing this to user space will be good.

Ramana

>
> Will
>

2018-10-23 10:23:32

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

On Tue, Oct 23, 2018 at 09:36:16AM +0100, Ramana Radhakrishnan wrote:
> On 19/10/2018 12:15, Catalin Marinas wrote:
> > On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote:
> >> diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> >> new file mode 100644
> >> index 000000000000..2aefedc31d9e
> >> --- /dev/null
> >> +++ b/arch/arm64/include/asm/pointer_auth.h
> >> @@ -0,0 +1,63 @@
> >> +// SPDX-License-Identifier: GPL-2.0
> >> +#ifndef __ASM_POINTER_AUTH_H
> >> +#define __ASM_POINTER_AUTH_H
> >> +
> >> +#include <linux/random.h>
> >> +
> >> +#include <asm/cpufeature.h>
> >> +#include <asm/sysreg.h>
> >> +
> >> +#ifdef CONFIG_ARM64_PTR_AUTH
> >> +/*
> >> + * Each key is a 128-bit quantity which is split across a pair of 64-bit
> >> + * registers (Lo and Hi).
> >> + */
> >> +struct ptrauth_key {
> >> + unsigned long lo, hi;
> >> +};
> >> +
> >> +/*
> >> + * We give each process its own instruction A key (APIAKey), which is shared by
> >> + * all threads. This is inherited upon fork(), and reinitialised upon exec*().
> >> + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
> >> + * instructions behaving as NOPs.
> >> + */
> >
> > I don't remember the past discussions but I assume the tools guys are ok
> > with a single key shared by multiple threads. Ramana, could you ack this
> > part, FTR?
>
> Sorry about the slow response, I've been traveling.
>
> Ack and Will's response covers the reasons why pretty well. A prctl call
> would be a good enhancement.

One minor "gotcha" with that is that the glibc prctl() wrapper would need to
be annotated not to use pointer auth, or we'd have to issue the syscall
in-line.

Will

2018-10-24 10:57:33

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On 19/10/2018 12:35, Catalin Marinas wrote:
> On Tue, Oct 16, 2018 at 05:14:39PM +0100, Kristina Martsenko wrote:
>> On 05/10/2018 10:04, Ramana Radhakrishnan wrote:
>>> On 05/10/2018 09:47, Kristina Martsenko wrote:
>>>> +Virtualization
>>>> +--------------
>>>> +
>>>> +Pointer authentication is not currently supported in KVM guests. KVM
>>>> +will mask the feature bits from ID_AA64ISAR1_EL1, and attempted use of
>>>> +the feature will result in an UNDEFINED exception being injected into
>>>> +the guest.
>>>
>>> However applications using instructions from the hint space will
>>> continue to work albeit without any protection (as they would just be
>>> nops) ?
>>
>> Mostly, yes. If the guest leaves SCTLR_EL1.EnIA unset (and
>> EnIB/EnDA/EnDB), then PAC* and AUT* instructions in the HINT space will
>> execute as NOPs. If the guest sets EnIA, then PAC*/AUT* instructions
>> will trap and KVM will inject an "Unknown reason" exception into the
>> guest (which will cause a Linux guest to send a SIGILL to the application).
>
> I think that part is fine. If KVM (a fairly recent version with CPUID
> sanitisation) does not enable ptr auth, the CPUID should not advertise
> this feature either so the guest kernel should not enable it. For the
> above instructions in the HINT space, they will just be NOPs. If the
> guest kernel enables the feature regardless of the CPUID information, it
> deserves to get an "Unknown reason" exception.
>
>> In the latter case we could instead pretend the instruction was a NOP
>> and not inject an exception, but trapping twice per every function would
>> probably be terrible for performance. The guest shouldn't be setting
>> EnIA anyway if ID_AA64ISAR1_EL1 reports that pointer authentication is
>> not present (because KVM has hidden it).
>
> I don't think we should. The SCTLR_EL1 bits are RES0 unless you know
> that the feature is present via CPUID.
>
>> The other special case is the XPACLRI instruction, which is also in the
>> HINT space. Currently it will trap and KVM will inject an exception into
>> the guest. We should probably change this to NOP instead, as that's what
>> applications will expect. Unfortunately there is no EnIA-like control to
>> make it NOP.
>
> Very good catch. Basically if EL2 doesn't know about ptr auth (older
> distro), EL1 may or may not know but leaves SCTLR_EL1 disabled (based on
> CPUID), the default HCR_EL2 is to trap (I'm ignoring EL3 as that's like
> to have ptr auth enabled, being built for the specific HW). So a user
> app considering XPACLRI a NOP (or inoffensive) will get a SIGILL
> (injected by the guest kernel following the injection of "Unknown
> reason" exception by KVM).
>
> Ramana, is XPACLRI commonly generated by gcc and expects it to be a NOP?
> Could we restrict it to only being used at run-time if the corresponding
> HWCAP is set? This means redefining this instruction as no longer in the
> NOP space.

Sorry to have missed this - I'm still catching up on email.

XPACLRI is used in the unwinder in exactly 2 places but not for
unwinding itself but for storing the actual return address in the data
structures, its not something I expect to be used very commonly so a
check there seems reasonable. The xpaclri is considered a nop in the
architecture as it is defined today. I don't like the idea of redefining
instructions as not in the nop space after it's been defined as being
so. We could investigate guarding the XPACLRI with a check with the
HWCAP. How many unwinders would you like us to fix ?

>
>> One option is for KVM to pretend the instruction was a NOP and return to
>> the guest. But if XPACLRI gets executed frequently, then the constant
>> trapping might hurt performance. I don't know how frequently it might
>> get used, as I don't know of any applications currently using it. From
>> what I understand, it may be used by userspace stack unwinders.

Yep. Probably one instruction per frame being unwound during exception
unwinding. And no trapping will be expensive even though it's *only* in
the exception unwind case.

>>
>> (Also worth noting - as far as I can tell there is no easy way for KVM
>> to know which pointer authentication instruction caused the trap, so we
>> may have to do something unusual like use "at s12e1r" to read guest
>> memory and check for XPACLRI.)
>
> Indeed, it's not an easy fix. As discussed (in the office), we can't
> even guarantee that the guest stage 1 translation is stable and points
> to the actual XPACLRI instruction.
>
>> The other option is to turn off trapping entirely. However then on a
>> big.LITTLE system with mismatched pointer authentication support
>> instructions will work intermittently on some CPUs but not others.
>
> That's another case but let's assume we never see such configurations ;).

That's a broken system by design :) !

Ramana
>

2018-11-02 06:02:46

by Jon Masters

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

On 10/19/18 1:45 PM, Will Deacon wrote:

>>> I think an alternative solution is to just disable trapping of pointer
>>> auth instructions in KVM. This will mean that the instructions will
>>> behave the same in the guest as they do in the host. HINT-space
>>> instructions (including XPACLRI) will behave as NOPs (or perform their
>>> function, if enabled by the guest), and will not trap.
>>
>> OK, so this means disabling the trap (during early EL2 setup) but still
>> sanitizing the CPUID not to report the feature to EL1 unless fully
>> supported on all CPUs.
>
> ... which is perfectly sensible, but not actually my main concern here.
> I'm worried about the possibility of distributions shipping *now* with
> userspace that's built with these instructions. That stuff is going to
> break if/when it encounters v8.3 hardware, and I don't think we can do
> much about it other than alert them to the potential issue.

FYI tracking this for RHEL. It's not a problem currently. I'll alert our
tools teams to hold off on any PAC work until this is figured out.

Jon.

--
Computer Architect | Sent with my Fedora powered laptop

2018-11-02 09:49:05

[permalink] [raw]

Subject: Re: [PATCH v5 11/17] arm64: docs: document pointer authentication

>> +
>> +When CONFIG_ARM64_PTR_AUTH is selected, and relevant HW support is
>> +present, the kernel will assign a random APIAKey value to each process
>> +at exec*() time. This key is shared by all threads within the process,
>> +and the key is preserved across fork(). Presence of functionality using
>> +APIAKey is advertised via HWCAP_APIA.
>
> It might be useful to include documentation here on how many bits of
> the address are being used for the PAC bits (I'm assuming it's 7?)

the number of bits available depends on the VA size, so you can't really
document a number.

Ramana

>
> -Kees
>

2018-11-13 16:19:44

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

(Sorry for the really late response!)

On 15/10/2018 23:42, Kees Cook wrote:
> On Fri, Oct 5, 2018 at 1:47 AM, Kristina Martsenko
> <[email protected]> wrote:
>> This series adds support for the ARMv8.3 pointer authentication
>> extension. The series contains Mark's original patches to enable pointer
>> authentication for userspace [1], followed by early RFC patches using
>> pointer authentication in the kernel.
>
> It wasn't obvious to me where the PAC mismatch exceptions will be
> caught. I'm mainly curious to compare the PAC exception handling to
> the existing stack-protector panic(). Can you point me to which
> routines manage that? (Perhaps I just missed it in the series...)

When the PAC authentication fails, it doesn't actually generate an
exception, it just flips a bit in the high-order bits of the pointer,
making the pointer invalid. Then when the pointer is dereferenced (e.g.
as a function return address), it generates the usual type of exception
for an invalid address.

So when a function return fails in user mode, the exception is handled
in __do_user_fault and a forced SIGSEGV is delivered to the task. When a
function return fails in kernel mode, the exception is handled in
__do_kernel_fault and the task is killed.

This is different from stack protector as we don't panic the kernel, we
just kill the task. It would be difficult to panic as we don't have a
reliable way of knowing that the exception was caused by a PAC
authentication failure (we just have an invalid pointer with a specific
bit flipped). We also don't print out any PAC-related warning.

Thanks,
Kristina

2018-11-13 23:10:58

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

On Tue, Nov 13, 2018 at 10:17 AM, Kristina Martsenko
<[email protected]> wrote:
> When the PAC authentication fails, it doesn't actually generate an
> exception, it just flips a bit in the high-order bits of the pointer,
> making the pointer invalid. Then when the pointer is dereferenced (e.g.
> as a function return address), it generates the usual type of exception
> for an invalid address.

Ah! Okay, thanks. I missed that detail. :)

What area of memory ends up being addressable with such bit flips?
(i.e. is the kernel making sure nothing executable ends up there?)

> So when a function return fails in user mode, the exception is handled
> in __do_user_fault and a forced SIGSEGV is delivered to the task. When a
> function return fails in kernel mode, the exception is handled in
> __do_kernel_fault and the task is killed.
>
> This is different from stack protector as we don't panic the kernel, we
> just kill the task. It would be difficult to panic as we don't have a
> reliable way of knowing that the exception was caused by a PAC
> authentication failure (we just have an invalid pointer with a specific
> bit flipped). We also don't print out any PAC-related warning.

There are other "guesses" in __do_kernel_fault(), I think? Could a
"PAC mismatch?" warning be included in the Oops if execution fails in
the address range that PAC failures would resolve into?

-Kees

--
Kees Cook

2018-11-14 15:55:31

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

On 13/11/2018 23:09, Kees Cook wrote:
> On Tue, Nov 13, 2018 at 10:17 AM, Kristina Martsenko
> <[email protected]> wrote:
>> When the PAC authentication fails, it doesn't actually generate an
>> exception, it just flips a bit in the high-order bits of the pointer,
>> making the pointer invalid. Then when the pointer is dereferenced (e.g.
>> as a function return address), it generates the usual type of exception
>> for an invalid address.
>
> Ah! Okay, thanks. I missed that detail. :)
>
> What area of memory ends up being addressable with such bit flips?
> (i.e. is the kernel making sure nothing executable ends up there?)

The address will be in between the user and kernel address ranges, so
it's guaranteed to be invalid and not address any memory.

Specifically, assuming a 48-bit VA configuration, user addresses must
have bits [55:48] clear and kernel addresses must have bits [63:48] set.
When authentication fails it will set two bits in those ranges to "10"
or "01", ensuring that the address is no longer a valid user or kernel
address.

>> So when a function return fails in user mode, the exception is handled
>> in __do_user_fault and a forced SIGSEGV is delivered to the task. When a
>> function return fails in kernel mode, the exception is handled in
>> __do_kernel_fault and the task is killed.
>>
>> This is different from stack protector as we don't panic the kernel, we
>> just kill the task. It would be difficult to panic as we don't have a
>> reliable way of knowing that the exception was caused by a PAC
>> authentication failure (we just have an invalid pointer with a specific
>> bit flipped). We also don't print out any PAC-related warning.
>
> There are other "guesses" in __do_kernel_fault(), I think? Could a
> "PAC mismatch?" warning be included in the Oops if execution fails in
> the address range that PAC failures would resolve into?

Sounds reasonable to me, I'll add a warning.

Thanks,
Kristina

2018-11-14 18:12:45

[permalink] [raw]

Subject: Re: [PATCH v5 07/17] arm64: add basic pointer authentication support

Hi all,

On Fri, Oct 19, 2018 at 12:24:04PM +0100, Will Deacon wrote:
> On Fri, Oct 19, 2018 at 12:15:43PM +0100, Catalin Marinas wrote:
> > On Fri, Oct 05, 2018 at 09:47:44AM +0100, Kristina Martsenko wrote:
> > > diff --git a/arch/arm64/include/asm/pointer_auth.h b/arch/arm64/include/asm/pointer_auth.h
> > > new file mode 100644
> > > index 000000000000..2aefedc31d9e
> > > --- /dev/null
> > > +++ b/arch/arm64/include/asm/pointer_auth.h
> > > @@ -0,0 +1,63 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +#ifndef __ASM_POINTER_AUTH_H
> > > +#define __ASM_POINTER_AUTH_H
> > > +
> > > +#include <linux/random.h>
> > > +
> > > +#include <asm/cpufeature.h>
> > > +#include <asm/sysreg.h>
> > > +
> > > +#ifdef CONFIG_ARM64_PTR_AUTH
> > > +/*
> > > + * Each key is a 128-bit quantity which is split across a pair of 64-bit
> > > + * registers (Lo and Hi).
> > > + */
> > > +struct ptrauth_key {
> > > + unsigned long lo, hi;
> > > +};
> > > +
> > > +/*
> > > + * We give each process its own instruction A key (APIAKey), which is shared by
> > > + * all threads. This is inherited upon fork(), and reinitialised upon exec*().
> > > + * All other keys are currently unused, with APIBKey, APDAKey, and APBAKey
> > > + * instructions behaving as NOPs.
> > > + */
> >
> > I don't remember the past discussions but I assume the tools guys are ok
> > with a single key shared by multiple threads. Ramana, could you ack this
> > part, FTR?
> >
> > (and it would help if someone from the Android and Chrome camps can
> > confirm)
>
> FWIW: I think we should be entertaining a prctl() interface to use a new
> key on a per-thread basis. Obviously, this would need to be used with care
> (e.g. you'd fork(); use the prctl() and then you'd better not return from
> the calling function!).
>
> Assuming we want this (Kees -- I was under the impression that everything in
> Android would end up with the same key otherwise?), then the question is
> do we want:
>
> - prctl() get/set operations for the key, or
> - prctl() set_random_key operation, or
> - both of the above?
>
> Part of the answer to that may lie in the requirements of CRIU, where I
> strongly suspect they need explicit get/set operations, although these
> could be gated on CONFIG_CHECKPOINT_RESTORE=y.

I managed to speak to the CRIU developers at LPC. The good news is that
their preference is for a ptrace()-based interface for getting and setting
the keys, so the only prctl() operation we need is to set a random key
(separately for A and B).

Will

2018-11-14 21:48:10

[permalink] [raw]

Subject: Re: [PATCH 00/17] ARMv8.3 pointer authentication support

On Tue, Nov 13, 2018 at 05:09:00PM -0600, Kees Cook wrote:
> On Tue, Nov 13, 2018 at 10:17 AM, Kristina Martsenko
> <[email protected]> wrote:
> > When the PAC authentication fails, it doesn't actually generate an
> > exception, it just flips a bit in the high-order bits of the pointer,
> > making the pointer invalid. Then when the pointer is dereferenced (e.g.
> > as a function return address), it generates the usual type of exception
> > for an invalid address.
>
> Ah! Okay, thanks. I missed that detail. :)
>
> What area of memory ends up being addressable with such bit flips?
> (i.e. is the kernel making sure nothing executable ends up there?)
>
> > So when a function return fails in user mode, the exception is handled
> > in __do_user_fault and a forced SIGSEGV is delivered to the task. When a
> > function return fails in kernel mode, the exception is handled in
> > __do_kernel_fault and the task is killed.
> >
> > This is different from stack protector as we don't panic the kernel, we
> > just kill the task. It would be difficult to panic as we don't have a
> > reliable way of knowing that the exception was caused by a PAC
> > authentication failure (we just have an invalid pointer with a specific
> > bit flipped). We also don't print out any PAC-related warning.
>
> There are other "guesses" in __do_kernel_fault(), I think? Could a
> "PAC mismatch?" warning be included in the Oops if execution fails in
> the address range that PAC failures would resolve into?

I'd personally prefer that we didn't try to guess if a fault is due to a failed
AUT*, even for logging.

Presently, it's not possible to distinguish between a fault resulting from a
failed AUT* and a fault which happens to have hte same bits/clear, so there are
false positives. The architecture may also change the precise details of the
faulting address, and we'd have false negatives in that case.

Given that, I think suggesting that a fault is due to a failed AUT* is liable
to make things more confusing.

Thanks,
Mark.

2018-11-14 22:49:06