[v1] KVM: s390x: fix memory overwrites when not using SCA entries
diff mbox

Message ID 20180306132758.21034-1-david@redhat.com
State New
Headers show

Commit Message

David Hildenbrand March 6, 2018, 1:27 p.m. UTC
Even if we don't have extended SCA support, we can have more than 64 CPUs
if we don't enable any HW features that might use the SCA entries.

Now, this works just fine, but we missed a return, which is why we
would actually store the SCA entries. If we have more than 64 CPUs, this
means writing outside of the basic SCA - bad.

Let's fix this. This allows > 64 CPUs when running nested (under vSIE)
without random crashes.

Fixes: a6940674c384 ("KVM: s390: allow 255 VCPUs when sca entries aren't used")
Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: David Hildenbrand <david@redhat.com>
---
 arch/s390/kvm/kvm-s390.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Cornelia Huck March 6, 2018, 1:29 p.m. UTC | #1
On Tue,  6 Mar 2018 14:27:58 +0100
David Hildenbrand <david@redhat.com> wrote:

> Even if we don't have extended SCA support, we can have more than 64 CPUs
> if we don't enable any HW features that might use the SCA entries.
> 
> Now, this works just fine, but we missed a return, which is why we
> would actually store the SCA entries. If we have more than 64 CPUs, this
> means writing outside of the basic SCA - bad.
> 
> Let's fix this. This allows > 64 CPUs when running nested (under vSIE)
> without random crashes.
> 
> Fixes: a6940674c384 ("KVM: s390: allow 255 VCPUs when sca entries aren't used")

cc: stable?

> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
>  arch/s390/kvm/kvm-s390.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
> index 77d7818130db..321bfbc67d3d 100644
> --- a/arch/s390/kvm/kvm-s390.c
> +++ b/arch/s390/kvm/kvm-s390.c
> @@ -2146,6 +2146,7 @@ static void sca_add_vcpu(struct kvm_vcpu *vcpu)
>  		/* we still need the basic sca for the ipte control */
>  		vcpu->arch.sie_block->scaoh = (__u32)(((__u64)sca) >> 32);
>  		vcpu->arch.sie_block->scaol = (__u32)(__u64)sca;
> +		return;
>  	}
>  	read_lock(&vcpu->kvm->arch.sca_lock);
>  	if (vcpu->kvm->arch.use_esca) {

Reviewed-by: Cornelia Huck <cohuck@redhat.com>
Christian Borntraeger March 6, 2018, 1:30 p.m. UTC | #2
7

On 03/06/2018 02:29 PM, Cornelia Huck wrote:
> On Tue,  6 Mar 2018 14:27:58 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> Even if we don't have extended SCA support, we can have more than 64 CPUs
>> if we don't enable any HW features that might use the SCA entries.
>>
>> Now, this works just fine, but we missed a return, which is why we
>> would actually store the SCA entries. If we have more than 64 CPUs, this
>> means writing outside of the basic SCA - bad.
>>
>> Let's fix this. This allows > 64 CPUs when running nested (under vSIE)
>> without random crashes.
>>
>> Fixes: a6940674c384 ("KVM: s390: allow 255 VCPUs when sca entries aren't used")
> 
> cc: stable?

yes.
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  arch/s390/kvm/kvm-s390.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 77d7818130db..321bfbc67d3d 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2146,6 +2146,7 @@ static void sca_add_vcpu(struct kvm_vcpu *vcpu)
>>  		/* we still need the basic sca for the ipte control */
>>  		vcpu->arch.sie_block->scaoh = (__u32)(((__u64)sca) >> 32);
>>  		vcpu->arch.sie_block->scaol = (__u32)(__u64)sca;
>> +		return;
>>  	}
>>  	read_lock(&vcpu->kvm->arch.sca_lock);
>>  	if (vcpu->kvm->arch.use_esca) {
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>

Thanks applied and queued for master.
David Hildenbrand March 6, 2018, 1:30 p.m. UTC | #3
On 06.03.2018 14:29, Cornelia Huck wrote:
> On Tue,  6 Mar 2018 14:27:58 +0100
> David Hildenbrand <david@redhat.com> wrote:
> 
>> Even if we don't have extended SCA support, we can have more than 64 CPUs
>> if we don't enable any HW features that might use the SCA entries.
>>
>> Now, this works just fine, but we missed a return, which is why we
>> would actually store the SCA entries. If we have more than 64 CPUs, this
>> means writing outside of the basic SCA - bad.
>>
>> Let's fix this. This allows > 64 CPUs when running nested (under vSIE)
>> without random crashes.
>>
>> Fixes: a6940674c384 ("KVM: s390: allow 255 VCPUs when sca entries aren't used")
> 
> cc: stable?

Think so!

If whoever picks this up can also fix the subject s/s390x/s390/, that
would be nice.

> 
>> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com>
>> Signed-off-by: David Hildenbrand <david@redhat.com>
>> ---
>>  arch/s390/kvm/kvm-s390.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
>> index 77d7818130db..321bfbc67d3d 100644
>> --- a/arch/s390/kvm/kvm-s390.c
>> +++ b/arch/s390/kvm/kvm-s390.c
>> @@ -2146,6 +2146,7 @@ static void sca_add_vcpu(struct kvm_vcpu *vcpu)
>>  		/* we still need the basic sca for the ipte control */
>>  		vcpu->arch.sie_block->scaoh = (__u32)(((__u64)sca) >> 32);
>>  		vcpu->arch.sie_block->scaol = (__u32)(__u64)sca;
>> +		return;
>>  	}
>>  	read_lock(&vcpu->kvm->arch.sca_lock);
>>  	if (vcpu->kvm->arch.use_esca) {
> 
> Reviewed-by: Cornelia Huck <cohuck@redhat.com>
>

Patch
diff mbox

diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c
index 77d7818130db..321bfbc67d3d 100644
--- a/arch/s390/kvm/kvm-s390.c
+++ b/arch/s390/kvm/kvm-s390.c
@@ -2146,6 +2146,7 @@  static void sca_add_vcpu(struct kvm_vcpu *vcpu)
 		/* we still need the basic sca for the ipte control */
 		vcpu->arch.sie_block->scaoh = (__u32)(((__u64)sca) >> 32);
 		vcpu->arch.sie_block->scaol = (__u32)(__u64)sca;
+		return;
 	}
 	read_lock(&vcpu->kvm->arch.sca_lock);
 	if (vcpu->kvm->arch.use_esca) {