| Message ID | 20200914133725.650221-1-vkuznets@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state() | expand |
On Mon, Sep 14, 2020 at 03:37:25PM +0200, Vitaly Kuznetsov wrote: > The save and ctl pointers are passed uninitialized to kfree() when > svm_set_nested_state() follows the 'goto out_set_gif' path. While > the issue could've been fixed by initializing these on-stack varialbles > to NULL, it seems preferable to eliminate 'out_set_gif' label completely > as it is not actually a failure path and duplicating a single svm_set_gif() > call doesn't look too bad. > > Fixes: 6ccbd29ade0d ("KVM: SVM: nested: Don't allocate VMCB structures on stack") > Addresses-Coverity: ("Uninitialized pointer read") > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Reported-by: Joerg Roedel <jroedel@suse.de> > Reported-by: Colin King <colin.king@canonical.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> > --- Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
On Mon, Sep 14, 2020 at 03:37:25PM +0200, Vitaly Kuznetsov wrote: > The save and ctl pointers are passed uninitialized to kfree() when > svm_set_nested_state() follows the 'goto out_set_gif' path. While > the issue could've been fixed by initializing these on-stack varialbles > to NULL, it seems preferable to eliminate 'out_set_gif' label completely > as it is not actually a failure path and duplicating a single svm_set_gif() > call doesn't look too bad. > > Fixes: 6ccbd29ade0d ("KVM: SVM: nested: Don't allocate VMCB structures on stack") > Addresses-Coverity: ("Uninitialized pointer read") > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Reported-by: Joerg Roedel <jroedel@suse.de> > Reported-by: Colin King <colin.king@canonical.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Acked-by: Joerg Roedel <jroedel@suse.de>
On 9/14/20 8:37 AM, Vitaly Kuznetsov wrote: > The save and ctl pointers are passed uninitialized to kfree() when > svm_set_nested_state() follows the 'goto out_set_gif' path. While > the issue could've been fixed by initializing these on-stack varialbles > to NULL, it seems preferable to eliminate 'out_set_gif' label completely > as it is not actually a failure path and duplicating a single svm_set_gif() > call doesn't look too bad. > > Fixes: 6ccbd29ade0d ("KVM: SVM: nested: Don't allocate VMCB structures on stack") > Addresses-Coverity: ("Uninitialized pointer read") > Reported-by: Dan Carpenter <dan.carpenter@oracle.com> > Reported-by: Joerg Roedel <jroedel@suse.de> > Reported-by: Colin King <colin.king@canonical.com> > Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> Tested-by: Tom Lendacky <thomas.lendacky@amd.com>
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c index 598a769f1961..67e6d053985d 100644 --- a/arch/x86/kvm/svm/nested.c +++ b/arch/x86/kvm/svm/nested.c @@ -1094,7 +1094,8 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu, if (!(kvm_state->flags & KVM_STATE_NESTED_GUEST_MODE)) { svm_leave_nested(svm); - goto out_set_gif; + svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET)); + return 0; } if (!page_address_valid(vcpu, kvm_state->hdr.svm.vmcb_pa)) @@ -1150,7 +1151,6 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu, if (!nested_svm_vmrun_msrpm(svm)) return -EINVAL; -out_set_gif: svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET)); ret = 0;
The save and ctl pointers are passed uninitialized to kfree() when svm_set_nested_state() follows the 'goto out_set_gif' path. While the issue could've been fixed by initializing these on-stack varialbles to NULL, it seems preferable to eliminate 'out_set_gif' label completely as it is not actually a failure path and duplicating a single svm_set_gif() call doesn't look too bad. Fixes: 6ccbd29ade0d ("KVM: SVM: nested: Don't allocate VMCB structures on stack") Addresses-Coverity: ("Uninitialized pointer read") Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Reported-by: Joerg Roedel <jroedel@suse.de> Reported-by: Colin King <colin.king@canonical.com> Signed-off-by: Vitaly Kuznetsov <vkuznets@redhat.com> --- arch/x86/kvm/svm/nested.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)