[RFC,03/13] powerpc/dexcr: Handle hashchk exception

  Recognise and pass the appropriate signal to the user program when a
hashchk instruction triggers. This is independent of allowing
configuration of DEXCR[NPHIE], as a hypervisor can enforce this aspect
regardless of the kernel.

Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
---
 arch/powerpc/include/asm/ppc-opcode.h |  1 +
 arch/powerpc/include/asm/processor.h  |  6 ++++++
 arch/powerpc/kernel/dexcr.c           | 22 ++++++++++++++++++++++
 arch/powerpc/kernel/traps.c           |  6 ++++++
 4 files changed, 35 insertions(+)
  

On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> Recognise and pass the appropriate signal to the user program when a
> hashchk instruction triggers. This is independent of allowing
> configuration of DEXCR[NPHIE], as a hypervisor can enforce this aspect
> regardless of the kernel.
>
> Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> ---
>  arch/powerpc/include/asm/ppc-opcode.h |  1 +
>  arch/powerpc/include/asm/processor.h  |  6 ++++++
>  arch/powerpc/kernel/dexcr.c           | 22 ++++++++++++++++++++++
>  arch/powerpc/kernel/traps.c           |  6 ++++++
>  4 files changed, 35 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/ppc-opcode.h b/arch/powerpc/include/asm/ppc-opcode.h
> index 21e33e46f4b8..89b316466ed1 100644
> --- a/arch/powerpc/include/asm/ppc-opcode.h
> +++ b/arch/powerpc/include/asm/ppc-opcode.h
> @@ -215,6 +215,7 @@
>  #define OP_31_XOP_STFSX	    663
>  #define OP_31_XOP_STFSUX    695
>  #define OP_31_XOP_STFDX     727
> +#define OP_31_XOP_HASHCHK   754
>  #define OP_31_XOP_STFDUX    759
>  #define OP_31_XOP_LHBRX     790
>  #define OP_31_XOP_LFIWAX    855
> diff --git a/arch/powerpc/include/asm/processor.h b/arch/powerpc/include/asm/processor.h
> index 0a8a793b8b8b..c17ec1e44c86 100644
> --- a/arch/powerpc/include/asm/processor.h
> +++ b/arch/powerpc/include/asm/processor.h
> @@ -448,10 +448,16 @@ void *exit_vmx_ops(void *dest);
>  
>  #ifdef CONFIG_PPC_BOOK3S_64
>  
> +bool is_hashchk_trap(struct pt_regs const *regs);
>  unsigned long get_thread_dexcr(struct thread_struct const *t);
>  
>  #else
>  
> +static inline bool is_hashchk_trap(struct pt_regs const *regs)
> +{
> +	return false;
> +}
> +
>  static inline unsigned long get_thread_dexcr(struct thread_struct const *t)
>  {
>  	return 0;
> diff --git a/arch/powerpc/kernel/dexcr.c b/arch/powerpc/kernel/dexcr.c
> index 32a0a69ff638..11515e67afac 100644
> --- a/arch/powerpc/kernel/dexcr.c
> +++ b/arch/powerpc/kernel/dexcr.c
> @@ -3,6 +3,9 @@
>  
>  #include <asm/cpu_has_feature.h>
>  #include <asm/cputable.h>
> +#include <asm/disassemble.h>
> +#include <asm/inst.h>
> +#include <asm/ppc-opcode.h>
>  #include <asm/processor.h>
>  #include <asm/reg.h>
>  
> @@ -19,6 +22,25 @@ static int __init dexcr_init(void)
>  }
>  early_initcall(dexcr_init);
>  
> +bool is_hashchk_trap(struct pt_regs const *regs)
> +{
> +	ppc_inst_t insn;
> +
> +	if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> +		return false;
> +
> +	if (get_user_instr(insn, (void __user *)regs->nip)) {
> +		WARN_ON(1);
> +		return false;
> +	}

Nice series, just starting to have a look at it.

You probably don't want a WARN_ON() here because it's user triggerable
and isn't necessarily even indiciating a problem or attack if the app
is doing code unmapping in order to get faults.

Check some of the other instruction emulation for what to do in case of
an EFAULT.

> +
> +	if (ppc_inst_primary_opcode(insn) == 31 &&
> +	    get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK)
> +		return true;
> +
> +	return false;
> +}
> +
>  unsigned long get_thread_dexcr(struct thread_struct const *t)
>  {
>  	return DEFAULT_DEXCR;
> diff --git a/arch/powerpc/kernel/traps.c b/arch/powerpc/kernel/traps.c
> index 9bdd79aa51cf..b83f5b382f24 100644
> --- a/arch/powerpc/kernel/traps.c
> +++ b/arch/powerpc/kernel/traps.c
> @@ -1516,6 +1516,12 @@ static void do_program_check(struct pt_regs *regs)
>  				return;
>  			}
>  		}
> +
> +		if (user_mode(regs) && is_hashchk_trap(regs)) {
> +			_exception(SIGILL, regs, ILL_ILLOPN, regs->nip);
> +			return;
> +		}

I guess ILLOPN makes sense. Do you know if any other archs do similar?

Thanks,
Nick
  

