linux-kernel - Re: [PATCH] KVM: x86: dynamically allocate large struct in em

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 25 May 2017 21:13:27 -0700
From:   Nick Desaulniers <nick.desaulniers@...il.com>
To:     Paolo Bonzini <pbonzini@...hat.com>
Cc:     Radim Krčmář <rkrcmar@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, x86@...nel.org,
        kvm@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] KVM: x86: dynamically allocate large struct in em_fxrstor

On Thu, May 25, 2017 at 04:07:08PM +0200, Paolo Bonzini wrote:
> I think we should do the fixup backwards.
> 
> That is:
> 
> - first do get_fpu
> 
> - if the fixup is necessary, i.e. ctxt->mode < X86EMUL_MODE_PROT64, do
> fxsave into &fxstate.
> 
> - then do segmented_read_std with the correct size, which is
>   - offsetof(struct fxregs_state, xmm_space[16]), i.e. 416
>     if ctxt->mode == X86EMUL_MODE_PROT64
>   - offsetof(struct fxregs_state, xmm_space[8]), i.e. 288
>     if ctxt->mode < X86EMUL_MODE_PROT64 and CR4.OSFXSR=1
>   - offsetof(struct fxregs_state, xmm_space[0]), i.e. 160
>     if ctxt->mode < X86EMUL_MODE_PROT64 and CR4.OSFXSR=0

but we still want to do a segmented_read_std with size 512 otherwise,
correct?

> - then check fx_state.mxcsr
> 
> - then do fxrstor

This sounds like we conditionally do the fxsave, but then always do the
fxrstor.  Is that ok? I guess the original code kind of does that as
well.

> - finally do put_fpu

Sounds straight forward.  I can see how fxsave and CR4.OSFXSR are
accessed in fxstor_fixup.  Is it ok to skip those memcpy's that would
otherwise occur when calling fxrstor_fixup() (which after these changes,
we would not be)?