On Sat, Jul 20, 2019 at 10:56:41AM +0200, Thomas Gleixner wrote:
> The recent fix for CR2 corruption introduced a new way to reliably corrupt
> the saved CR2 value.
>
> CR2 is saved early in the entry code in RDX, which is the third argument to
> the fault handling functions. But it missed that between saving and
> invoking the fault handler enter_from_user_mode() can be called. RDX is a
> caller saved register so the invoked function can freely clobber it with
> the obvious consequences.
>
> The TRACE_IRQS_OFF call is safe as it calls through the thunk which
> preserves RDX.
>
> Store CR2 in R12 instead which is a callee saved register and move R12 to
> RDX just before calling the fault handler.
>
> Fixes: a0d14b8909de ("x86/mm, tracing: Fix CR2 corruption")
> Reported-by: Sean Christopherson <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
D'0h :-( Sorry about that.
Acked-by: Peter Zijlstra (Intel) <[email protected]>
> ---
> arch/x86/entry/entry_64.S | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> --- a/arch/x86/entry/entry_64.S
> +++ b/arch/x86/entry/entry_64.S
> @@ -875,7 +875,12 @@ apicinterrupt IRQ_WORK_VECTOR irq_work
> UNWIND_HINT_REGS
>
> .if \read_cr2
> - GET_CR2_INTO(%rdx); /* can clobber %rax */
> + /*
> + * Store CR2 early so subsequent faults cannot clobber it. Use R12 as
> + * intermediate storage as RDX can be clobbered in
> enter_from_user_mode().
> + * GET_CR2_INTO can clobber RAX.
> + */
> + GET_CR2_INTO(%r12);
> .endif
>
> .if \shift_ist != -1
> @@ -904,6 +909,10 @@ apicinterrupt IRQ_WORK_VECTOR irq_work
> subq $\ist_offset, CPU_TSS_IST(\shift_ist)
> .endif
>
> + .if \read_cr2
> + movq %r12, %rdx /* Move CR2 into 3rd argument */
> + .endif
> +
> call \do_sym
>
> .if \shift_ist != -1
