Re: [PATCH Dovetail v2 1/6] arm: irq_pipeline: Fix fault_{entry,exit} bypass in do_kernel_address_page_fault

Florian Bezdeka <[email protected]> writes:

> In case do_kernel_address_page_fault() was entered over supervisor mode
> the fault_{entry,exit} machinery was bypassed. As a result the inband
> IRQ mask might have been touched from the oob stage.
>
> There is another problem with interrupts_enabled() in the supervisor
> mode code path. That one will be addressed in a separate patch.
>
> Signed-off-by: Florian Bezdeka <[email protected]>
> ---
>  arch/arm/mm/fault.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mm/fault.c b/arch/arm/mm/fault.c
> index 49f5d628c4c9a91a3e879af0d541fd76665d78fe..7a5c0a5a825df5f76eec6aa341d910c3efa63f88 100644
> --- a/arch/arm/mm/fault.c
> +++ b/arch/arm/mm/fault.c
> @@ -382,6 +382,8 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
>  {
>  	unsigned long irqflags;
>  
> +	irqflags = fault_entry(ARM_TRAP_ACCESS, regs);
> +
>  	if (user_mode(regs)) {
>  		/*
>  		 * Fault from user mode for a kernel space address. User mode
> @@ -391,9 +393,7 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
>  		 * Note that __do_user_fault() will enable interrupts.
>  		 */
>  		harden_branch_predictor();

By doing so, you move the Spectre mitigation after the fault entry
handling, which means past the in-band switch.

> -		irqflags = fault_entry(ARM_TRAP_ACCESS, regs);
>  		__do_user_fault(addr, fsr, SIGSEGV, SEGV_MAPERR, regs);
> -		fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
>  	} else {
>  		/*
>  		 * Fault from kernel mode. Enable interrupts if they were
> @@ -409,6 +409,8 @@ do_kernel_address_page_fault(struct mm_struct *mm, unsigned long addr,
>  		__do_kernel_fault(mm, addr, fsr, regs);
>  	}
>  
> +	fault_exit(ARM_TRAP_ACCESS, regs, irqflags);
> +
>  	return 0;
>  }

-- 
Philippe.