| Message ID | 20220621150902.46126-7-mlevitsk@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | SMM emulation and interrupt shadow fixes | expand |
On Tue, Jun 21, 2022, Maxim Levitsky wrote: > On 64 bit host, if the guest doesn't have X86_FEATURE_LM, we would s/we would/KVM will > access 16 gprs to 32-bit smram image, causing out-ouf-bound ram > access. > > On 32 bit host, the rsm_load_state_64/enter_smm_save_state_64 > is compiled out, thus access overflow can't happen. > > Fixes: b443183a25ab61 ("KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM") Argh, I forgot that this one of the like five places KVM actually respects the long mode flag. Even worse, I fixed basically the same thing a while back, commit b68f3cc7d978 ("KVM: x86: Always use 32-bit SMRAM save state for 32-bit kernels"). We should really harden put_smstate() and GET_SMSTATE()... > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> > --- Nits aside, Reviewed-by: Sean Christopherson <seanjc@google.com> > arch/x86/kvm/emulate.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c > index 002687d17f9364..ce186aebca8e83 100644 > --- a/arch/x86/kvm/emulate.c > +++ b/arch/x86/kvm/emulate.c > @@ -2469,7 +2469,7 @@ static int rsm_load_state_32(struct x86_emulate_ctxt *ctxt, > ctxt->eflags = GET_SMSTATE(u32, smstate, 0x7ff4) | X86_EFLAGS_FIXED; > ctxt->_eip = GET_SMSTATE(u32, smstate, 0x7ff0); > > - for (i = 0; i < NR_EMULATOR_GPRS; i++) > + for (i = 0; i < 8; i++) > *reg_write(ctxt, i) = GET_SMSTATE(u32, smstate, 0x7fd0 + i * 4); > > val = GET_SMSTATE(u32, smstate, 0x7fcc); > @@ -2526,7 +2526,7 @@ static int rsm_load_state_64(struct x86_emulate_ctxt *ctxt, > u16 selector; > int i, r; > > - for (i = 0; i < NR_EMULATOR_GPRS; i++) > + for (i = 0; i < 16; i++) > *reg_write(ctxt, i) = GET_SMSTATE(u64, smstate, 0x7ff8 - i * 8); > > ctxt->_eip = GET_SMSTATE(u64, smstate, 0x7f78); > -- > 2.26.3 >
On Thu, Jul 21, 2022, Sean Christopherson wrote: > On Tue, Jun 21, 2022, Maxim Levitsky wrote: > > On 64 bit host, if the guest doesn't have X86_FEATURE_LM, we would > > s/we would/KVM will > > > access 16 gprs to 32-bit smram image, causing out-ouf-bound ram > > access. > > > > On 32 bit host, the rsm_load_state_64/enter_smm_save_state_64 > > is compiled out, thus access overflow can't happen. > > > > Fixes: b443183a25ab61 ("KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM") > > Argh, I forgot that this one of the like five places KVM actually respects the > long mode flag. Even worse, I fixed basically the same thing a while back, > commit b68f3cc7d978 ("KVM: x86: Always use 32-bit SMRAM save state for 32-bit kernels"). > > We should really harden put_smstate() and GET_SMSTATE()... Or I could read the next few patches and see that they go away...
diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c index 002687d17f9364..ce186aebca8e83 100644 --- a/arch/x86/kvm/emulate.c +++ b/arch/x86/kvm/emulate.c @@ -2469,7 +2469,7 @@ static int rsm_load_state_32(struct x86_emulate_ctxt *ctxt, ctxt->eflags = GET_SMSTATE(u32, smstate, 0x7ff4) | X86_EFLAGS_FIXED; ctxt->_eip = GET_SMSTATE(u32, smstate, 0x7ff0); - for (i = 0; i < NR_EMULATOR_GPRS; i++) + for (i = 0; i < 8; i++) *reg_write(ctxt, i) = GET_SMSTATE(u32, smstate, 0x7fd0 + i * 4); val = GET_SMSTATE(u32, smstate, 0x7fcc); @@ -2526,7 +2526,7 @@ static int rsm_load_state_64(struct x86_emulate_ctxt *ctxt, u16 selector; int i, r; - for (i = 0; i < NR_EMULATOR_GPRS; i++) + for (i = 0; i < 16; i++) *reg_write(ctxt, i) = GET_SMSTATE(u64, smstate, 0x7ff8 - i * 8); ctxt->_eip = GET_SMSTATE(u64, smstate, 0x7f78);
On 64 bit host, if the guest doesn't have X86_FEATURE_LM, we would access 16 gprs to 32-bit smram image, causing out-ouf-bound ram access. On 32 bit host, the rsm_load_state_64/enter_smm_save_state_64 is compiled out, thus access overflow can't happen. Fixes: b443183a25ab61 ("KVM: x86: Reduce the number of emulator GPRs to '8' for 32-bit KVM") Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com> --- arch/x86/kvm/emulate.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)