| Message ID | 20210326000235.370514-2-tony.luck@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Fix machine check recovery for copy_from_user | expand |
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.
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