Benjamin Gray <bg...@linux.ibm.com> writes:
> To determine if a trap was caused by a HASHCHK instruction, we inspect
> the user instruction that triggered the trap. However this may sleep
> if the page needs to be faulted in.
It would be good to include the output from the might_sleep() check that
failed.
And I think this was found by syzkaller? If so we should say so.
cheers
> Move the HASHCHK handler logic to after we allow IRQs, which is fine
> because we are only interested in HASHCHK if it's a user space trap.
>
> Fixes: 5bcba4e6c13f ("powerpc/dexcr: Handle hashchk exception")
> Signed-off-by: Benjamin Gray <bg...@linux.ibm.com>
> ---
> arch/powerpc/kernel/traps.c | 56 ++++++++++++++++++++++++-------------
> 1 file changed, 36 insertions(+), 20 deletions(-)
>
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index f5ce282dc4b8..32b5e841ea97 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1512,23 +1512,11 @@ static void do_program_check(struct pt_regs *regs)
> return;
> }
>
> - if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE) && user_mode(regs)) {
> - ppc_inst_t insn;
> -
> - if (get_user_instr(insn, (void __user *)regs->nip)) {
> - _exception(SIGSEGV, regs, SEGV_MAPERR,
> regs->nip);
> - return;
> - }
> -
> - if (ppc_inst_primary_opcode(insn) == 31 &&
> - get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK) {
> - _exception(SIGILL, regs, ILL_ILLOPN, regs->nip);
> - return;
> - }
> + /* User mode considers other cases after enabling IRQs */
> + if (!user_mode(regs)) {
> + _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip);
> + return;
> }
> -
> - _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip);
> - return;
> }
> #ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> if (reason & REASON_TM) {
> @@ -1561,16 +1549,44 @@ static void do_program_check(struct pt_regs *regs)
>
> /*
> * If we took the program check in the kernel skip down to sending a
> - * SIGILL. The subsequent cases all relate to emulating instructions
> - * which we should only do for userspace. We also do not want to enable
> - * interrupts for kernel faults because that might lead to further
> - * faults, and loose the context of the original exception.
> + * SIGILL. The subsequent cases all relate to user space, such as
> + * emulating instructions which we should only do for user space. We
> + * also do not want to enable interrupts for kernel faults because that
> + * might lead to further faults, and loose the context of the original
> + * exception.
> */
> if (!user_mode(regs))
> goto sigill;
>
> interrupt_cond_local_irq_enable(regs);
>
> + /*
> + * (reason & REASON_TRAP) is mostly handled before enabling IRQs,
> + * except get_user_instr() can sleep so we cannot reliably inspect the
> + * current instruction in that context. Now that we know we are
> + * handling a user space trap and can sleep, we can check if the trap
> + * was a hashchk failure.
> + */
> + if (reason & REASON_TRAP) {
> + if (cpu_has_feature(CPU_FTR_DEXCR_NPHIE)) {
> + ppc_inst_t insn;
> +
> + if (get_user_instr(insn, (void __user *)regs->nip)) {
> + _exception(SIGSEGV, regs, SEGV_MAPERR,
> regs->nip);
> + return;
> + }
> +
> + if (ppc_inst_primary_opcode(insn) == 31 &&
> + get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK) {
> + _exception(SIGILL, regs, ILL_ILLOPN, regs->nip);
> + return;
> + }
> + }
> +
> + _exception(SIGTRAP, regs, TRAP_BRKPT, regs->nip);
> + return;
> + }
> +
> /* (reason & REASON_ILLEGAL) would be the obvious thing here,
> * but there seems to be a hardware bug on the 405GP (RevD)
> * that means ESR is sometimes set incorrectly - either to
> --
> 2.41.0
