Messages in this thread Patch in this message | | | Date | Wed, 16 Jun 2021 17:02:45 +0200 | From | Borislav Petkov <> | Subject | Re: [patch V2 09/52] x86/fpu: Reject invalid MXCSR values in copy_kernel_to_xstate() |
| |
On Mon, Jun 14, 2021 at 05:44:17PM +0200, Thomas Gleixner wrote: > Instead of masking out reserved bits, check them and reject the provided > state as invalid if not zero. > > Suggested-by: Andy Lutomirski <luto@kernel.org> > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > --- > V2: New patch > --- > arch/x86/kernel/fpu/xstate.c | 11 ++++++++--- > 1 file changed, 8 insertions(+), 3 deletions(-) > > --- a/arch/x86/kernel/fpu/xstate.c > +++ b/arch/x86/kernel/fpu/xstate.c > @@ -1166,6 +1166,14 @@ int copy_kernel_to_xstate(struct xregs_s > if (validate_user_xstate_header(&hdr)) > return -EINVAL; > > + if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
Since we're cleaning up this FPU stinking pile - that function needs to have a verb in the name, something like:
if (xfeatures_mxcsr_quirk_needed(...))
but that's unrelated to here and as a note to whoever gets to get to it first.
> + const u32 *mxcsr = kbuf + offsetof(struct fxregs_state, mxcsr); > + > + /* Reserved bits in MXCSR must be zero. */ > + if (*mxcsr & ~mxcsr_feature_mask) > + return -EINVAL; > + }
Btw, that function has another
if (xfeatures_mxcsr_quirk(hdr.xfeatures)) {
branch already below the loop.
Should we merge both? Diff ontop of yours:
--- diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c index 5d032c48f39d..30022d3fcd4a 100644 --- a/arch/x86/kernel/fpu/xstate.c +++ b/arch/x86/kernel/fpu/xstate.c @@ -1172,6 +1172,11 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) /* Reserved bits in MXCSR must be zero. */ if (*mxcsr & ~mxcsr_feature_mask) return -EINVAL; + + offset = offsetof(struct fxregs_state, mxcsr); + size = MXCSR_AND_FLAGS_SIZE; + + memcpy(&xsave->i387.mxcsr, kbuf + offset, size); } for (i = 0; i < XFEATURE_MAX; i++) { @@ -1187,12 +1192,6 @@ int copy_kernel_to_xstate(struct xregs_state *xsave, const void *kbuf) } } - if (xfeatures_mxcsr_quirk(hdr.xfeatures)) { - offset = offsetof(struct fxregs_state, mxcsr); - size = MXCSR_AND_FLAGS_SIZE; - memcpy(&xsave->i387.mxcsr, kbuf + offset, size); - } - /* * The state that came in from userspace was user-state only. * Mask all the user states out of 'xfeatures': -- Regards/Gruss, Boris.
SUSE Software Solutions Germany GmbH, GF: Felix Imendörffer, HRB 36809, AG Nürnberg
| |