[Qemu-devel] [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Qemu-devel] [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1185 bytes --]

KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
small for all MSRs. But this is precisely the error we trigger with the
initial request in order to obtain that size. Do not fail in that case.

This caused a subtle corruption of the guest state as MSR_STAR was not
properly saved/restored. The corruption became visible with latest kvm
optimizing the MSR updates.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---

 target-i386/kvm.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 3b61a7f..88b504c 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -244,9 +244,9 @@ static int kvm_has_msr_star(CPUState *env)
          * save/restore */
         msr_list.nmsrs = 0;
         ret = kvm_ioctl(env->kvm_state, KVM_GET_MSR_INDEX_LIST, &msr_list);
-        if (ret < 0)
+        if (ret < 0 && ret != -E2BIG) {
             return 0;
-
+        }
         /* Old kernel modules had a bug and could write beyond the provided
            memory. Allocate at least a safe amount of 1K. */
         kvm_msr_list = qemu_mallocz(MAX(1024, sizeof(msr_list) +


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] [FOR 0.12][FOR 0.11][PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 1361 bytes --]

Sorry, this is of course a critical fix for all branches that have KVM
support.

Jan

Jan Kiszka wrote:
> KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
> small for all MSRs. But this is precisely the error we trigger with the
> initial request in order to obtain that size. Do not fail in that case.
> 
> This caused a subtle corruption of the guest state as MSR_STAR was not
> properly saved/restored. The corruption became visible with latest kvm
> optimizing the MSR updates.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> 
>  target-i386/kvm.c |    4 ++--
>  1 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 3b61a7f..88b504c 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -244,9 +244,9 @@ static int kvm_has_msr_star(CPUState *env)
>           * save/restore */
>          msr_list.nmsrs = 0;
>          ret = kvm_ioctl(env->kvm_state, KVM_GET_MSR_INDEX_LIST, &msr_list);
> -        if (ret < 0)
> +        if (ret < 0 && ret != -E2BIG) {
>              return 0;
> -
> +        }
>          /* Old kernel modules had a bug and could write beyond the provided
>             memory. Allocate at least a safe amount of 1K. */
>          kvm_msr_list = qemu_mallocz(MAX(1024, sizeof(msr_list) +
> 



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

Re: [Qemu-devel] [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Avi Kivity]

On 12/06/2009 04:51 PM, Jan Kiszka wrote:
> KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
> small for all MSRs. But this is precisely the error we trigger with the
> initial request in order to obtain that size. Do not fail in that case.
>
> This caused a subtle corruption of the guest state as MSR_STAR was not
> properly saved/restored. The corruption became visible with latest kvm
> optimizing the MSR updates.
>    

Strong ack.  Anthony, please apply ASAP.  This is worthy of a 0.11.2, IMO.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Avi Kivity]

On 12/06/2009 04:51 PM, Jan Kiszka wrote:
> KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
> small for all MSRs. But this is precisely the error we trigger with the
> initial request in order to obtain that size. Do not fail in that case.
>
> This caused a subtle corruption of the guest state as MSR_STAR was not
> properly saved/restored. The corruption became visible with latest kvm
> optimizing the MSR updates.
>    

I applied this to qemu-kvm since it missed 0.12-rc1.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Jan Kiszka]

[-- Attachment #1: Type: text/plain, Size: 660 bytes --]

Avi Kivity wrote:
> On 12/06/2009 04:51 PM, Jan Kiszka wrote:
>> KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
>> small for all MSRs. But this is precisely the error we trigger with the
>> initial request in order to obtain that size. Do not fail in that case.
>>
>> This caused a subtle corruption of the guest state as MSR_STAR was not
>> properly saved/restored. The corruption became visible with latest kvm
>> optimizing the MSR updates.
>>    
> 
> I applied this to qemu-kvm since it missed 0.12-rc1.
> 

OK, but this part is unused in qemu-kvm as we use our own (unaffected)
implementation there so far.

Jan


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 257 bytes --]

[Qemu-devel] Re: [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Avi Kivity]

On 12/07/2009 10:24 AM, Jan Kiszka wrote:
>
>> I applied this to qemu-kvm since it missed 0.12-rc1.
>>
>>      
> OK, but this part is unused in qemu-kvm as we use our own (unaffected)
> implementation there so far.
>    

#include <toldyouso.h>

Will unapply.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Anthony Liguori]

Avi Kivity wrote:
> On 12/06/2009 04:51 PM, Jan Kiszka wrote:
>> KVM_GET_MSR_INDEX_LIST returns -E2BIG when the provided space is too
>> small for all MSRs. But this is precisely the error we trigger with the
>> initial request in order to obtain that size. Do not fail in that case.
>>
>> This caused a subtle corruption of the guest state as MSR_STAR was not
>> properly saved/restored. The corruption became visible with latest kvm
>> optimizing the MSR updates.
>>    
>
> I applied this to qemu-kvm since it missed 0.12-rc1.

I've got it in my tree.  It will be a part of 0.12-rc2 and I'll push to 
master today.

Don't know when 0.11.2 will happen but I'll also push it to the 
stable-0.11 branch.

-- 
Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Avi Kivity]

On 12/07/2009 04:47 PM, Anthony Liguori wrote:
>
> Don't know when 0.11.2 will happen but I'll also push it to the 
> stable-0.11 branch.
>

I think this is worthy of a 0.11.2 all by itself.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] kvm: x86: Fix initial kvm_has_msr_star

[Anthony Liguori]

Avi Kivity wrote:
> On 12/07/2009 04:47 PM, Anthony Liguori wrote:
>>
>> Don't know when 0.11.2 will happen but I'll also push it to the 
>> stable-0.11 branch.
>>
>
> I think this is worthy of a 0.11.2 all by itself.

Understood.  I'll see what I can do today.

-- 
Regards,

Anthony Liguori