Re: [PATCH] KVM: s390: pci: fix GAIT table indexing due to double-scaling pointer arithmetic

[Christian Borntraeger]

Am 15.04.26 um 11:26 schrieb Junrui Luo:
> kvm_s390_pci_aif_enable(), kvm_s390_pci_aif_disable(), and
> aen_host_forward() index the GAIT by manually multiplying the index
> with sizeof(struct zpci_gaite).
>
> Since aift->gait is already a struct zpci_gaite pointer, this
> double-scales the offset, accessing element aisb*16 instead of aisb.
>
> This causes out-of-bounds accesses when aisb >= 32 (with
> ZPCI_NR_DEVICES=512)
>
> Fix by removing the erroneous sizeof multiplication.
>
> Fixes: 3c5a1b6f0a18 ("KVM: s390: pci: provide routines for enabling/disabling interrupt forwarding")
> Fixes: 73f91b004321 ("KVM: s390: pci: enable host forwarding of Adapter Event Notifications")
> Reported-by: Yuhao Jiang <danisjiang@xxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Junrui Luo <moonafterrain@xxxxxxxxxxx>

looks good to me.
Reviewed-by: Christian Borntraeger <borntraeger@xxxxxxxxxxxxx>

Out of interest, was this found by static code checking or AI by any chance?



@Matt, can you test/review this as well?

> ---
>   arch/s390/kvm/interrupt.c | 3 +--
>   arch/s390/kvm/pci.c       | 6 ++----
>   2 files changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
> index 7cb8ce833b62..f48f25c7dc8f 100644
> --- a/arch/s390/kvm/interrupt.c
> +++ b/arch/s390/kvm/interrupt.c
> @@ -3307,8 +3307,7 @@ static void aen_host_forward(unsigned long si)
>   	struct zpci_gaite *gaite;
>   	struct kvm *kvm;
>   
> -	gaite = (struct zpci_gaite *)aift->gait +
> -		(si * sizeof(struct zpci_gaite));
> +	gaite = aift->gait + si;
>   	if (gaite->count == 0)
>   		return;
>   	if (gaite->aisb != 0)
> diff --git a/arch/s390/kvm/pci.c b/arch/s390/kvm/pci.c
> index 86d93e8dddae..eed45af1a92d 100644
> --- a/arch/s390/kvm/pci.c
> +++ b/arch/s390/kvm/pci.c
> @@ -290,8 +290,7 @@ static int kvm_s390_pci_aif_enable(struct zpci_dev *zdev, struct zpci_fib *fib,
>   				    phys_to_virt(fib->fmt0.aibv));
>   
>   	spin_lock_irq(&aift->gait_lock);
> -	gaite = (struct zpci_gaite *)aift->gait + (zdev->aisb *
> -						   sizeof(struct zpci_gaite));
> +	gaite = aift->gait + zdev->aisb;
>   
>   	/* If assist not requested, host will get all alerts */
>   	if (assist)
> @@ -357,8 +356,7 @@ static int kvm_s390_pci_aif_disable(struct zpci_dev *zdev, bool force)
>   	if (zdev->kzdev->fib.fmt0.aibv == 0)
>   		goto out;
>   	spin_lock_irq(&aift->gait_lock);
> -	gaite = (struct zpci_gaite *)aift->gait + (zdev->aisb *
> -						   sizeof(struct zpci_gaite));
> +	gaite = aift->gait + zdev->aisb;
>   	isc = gaite->gisc;
>   	gaite->count--;
>   	if (gaite->count == 0) {