On 2019-06-05 10:32:57 [-0700], Eric Biggers wrote:
> As I said, the commit looks broken to me. save_fsave_header() reads from
> tsk->thread.fpu.state.fxsave, which due to that commit isn't being updated with
> the latest registers. Am I missing something? Note the comment you deleted:
So if your system uses fxsr() then that function shouldn't matter. If
your system uses xsave() (which I believe it does) then the first
section is the "fxregs state" which is the same as in fxsr's case (see
struct xregs_state). So it shouldn't make a difference and that is why I
strongly assumed it is a miss-merge. However it makes a difference…
So the hunk at the end should make things work again (my FPU test case
passes). I don't know why we convert things forth and back in the signal
handler but I think something here is different for xsave's legacy area
vs fxsave.
diff --git a/arch/x86/kernel/fpu/signal.c b/arch/x86/kernel/fpu/signal.c
index 060d6188b4533..c653c9920c5e0 100644
--- a/arch/x86/kernel/fpu/signal.c
+++ b/arch/x86/kernel/fpu/signal.c
@@ -62,16 +62,7 @@ static inline int save_fsave_header(struct task_struct *tsk, void __user *buf)
struct user_i387_ia32_struct env;
struct _fpstate_32 __user *fp = buf;
- convert_from_fxsr(&env, tsk);
-
- if (__copy_to_user(buf, &env, sizeof(env)) ||
- __put_user(xsave->i387.swd, &fp->status) ||
- __put_user(X86_FXSR_MAGIC, &fp->magic))
- return -1;
- } else {
- struct fregs_state __user *fp = buf;
- u32 swd;
- if (__get_user(swd, &fp->swd) || __put_user(swd, &fp->status))
+ if (__put_user(X86_FXSR_MAGIC, &fp->magic))
return -1;
}
@@ -236,9 +227,6 @@ sanitize_restored_xstate(union fpregs_state *state,
* reasons.
*/
xsave->i387.mxcsr &= mxcsr_feature_mask;
-
- if (ia32_env)
- convert_to_fxsr(&state->fxsave, ia32_env);
}
}
> - Eric
Sebastian
