Re: [PATCH v6 32/49] KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Failure to clear lpi_ist_restore_pending when IST_BASER.Valid is cleared by userspace, permanently preventing VM execution.
- [Medium] KVM_HAS_DEVICE_ATTR fails with -EINVAL for KVM_DEV_ARM_VGIC_GRP_IRS_REGS if no VCPUs are created yet.
- [Medium] Userspace can bypass KVM's intentional 16-bit limit for GICV5_IRS_IDR2.ID_BITS, potentially causing oversized memory allocations.
--

commit 114e2a53a8a49611a9a0cdd053417985c38f2cd2
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Handle userspace accesses to IRS MMIO region

As part of saving and restoring the state of a GICv5-based system,
userspace must save and restore the IRS MMIO registers. These include
important information such as the guest IST configuration, and KVM
must present consistent state to the guest after migration.

> diff --git a/arch/arm64/kvm/vgic/vgic-irs-v5.c b/arch/arm64/kvm/vgic/vgic-irs-v5.c
> index 06d2deb770a6d..9e9853b88ce2a 100644
> --- a/arch/arm64/kvm/vgic/vgic-irs-v5.c
> +++ b/arch/arm64/kvm/vgic/vgic-irs-v5.c
[ ... ]
> @@ -431,90 +489,231 @@ static void vgic_v5_mmio_write_irs_ist(struct kvm_vcpu *vcpu, gpa_t addr,
[ ... ]
> +static int vgic_v5_mmio_uaccess_write_irs(struct kvm_vcpu *vcpu, gpa_t addr,
> +					  unsigned int len, unsigned long val)
> +{
[ ... ]
> +	case GICV5_IRS_IDR2:
> +		/* We always support LPIs */
> +		if (!(val & GICV5_IRS_IDR2_LPI))
> +			return -EINVAL;
[ ... ]
> +		/* We can't present more bits than we have support for in HW */
> +		if (FIELD_GET(GICV5_IRS_IDR2_ID_BITS, val) >
> +		    vgic_v5_irs_ist_id_bits(&irs_caps))
> +			return -EINVAL;

[Severity: Medium]
Can userspace bypass KVM's intentional 16-bit limit for
GICV5_IRS_IDR2.ID_BITS here?

KVM caps this to 16 during initialization, but this check appears to allow
userspace to restore a value up to the hardware maximum. This could
potentially cause architecturally inconsistent state and oversized host memory
allocations.

> +
> +		/* Min LPI ID bits must be greater than or equal to the HW */
> +		if (FIELD_GET(GICV5_IRS_IDR2_MIN_LPI_ID_BITS, val) <
> +		    vgic_v5_irs_min_lpi_id_bits(&irs_caps))
> +			return -EINVAL;
[ ... ]
> +		irs_data->idr2.id_bits = FIELD_GET(GICV5_IRS_IDR2_ID_BITS, val);
> +		break;
[ ... ]
> +	case GICV5_IRS_IST_BASER:
> +		if (irs_data->ist_baser.valid &&
> +		    !vgic_v5_ist_baser_matches(irs_data, val))
> +			return -EINVAL;
> +
> +		if (!irs_data->ist_baser.valid &&
> +		    (val & GICV5_IRS_IST_BASER_VALID))
> +			irs_data->lpi_ist_restore_pending = true;
> +
> +		vgic_v5_update_irs_ist_baser(irs_data, val);
> +		break;

[Severity: High]
If userspace sets GICV5_IRS_IST_BASER with Valid=1 (setting
lpi_ist_restore_pending=true) and later clears it by setting Valid=0, does
lpi_ist_restore_pending remain true?

If the state machine fails to clear this pending flag, it appears this would
cause vgic_v5_map_resources() to fail unconditionally and permanently prevent
VM execution.

> +	case GICV5_IRS_IST_CFGR:
[ ... ]
> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> index 599ab4efc1db5..f1f1fcb08161f 100644
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> @@ -786,6 +786,9 @@ int vgic_v5_parse_attr(struct kvm_device *dev, struct kvm_device_attr *attr,
>  		mpidr_reg = VGIC_TO_MPIDR(vgic_mpidr);
>  		reg_attr->vcpu = kvm_mpidr_to_vcpu(dev->kvm, mpidr_reg);
>  		break;
> +	case KVM_DEV_ARM_VGIC_GRP_IRS_REGS:
> +		reg_attr->vcpu = kvm_get_vcpu(dev->kvm, 0);
> +		break;
>  	default:
>  		return -EINVAL;
>  	}

[Severity: Medium]
If userspace probes for the KVM_DEV_ARM_VGIC_GRP_IRS_REGS capability before
creating any VCPUs, does this unconditionally fetch VCPU 0 and subsequently
fail the attribute parsing with -EINVAL?

It looks like this breaks the standard KVM capability probing sequence where
userspace calls KVM_HAS_DEVICE_ATTR on a newly created VGIC device before
calling KVM_CREATE_VCPU.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904113404.4051341-1-sascha.bischoff@arm.com?part=32