All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state()
@ 2020-09-14 13:37 Vitaly Kuznetsov
  2020-09-14 15:10 ` Sean Christopherson
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Vitaly Kuznetsov @ 2020-09-14 13:37 UTC (permalink / raw)
  To: x86
  Cc: kvm, linux-kernel, Borislav Petkov, Paolo Bonzini,
	Sean Christopherson, Wanpeng Li, Jim Mattson, Joerg Roedel,
	Dan Carpenter, Colin King, Thomas Gleixner, Ingo Molnar

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(-)

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;
-- 
2.25.4


^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state()
  2020-09-14 13:37 [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state() Vitaly Kuznetsov
@ 2020-09-14 15:10 ` Sean Christopherson
  2020-09-14 15:44 ` Joerg Roedel
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Sean Christopherson @ 2020-09-14 15:10 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: x86, kvm, linux-kernel, Borislav Petkov, Paolo Bonzini,
	Wanpeng Li, Jim Mattson, Joerg Roedel, Dan Carpenter, Colin King,
	Thomas Gleixner, Ingo Molnar

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state()
  2020-09-14 13:37 [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state() Vitaly Kuznetsov
  2020-09-14 15:10 ` Sean Christopherson
@ 2020-09-14 15:44 ` Joerg Roedel
  2020-09-14 16:03 ` Tom Lendacky
  2020-09-14 17:16 ` [tip: x86/seves] KVM: nSVM: Avoid " tip-bot2 for Vitaly Kuznetsov
  3 siblings, 0 replies; 5+ messages in thread
From: Joerg Roedel @ 2020-09-14 15:44 UTC (permalink / raw)
  To: Vitaly Kuznetsov
  Cc: x86, kvm, linux-kernel, Borislav Petkov, Paolo Bonzini,
	Sean Christopherson, Wanpeng Li, Jim Mattson, Dan Carpenter,
	Colin King, Thomas Gleixner, Ingo Molnar

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>


^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state()
  2020-09-14 13:37 [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state() Vitaly Kuznetsov
  2020-09-14 15:10 ` Sean Christopherson
  2020-09-14 15:44 ` Joerg Roedel
@ 2020-09-14 16:03 ` Tom Lendacky
  2020-09-14 17:16 ` [tip: x86/seves] KVM: nSVM: Avoid " tip-bot2 for Vitaly Kuznetsov
  3 siblings, 0 replies; 5+ messages in thread
From: Tom Lendacky @ 2020-09-14 16:03 UTC (permalink / raw)
  To: Vitaly Kuznetsov, x86
  Cc: kvm, linux-kernel, Borislav Petkov, Paolo Bonzini,
	Sean Christopherson, Wanpeng Li, Jim Mattson, Joerg Roedel,
	Dan Carpenter, Colin King, Thomas Gleixner, Ingo Molnar

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>

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [tip: x86/seves] KVM: nSVM: Avoid freeing uninitialized pointers in svm_set_nested_state()
  2020-09-14 13:37 [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state() Vitaly Kuznetsov
                   ` (2 preceding siblings ...)
  2020-09-14 16:03 ` Tom Lendacky
@ 2020-09-14 17:16 ` tip-bot2 for Vitaly Kuznetsov
  3 siblings, 0 replies; 5+ messages in thread
From: tip-bot2 for Vitaly Kuznetsov @ 2020-09-14 17:16 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Dan Carpenter, Joerg Roedel, Colin King, Vitaly Kuznetsov,
	Borislav Petkov, Sean Christopherson, Tom Lendacky, x86, LKML

The following commit has been merged into the x86/seves branch of tip:

Commit-ID:     e6eb15c9ba3165698488ae5c34920eea20eaa38e
Gitweb:        https://git.kernel.org/tip/e6eb15c9ba3165698488ae5c34920eea20eaa38e
Author:        Vitaly Kuznetsov <vkuznets@redhat.com>
AuthorDate:    Mon, 14 Sep 2020 15:37:25 +02:00
Committer:     Borislav Petkov <bp@suse.de>
CommitterDate: Mon, 14 Sep 2020 18:08:54 +02:00

KVM: nSVM: Avoid freeing uninitialized pointers in svm_set_nested_state()

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.

 [ bp: Drop obscure Addresses-Coverity: tag. ]

Fixes: 6ccbd29ade0d ("KVM: SVM: nested: Don't allocate VMCB structures on stack")
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>
Signed-off-by: Borislav Petkov <bp@suse.de>
Reviewed-by: Sean Christopherson <sean.j.christopherson@intel.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
Tested-by: Tom Lendacky <thomas.lendacky@amd.com>
Link: https://lkml.kernel.org/r/20200914133725.650221-1-vkuznets@redhat.com
---
 arch/x86/kvm/svm/nested.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 2803662..d1ae94f 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1092,7 +1092,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))
@@ -1145,7 +1146,6 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
 	load_nested_vmcb_control(svm, ctl);
 	nested_prepare_vmcb_control(svm);
 
-out_set_gif:
 	svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
 
 	ret = 0;

^ permalink raw reply related	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-09-14 17:19 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-14 13:37 [PATCH tip] KVM: nSVM: avoid freeing uninitialized pointers in svm_set_nested_state() Vitaly Kuznetsov
2020-09-14 15:10 ` Sean Christopherson
2020-09-14 15:44 ` Joerg Roedel
2020-09-14 16:03 ` Tom Lendacky
2020-09-14 17:16 ` [tip: x86/seves] KVM: nSVM: Avoid " tip-bot2 for Vitaly Kuznetsov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.