| Message ID | 20190117195558.110516-1-jmattson@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: nVMX: Fix size checks in vmx_set_nested_state | expand |
On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: > > The size checks in vmx_nested_state are wrong because the calculations > are made based on the size of a pointer to a struct kvm_nested_state > rather than the size of a struct kvm_nested_state. > > Reported-by: Felix Wilhelm <fwilhelm@google.com> > Signed-off-by: Jim Mattson <jmattson@google.com> > Reviewed-by: Drew Schmitt <dasch@google.com> > Reviewed-by: Marc Orr <marcorr@google.com> > Reviewed-by: Peter Shier <pshier@google.com> > --- > arch/x86/kvm/vmx/nested.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > index 2616bd2c7f2c..3bb49ad91d0c 100644 > --- a/arch/x86/kvm/vmx/nested.c > +++ b/arch/x86/kvm/vmx/nested.c > @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > return ret; > > /* Empty 'VMXON' state is permitted */ > - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) > + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) > return 0; > > if (kvm_state->vmx.vmcs_pa != -1ull) { > @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > vmcs12->vmcs_link_pointer != -1ull) { > struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); > > - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) > + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) > return -EINVAL; > > if (copy_from_user(shadow_vmcs12, > -- > 2.20.1.97.g81188d93c3-goog Ping.
On Wed, Jan 30, 2019 at 11:52 AM Jim Mattson <jmattson@google.com> wrote: > > On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: > > > > The size checks in vmx_nested_state are wrong because the calculations > > are made based on the size of a pointer to a struct kvm_nested_state > > rather than the size of a struct kvm_nested_state. > > > > Reported-by: Felix Wilhelm <fwilhelm@google.com> > > Signed-off-by: Jim Mattson <jmattson@google.com> > > Reviewed-by: Drew Schmitt <dasch@google.com> > > Reviewed-by: Marc Orr <marcorr@google.com> > > Reviewed-by: Peter Shier <pshier@google.com> > > --- > > arch/x86/kvm/vmx/nested.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > index 2616bd2c7f2c..3bb49ad91d0c 100644 > > --- a/arch/x86/kvm/vmx/nested.c > > +++ b/arch/x86/kvm/vmx/nested.c > > @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > > return ret; > > > > /* Empty 'VMXON' state is permitted */ > > - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) > > + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) > > return 0; > > > > if (kvm_state->vmx.vmcs_pa != -1ull) { > > @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > > vmcs12->vmcs_link_pointer != -1ull) { > > struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); > > > > - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) > > + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) > > return -EINVAL; > > > > if (copy_from_user(shadow_vmcs12, > > -- > > 2.20.1.97.g81188d93c3-goog > > Ping. Ping again?
On 02/12/2019 10:09 AM, Jim Mattson wrote: > On Wed, Jan 30, 2019 at 11:52 AM Jim Mattson <jmattson@google.com> wrote: >> On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: >>> The size checks in vmx_nested_state are wrong because the calculations >>> are made based on the size of a pointer to a struct kvm_nested_state >>> rather than the size of a struct kvm_nested_state. >>> >>> Reported-by: Felix Wilhelm <fwilhelm@google.com> >>> Signed-off-by: Jim Mattson <jmattson@google.com> >>> Reviewed-by: Drew Schmitt <dasch@google.com> >>> Reviewed-by: Marc Orr <marcorr@google.com> >>> Reviewed-by: Peter Shier <pshier@google.com> >>> --- >>> arch/x86/kvm/vmx/nested.c | 4 ++-- >>> 1 file changed, 2 insertions(+), 2 deletions(-) >>> >>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >>> index 2616bd2c7f2c..3bb49ad91d0c 100644 >>> --- a/arch/x86/kvm/vmx/nested.c >>> +++ b/arch/x86/kvm/vmx/nested.c >>> @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >>> return ret; >>> >>> /* Empty 'VMXON' state is permitted */ >>> - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) >>> + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) >>> return 0; >>> >>> if (kvm_state->vmx.vmcs_pa != -1ull) { >>> @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >>> vmcs12->vmcs_link_pointer != -1ull) { >>> struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); >>> >>> - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) >>> + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) >>> return -EINVAL; >>> >>> if (copy_from_user(shadow_vmcs12, >>> -- >>> 2.20.1.97.g81188d93c3-goog >> Ping. > Ping again? Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com>
On Tue, Feb 12, 2019 at 10:43 AM Krish Sadhukhan <krish.sadhukhan@oracle.com> wrote: > > > > On 02/12/2019 10:09 AM, Jim Mattson wrote: > > On Wed, Jan 30, 2019 at 11:52 AM Jim Mattson <jmattson@google.com> wrote: > >> On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: > >>> The size checks in vmx_nested_state are wrong because the calculations > >>> are made based on the size of a pointer to a struct kvm_nested_state > >>> rather than the size of a struct kvm_nested_state. > >>> > >>> Reported-by: Felix Wilhelm <fwilhelm@google.com> > >>> Signed-off-by: Jim Mattson <jmattson@google.com> > >>> Reviewed-by: Drew Schmitt <dasch@google.com> > >>> Reviewed-by: Marc Orr <marcorr@google.com> > >>> Reviewed-by: Peter Shier <pshier@google.com> > >>> --- > >>> arch/x86/kvm/vmx/nested.c | 4 ++-- > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > >>> > >>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > >>> index 2616bd2c7f2c..3bb49ad91d0c 100644 > >>> --- a/arch/x86/kvm/vmx/nested.c > >>> +++ b/arch/x86/kvm/vmx/nested.c > >>> @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > >>> return ret; > >>> > >>> /* Empty 'VMXON' state is permitted */ > >>> - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) > >>> + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) > >>> return 0; > >>> > >>> if (kvm_state->vmx.vmcs_pa != -1ull) { > >>> @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > >>> vmcs12->vmcs_link_pointer != -1ull) { > >>> struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); > >>> > >>> - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) > >>> + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) > >>> return -EINVAL; > >>> > >>> if (copy_from_user(shadow_vmcs12, > >>> -- > >>> 2.20.1.97.g81188d93c3-goog > >> Ping. > > Ping again? > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> Ping again, just for grins.
On Tue, Mar 12, 2019 at 8:42 PM Jim Mattson <jmattson@google.com> wrote: > > On Tue, Feb 12, 2019 at 10:43 AM Krish Sadhukhan > <krish.sadhukhan@oracle.com> wrote: > > > > > > > > On 02/12/2019 10:09 AM, Jim Mattson wrote: > > > On Wed, Jan 30, 2019 at 11:52 AM Jim Mattson <jmattson@google.com> wrote: > > >> On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: > > >>> The size checks in vmx_nested_state are wrong because the calculations > > >>> are made based on the size of a pointer to a struct kvm_nested_state > > >>> rather than the size of a struct kvm_nested_state. > > >>> > > >>> Reported-by: Felix Wilhelm <fwilhelm@google.com> > > >>> Signed-off-by: Jim Mattson <jmattson@google.com> > > >>> Reviewed-by: Drew Schmitt <dasch@google.com> > > >>> Reviewed-by: Marc Orr <marcorr@google.com> > > >>> Reviewed-by: Peter Shier <pshier@google.com> > > >>> --- > > >>> arch/x86/kvm/vmx/nested.c | 4 ++-- > > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > > >>> > > >>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > >>> index 2616bd2c7f2c..3bb49ad91d0c 100644 > > >>> --- a/arch/x86/kvm/vmx/nested.c > > >>> +++ b/arch/x86/kvm/vmx/nested.c > > >>> @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > > >>> return ret; > > >>> > > >>> /* Empty 'VMXON' state is permitted */ > > >>> - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) > > >>> + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) > > >>> return 0; > > >>> > > >>> if (kvm_state->vmx.vmcs_pa != -1ull) { > > >>> @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > > >>> vmcs12->vmcs_link_pointer != -1ull) { > > >>> struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); > > >>> > > >>> - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) > > >>> + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) > > >>> return -EINVAL; > > >>> > > >>> if (copy_from_user(shadow_vmcs12, > > >>> -- > > >>> 2.20.1.97.g81188d93c3-goog > > >> Ping. > > > Ping again? > > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > > Ping again, just for grins. Ping.
On Fri, Mar 22, 2019 at 9:53 AM Jim Mattson <jmattson@google.com> wrote: > > On Tue, Mar 12, 2019 at 8:42 PM Jim Mattson <jmattson@google.com> wrote: > > > > On Tue, Feb 12, 2019 at 10:43 AM Krish Sadhukhan > > <krish.sadhukhan@oracle.com> wrote: > > > > > > > > > > > > On 02/12/2019 10:09 AM, Jim Mattson wrote: > > > > On Wed, Jan 30, 2019 at 11:52 AM Jim Mattson <jmattson@google.com> wrote: > > > >> On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: > > > >>> The size checks in vmx_nested_state are wrong because the calculations > > > >>> are made based on the size of a pointer to a struct kvm_nested_state > > > >>> rather than the size of a struct kvm_nested_state. > > > >>> > > > >>> Reported-by: Felix Wilhelm <fwilhelm@google.com> > > > >>> Signed-off-by: Jim Mattson <jmattson@google.com> > > > >>> Reviewed-by: Drew Schmitt <dasch@google.com> > > > >>> Reviewed-by: Marc Orr <marcorr@google.com> > > > >>> Reviewed-by: Peter Shier <pshier@google.com> > > > >>> --- > > > >>> arch/x86/kvm/vmx/nested.c | 4 ++-- > > > >>> 1 file changed, 2 insertions(+), 2 deletions(-) > > > >>> > > > >>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c > > > >>> index 2616bd2c7f2c..3bb49ad91d0c 100644 > > > >>> --- a/arch/x86/kvm/vmx/nested.c > > > >>> +++ b/arch/x86/kvm/vmx/nested.c > > > >>> @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > > > >>> return ret; > > > >>> > > > >>> /* Empty 'VMXON' state is permitted */ > > > >>> - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) > > > >>> + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) > > > >>> return 0; > > > >>> > > > >>> if (kvm_state->vmx.vmcs_pa != -1ull) { > > > >>> @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, > > > >>> vmcs12->vmcs_link_pointer != -1ull) { > > > >>> struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); > > > >>> > > > >>> - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) > > > >>> + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) > > > >>> return -EINVAL; > > > >>> > > > >>> if (copy_from_user(shadow_vmcs12, > > > >>> -- > > > >>> 2.20.1.97.g81188d93c3-goog > > > >> Ping. > > > > Ping again? > > > Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> > > > > Ping again, just for grins. > > Ping. This seems pretty straightforward. Am I missing something?
On 30/04/19 23:46, Jim Mattson wrote: > On Fri, Mar 22, 2019 at 9:53 AM Jim Mattson <jmattson@google.com> wrote: >> >> On Tue, Mar 12, 2019 at 8:42 PM Jim Mattson <jmattson@google.com> wrote: >>> >>> On Tue, Feb 12, 2019 at 10:43 AM Krish Sadhukhan >>> <krish.sadhukhan@oracle.com> wrote: >>>> >>>> >>>> >>>> On 02/12/2019 10:09 AM, Jim Mattson wrote: >>>>> On Wed, Jan 30, 2019 at 11:52 AM Jim Mattson <jmattson@google.com> wrote: >>>>>> On Thu, Jan 17, 2019 at 11:56 AM Jim Mattson <jmattson@google.com> wrote: >>>>>>> The size checks in vmx_nested_state are wrong because the calculations >>>>>>> are made based on the size of a pointer to a struct kvm_nested_state >>>>>>> rather than the size of a struct kvm_nested_state. >>>>>>> >>>>>>> Reported-by: Felix Wilhelm <fwilhelm@google.com> >>>>>>> Signed-off-by: Jim Mattson <jmattson@google.com> >>>>>>> Reviewed-by: Drew Schmitt <dasch@google.com> >>>>>>> Reviewed-by: Marc Orr <marcorr@google.com> >>>>>>> Reviewed-by: Peter Shier <pshier@google.com> >>>>>>> --- >>>>>>> arch/x86/kvm/vmx/nested.c | 4 ++-- >>>>>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>>>>> >>>>>>> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c >>>>>>> index 2616bd2c7f2c..3bb49ad91d0c 100644 >>>>>>> --- a/arch/x86/kvm/vmx/nested.c >>>>>>> +++ b/arch/x86/kvm/vmx/nested.c >>>>>>> @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >>>>>>> return ret; >>>>>>> >>>>>>> /* Empty 'VMXON' state is permitted */ >>>>>>> - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) >>>>>>> + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) >>>>>>> return 0; >>>>>>> >>>>>>> if (kvm_state->vmx.vmcs_pa != -1ull) { >>>>>>> @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, >>>>>>> vmcs12->vmcs_link_pointer != -1ull) { >>>>>>> struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); >>>>>>> >>>>>>> - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) >>>>>>> + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) >>>>>>> return -EINVAL; >>>>>>> >>>>>>> if (copy_from_user(shadow_vmcs12, >>>>>>> -- >>>>>>> 2.20.1.97.g81188d93c3-goog >>>>>> Ping. >>>>> Ping again? >>>> Reviewed-by: Krish Sadhukhan <krish.sadhukhan@oracle.com> >>> >>> Ping again, just for grins. >> >> Ping. > > This seems pretty straightforward. Am I missing something? Nope, I just missed it again and again. I've queued it now. Paolo
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c index 2616bd2c7f2c..3bb49ad91d0c 100644 --- a/arch/x86/kvm/vmx/nested.c +++ b/arch/x86/kvm/vmx/nested.c @@ -5351,7 +5351,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, return ret; /* Empty 'VMXON' state is permitted */ - if (kvm_state->size < sizeof(kvm_state) + sizeof(*vmcs12)) + if (kvm_state->size < sizeof(*kvm_state) + sizeof(*vmcs12)) return 0; if (kvm_state->vmx.vmcs_pa != -1ull) { @@ -5395,7 +5395,7 @@ static int vmx_set_nested_state(struct kvm_vcpu *vcpu, vmcs12->vmcs_link_pointer != -1ull) { struct vmcs12 *shadow_vmcs12 = get_shadow_vmcs12(vcpu); - if (kvm_state->size < sizeof(kvm_state) + 2 * sizeof(*vmcs12)) + if (kvm_state->size < sizeof(*kvm_state) + 2 * sizeof(*vmcs12)) return -EINVAL; if (copy_from_user(shadow_vmcs12,