On Wed, Jun 02, 2021 at 11:55:46AM +0200, Thomas Gleixner wrote:
From: Andy Lutomirski luto@kernel.org
If XRSTOR fails due to sufficiently complicated paging errors (e.g. concurrent TLB invalidation),
I can't connect "concurrent TLB invalidation" to "sufficiently complicated paging errors". Can you elaborate pls?
it may fault with #PF but still modify portions of the user extended state.
Yikes, leaky leaky insn.
If this happens in __fpu_restore_sig() with a victim task's FPU registers loaded and the task is preempted by the victim task,
This is probably meaning another task but the only task mentioned here is a "victim task"?
the victim task resumes with partially corrupt extended state.
Invalidate the FPU registers when XRSTOR fails to prevent this scenario.
Fixes: 1d731e731c4c ("x86/fpu: Add a fastpath to __fpu__restore_sig()") Signed-off-by: Andy Lutomirski luto@kernel.org Signed-off-by: Thomas Gleixner tglx@linutronix.de Cc: stable@vger.kernel.org
arch/x86/kernel/fpu/signal.c | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+)
--- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -369,6 +369,27 @@ static int __fpu__restore_sig(void __use fpregs_unlock(); return 0; }
if (test_thread_flag(TIF_NEED_FPU_LOAD)) {/** The FPU registers do not belong to current, and* we just did an FPU restore operation, restricted
Please get rid of the "we"-personal pronouns formulations.
* to the user portion of the register file, and
"register file"? That sounds like comment which belongs in microcode but not in software. :-)
* failed. In the event that the ucode was* unfriendly and modified the registers at all, we* need to make sure that we aren't corrupting an* innocent non-current task's registers.*/__cpu_invalidate_fpregs_state();} else {/** As above, we may have just clobbered current's* user FPU state. We will either successfully* load it or clear it below, so no action is* required here.*/}
I'm wondering if that comment can simply be above the TIF_NEED_FPU_LOAD testing, standalone, instead of having it in an empty else? And then get rid of that else.
Thx.