diff mbox

[4/6] kvm-s390: Unlink vcpu on destroy

Message ID 1241534358-32172-5-git-send-email-ehrhardt@linux.vnet.ibm.com (mailing list archive)
State New, archived
Headers show

Commit Message

ehrhardt@linux.vnet.ibm.com May 5, 2009, 2:39 p.m. UTC
From: Carsten Otte <cotte@de.ibm.com>

This patch makes sure we do unlink a vcpu's sie control block
from the system control area in kvm_arch_vcpu_destroy. This
prevents illegal accesses to the sie control block from other
virtual cpus after free.

Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
Signed-off-by: Carsten Otte <cotte@de.ibm.com>
---
 arch/s390/kvm/kvm-s390.c |    9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Avi Kivity May 6, 2009, 12:11 p.m. UTC | #1
ehrhardt@linux.vnet.ibm.com wrote:
> From: Carsten Otte <cotte@de.ibm.com>
>
> This patch makes sure we do unlink a vcpu's sie control block
> from the system control area in kvm_arch_vcpu_destroy. This
> prevents illegal accesses to the sie control block from other
> virtual cpus after free.
>
> Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
> Signed-off-by: Carsten Otte <cotte@de.ibm.com>
> ---
>  arch/s390/kvm/kvm-s390.c |    9 +++++++--
>  1 file changed, 7 insertions(+), 2 deletions(-)
>
> Index: kvm/arch/s390/kvm/kvm-s390.c
> ===================================================================
> --- kvm.orig/arch/s390/kvm/kvm-s390.c
> +++ kvm/arch/s390/kvm/kvm-s390.c
> @@ -195,6 +195,9 @@ out_nokvm:
>  void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>  {
>  	VCPU_EVENT(vcpu, 3, "%s", "free cpu");
> +	if (vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda ==
> +		(__u64) vcpu->arch.sie_block)
> +		vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda = 0;
>  	free_page((unsigned long)(vcpu->arch.sie_block));
>
>   

If this is accessed by hardware on a different cpu, don't you need a 
memory barrier here?
ehrhardt@linux.vnet.ibm.com May 11, 2009, 1 p.m. UTC | #2
Avi Kivity wrote:
> ehrhardt@linux.vnet.ibm.com wrote:
>> From: Carsten Otte <cotte@de.ibm.com>
>>
>> This patch makes sure we do unlink a vcpu's sie control block
>> from the system control area in kvm_arch_vcpu_destroy. This
>> prevents illegal accesses to the sie control block from other
>> virtual cpus after free.
>>
>> Reported-by: Mijo Safradin <mijo@linux.vnet.ibm.com>
>> Signed-off-by: Carsten Otte <cotte@de.ibm.com>
>> ---
>>  arch/s390/kvm/kvm-s390.c |    9 +++++++--
>>  1 file changed, 7 insertions(+), 2 deletions(-)
>>
>> Index: kvm/arch/s390/kvm/kvm-s390.c
>> ===================================================================
>> --- kvm.orig/arch/s390/kvm/kvm-s390.c
>> +++ kvm/arch/s390/kvm/kvm-s390.c
>> @@ -195,6 +195,9 @@ out_nokvm:
>>  void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
>>  {
>>      VCPU_EVENT(vcpu, 3, "%s", "free cpu");
>> +    if (vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda ==
>> +        (__u64) vcpu->arch.sie_block)
>> +        vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda = 0;
>>      free_page((unsigned long)(vcpu->arch.sie_block));
>>
>>   
>
> If this is accessed by hardware on a different cpu, don't you need a 
> memory barrier here?
>
>
Right, will be in v2
diff mbox

Patch

Index: kvm/arch/s390/kvm/kvm-s390.c
===================================================================
--- kvm.orig/arch/s390/kvm/kvm-s390.c
+++ kvm/arch/s390/kvm/kvm-s390.c
@@ -195,6 +195,9 @@  out_nokvm:
 void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
 {
 	VCPU_EVENT(vcpu, 3, "%s", "free cpu");
+	if (vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda ==
+		(__u64) vcpu->arch.sie_block)
+		vcpu->kvm->arch.sca->cpu[vcpu->vcpu_id].sda = 0;
 	free_page((unsigned long)(vcpu->arch.sie_block));
 	kvm_vcpu_uninit(vcpu);
 	kfree(vcpu);
@@ -307,8 +310,10 @@  struct kvm_vcpu *kvm_arch_vcpu_create(st
 
 	vcpu->arch.sie_block->icpua = id;
 	BUG_ON(!kvm->arch.sca);
-	BUG_ON(kvm->arch.sca->cpu[id].sda);
-	kvm->arch.sca->cpu[id].sda = (__u64) vcpu->arch.sie_block;
+	if (!kvm->arch.sca->cpu[id].sda)
+		kvm->arch.sca->cpu[id].sda = (__u64) vcpu->arch.sie_block;
+	else
+		BUG_ON(!kvm->vcpus[id]); /* vcpu does already exist */
 	vcpu->arch.sie_block->scaoh = (__u32)(((__u64)kvm->arch.sca) >> 32);
 	vcpu->arch.sie_block->scaol = (__u32)(__u64)kvm->arch.sca;