On Tue, 2022-11-29 at 20:39 +1000, Nicholas Piggin wrote:
> On Mon Nov 28, 2022 at 12:44 PM AEST, Benjamin Gray wrote:
> > Recognise and pass the appropriate signal to the user program when
> > a
> > hashchk instruction triggers. This is independent of allowing
> > configuration of DEXCR[NPHIE], as a hypervisor can enforce this
> > aspect
> > regardless of the kernel.
> > 
> > Signed-off-by: Benjamin Gray <bgray@linux.ibm.com>
> > ---
> >  arch/powerpc/include/asm/ppc-opcode.h |  1 +
> >  arch/powerpc/include/asm/processor.h  |  6 ++++++
> >  arch/powerpc/kernel/dexcr.c           | 22 ++++++++++++++++++++++
> >  arch/powerpc/kernel/traps.c           |  6 ++++++
> >  4 files changed, 35 insertions(+)
> > 
> > diff --git a/arch/powerpc/include/asm/ppc-opcode.h
> > b/arch/powerpc/include/asm/ppc-opcode.h
> > index 21e33e46f4b8..89b316466ed1 100644
> > --- a/arch/powerpc/include/asm/ppc-opcode.h
> > +++ b/arch/powerpc/include/asm/ppc-opcode.h
> > @@ -215,6 +215,7 @@
> >  #define OP_31_XOP_STFSX            663
> >  #define OP_31_XOP_STFSUX    695
> >  #define OP_31_XOP_STFDX     727
> > +#define OP_31_XOP_HASHCHK   754
> >  #define OP_31_XOP_STFDUX    759
> >  #define OP_31_XOP_LHBRX     790
> >  #define OP_31_XOP_LFIWAX    855
> > diff --git a/arch/powerpc/include/asm/processor.h
> > b/arch/powerpc/include/asm/processor.h
> > index 0a8a793b8b8b..c17ec1e44c86 100644
> > --- a/arch/powerpc/include/asm/processor.h
> > +++ b/arch/powerpc/include/asm/processor.h
> > @@ -448,10 +448,16 @@ void *exit_vmx_ops(void *dest);
> >  
> >  #ifdef CONFIG_PPC_BOOK3S_64
> >  
> > +bool is_hashchk_trap(struct pt_regs const *regs);
> >  unsigned long get_thread_dexcr(struct thread_struct const *t);
> >  
> >  #else
> >  
> > +static inline bool is_hashchk_trap(struct pt_regs const *regs)
> > +{
> > +       return false;
> > +}
> > +
> >  static inline unsigned long get_thread_dexcr(struct thread_struct
> > const *t)
> >  {
> >         return 0;
> > diff --git a/arch/powerpc/kernel/dexcr.c
> > b/arch/powerpc/kernel/dexcr.c
> > index 32a0a69ff638..11515e67afac 100644
> > --- a/arch/powerpc/kernel/dexcr.c
> > +++ b/arch/powerpc/kernel/dexcr.c
> > @@ -3,6 +3,9 @@
> >  
> >  #include <asm/cpu_has_feature.h>
> >  #include <asm/cputable.h>
> > +#include <asm/disassemble.h>
> > +#include <asm/inst.h>
> > +#include <asm/ppc-opcode.h>
> >  #include <asm/processor.h>
> >  #include <asm/reg.h>
> >  
> > @@ -19,6 +22,25 @@ static int __init dexcr_init(void)
> >  }
> >  early_initcall(dexcr_init);
> >  
> > +bool is_hashchk_trap(struct pt_regs const *regs)
> > +{
> > +       ppc_inst_t insn;
> > +
> > +       if (!cpu_has_feature(CPU_FTR_DEXCR_NPHIE))
> > +               return false;
> > +
> > +       if (get_user_instr(insn, (void __user *)regs->nip)) {
> > +               WARN_ON(1);
> > +               return false;
> > +       }
> 
> Nice series, just starting to have a look at it.
> 
> You probably don't want a WARN_ON() here because it's user
> triggerable
> and isn't necessarily even indiciating a problem or attack if the app
> is doing code unmapping in order to get faults.
> 
> Check some of the other instruction emulation for what to do in case
> of
> an EFAULT.

Alright, I'll take a look

> > +
> > +       if (ppc_inst_primary_opcode(insn) == 31 &&
> > +           get_xop(ppc_inst_val(insn)) == OP_31_XOP_HASHCHK)
> > +               return true;
> > +
> > +       return false;
> > +}
> > +
> >  unsigned long get_thread_dexcr(struct thread_struct const *t)
> >  {
> >         return DEFAULT_DEXCR;
> > diff --git a/arch/powerpc/kernel/traps.c
> > b/arch/powerpc/kernel/traps.c
> > index 9bdd79aa51cf..b83f5b382f24 100644
> > --- a/arch/powerpc/kernel/traps.c
> > +++ b/arch/powerpc/kernel/traps.c
> > @@ -1516,6 +1516,12 @@ static void do_program_check(struct pt_regs
> > *regs)
> >                                 return;
> >                         }
> >                 }
> > +
> > +               if (user_mode(regs) && is_hashchk_trap(regs)) {
> > +                       _exception(SIGILL, regs, ILL_ILLOPN, regs-
> > >nip);
> > +                       return;
> > +               }
> 
> I guess ILLOPN makes sense. Do you know if any other archs do
> similar?

Ah sorry, when refactoring Chris' patches I forgot to put back in the
commit message that this is how ARM reports their similar check
failure. For example, their FPAC handler in
arch/arm64/kernel/traps.c:518 does this.