| 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 */ }
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(-)