Re: [PATCH v2 1/4] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them

[Yao Yuan]

On Mon, Aug 24, 2026 at 08:42:42AM +0800, Fuad Tabba wrote:
> The GICv4 allocation and teardown paths free their irq domains and
> fwnodes but leave the pointers set, and the allocation error paths test
> those pointers before removing them. struct its_vm and struct its_vpe
> are embedded in KVM's per-VM and per-vCPU state, so nothing re-zeroes
> them between two attempts, and an error path taken after an earlier one
> already freed the domain calls irq_domain_remove() on freed memory.

Hi Tabba,

Real nice finding!

Reviewed-by: Yuan Yao <yaoyuan@xxxxxxxxxxxxxxxxx>

>
> Reaching this takes two allocation failures, one to leave the stale
> pointer behind and one to send the next attempt down the error path.
>
> Fixes: 7de5c0af9c7c ("irqchip/gic-v4: Add per-VM VPE domain creation")
> Fixes: 6d31b6ff985d ("irqchip/gic-v4.1: Add VSGI allocation/teardown")
> Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
> ---
>  drivers/irqchip/irq-gic-v4.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/irqchip/irq-gic-v4.c b/drivers/irqchip/irq-gic-v4.c
> index 8455b4a5fbb0d..754839e409f88 100644
> --- a/drivers/irqchip/irq-gic-v4.c
> +++ b/drivers/irqchip/irq-gic-v4.c
> @@ -147,10 +147,14 @@ static int its_alloc_vcpu_sgis(struct its_vpe *vpe, int idx)
>  	return 0;
>
>  err:
> -	if (vpe->sgi_domain)
> +	if (vpe->sgi_domain) {
>  		irq_domain_remove(vpe->sgi_domain);
> -	if (vpe->fwnode)
> +		vpe->sgi_domain = NULL;
> +	}
> +	if (vpe->fwnode) {
>  		irq_domain_free_fwnode(vpe->fwnode);
> +		vpe->fwnode = NULL;
> +	}
>  	kfree(name);
>  	return -ENOMEM;
>  }
> @@ -191,10 +195,14 @@ int its_alloc_vcpu_irqs(struct its_vm *vm)
>  	return 0;
>
>  err:
> -	if (vm->domain)
> +	if (vm->domain) {
>  		irq_domain_remove(vm->domain);
> -	if (vm->fwnode)
> +		vm->domain = NULL;
> +	}
> +	if (vm->fwnode) {
>  		irq_domain_free_fwnode(vm->fwnode);
> +		vm->fwnode = NULL;
> +	}
>
>  	return -ENOMEM;
>  }
> @@ -215,6 +223,8 @@ static void its_free_sgi_irqs(struct its_vm *vm)
>  		irq_domain_free_irqs(irq, 16);
>  		irq_domain_remove(vm->vpes[i]->sgi_domain);
>  		irq_domain_free_fwnode(vm->vpes[i]->fwnode);
> +		vm->vpes[i]->sgi_domain = NULL;
> +		vm->vpes[i]->fwnode = NULL;
>  	}
>  }
>
> @@ -224,6 +234,8 @@ void its_free_vcpu_irqs(struct its_vm *vm)
>  	irq_domain_free_irqs(vm->vpes[0]->irq, vm->nr_vpes);
>  	irq_domain_remove(vm->domain);
>  	irq_domain_free_fwnode(vm->fwnode);
> +	vm->domain = NULL;
> +	vm->fwnode = NULL;
>  }
>
>  static int its_send_vpe_cmd(struct its_vpe *vpe, struct its_cmd_info *info)
> --
> 2.39.5
>