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

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

[Qingshuang Fu]

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
-- 
2.25.1

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

[Herve Codina]

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é

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

[Thomas Gleixner]

On Fri, Sep 04 2026 at 00:37, Herve Codina wrote:
> On Wed,  2 Sep 2026 14:59:39 +0800
> Qingshuang Fu <fuqingshuang@kylinos.cn> wrote:
>> 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;
> 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(). 

Yes, in the 'if (info->dgc_info)' branch.

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

[Qingshuang Fu]

在 2026/9/4 14:26, Thomas Gleixner 写道:
> On Fri, Sep 04 2026 at 00:37, Herve Codina wrote:
>> On Wed,  2 Sep 2026 14:59:39 +0800
>> Qingshuang Fu <fuqingshuang@kylinos.cn> wrote:
>>> 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;
>> 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().
> 
> Yes, in the 'if (info->dgc_info)' branch.

Hi,

Thank you very much for your valuable suggestion.

I have posted the v2 patch here:
https://lore.kernel.org/all/20260904070943.934476-1-fuqingshuang@kylinos.cn/

In v2, IRQ_DOMAIN_FLAG_DESTROY_GC is now set inside the info->dgc_info
branch within __irq_domain_instantiate(), right after 
irq_domain_alloc_generic_chips()
succeeds.

Now only domains created via irq_domain_instantiate() with dgc_info get
IRQ_DOMAIN_FLAG_DESTROY_GC automatically. Explicit callers of
irq_domain_alloc_generic_chips() remain untouched and are still responsible
for cleaning‑up generic chips manually.

Regards,
Qingshuang Fu

[PATCH v2] irqdomain: Auto-set IRQ_DOMAIN_FLAG_DESTROY_GC in __irq_domain_instantiate()

[Qingshuang Fu]

When a driver uses irq_domain_instantiate() with dgc_info to create
generic irq chips, the domain is expected to automatically clean up
the generic chips when irq_domain_remove() is called. However,
__irq_domain_instantiate() does not set the IRQ_DOMAIN_FLAG_DESTROY_GC
flag which is required by irq_domain_remove() to trigger cleanup.

Currently all existing callers of irq_domain_instantiate() with dgc_info
manually set this flag, which is error-prone. If a future caller forgets to
set the flag, generic chips allocated by irq_domain_alloc_generic_chips()
will leak on domain removal.

Set IRQ_DOMAIN_FLAG_DESTROY_GC right after irq_domain_alloc_generic_chips()
succeeds inside __irq_domain_instantiate().

This makes automatic cleanup the default for all users that provide
dgc_info via irq_domain_instantiate().

This is the correct location for several reasons:
- irq_domain_instantiate() is a high-level wrapper which internally
  allocates the generic chips, so it should also take responsibility
  for arranging their cleanup.
- Automatically setting the flag inside irq_domain_alloc_generic_chips()
  would affect legacy callers like __irq_alloc_domain_generic_chips(),
  some of which perform custom manual cleanup and could hit use-after-free.

