Re: [PATCH v7 05/19] iov_iter: Introduce fault_in_iov_iter_writeable
From: Al Viro <viro@zeniv.linux.org.uk>
Date: 2021-08-28 23:02:46
Also in:
lkml, ocfs2-devel
Subsystem:
the rest, x86 architecture (32-bit and 64-bit) · Maintainers:
Linus Torvalds, Thomas Gleixner, Ingo Molnar, Borislav Petkov, Dave Hansen
On Sat, Aug 28, 2021 at 10:19:04PM +0000, Al Viro wrote:
How about taking __clear_user() out of copy_fpregs_to_sigframe() and replacing the call of fault_in_pages_writeable() with if (!clear_user(buf_fx, fpu_user_xstate_size)) goto retry; return -EFAULT; in the caller?
Something like this (completely untested) Lift __clear_user() out of copy_fpregs_to_sigframe(), do not confuse EFAULT with X86_TRAP_PF, don't bother with fault_in_pages_writeable() (pointless, since now __clear_user() on error is not under pagefault_disable()). And don't bother with retries on anything other than #PF...
diff --git a/arch/x86/include/asm/fpu/internal.h b/arch/x86/include/asm/fpu/internal.h
index 5a18694a89b2..71c6621a262f 100644
--- a/arch/x86/include/asm/fpu/internal.h
+++ b/arch/x86/include/asm/fpu/internal.h@@ -17,6 +17,7 @@ #include <linux/mm.h> #include <asm/user.h> +#include <asm/trapnr.h> #include <asm/fpu/api.h> #include <asm/fpu/xstate.h> #include <asm/fpu/xcr.h>
@@ -345,7 +346,7 @@ static inline int xsave_to_user_sigframe(struct xregs_state __user *buf) */ err = __clear_user(&buf->header, sizeof(buf->header)); if (unlikely(err)) - return -EFAULT; + return -X86_TRAP_PF; stac(); XSTATE_OP(XSAVE, buf, lmask, hmask, err);
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 445c57c9c539..611b9ed9c06b 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c@@ -135,18 +135,12 @@ static inline int save_xstate_epilog(void __user *buf, int ia32_frame) static inline int copy_fpregs_to_sigframe(struct xregs_state __user *buf) { - int err; - if (use_xsave()) - err = xsave_to_user_sigframe(buf); - else if (use_fxsr()) - err = fxsave_to_user_sigframe((struct fxregs_state __user *) buf); + return xsave_to_user_sigframe(buf); + if (use_fxsr()) + return fxsave_to_user_sigframe((struct fxregs_state __user *) buf); else - err = fnsave_to_user_sigframe((struct fregs_state __user *) buf); - - if (unlikely(err) && __clear_user(buf, fpu_user_xstate_size)) - err = -EFAULT; - return err; + return fnsave_to_user_sigframe((struct fregs_state __user *) buf); } /*
@@ -205,9 +199,10 @@ int copy_fpstate_to_sigframe(void __user *buf, void __user *buf_fx, int size) fpregs_unlock(); if (ret) { - if (!fault_in_pages_writeable(buf_fx, fpu_user_xstate_size)) + if (!__clear_user(buf_fx, fpu_user_xstate_size) && + ret == -X86_TRAP_PF) goto retry; - return -EFAULT; + return -1; } /* Save the fsave header for the 32-bit frames. */
@@ -275,7 +270,7 @@ static int restore_fpregs_from_user(void __user *buf, u64 xrestore, fpregs_unlock(); /* Try to handle #PF, but anything else is fatal. */ - if (ret != -EFAULT) + if (ret != -X86_TRAP_PF) return -EINVAL; ret = fault_in_pages_readable(buf, size);