| Message ID | 20210606001323.067157324@linutronix.de |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
On Sun, Jun 06, 2021 at 01:47:44AM +0200, Thomas Gleixner wrote: > From: Thomas Gleixner <tglx@linutronix.de> > > The non-compacted slowpath uses __copy_from_user() and copies the entire > user buffer into the kernel buffer, verbatim. This means that the kernel > buffer may now contain entirely invalid state on which XRSTOR will #GP. > validate_user_xstate_header() can detect some of that corruption, but that > leaves the onus on callers to clear the buffer. > > Prior to XSAVES support it was possible just to reinitialize the buffer, > completely, but with supervisor states that is not longer possible as the > buffer clearing code split got it backwards. Fixing that is possible, but > not corrupting the state in the first place is more robust. > > Avoid corruption of the kernel XSAVE buffer by using copy_user_to_xstate() > which validates the XSAVE header contents before copying the actual states > to the kernel. copy_user_to_xstate() was previously only called for > compacted-format kernel buffers, but it works for both compacted and > non-compacted forms. > > Using it for the non-compacted form is slower because of multiple > __copy_from_user() operations, but that cost is less important than robust > code in an already slow path. > > [ Changelog polished by Dave Hansen ] > > Fixes: b860eb8dce59 ("x86/fpu/xstate: Define new functions for clearing fpregs and xstates") > Reported-by: syzbot+2067e764dbcd10721e2e@syzkaller.appspotmail.com > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Cc: stable@vger.kernel.org > --- > V2: Removed the make validate_user_xstate_header() static hunks (Borislav) > --- > arch/x86/kernel/fpu/signal.c | 9 +-------- > 1 file changed, 1 insertion(+), 8 deletions(-) Very nice. Reviewed-by: Borislav Petkov <bp@suse.de>
--- a/arch/x86/kernel/fpu/signal.c +++ b/arch/x86/kernel/fpu/signal.c @@ -405,14 +405,7 @@ static int __fpu__restore_sig(void __use if (use_xsave() && !fx_only) { u64 init_bv = xfeatures_mask_user() & ~user_xfeatures; - if (using_compacted_format()) { - ret = copy_user_to_xstate(&fpu->state.xsave, buf_fx); - } else { - ret = __copy_from_user(&fpu->state.xsave, buf_fx, state_size); - - if (!ret && state_size > offsetof(struct xregs_state, header)) - ret = validate_user_xstate_header(&fpu->state.xsave.header); - } + ret = copy_user_to_xstate(&fpu->state.xsave, buf_fx); if (ret) goto err_out;