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
