| Subject | Re: [patch 06/41] x86/fpu: Sanitize xstateregs_set() | From | Andy Lutomirski <> | Date | Fri, 11 Jun 2021 11:45:52 -0700 |
| |
On 6/11/21 9:15 AM, Thomas Gleixner wrote: > xstateregs_set() operates on a stopped task and tries to copy the provided > buffer into the tasks fpu.state.xsave buffer. > > Any error while copying or invalid state detected after copying results in > wiping the target tasks FPU state completely including supervisor states. > > That's just wrong. The caller supplied invalid data or has a problem with > unmapped memory, so there is absolutely no justification to corrupt the > target state. > > @@ -1146,14 +1146,16 @@ int copy_kernel_to_xstate(struct xregs_s > */ > xsave->header.xfeatures |= hdr.xfeatures; > > + /* mxcsr reserved bits must be masked to zero for security reasons. */ > + xsave->i387.mxcsr &= mxcsr_feature_mask;
This comment is vague. At least it should say:
A subsequent XRSTOR(S) will fail if MXCSR has bits set that are not accepted by the current CPU. Mask out unsupported bits.
But a much nicer fix IMO would be to just return an error.
--Andy
|