On Mon, May 31, 2021, at 11:56 AM, Thomas Gleixner wrote:
On Mon, May 31 2021 at 12:01, Thomas Gleixner wrote:
On Sat, May 29 2021 at 22:12, Andy Lutomirski wrote:
/*
- Clear the FPU state back to init state.
- Called by sys_execve(), by the signal handler code and by various
- error paths.
- Reset current's user FPU states to the init states. The caller promises
- that current's supervisor states (in memory or CPU regs as appropriate)
*/
- as well as the XSAVE header in memory are intact.
-static void fpu__clear(struct fpu *fpu, bool user_only) +void fpu__clear_user_states(struct fpu *fpu) { WARN_ON_FPU(fpu != ¤t->thread.fpu); if (!static_cpu_has(X86_FEATURE_FPU)) {
This can only be safely called if XSAVES is available. So this check is bogus as it actually should check for !XSAVES. And if at all it should be:
if (WARN_ON_ONCE(!XSAVES)) ....
This is exactly the stuff which causes subtle problems down the road.
I have no idea why you are insisting on having this conditional at the call site. It's just an invitation for trouble because someone finds this function and calls it unconditionally. And he will miss the 'promise' part in the comment as I did.
And of course there is:
__fpu__restore_sig()
if (!buf) { fpu__clear_user_states(fpu); return 0; }
and
handle_signal()
if (!failed) fpu__clear_user_states(fpu);
This looks okay.
Really there are two callers of fpu__clear_all() that are special:
execve: Just in case some part of the xstate buffer mode that’s supposed to be invariant got corrupted or in case there is some side channel that can leak the INIT-but-not-zeroed contents of a state to user code, we should really wipe the memory completely across privilege boundaries.
__fpu__restore_sig: the utterly daft copy from user space needs special recovery.
Maybe the right solution is to rename it. Instead of fpu__clear_all(), how about fpu__wipe_and_reset()?
which invoke that function unconditionally.
Thanks,
tglx