KVM: s390: Fix cleanup in kvm_s390_pv_create_cpu()

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")
Reviewed-by: Steffen Eiden <seiden@linux.ibm.com>
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com>
Message-ID: <20260803124040.126471-14-imbrenda@linux.ibm.com>

diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c
index dc204b521052c48beb24a3cc440f03e5486463b3..b02e0159d3cd2199f4517d86d16a21d376a5efa3 100644 (file)
--- 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;
        }