Messages in this thread | | | From | Thomas Gleixner <> | Subject | Re: [patch V2 09/52] x86/fpu: Reject invalid MXCSR values in copy_kernel_to_xstate() | Date | Thu, 17 Jun 2021 01:51:50 +0200 |
| |
On Wed, Jun 16 2021 at 17:02, Borislav Petkov wrote: > 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:
No, because the first usage is wrong. I found that while looking through this stuff again. Sigh...
| |