Re: [PATCH v10 8/8] gpio: pl061: add Ambarella register-layout variant

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] gpio: devres LIFO order UAF on irq.domain during teardown
--

commit a177d737541186c7b1e51ef9f245ebda1871ffa1
Author: Long Zhao <longzhao@ambarella.com>

gpio: pl061: add Ambarella register-layout variant

Add a 32-bit, 32-line layout with DAT/DIR/IS/IBE/IEV/IE/MIS/IC plus
MASK and ENABLE. Probe unmasks the bank and enables the controller.

> diff --git a/drivers/gpio/gpio-pl061.c b/drivers/gpio/gpio-pl061.c
> index 3822faa3e2768..075af7985d893 100644
> --- a/drivers/gpio/gpio-pl061.c
> +++ b/drivers/gpio/gpio-pl061.c
[ ... ]
> @@ -347,6 +393,13 @@ static const struct pl061_drvdata pl061_arm = {
>  	.reg_mask_xlate		= pl061_arm_reg_mask_xlate,
>  };
>  
> +static const struct pl061_drvdata pl061_ambarella = {
> +	.regmap_config		= &pl061_ambarella_regmap_config,
> +	.regs			= &pl061_ambarella_regs,
> +	.ngpio			= AMBARELLA_GPIO_NR,
> +	.clear_irq_on_type	= true,
> +};
> +
>  static int pl061_probe(struct amba_device *adev, const struct amba_id *id)
>  {
>  	struct gpio_regmap_config config = {};

[Severity: High]
Can this lead to a use-after-free during device teardown due to devres
ordering?

While this patch adds the new Ambarella variant, the shared pl061_probe()
function registers devres actions in an order that might be unsafe during
teardown.

In pl061_probe(), the gpiochip is registered first:

drivers/gpio/gpio-pl061.c:pl061_probe() {
        ...
        gpio_regmap = devm_gpio_regmap_register(dev, &config);
        ...
        ret = devm_add_action_or_reset(dev, pl061_remove_irq, pl061);
        ...
}

Because devres executes actions in reverse order of registration (LIFO),
pl061_remove_irq() will run first during a late probe failure or device
removal, calling irq_domain_remove() and freeing the IRQ domain.

At this point, the gpiochip is still registered. If a concurrent
call to gpiod_to_irq() acquires the valid gpio_chip, it will call into:

drivers/gpio/gpiolib.c:gpiochip_to_irq() {
        struct irq_domain *domain = gc->irq.domain;
        ...
        return irq_create_mapping(domain, offset);
}

Will this dereference the newly freed gc->irq.domain before the gpiochip
itself is unregistered? Should the IRQ teardown action be registered before
the gpiochip registration to ensure the chip is removed first?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015-cv75-gpio-v10-0-3fe3eb315a55@ambarella.com?part=8