Re: [PATCH 6.6.y 2/2] gpio: rockchip: fix generic IRQ chip leak on remove

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpio: rockchip: UAF in rockchip_gpio_to_irq due to teardown order
- [High] gpio: rockchip: missing irq mapping disposal causes UAF
--

commit 7ad98fc33c5f3a1c30b1b0722c11e01bbf5172c9
Author: Marco Scardovi <scardracs@disroot.org>
Subject: gpio: rockchip: fix generic IRQ chip leak on remove

This patch addresses a resource leak by explicitly removing the generic
chips and the IRQ domain in rockchip_gpio_remove().

Link: https://patch.msgid.link/20260607230504.35392-2-scardracs@disroot.org

> diff --git a/drivers/gpio/gpio-rockchip.c b/drivers/gpio/gpio-rockchip.c
> index e0e4f3ed5fdd4..86cf3cbcdb403 100644
> --- a/drivers/gpio/gpio-rockchip.c
> +++ b/drivers/gpio/gpio-rockchip.c
> @@ -786,8 +786,10 @@ static int rockchip_gpio_remove(struct platform_device *pdev)
>  	struct rockchip_pin_bank *bank = platform_get_drvdata(pdev);
>  
>  	irq_set_chained_handler_and_data(bank->irq, NULL, NULL);
> -	if (bank->domain)
> +	if (bank->domain) {
> +		irq_domain_remove_generic_chips(bank->domain);
>  		irq_domain_remove(bank->domain);

[Severity: High]
Does removing the IRQ domain before calling gpiochip_remove() introduce a
use-after-free race condition?

Because the GPIO chip is still registered and active in gpiolib, concurrent
operations (like a sysfs write configuring an edge trigger) can still invoke
gpiod_to_irq(). This triggers the driver's .to_irq callback:

rockchip_gpio_to_irq() {
    ...
    virq = irq_create_mapping(bank->domain, offset);
    ...
}

Since bank->domain has already been freed, this will dereference a dangling
pointer. Would reversing the teardown order to call gpiochip_remove() before
freeing the domain unregister sysfs nodes, drain pending operations, and
safely close this race window?

[Severity: High]
Does removing the IRQ domain here without disposing of active mappings leak
those mappings and cause a use-after-free in the core IRQ subsystem?

When the domain is removed without iterating over active pins to call
irq_dispose_mapping(), the core IRQ code hits a warning:

irq_domain_remove() {
    ...
    WARN_ON(!radix_tree_empty(&domain->revmap_tree));
    ...
    irq_domain_free(domain);
}

Active IRQ descriptors will retain a dangling pointer to the freed domain in
their irq_data->domain fields. When consumer drivers subsequently unbind and
free their IRQ, irq_dispose_mapping() accesses the freed domain pointer to
check hierarchy flags, leading to a crash.

Should the active mappings be disposed of before removing the domain?

> +	}
>  	gpiochip_remove(&bank->gpio_chip);
>  
>  	return 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261008191824.98662-1-artem@trailofbits.com?part=2