Messages in this thread | | | Date | Tue, 5 Oct 2021 15:11:47 +0200 | Subject | Re: [PATCH v5 02/14] KVM: s390: pv: avoid double free of sida page | From | Janosch Frank <> |
| |
On 9/20/21 15:24, Claudio Imbrenda wrote: > If kvm_s390_pv_destroy_cpu is called more than once, we risk calling > free_page on a random page, since the sidad field is aliased with the > gbea, which is not guaranteed to be zero. > > This can happen, for example, if userspace calls the KVM_PV_DISABLE > IOCTL, and it fails, and then userspace calls the same IOCTL again. > This scenario is only possible if KVM has some serious bug or if the > hardware is broken. > > The solution is to simply return successfully immediately if the vCPU > was already non secure. > > Signed-off-by: Claudio Imbrenda <imbrenda@linux.ibm.com> > Fixes: 19e1227768863a1469797c13ef8fea1af7beac2c ("KVM: S390: protvirt: Introduce instruction data area bounce buffer")
Reviewed-by: Janosch Frank <frankja@linux.ibm.com>
> --- > arch/s390/kvm/pv.c | 19 +++++++++---------- > 1 file changed, 9 insertions(+), 10 deletions(-) > > diff --git a/arch/s390/kvm/pv.c b/arch/s390/kvm/pv.c > index c8841f476e91..0a854115100b 100644 > --- a/arch/s390/kvm/pv.c > +++ b/arch/s390/kvm/pv.c > @@ -16,18 +16,17 @@ > > int kvm_s390_pv_destroy_cpu(struct kvm_vcpu *vcpu, u16 *rc, u16 *rrc) > { > - int cc = 0; > + int cc; > > - if (kvm_s390_pv_cpu_get_handle(vcpu)) { > - cc = uv_cmd_nodata(kvm_s390_pv_cpu_get_handle(vcpu), > - UVC_CMD_DESTROY_SEC_CPU, rc, rrc); > + if (!kvm_s390_pv_cpu_get_handle(vcpu)) > + return 0; > + > + cc = uv_cmd_nodata(kvm_s390_pv_cpu_get_handle(vcpu), UVC_CMD_DESTROY_SEC_CPU, rc, rrc); > + > + KVM_UV_EVENT(vcpu->kvm, 3, "PROTVIRT DESTROY VCPU %d: rc %x rrc %x", > + vcpu->vcpu_id, *rc, *rrc); > + WARN_ONCE(cc, "protvirt destroy cpu failed rc %x rrc %x", *rc, *rrc); > > - KVM_UV_EVENT(vcpu->kvm, 3, > - "PROTVIRT DESTROY VCPU %d: rc %x rrc %x", > - vcpu->vcpu_id, *rc, *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, >
| |