Re: [PATCH 1/4] x86/mce: Fix copyin code to return -EFAULT on machine check.

[Borislav Petkov]

On Thu, Mar 25, 2021 at 05:02:32PM -0700, Tony Luck wrote:
> When copy from user fails due to a machine check on poison reading
> user data it should return an error code.
> 
> ---
> 
> Separate patch just now, but likely needs to be combined with patches
> to iteration code for bisection safety.
> ---
>  arch/x86/lib/copy_user_64.S | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/x86/lib/copy_user_64.S b/arch/x86/lib/copy_user_64.S
> index 77b9b2a3b5c8..2987118c541a 100644
> --- a/arch/x86/lib/copy_user_64.S
> +++ b/arch/x86/lib/copy_user_64.S
> @@ -14,6 +14,7 @@
>  #include <asm/alternative-asm.h>
>  #include <asm/asm.h>
>  #include <asm/smap.h>
> +#include <asm/errno.h>
>  #include <asm/export.h>
>  #include <asm/trapnr.h>
>  
> @@ -237,18 +238,21 @@ SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
>  	cmp $X86_TRAP_MC,%eax		/* check if X86_TRAP_MC */
>  	je 3f
>  1:	rep movsb
> -2:	mov %ecx,%eax
> +	mov %ecx,%eax
> +	ASM_CLAC
> +	ret
> +
> +2:
> +	cmp $X86_TRAP_MC,%eax
> +	je 3f
> +	mov %ecx,%eax
>  	ASM_CLAC
>  	ret
>  
>  	/*
> -	 * Return zero to pretend that this copy succeeded. This
> -	 * is counter-intuitive, but needed to prevent the code
> -	 * in lib/iov_iter.c from retrying and running back into
> -	 * the poison cache line again. The machine check handler
> -	 * will ensure that a SIGBUS is sent to the task.
> +	 * Return -EFAULT for the machine check cases
>  	 */
> -3:	xorl %eax,%eax
> +3:	movl $-EFAULT,%eax
>  	ASM_CLAC
>  	ret

Right, looks ok after swapping all that
fault-handlers-rerouting-based-on-labels thing back in. You'd need to
update the blubber in the comment above it that we're returning -EFAULT
now too.

Also, you might wanna converge the exit paths to a single "out" label
if the unconditional JMPs are not that big of a deal (pasting the whole
thing and not as a diff because diff is unreadable):

SYM_CODE_START_LOCAL(.Lcopy_user_handle_tail)
        movl %edx,%ecx
        cmp $X86_TRAP_MC,%eax           /* check if X86_TRAP_MC */
        je 3f

1:      rep movsb
        mov %ecx,%eax
        jmp out

2:      cmp $X86_TRAP_MC,%eax
        je 3f
        mov %ecx,%eax
        jmp out

        /* Return -EFAULT for the machine check cases */
3:      movl $-EFAULT,%eax

out:
        ASM_CLAC
        ret

        _ASM_EXTABLE_CPY(1b, 2b)
SYM_CODE_END(.Lcopy_user_handle_tail)

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette