[PATCH] regmap: irq: Free the irqdomain we create

[PATCH] regmap: irq: Free the irqdomain we create

[Mark Brown]

When domain support was added to regmap-irq it was not possible to
remove domains, this was added later by 8ee99ada293b (irqdomain: Support
removal of IRQ domains.).  We did update the main removal path to free
the domain but forgot the error unwinding case during creation that is
now in regmap_add_irq_chip_fwnode() after some code motion, meaning that
errors during instantiation result in an unused irqchip being left
hanging around.  Add the missing irq_remove_domain() call where the
comment says it should be.

Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
Reported-by: Thomas Gleixner <tglx@kernel.org>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 drivers/base/regmap/regmap-irq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
index 99b55b1053ee..715eb9e7aa2a 100644
--- a/drivers/base/regmap/regmap-irq.c
+++ b/drivers/base/regmap/regmap-irq.c
@@ -963,7 +963,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
 	return 0;
 
 err_domain:
-	/* Should really dispose of the domain but... */
+	irq_domain_remove(d->domain);
 err_mutex:
 	mutex_destroy(&d->lock);
 	lockdep_unregister_key(&d->lock_key);

---
base-commit: cee9395acd8043be0644b25c34bfa86623f2b935
change-id: 20260831-regmap-irq-deallocate-domain-f1902d654fd9

Best regards,
--  
Mark Brown <broonie@kernel.org>

Re: [PATCH] regmap: irq: Free the irqdomain we create

[Mark Brown]

On Tue, 01 Sep 2026 22:55:42 +0100, Mark Brown wrote:
> regmap: irq: Free the irqdomain we create

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/regmap.git for-7.3

Thanks!

[1/1] regmap: irq: Free the irqdomain we create
      https://git.kernel.org/broonie/regmap/c/2e42cade8ff1

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

Re: [PATCH] regmap: irq: Free the irqdomain we create

[Thomas Gleixner]

On Tue, Sep 01 2026 at 22:55, Mark Brown wrote:
> When domain support was added to regmap-irq it was not possible to
> remove domains, this was added later by 8ee99ada293b (irqdomain: Support
> removal of IRQ domains.).  We did update the main removal path to free
> the domain but forgot the error unwinding case during creation that is
> now in regmap_add_irq_chip_fwnode() after some code motion, meaning that
> errors during instantiation result in an unused irqchip being left
> hanging around.  Add the missing irq_remove_domain() call where the
> comment says it should be.
>
> Reported-by: Farhad Alemi <farhad.alemi@berkeley.edu>
> Reported-by: Thomas Gleixner <tglx@kernel.org>
> Signed-off-by: Mark Brown <broonie@kernel.org>
> ---
>  drivers/base/regmap/regmap-irq.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/base/regmap/regmap-irq.c b/drivers/base/regmap/regmap-irq.c
> index 99b55b1053ee..715eb9e7aa2a 100644
> --- a/drivers/base/regmap/regmap-irq.c
> +++ b/drivers/base/regmap/regmap-irq.c
> @@ -963,7 +963,7 @@ int regmap_add_irq_chip_fwnode(struct fwnode_handle *fwnode,
>  	return 0;
>  
>  err_domain:
> -	/* Should really dispose of the domain but... */
> +	irq_domain_remove(d->domain);

That won't work.

The case which is affected is the one which allocates interrupts
upfront via alloc_irq_descs().

In that case the domain creation will associate allocated interrupts
because info.virq_base is > 0.

This wont trigger the WARN_ON() in irq_domain_remove() because it's a
fixed sized linear domain, but irq_domain_remove() will leak the
interrupt descriptors which still have a reference (pointer) to the irq
chip and the domain. So the same UAF is still there :)

What you need to do before removing the domain is

     if (irq_base > 0)
     	irq_domain_free_irqs(irq_base, chip->num_irqs);

Thanks,

        tglx

Re: [PATCH] regmap: irq: Free the irqdomain we create

[Thomas Gleixner]

On Fri, Sep 04 2026 at 21:00, Thomas Gleixner wrote:
> On Tue, Sep 01 2026 at 22:55, Mark Brown wrote:
>>  err_domain:
>> -	/* Should really dispose of the domain but... */
>> +	irq_domain_remove(d->domain);
>
> That won't work.
>
> The case which is affected is the one which allocates interrupts
> upfront via alloc_irq_descs().
>
> In that case the domain creation will associate allocated interrupts
> because info.virq_base is > 0.
>
> This wont trigger the WARN_ON() in irq_domain_remove() because it's a
> fixed sized linear domain, but irq_domain_remove() will leak the
> interrupt descriptors which still have a reference (pointer) to the irq
> chip and the domain. So the same UAF is still there :)
>
> What you need to do before removing the domain is
>
>      if (irq_base > 0)
>      	irq_domain_free_irqs(irq_base, chip->num_irqs);

Hit send too fast. That stupidly works only when hierarchical domains
are enabled.

So you need:

      if (irq_base > 0) {
      	  for (unsigned int i = 0; i < chip->num_irqs; i++)
          	irq_dispose_mapping(irq_base + i);
      }
      irqdomain_remove_domain();

Thanks,

        tglx

Re: [PATCH] regmap: irq: Free the irqdomain we create

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 564 bytes --]

On Fri, Sep 04, 2026 at 09:05:26PM +0200, Thomas Gleixner wrote:
> On Fri, Sep 04 2026 at 21:00, Thomas Gleixner wrote:

> >      if (irq_base > 0)
> >      	irq_domain_free_irqs(irq_base, chip->num_irqs);

> Hit send too fast. That stupidly works only when hierarchical domains
> are enabled.

> So you need:

>       if (irq_base > 0) {
>       	  for (unsigned int i = 0; i < chip->num_irqs; i++)
>           	irq_dispose_mapping(irq_base + i);
>       }
>       irqdomain_remove_domain();

Ah, thanks - I'd expected removing the domain to clean everything up.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]