Messages in this thread | | | Date | Mon, 2 May 2016 14:18:17 -0700 | From | Yu-cheng Yu <> | Subject | Re: [PATCH v4 04/10] x86/xsaves: Introduce a new check that allows correct xstates copy from kernel to user directly |
| |
On Mon, May 02, 2016 at 10:33:10AM -0700, Dave Hansen wrote: > On 05/02/2016 10:19 AM, Yu-cheng Yu wrote: > > On Mon, May 02, 2016 at 09:43:47AM -0700, Dave Hansen wrote: > >>> If (fpu.fpstate_active == 0), then the task does not use FPU; we don't > >>> want to save these registers, right? > >> > >> No. It's possible to have fpstate_active=0 while fpregs_active=1. Such > >> a task uses the FPU, but just hasn't done an XSAVE* to save the register > >> content to the fpstate buffer. > >> > >> Note, this is just theoretical, and does not happen in this particular > >> call path today. > > > > What about... > > > > static int may_copy_fpregs_to_sigframe(void) > > { > > if (fpregs_active()) > > return 1; > > > > WARN_ONCE(!current->thread.fpu.fpstate_active, > > "direct FPU save with no math use\n"); > > > > if (boot_cpu_has(X86_FEATURE_XSAVES)) > > return 1; > > > > return 0; > > } > > I don't think that changes anything. We still have a check in there > that has no purpose. You've changed the ordering so that the specific > example that I pointed out no longer triggers it. But, the underlying > issue remains.
Before Linux gets into copy_fpstate_to_sigframe(), current->thread.fpu.fpstate_active must be true. For eagerfpu, fpregs_active() must also be true. For lazyfpu, once we try to do FSAVE/FXSAVE/XSAVE, fpregs_active() will become true as well.
We should have not based on boot_cpu_has(X86_FEATURE_XSAVES) at all.
Why don't we make it simple and always copy_fpregs_to_signal_frame()? Or, only for the lazy case, i.e. !fpregs_active(), we do __copy_to_user().
Anyway, I think we can just replace may_copy_fpregs_to_sigframe() with !fpregs_active().
Comments?
| |