Re: [PATCH] KVM: s390/diag: fix racy access of physical cpu number in diag 9c handler

[Claudio Imbrenda <imbrenda@linux.ibm.com>]

On Tue, 23 May 2023 16:05:00 +0200
Christian Borntraeger <borntraeger@linux.ibm.com> wrote:

> We do check for target CPU == -1, but this might change at the time we
> are going to use it. Hold the physical target CPU in a local variable to
> avoid out-of-bound accesses to the cpu arrays.
> 
> Cc: Pierre Morel <pmorel@linux.ibm.com>
> Fixes: 87e28a15c42c ("KVM: s390: diag9c (directed yield) forwarding")
> Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com>
> Reviewed-by: Nico Boehr <nrb@linux.ibm.com>
> Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

Reviewed-by: Claudio Imbrenda <imbrenda@linux.ibm.com>

> Signed-off-by: Christian Borntraeger <borntraeger@linux.ibm.com>
> ---
>  arch/s390/kvm/diag.c | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/s390/kvm/diag.c b/arch/s390/kvm/diag.c
> index 807fa9da1e72..3c65b8258ae6 100644
> --- a/arch/s390/kvm/diag.c
> +++ b/arch/s390/kvm/diag.c
> @@ -166,6 +166,7 @@ static int diag9c_forwarding_overrun(void)
>  static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
>  {
>  	struct kvm_vcpu *tcpu;
> +	int tcpu_cpu;
>  	int tid;
>  
>  	tid = vcpu->run->s.regs.gprs[(vcpu->arch.sie_block->ipa & 0xf0) >> 4];
> @@ -181,14 +182,15 @@ static int __diag_time_slice_end_directed(struct kvm_vcpu *vcpu)
>  		goto no_yield;
>  
>  	/* target guest VCPU already running */
> -	if (READ_ONCE(tcpu->cpu) >= 0) {
> +	tcpu_cpu = READ_ONCE(tcpu->cpu);
> +	if (tcpu_cpu >= 0) {
>  		if (!diag9c_forwarding_hz || diag9c_forwarding_overrun())
>  			goto no_yield;
>  
>  		/* target host CPU already running */
> -		if (!vcpu_is_preempted(tcpu->cpu))
> +		if (!vcpu_is_preempted(tcpu_cpu))
>  			goto no_yield;
> -		smp_yield_cpu(tcpu->cpu);
> +		smp_yield_cpu(tcpu_cpu);
>  		VCPU_EVENT(vcpu, 5,
>  			   "diag time slice end directed to %d: yield forwarded",
>  			   tid);