Fixes: e6f67ce32e8e ("irqdomain: Add support for generic irq chips creation before publishing a domain")
Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
---
Changes in v2:
- Move setting of IRQ_DOMAIN_FLAG_DESTROY_GC from irq_domain_alloc_generic_chips()
  into the info->dgc_info branch inside __irq_domain_instantiate(), as suggested.
  This avoids unconditional flag-setting which could cause use-after-free for
  legacy drivers with custom generic chip cleanup paths.

 kernel/irq/irqdomain.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 57c819da30c2..4fdcb6df5306 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -344,6 +344,7 @@ static struct irq_domain *__irq_domain_instantiate(const struct irq_domain_info
 		err = irq_domain_alloc_generic_chips(domain, info->dgc_info);
 		if (err)
 			goto err_domain_free;
+		domain->flags |= IRQ_DOMAIN_FLAG_DESTROY_GC;
 	}
 
 	if (info->init) {

base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3
-- 
2.25.1

Re: [PATCH v2] irqdomain: Auto-set IRQ_DOMAIN_FLAG_DESTROY_GC in __irq_domain_instantiate()

[Herve Codina]

Hi Qingshuang,

On Fri,  4 Sep 2026 15:09:43 +0800
Qingshuang Fu <fuqingshuang@kylinos.cn> wrote:

> When a driver uses irq_domain_instantiate() with dgc_info to create
> generic irq chips, the domain is expected to automatically clean up
> the generic chips when irq_domain_remove() is called. However,
> __irq_domain_instantiate() does not set the IRQ_DOMAIN_FLAG_DESTROY_GC
> flag which is required by irq_domain_remove() to trigger cleanup.
> 
> Currently all existing callers of irq_domain_instantiate() with dgc_info
> manually set this flag, which is error-prone. If a future caller forgets to
> set the flag, generic chips allocated by irq_domain_alloc_generic_chips()
> will leak on domain removal.
> 
> Set IRQ_DOMAIN_FLAG_DESTROY_GC right after irq_domain_alloc_generic_chips()
> succeeds inside __irq_domain_instantiate().
> 
> This makes automatic cleanup the default for all users that provide
> dgc_info via irq_domain_instantiate().
> 
> This is the correct location for several reasons:
> - irq_domain_instantiate() is a high-level wrapper which internally
>   allocates the generic chips, so it should also take responsibility
>   for arranging their cleanup.
> - Automatically setting the flag inside irq_domain_alloc_generic_chips()
>   would affect legacy callers like __irq_alloc_domain_generic_chips(),
>   some of which perform custom manual cleanup and could hit use-after-free.
> 
> Fixes: e6f67ce32e8e ("irqdomain: Add support for generic irq chips creation before publishing a domain")
> Signed-off-by: Qingshuang Fu <fuqingshuang@kylinos.cn>
> ---
> Changes in v2:
> - Move setting of IRQ_DOMAIN_FLAG_DESTROY_GC from irq_domain_alloc_generic_chips()
>   into the info->dgc_info branch inside __irq_domain_instantiate(), as suggested.
>   This avoids unconditional flag-setting which could cause use-after-free for
>   legacy drivers with custom generic chip cleanup paths.
> 
>  kernel/irq/irqdomain.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 57c819da30c2..4fdcb6df5306 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -344,6 +344,7 @@ static struct irq_domain *__irq_domain_instantiate(const struct irq_domain_info
>  		err = irq_domain_alloc_generic_chips(domain, info->dgc_info);
>  		if (err)
>  			goto err_domain_free;
> +		domain->flags |= IRQ_DOMAIN_FLAG_DESTROY_GC;
>  	}
>  
>  	if (info->init) {
> 
> base-commit: a500db7819c50db59e55f1b4fa1c3baa5a2616f3

Ok, the flag is now set automatically in __irq_domain_instantiate() depending
on the setting of info->dgc_info.

Callers of __irq_domain_instantiate() have no more reason to set explicitly
this flag. I mean as soon as info->dgc_info is set, having also
info->domain_flag set with IRQ_DOMAIN_FLAG_DESTROY_GC is a redundant
information which can lead to confusion.

Users can ask "Why setting IRQ_DOMAIN_FLAG_DESTROY_GC in info->domain_flag
is needed whereas it is automatically set ?".

IMHO some patches in the series can be added removing the setting of
IRQ_DOMAIN_FLAG_DESTROY_GC at caller side in info->domain_flag when
info->dgc_info is set.

Best regards,
Hervé

Re: [PATCH v2] irqdomain: Auto-set IRQ_DOMAIN_FLAG_DESTROY_GC in __irq_domain_instantiate()

[Thomas Gleixner]

On Fri, Sep 04 2026 at 15:09, Qingshuang Fu wrote:
> When a driver uses irq_domain_instantiate() with dgc_info to create
> generic irq chips, the domain is expected to automatically clean up
> the generic chips when irq_domain_remove() is called. However,
> __irq_domain_instantiate() does not set the IRQ_DOMAIN_FLAG_DESTROY_GC
> flag which is required by irq_domain_remove() to trigger cleanup.
>
> Currently all existing callers of irq_domain_instantiate() with dgc_info
> manually set this flag, which is error-prone. If a future caller forgets to
> set the flag, generic chips allocated by irq_domain_alloc_generic_chips()
> will leak on domain removal.
>
> Set IRQ_DOMAIN_FLAG_DESTROY_GC right after irq_domain_alloc_generic_chips()
> succeeds inside __irq_domain_instantiate().
>
> This makes automatic cleanup the default for all users that provide
> dgc_info via irq_domain_instantiate().
>
> This is the correct location for several reasons:
> - irq_domain_instantiate() is a high-level wrapper which internally
>   allocates the generic chips, so it should also take responsibility
>   for arranging their cleanup.
> - Automatically setting the flag inside irq_domain_alloc_generic_chips()
>   would affect legacy callers like __irq_alloc_domain_generic_chips(),
>   some of which perform custom manual cleanup and could hit use-after-free.
>
> Fixes: e6f67ce32e8e ("irqdomain: Add support for generic irq chips creation before publishing a domain")

This does not fix anything.

All users which invoke irq_domain_instantiate() with info->dgc_info
populated set the flag, no?

And instead of adding a bogus Fixes tag, you could have made a series
which removes the redundant flag from the three perfectly fine working
drivers.