* [PATCH] kvm: svm: fix unsigned compare less than zero comparison
@ 2016-09-19 6:11 Colin King
2016-09-19 10:06 ` Joerg Roedel
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Colin King @ 2016-09-19 6:11 UTC (permalink / raw)
To: Joerg Roedel, Paolo Bonzini, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86, kvm
Cc: linux-kernel
From: Colin Ian King <colin.king@canonical.com>
vm_data->avic_vm_id is a u32, so the check for a error
return (less than zero) such as -EAGAIN from
avic_get_next_vm_id currently has no effect whatsoever.
Fix this by using a temporary int for the comparison
and assign vm_data->avic_vm_id to this. I used an explicit
u32 cast in the assignment to show why vm_data->avic_vm_id
cannot be used in the assign/compare steps.
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
arch/x86/kvm/svm.c | 9 +++++----
1 file changed, 5 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
index 1b66c5a..2ca66aa 100644
--- a/arch/x86/kvm/svm.c
+++ b/arch/x86/kvm/svm.c
@@ -1419,7 +1419,7 @@ static void avic_vm_destroy(struct kvm *kvm)
static int avic_vm_init(struct kvm *kvm)
{
unsigned long flags;
- int err = -ENOMEM;
+ int vm_id, err = -ENOMEM;
struct kvm_arch *vm_data = &kvm->arch;
struct page *p_page;
struct page *l_page;
@@ -1427,9 +1427,10 @@ static int avic_vm_init(struct kvm *kvm)
if (!avic)
return 0;
- vm_data->avic_vm_id = avic_get_next_vm_id();
- if (vm_data->avic_vm_id < 0)
- return vm_data->avic_vm_id;
+ vm_id = avic_get_next_vm_id();
+ if (vm_id < 0)
+ return vm_id;
+ vm_data->avic_vm_id = (u32)vm_id;
/* Allocating physical APIC ID table (4KB) */
p_page = alloc_page(GFP_KERNEL);
--
2.9.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] kvm: svm: fix unsigned compare less than zero comparison
2016-09-19 6:11 [PATCH] kvm: svm: fix unsigned compare less than zero comparison Colin King
@ 2016-09-19 10:06 ` Joerg Roedel
2016-09-19 11:22 ` Paolo Bonzini
2016-09-20 2:50 ` Suravee Suthikulpanit
2 siblings, 0 replies; 4+ messages in thread
From: Joerg Roedel @ 2016-09-19 10:06 UTC (permalink / raw)
To: Colin King
Cc: Paolo Bonzini, Radim Krčmář, Thomas Gleixner,
Ingo Molnar, H . Peter Anvin, x86, kvm, linux-kernel
On Mon, Sep 19, 2016 at 07:11:59AM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> vm_data->avic_vm_id is a u32, so the check for a error
> return (less than zero) such as -EAGAIN from
> avic_get_next_vm_id currently has no effect whatsoever.
> Fix this by using a temporary int for the comparison
> and assign vm_data->avic_vm_id to this. I used an explicit
> u32 cast in the assignment to show why vm_data->avic_vm_id
> cannot be used in the assign/compare steps.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Acked-by: Joerg Roedel <jroedel@suse.de>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] kvm: svm: fix unsigned compare less than zero comparison
2016-09-19 6:11 [PATCH] kvm: svm: fix unsigned compare less than zero comparison Colin King
2016-09-19 10:06 ` Joerg Roedel
@ 2016-09-19 11:22 ` Paolo Bonzini
2016-09-20 2:50 ` Suravee Suthikulpanit
2 siblings, 0 replies; 4+ messages in thread
From: Paolo Bonzini @ 2016-09-19 11:22 UTC (permalink / raw)
To: Colin King, Joerg Roedel, Radim Krčmář,
Thomas Gleixner, Ingo Molnar, H . Peter Anvin, x86, kvm
Cc: linux-kernel
On 19/09/2016 08:11, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> vm_data->avic_vm_id is a u32, so the check for a error
> return (less than zero) such as -EAGAIN from
> avic_get_next_vm_id currently has no effect whatsoever.
> Fix this by using a temporary int for the comparison
> and assign vm_data->avic_vm_id to this. I used an explicit
> u32 cast in the assignment to show why vm_data->avic_vm_id
> cannot be used in the assign/compare steps.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> arch/x86/kvm/svm.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 1b66c5a..2ca66aa 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1419,7 +1419,7 @@ static void avic_vm_destroy(struct kvm *kvm)
> static int avic_vm_init(struct kvm *kvm)
> {
> unsigned long flags;
> - int err = -ENOMEM;
> + int vm_id, err = -ENOMEM;
> struct kvm_arch *vm_data = &kvm->arch;
> struct page *p_page;
> struct page *l_page;
> @@ -1427,9 +1427,10 @@ static int avic_vm_init(struct kvm *kvm)
> if (!avic)
> return 0;
>
> - vm_data->avic_vm_id = avic_get_next_vm_id();
> - if (vm_data->avic_vm_id < 0)
> - return vm_data->avic_vm_id;
> + vm_id = avic_get_next_vm_id();
> + if (vm_id < 0)
> + return vm_id;
> + vm_data->avic_vm_id = (u32)vm_id;
>
> /* Allocating physical APIC ID table (4KB) */
> p_page = alloc_page(GFP_KERNEL);
>
Applying the patch, thanks.
Paolo
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] kvm: svm: fix unsigned compare less than zero comparison
2016-09-19 6:11 [PATCH] kvm: svm: fix unsigned compare less than zero comparison Colin King
2016-09-19 10:06 ` Joerg Roedel
2016-09-19 11:22 ` Paolo Bonzini
@ 2016-09-20 2:50 ` Suravee Suthikulpanit
2 siblings, 0 replies; 4+ messages in thread
From: Suravee Suthikulpanit @ 2016-09-20 2:50 UTC (permalink / raw)
To: Colin King, Joerg Roedel, Paolo Bonzini,
Radim Krčmář, Thomas Gleixner, Ingo Molnar,
H . Peter Anvin, x86, kvm
Cc: linux-kernel
Hi,
On 9/19/16 13:11, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> vm_data->avic_vm_id is a u32, so the check for a error
> return (less than zero) such as -EAGAIN from
> avic_get_next_vm_id currently has no effect whatsoever.
> Fix this by using a temporary int for the comparison
> and assign vm_data->avic_vm_id to this. I used an explicit
> u32 cast in the assignment to show why vm_data->avic_vm_id
> cannot be used in the assign/compare steps.
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> arch/x86/kvm/svm.c | 9 +++++----
> 1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c
> index 1b66c5a..2ca66aa 100644
> --- a/arch/x86/kvm/svm.c
> +++ b/arch/x86/kvm/svm.c
> @@ -1419,7 +1419,7 @@ static void avic_vm_destroy(struct kvm *kvm)
> static int avic_vm_init(struct kvm *kvm)
> {
> unsigned long flags;
> - int err = -ENOMEM;
> + int vm_id, err = -ENOMEM;
> struct kvm_arch *vm_data = &kvm->arch;
> struct page *p_page;
> struct page *l_page;
> @@ -1427,9 +1427,10 @@ static int avic_vm_init(struct kvm *kvm)
> if (!avic)
> return 0;
>
> - vm_data->avic_vm_id = avic_get_next_vm_id();
> - if (vm_data->avic_vm_id < 0)
> - return vm_data->avic_vm_id;
> + vm_id = avic_get_next_vm_id();
> + if (vm_id < 0)
> + return vm_id;
> + vm_data->avic_vm_id = (u32)vm_id;
>
> /* Allocating physical APIC ID table (4KB) */
> p_page = alloc_page(GFP_KERNEL);
>
Thanks for catching this.
Suravee
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-09-20 2:47 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-19 6:11 [PATCH] kvm: svm: fix unsigned compare less than zero comparison Colin King
2016-09-19 10:06 ` Joerg Roedel
2016-09-19 11:22 ` Paolo Bonzini
2016-09-20 2:50 ` Suravee Suthikulpanit
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).