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

Qingshuang Fu posted 1 patch 3 weeks ago
kernel/irq/irqdomain.c | 1 +
1 file changed, 1 insertion(+)
[PATCH v2] irqdomain: Auto-set IRQ_DOMAIN_FLAG_DESTROY_GC in __irq_domain_instantiate()
Posted by Qingshuang Fu 3 weeks ago
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()
Posted by Thomas Gleixner 3 weeks ago
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.
Re: [PATCH v2] irqdomain: Auto-set IRQ_DOMAIN_FLAG_DESTROY_GC in __irq_domain_instantiate()
Posted by Herve Codina 3 weeks ago
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é