If creating a protected vCPU in kvm_s390_pv_create_cpu() fails, kvm_s390_pv_destroy_cpu() was called, which checks whether the vCPU has a PV handle and exits doing nothing otherwise. At that point, due to not having created the protected vCPU, the PV handle will not be set, and kvm_s390_pv_destroy_cpu() will do nothing, thus leaking the allocated memory. Fix by factoring out the code to free and reset a PV vCPU; call it from kvm_s390_pv_destroy_cpu() and kvm_s390_pv_create_cpu(). Opportunistically fix the return value of kvm_s390_pv_destroy_cpu() in case of errors: return -EIO instead if EIO. Fixes: d4074324b07a ("KVM: s390: pv: avoid double free of sida page") Signed-off-by: Claudio Imbrenda Reviewed-by: Steffen Eiden --- arch/s390/kvm/pv.c | 41 +++++++++++++++++++++-------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c index dc204b521052..b02e0159d3cd 100644 --- a/arch/s390/kvm/pv.c +++ b/arch/s390/kvm/pv.c @@ -244,6 +244,24 @@ static void kvm_s390_clear_pv_state(struct kvm *kvm) kvm->arch.pv.stor_var = NULL; } +static void kvm_s390_pv_dispose_cpu(struct kvm_vcpu *vcpu, bool free_stor_base) +{ + if (free_stor_base) + free_pages(vcpu->arch.pv.stor_base, get_order(uv_info.guest_cpu_stor_len)); + free_page((unsigned long)sida_addr(vcpu->arch.sie_block)); + vcpu->arch.sie_block->pv_handle_cpu = 0; + vcpu->arch.sie_block->pv_handle_config = 0; + memset(&vcpu->arch.pv, 0, sizeof(vcpu->arch.pv)); + vcpu->arch.sie_block->sdf = 0; + /* + * The sidad field (for sdf == 2) is now the gbea field (for sdf == 0). + * Use the reset value of gbea to avoid leaking the kernel pointer of + * the just freed sida. + */ + vcpu->arch.sie_block->gbea = 1; + kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); +} + int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) { int cc; @@ -258,24 +276,9 @@ int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) WARN_ONCE(cc, "protvirt destroy cpu failed rc %x rrc %x", *rc, *rrc); /* Intended memory leak for something that should never happen. */ - if (!cc) - free_pages(vcpu->arch.pv.stor_base, - get_order(uv_info.guest_cpu_stor_len)); - - free_page((unsigned long)sida_addr(vcpu->arch.sie_block)); - vcpu->arch.sie_block->pv_handle_cpu = 0; - vcpu->arch.sie_block->pv_handle_config = 0; - memset(&vcpu->arch.pv, 0, sizeof(vcpu->arch.pv)); - vcpu->arch.sie_block->sdf = 0; - /* - * The sidad field (for sdf == 2) is now the gbea field (for sdf == 0). - * Use the reset value of gbea to avoid leaking the kernel pointer of - * the just freed sida. - */ - vcpu->arch.sie_block->gbea = 1; - kvm_make_request(KVM_REQ_TLB_FLUSH, vcpu); + kvm_s390_pv_dispose_cpu(vcpu, !cc); - return cc ? EIO : 0; + return cc ? -EIO : 0; } int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) @@ -319,9 +322,7 @@ int kvm_s390_pv_create_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) uvcb.header.rrc); if (cc) { - u16 dummy; - - kvm_s390_pv_destroy_cpu(vcpu, &dummy, &dummy); + kvm_s390_pv_dispose_cpu(vcpu, true); return -EIO; } -- 2.55.0