Message ID | 20210415080127.1061275-1-hca@linux.ibm.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | KVM: s390: fix guarded storage control register handling | expand |
On 15.04.21 10:01, Heiko Carstens wrote: > store_regs_fmt2() has an ordering problem: first the guarded storage > facility is enabled on the local cpu, then preemption disabled, and > then the STGSC (store guarded storage controls) instruction is > executed. > > If the process gets scheduled away between enabling the guarded > storage facility and before preemption is disabled, this might lead to > a special operation exception and therefore kernel crash as soon as > the process is scheduled back and the STGSC instruction is executed. > > Fixes: 4e0b1ab72b8a ("KVM: s390: gs support for kvm guests") > Cc: <stable@vger.kernel.org> # 4.12 > Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 2f09e9d7dc95..24ad447e648c 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4307,16 +4307,16 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu) > kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; > kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val; > if (MACHINE_HAS_GS) { > + preempt_disable(); > __ctl_set_bit(2, 4); > if (vcpu->arch.gs_enabled) > save_gs_cb(current->thread.gs_cb); > - preempt_disable(); > current->thread.gs_cb = vcpu->arch.host_gscb; > restore_gs_cb(vcpu->arch.host_gscb); > - preempt_enable(); > if (!vcpu->arch.host_gscb) > __ctl_clear_bit(2, 4); > vcpu->arch.host_gscb = NULL; > + preempt_enable(); > } > /* SIE will save etoken directly into SDNX and therefore kvm_run */ > } > LGTM Reviewed-by: David Hildenbrand <david@redhat.com>
On 4/15/21 10:01 AM, Heiko Carstens wrote: > store_regs_fmt2() has an ordering problem: first the guarded storage > facility is enabled on the local cpu, then preemption disabled, and > then the STGSC (store guarded storage controls) instruction is > executed. > > If the process gets scheduled away between enabling the guarded > storage facility and before preemption is disabled, this might lead to > a special operation exception and therefore kernel crash as soon as > the process is scheduled back and the STGSC instruction is executed. > > Fixes: 4e0b1ab72b8a ("KVM: s390: gs support for kvm guests") > Cc: <stable@vger.kernel.org> # 4.12 > Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com> Reviewed-by: Janosch Frank <frankja@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 2f09e9d7dc95..24ad447e648c 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4307,16 +4307,16 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu) > kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; > kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val; > if (MACHINE_HAS_GS) { > + preempt_disable(); > __ctl_set_bit(2, 4); > if (vcpu->arch.gs_enabled) > save_gs_cb(current->thread.gs_cb); > - preempt_disable(); > current->thread.gs_cb = vcpu->arch.host_gscb; > restore_gs_cb(vcpu->arch.host_gscb); > - preempt_enable(); > if (!vcpu->arch.host_gscb) > __ctl_clear_bit(2, 4); > vcpu->arch.host_gscb = NULL; > + preempt_enable(); > } > /* SIE will save etoken directly into SDNX and therefore kvm_run */ > } >
On Thu, 15 Apr 2021 10:01:27 +0200 Heiko Carstens <hca@linux.ibm.com> wrote: > store_regs_fmt2() has an ordering problem: first the guarded storage > facility is enabled on the local cpu, then preemption disabled, and > then the STGSC (store guarded storage controls) instruction is > executed. > > If the process gets scheduled away between enabling the guarded > storage facility and before preemption is disabled, this might lead to > a special operation exception and therefore kernel crash as soon as > the process is scheduled back and the STGSC instruction is executed. > > Fixes: 4e0b1ab72b8a ("KVM: s390: gs support for kvm guests") > Cc: <stable@vger.kernel.org> # 4.12 > Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com> > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) Reviewed-by: Cornelia Huck <cohuck@redhat.com>
On 15.04.21 10:01, Heiko Carstens wrote: > store_regs_fmt2() has an ordering problem: first the guarded storage > facility is enabled on the local cpu, then preemption disabled, and > then the STGSC (store guarded storage controls) instruction is > executed. > > If the process gets scheduled away between enabling the guarded > storage facility and before preemption is disabled, this might lead to > a special operation exception and therefore kernel crash as soon as > the process is scheduled back and the STGSC instruction is executed. > > Fixes: 4e0b1ab72b8a ("KVM: s390: gs support for kvm guests") > Cc: <stable@vger.kernel.org> # 4.12 > Reviewed-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Heiko Carstens <hca@linux.ibm.com> Thanks applied. Will queue for the s390kvm tree after the CI/regression has finished. > --- > arch/s390/kvm/kvm-s390.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c > index 2f09e9d7dc95..24ad447e648c 100644 > --- a/arch/s390/kvm/kvm-s390.c > +++ b/arch/s390/kvm/kvm-s390.c > @@ -4307,16 +4307,16 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu) > kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; > kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val; > if (MACHINE_HAS_GS) { > + preempt_disable(); > __ctl_set_bit(2, 4); > if (vcpu->arch.gs_enabled) > save_gs_cb(current->thread.gs_cb); > - preempt_disable(); > current->thread.gs_cb = vcpu->arch.host_gscb; > restore_gs_cb(vcpu->arch.host_gscb); > - preempt_enable(); > if (!vcpu->arch.host_gscb) > __ctl_clear_bit(2, 4); > vcpu->arch.host_gscb = NULL; > + preempt_enable(); > } > /* SIE will save etoken directly into SDNX and therefore kvm_run */ > } >
diff --git a/arch/s390/kvm/kvm-s390.c b/arch/s390/kvm/kvm-s390.c index 2f09e9d7dc95..24ad447e648c 100644 --- a/arch/s390/kvm/kvm-s390.c +++ b/arch/s390/kvm/kvm-s390.c @@ -4307,16 +4307,16 @@ static void store_regs_fmt2(struct kvm_vcpu *vcpu) kvm_run->s.regs.bpbc = (vcpu->arch.sie_block->fpf & FPF_BPBC) == FPF_BPBC; kvm_run->s.regs.diag318 = vcpu->arch.diag318_info.val; if (MACHINE_HAS_GS) { + preempt_disable(); __ctl_set_bit(2, 4); if (vcpu->arch.gs_enabled) save_gs_cb(current->thread.gs_cb); - preempt_disable(); current->thread.gs_cb = vcpu->arch.host_gscb; restore_gs_cb(vcpu->arch.host_gscb); - preempt_enable(); if (!vcpu->arch.host_gscb) __ctl_clear_bit(2, 4); vcpu->arch.host_gscb = NULL; + preempt_enable(); } /* SIE will save etoken directly into SDNX and therefore kvm_run */ }