Re: [PATCH v2 1/4] irqchip/gic-v4: Clear the domain and fwnode pointers after freeing them
Yao Yuan <[email protected]>
| Newsgroups | org.infradead.lists.linux-arm-kernel,dev.linux.lists.kvmarm,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <xsuxicnwpzpogl7wlh7atdjw75irdd54oytivalf223jfdbfmz@36tvycw2qt6y> |
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 <[email protected]> > > 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 <[email protected]> > --- > 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 >