Re: [PATCH v6 22/49] KVM: arm64: gic-v5: Register the IRS IODEV

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Memory leak of the `dist->spis` array if GICv5 initialization is retried after a failure.
--

commit 67a160bda61ea35d934ec550a4467b5c6ecad827
Author: Sascha Bischoff <Sascha.Bischoff@arm.com>

KVM: arm64: gic-v5: Register the IRS IODEV

Now that we have an emulated IRS, it needs to be registered, which
ensures that guest accesses to the MMIO regions handled by the device
are handled appropriately in KVM. Therefore, as part of
vgic_map_resources, the GICv5 IRS IODEV is registered. If the address
for the IRS is not provided, bail out reporting an error - this is not
a supported config.

> diff --git a/arch/arm64/kvm/vgic/vgic-kvm-device.c b/arch/arm64/kvm/vgic/vgic-kvm-device.c
> index 90be99443df3b..8c2c6886af4e3 100644
> --- a/arch/arm64/kvm/vgic/vgic-kvm-device.c
> +++ b/arch/arm64/kvm/vgic/vgic-kvm-device.c

[ ... ]

> @@ -748,21 +778,25 @@ static int vgic_v5_set_attr(struct kvm_device *dev,
>  {
>  	switch (attr->group) {
>  	case KVM_DEV_ARM_VGIC_GRP_ADDR:
> +		break;
>  	case KVM_DEV_ARM_VGIC_GRP_CPU_SYSREGS:
> -	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
>  		return -ENXIO;
> +	case KVM_DEV_ARM_VGIC_GRP_NR_IRQS:
> +		break;
>  	case KVM_DEV_ARM_VGIC_GRP_CTRL:
>  		switch (attr->attr) {
>  		case KVM_DEV_ARM_VGIC_CTRL_INIT:
> -			return vgic_set_common_attr(dev, attr);
> +			break;

[Severity: Medium]
If userspace calls KVM_DEV_ARM_VGIC_CTRL_INIT and the routing setup fails in
vgic_init(), does this leave dist->spis leaking if userspace retries the
initialization?

When vgic_init() aborts due to a failure in
kvm_vgic_setup_default_irq_routing(), it calls vgic_v5_teardown():

arch/arm64/kvm/vgic/vgic-init.c:vgic_init() {
        ...
        if (vgic_is_v5(kvm))
                vgic_v5_teardown(kvm);

        return ret;
}

However, it appears vgic_v5_teardown() does not free the dist->spis array.

If userspace retries the init call, kvm_vgic_v5_irs_init() unconditionally
allocates a new array over the old one:

arch/arm64/kvm/vgic/vgic-irs-v5.c:kvm_vgic_v5_irs_init() {
        ...
        dist->spis = kcalloc(nr_spis, sizeof(struct vgic_irq), GFP_KERNEL_ACCOUNT);
        ...
}

Can this allow a misconfigured or malicious userspace to repeatedly leak
memory (up to ~200KB per call) by retrying the initialization after a failure?

>  		case KVM_DEV_ARM_VGIC_USERSPACE_PPIS:
>  		default:
>  			return -ENXIO;
>  		}
> +		break;
>  	default:
>  		return -ENXIO;
>  	}

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