Re: [PATCH] genirq/generic-chip: Set IRQ_DOMAIN_FLAG_DESTROY_GC in irq_domain_alloc_generic_chips()

[Herve Codina <herve.codina@bootlin.com>]

Qingshuang,

On Wed,  2 Sep 2026 14:59:39 +0800
Qingshuang Fu <fuqingshuang@kylinos.cn> wrote:

> irq_domain_alloc_generic_chips() allocates generic irq chips and stores
> them via d->gc. However, it does not set the IRQ_DOMAIN_FLAG_DESTROY_GC
> flag on the domain. This means that when irq_domain_remove() is later
> called, the generic chips are not freed because the check for
> IRQ_DOMAIN_FLAG_DESTROY_GC fails, resulting in a memory leak.
> 
> Currently, every caller of irq_domain_alloc_generic_chips(), including
> irq_domain_instantiate() when supplied with dgc_info, must manually set
> this flag. If a caller forgets to do so, the allocated generic chips
> will silently leak on domain removal.
> 
> Fix this by setting IRQ_DOMAIN_FLAG_DESTROY_GC in
> irq_domain_alloc_generic_chips() right after d->gc is assigned. This
> ensures that any domain using generic chips will automatically have the
> chips cleaned up when the domain is removed.
> 
> The flag is set using an idempotent OR‑operation, so existing callers
> which already set this flag manually remain unaffected. Even if the
> allocation fails halfway and frees the gc memory in the error path,
> irq_domain_remove_generic_chips() checks d->gc for NULL before
> proceeding, thus no double‑free can occur. Setting the flag here is
> always safe.
> 
> Fixes: e6f67ce32e8e ("irqdomain: Add support for generic irq chips creation before publishing a domain")
> Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
> ---
>  kernel/irq/generic-chip.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/irq/generic-chip.c b/kernel/irq/generic-chip.c
> index 2c8bc6ce082e..13b634053a3e 100644
> --- a/kernel/irq/generic-chip.c
> +++ b/kernel/irq/generic-chip.c
> @@ -308,6 +308,7 @@ int irq_domain_alloc_generic_chips(struct irq_domain *d,
>  	dgc->gc_flags = info->gc_flags;
>  	dgc->exit = info->exit;
>  	d->gc = dgc;
> +	d->flags |= IRQ_DOMAIN_FLAG_DESTROY_GC;
>  
>  	/* Calc pointer to the first generic chip */
>  	tmp += dgc_sz;
> 
> base-commit: 89a312991dc6e638a36adc43ccb91dbc25504c04

Not sure that having this flag always set is a good idea without any other changes
on IRQ generic chips users.

I am pretty sure that calling unconditionally irq_domain_remove_generic_chips()
in irq_domain_remove() in all drivers can lead to some use-after-free issues.

Sashiko has reported one case but maybe some other are present.
Can you double check on your side?

Also, the flag was not intended to avoid needed (and consistent) operation in
the code. I mean, if the code call this kind of sequence at init/probe:
--- 8< ---
irq_create_domain(); /* or similar functions */
irq_domain_alloc_generic_chips();
--- 8< ---

In  order to be consistent, It should also call.
--- 8< ---
irq_domain_remove_generic_chips();
irq_remove_domain();
--- 8< ---

The flag has been intended to be use with irq_domain_instantiate() which can call
irq_domain_alloc_generic_chips() internally. Callers of irq_domain_instantiate()
call only irq_remove_domain().

Indeed calling only irq_domain_instantiate() and calling both
irq_domain_remove_generic_chips() followed by irq_remove_domain() is, in that case,
not consistent too.

IHMO, if the flag IRQ_DOMAIN_FLAG_DESTROY_GC need to be automatically set
somewhere, it could be in irq_domain_instantiate(). 

Best regards,
Hervé