| Message ID | 1586842314-19527-3-git-send-email-amit.kachhap@arm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: add Armv8.6 pointer authentication | expand |
On Tue, Apr 14, 2020 at 11:01:52AM +0530, Amit Daniel Kachhap wrote: > This patch add changes for Pointer Authentication enhanced features > mandatory for Armv8.6. These features are, > > * Uses an enhanced PAC generation logic which hardens finding the correct > PAC value of the authenticated pointer. However, no code change done > for this. > > * Fault(FPAC) is generated now when the ptrauth authentication instruction > fails in authenticating the PAC present in the address. This is different > from earlier case when such failures just adds an error code in the top > byte and waits for subsequent load/store to abort. The ptrauth > instructions which may cause this fault are autiasp, retaa etc. > > The above features are now represented by additional configurations > for the Address Authentication cpufeature. > > The fault received in the kernel due to FPAC is treated as Illegal > instruction and hence signal SIGILL is injected with ILL_ILLOPN as the > signal code. Note that this is different from earlier ARMv8.3 ptrauth > where signal SIGSEGV is issued due to Pointer authentication failures. Sorry if it was discussed before. Was there any reasoning behind choosing ILL_ILLOPN vs something else like ILL_ILLADR? > diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c > index cf402be5c573..0ef9e9880194 100644 > --- a/arch/arm64/kernel/traps.c > +++ b/arch/arm64/kernel/traps.c > @@ -411,6 +411,23 @@ void do_undefinstr(struct pt_regs *regs) > } > NOKPROBE_SYMBOL(do_undefinstr); > > +void do_ptrauth_fault(struct pt_regs *regs, unsigned long esr) > +{ > + const char *desc; > + > + BUG_ON(!user_mode(regs)); > + > + /* Even if we chose not to use PTRAUTH, the hardware might still trap */ > + if (unlikely(!(system_supports_address_auth()))) { Nitpick: no need for braces around system_supports_address_auth(). > + force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc); > + return; > + } So when do we execute this path? Is it on a big.LITTLE system where some CPUs don't have the 8.6 behaviour? It's the same AUT instruction that triggered it, so I don't think we should report a different ILL code. It's a bit unfortunate that this new ptrauth feature doesn't have an opt-in, so user-space would have to cope with both behaviours. In this case I don't see why we need to differentiate on system_supports_address_auth(). While the new behaviour is a lot more useful in practice, I wonder whether we could still emulate the old one by setting regs->pc to a faulting address and returning to user. > + > + desc = "pointer authentication fault"; > + arm64_notify_die(desc, regs, SIGILL, ILL_ILLOPN, (void __user *)regs->pc, esr); Nitpick: you could pass the string directly, no need for an additional variable.
Hi, On 5/6/20 10:01 PM, Catalin Marinas wrote: > On Tue, Apr 14, 2020 at 11:01:52AM +0530, Amit Daniel Kachhap wrote: >> This patch add changes for Pointer Authentication enhanced features >> mandatory for Armv8.6. These features are, >> >> * Uses an enhanced PAC generation logic which hardens finding the correct >> PAC value of the authenticated pointer. However, no code change done >> for this. >> >> * Fault(FPAC) is generated now when the ptrauth authentication instruction >> fails in authenticating the PAC present in the address. This is different >> from earlier case when such failures just adds an error code in the top >> byte and waits for subsequent load/store to abort. The ptrauth >> instructions which may cause this fault are autiasp, retaa etc. >> >> The above features are now represented by additional configurations >> for the Address Authentication cpufeature. >> >> The fault received in the kernel due to FPAC is treated as Illegal >> instruction and hence signal SIGILL is injected with ILL_ILLOPN as the >> signal code. Note that this is different from earlier ARMv8.3 ptrauth >> where signal SIGSEGV is issued due to Pointer authentication failures. > > Sorry if it was discussed before. Was there any reasoning behind > choosing ILL_ILLOPN vs something else like ILL_ILLADR? No it was not discussed earlier. I used it as I thought that autiasp failed here due to incorrect operands provided (sp, key, lr). Although sp and lr are addresses. > >> diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c >> index cf402be5c573..0ef9e9880194 100644 >> --- a/arch/arm64/kernel/traps.c >> +++ b/arch/arm64/kernel/traps.c >> @@ -411,6 +411,23 @@ void do_undefinstr(struct pt_regs *regs) >> } >> NOKPROBE_SYMBOL(do_undefinstr); >> >> +void do_ptrauth_fault(struct pt_regs *regs, unsigned long esr) >> +{ >> + const char *desc; >> + >> + BUG_ON(!user_mode(regs)); >> + >> + /* Even if we chose not to use PTRAUTH, the hardware might still trap */ >> + if (unlikely(!(system_supports_address_auth()))) { > > Nitpick: no need for braces around system_supports_address_auth(). ok > >> + force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc); >> + return; >> + } > > So when do we execute this path? Is it on a big.LITTLE system where some > CPUs don't have the 8.6 behaviour? It's the same AUT instruction that > triggered it, so I don't think we should report a different ILL code. > > It's a bit unfortunate that this new ptrauth feature doesn't have an > opt-in, so user-space would have to cope with both behaviours. In this > case I don't see why we need to differentiate on > system_supports_address_auth(). I was referring to some similar checks present in do_sve_acc in file arch/arm64/kernel/fpsimd.c to gaurd some unknown situations. Anyway I should probably drop this as EC value is already matched. > > While the new behaviour is a lot more useful in practice, I wonder > whether we could still emulate the old one by setting regs->pc to a > faulting address and returning to user. However even if we set regs->pc to the faulting lr address but this lr may not be same as earlier one as theoretically there can be two autia instructions so I am not sure if complete emulation is possible. Also other work will be change ESR value, set error pattern in the faulting address etc when the error pattern is itself not defined. > >> + >> + desc = "pointer authentication fault"; >> + arm64_notify_die(desc, regs, SIGILL, ILL_ILLOPN, (void __user *)regs->pc, esr); > > Nitpick: you could pass the string directly, no need for an additional > variable. ok Amit >
On Thu, May 07, 2020 at 08:58:51PM +0530, Amit Kachhap wrote: > On 5/6/20 10:01 PM, Catalin Marinas wrote: > > On Tue, Apr 14, 2020 at 11:01:52AM +0530, Amit Daniel Kachhap wrote: > > > This patch add changes for Pointer Authentication enhanced features > > > mandatory for Armv8.6. These features are, > > > > > > * Uses an enhanced PAC generation logic which hardens finding the correct > > > PAC value of the authenticated pointer. However, no code change done > > > for this. > > > > > > * Fault(FPAC) is generated now when the ptrauth authentication instruction > > > fails in authenticating the PAC present in the address. This is different > > > from earlier case when such failures just adds an error code in the top > > > byte and waits for subsequent load/store to abort. The ptrauth > > > instructions which may cause this fault are autiasp, retaa etc. > > > > > > The above features are now represented by additional configurations > > > for the Address Authentication cpufeature. > > > > > > The fault received in the kernel due to FPAC is treated as Illegal > > > instruction and hence signal SIGILL is injected with ILL_ILLOPN as the > > > signal code. Note that this is different from earlier ARMv8.3 ptrauth > > > where signal SIGSEGV is issued due to Pointer authentication failures. [...] > > While the new behaviour is a lot more useful in practice, I wonder > > whether we could still emulate the old one by setting regs->pc to a > > faulting address and returning to user. > > However even if we set regs->pc to the faulting lr address but this lr > may not be same as earlier one as theoretically there can be two autia > instructions so I am not sure if complete emulation is possible. Also other > work will be change ESR value, set error pattern in the faulting address etc > when the error pattern is itself not defined. You are right, we can't fully emulate this. I was thinking of getting a fake faulting PC and returning to it so that we trigger a SIGSEGV. Anyway, I don't think it buys us anything and I really doubt we have user space relying on the 8.3 behaviour.
diff --git a/arch/arm64/include/asm/esr.h b/arch/arm64/include/asm/esr.h index 6a395a7e6707..f4faff45edd1 100644 --- a/arch/arm64/include/asm/esr.h +++ b/arch/arm64/include/asm/esr.h @@ -35,7 +35,9 @@ #define ESR_ELx_EC_SYS64 (0x18) #define ESR_ELx_EC_SVE (0x19) #define ESR_ELx_EC_ERET (0x1a) /* EL2 only */ -/* Unallocated EC: 0x1b - 0x1E */ +/* Unallocated EC: 0x1B */ +#define ESR_ELx_EC_FPAC (0x1C) /* EL1 and above */ +/* Unallocated EC: 0x1D - 0x1E */ #define ESR_ELx_EC_IMP_DEF (0x1f) /* EL3 only */ #define ESR_ELx_EC_IABT_LOW (0x20) #define ESR_ELx_EC_IABT_CUR (0x21) diff --git a/arch/arm64/include/asm/exception.h b/arch/arm64/include/asm/exception.h index 7a6e81ca23a8..de76772fac81 100644 --- a/arch/arm64/include/asm/exception.h +++ b/arch/arm64/include/asm/exception.h @@ -46,4 +46,5 @@ void bad_el0_sync(struct pt_regs *regs, int reason, unsigned int esr); void do_cp15instr(unsigned int esr, struct pt_regs *regs); void do_el0_svc(struct pt_regs *regs); void do_el0_svc_compat(struct pt_regs *regs); +void do_ptrauth_fault(struct pt_regs *regs, unsigned long esr); #endif /* __ASM_EXCEPTION_H */ diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h index ebc622432831..53904b968e5d 100644 --- a/arch/arm64/include/asm/sysreg.h +++ b/arch/arm64/include/asm/sysreg.h @@ -621,14 +621,22 @@ #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 +#define ID_AA64ISAR1_APA_NI 0x0 +#define ID_AA64ISAR1_APA_ARCHITECTED 0x1 +#define ID_AA64ISAR1_APA_ARCH_EPAC 0x2 +#define ID_AA64ISAR1_APA_ARCH_EPAC2 0x3 +#define ID_AA64ISAR1_APA_ARCH_EPAC2_FPAC 0x4 +#define ID_AA64ISAR1_APA_ARCH_EPAC2_FPAC_CMB 0x5 +#define ID_AA64ISAR1_API_NI 0x0 +#define ID_AA64ISAR1_API_IMP_DEF 0x1 +#define ID_AA64ISAR1_API_IMP_DEF_EPAC 0x2 +#define ID_AA64ISAR1_API_IMP_DEF_EPAC2 0x3 +#define ID_AA64ISAR1_API_IMP_DEF_EPAC2_FPAC 0x4 +#define ID_AA64ISAR1_API_IMP_DEF_EPAC2_FPAC_CMB 0x5 +#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 diff --git a/arch/arm64/kernel/entry-common.c b/arch/arm64/kernel/entry-common.c index c839b5bf1904..870ff488708b 100644 --- a/arch/arm64/kernel/entry-common.c +++ b/arch/arm64/kernel/entry-common.c @@ -15,6 +15,7 @@ #include <asm/exception.h> #include <asm/kprobes.h> #include <asm/mmu.h> +#include <asm/pointer_auth.h> #include <asm/sysreg.h> static void notrace el1_abort(struct pt_regs *regs, unsigned long esr) @@ -66,6 +67,13 @@ static void notrace el1_dbg(struct pt_regs *regs, unsigned long esr) } NOKPROBE_SYMBOL(el1_dbg); +static void notrace el1_fpac(struct pt_regs *regs, unsigned long esr) +{ + local_daif_inherit(regs); + do_ptrauth_fault(regs, esr); +} +NOKPROBE_SYMBOL(el1_fpac); + asmlinkage void notrace el1_sync_handler(struct pt_regs *regs) { unsigned long esr = read_sysreg(esr_el1); @@ -92,6 +100,9 @@ asmlinkage void notrace el1_sync_handler(struct pt_regs *regs) case ESR_ELx_EC_BRK64: el1_dbg(regs, esr); break; + case ESR_ELx_EC_FPAC: + el1_fpac(regs, esr); + break; default: el1_inv(regs, esr); }; @@ -219,6 +230,14 @@ static void notrace el0_svc(struct pt_regs *regs) } NOKPROBE_SYMBOL(el0_svc); +static void notrace el0_fpac(struct pt_regs *regs, unsigned long esr) +{ + user_exit_irqoff(); + local_daif_restore(DAIF_PROCCTX); + do_ptrauth_fault(regs, esr); +} +NOKPROBE_SYMBOL(el0_fpac); + asmlinkage void notrace el0_sync_handler(struct pt_regs *regs) { unsigned long esr = read_sysreg(esr_el1); @@ -261,6 +280,9 @@ asmlinkage void notrace el0_sync_handler(struct pt_regs *regs) case ESR_ELx_EC_BRK64: el0_dbg(regs, esr); break; + case ESR_ELx_EC_FPAC: + el0_fpac(regs, esr); + break; default: el0_inv(regs, esr); } @@ -324,6 +346,9 @@ asmlinkage void notrace el0_sync_compat_handler(struct pt_regs *regs) case ESR_ELx_EC_BKPT32: el0_dbg(regs, esr); break; + case ESR_ELx_EC_FPAC: + el0_fpac(regs, esr); + break; default: el0_inv(regs, esr); } diff --git a/arch/arm64/kernel/traps.c b/arch/arm64/kernel/traps.c index cf402be5c573..0ef9e9880194 100644 --- a/arch/arm64/kernel/traps.c +++ b/arch/arm64/kernel/traps.c @@ -411,6 +411,23 @@ void do_undefinstr(struct pt_regs *regs) } NOKPROBE_SYMBOL(do_undefinstr); +void do_ptrauth_fault(struct pt_regs *regs, unsigned long esr) +{ + const char *desc; + + BUG_ON(!user_mode(regs)); + + /* Even if we chose not to use PTRAUTH, the hardware might still trap */ + if (unlikely(!(system_supports_address_auth()))) { + force_signal_inject(SIGILL, ILL_ILLOPC, regs->pc); + return; + } + + desc = "pointer authentication fault"; + arm64_notify_die(desc, regs, SIGILL, ILL_ILLOPN, (void __user *)regs->pc, esr); +} +NOKPROBE_SYMBOL(do_ptrauth_fault); + #define __user_cache_maint(insn, address, res) \ if (address >= user_addr_max()) { \ res = -EFAULT; \ @@ -763,6 +780,7 @@ static const char *esr_class_str[] = { [ESR_ELx_EC_SYS64] = "MSR/MRS (AArch64)", [ESR_ELx_EC_SVE] = "SVE", [ESR_ELx_EC_ERET] = "ERET/ERETAA/ERETAB", + [ESR_ELx_EC_FPAC] = "FPAC", [ESR_ELx_EC_IMP_DEF] = "EL3 IMP DEF", [ESR_ELx_EC_IABT_LOW] = "IABT (lower EL)", [ESR_ELx_EC_IABT_CUR] = "IABT (current EL)",
This patch add changes for Pointer Authentication enhanced features mandatory for Armv8.6. These features are, * Uses an enhanced PAC generation logic which hardens finding the correct PAC value of the authenticated pointer. However, no code change done for this. * Fault(FPAC) is generated now when the ptrauth authentication instruction fails in authenticating the PAC present in the address. This is different from earlier case when such failures just adds an error code in the top byte and waits for subsequent load/store to abort. The ptrauth instructions which may cause this fault are autiasp, retaa etc. The above features are now represented by additional configurations for the Address Authentication cpufeature. The fault received in the kernel due to FPAC is treated as Illegal instruction and hence signal SIGILL is injected with ILL_ILLOPN as the signal code. Note that this is different from earlier ARMv8.3 ptrauth where signal SIGSEGV is issued due to Pointer authentication failures. Signed-off-by: Amit Daniel Kachhap <amit.kachhap@arm.com> --- arch/arm64/include/asm/esr.h | 4 +++- arch/arm64/include/asm/exception.h | 1 + arch/arm64/include/asm/sysreg.h | 24 ++++++++++++++++-------- arch/arm64/kernel/entry-common.c | 25 +++++++++++++++++++++++++ arch/arm64/kernel/traps.c | 18 ++++++++++++++++++ 5 files changed, 63 insertions(+), 9 deletions(-)