Re: [PATCH v4 05/13] gpio: adp5585: use devm_blocking_notifier_chain_register()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Use-after-free of the gpiochip's IRQ domain during device unbinding due to incorrect devres registration order.
--

--- Patch [5]: [PATCH v4 05/13] gpio: adp5585: use devm_blocking_notifier_chain_register() ---
commit 44c510dbb3ca8a6273606da82607128231bd2e31
Author: Eliav Farber <[email protected]>

gpio: adp5585: use devm_blocking_notifier_chain_register()

This commit replaces the manual blocking_notifier_chain_register() and
devm_add_action_or_reset() pattern with a single call to
devm_blocking_notifier_chain_register(), dropping the standalone
unregister callback.

> diff --git a/drivers/gpio/gpio-adp5585.c b/drivers/gpio/gpio-adp5585.c
> --- a/drivers/gpio/gpio-adp5585.c
> +++ b/drivers/gpio/gpio-adp5585.c

[ ... ]

> @@ -450,13 +440,9 @@ static int adp5585_gpio_probe(struct platform_device *pdev)
>  		girq->threaded = true;
>  
>  		adp5585_gpio->nb.notifier_call = adp5585_gpio_key_event;
> -		ret = blocking_notifier_chain_register(&adp5585->event_notifier,
> -						       &adp5585_gpio->nb);
> -		if (ret)
> -			return ret;
> -
> -		ret = devm_add_action_or_reset(dev, adp5585_gpio_unreg_notifier,
> -					       adp5585_gpio);
> +		ret = devm_blocking_notifier_chain_register(dev,
> +							    &adp5585->event_notifier,
> +							    &adp5585_gpio->nb);
[Severity: High]
This is a pre-existing issue, but does this devres registration order create
a use-after-free during device unbind?

Because the notifier is registered before the GPIO chip is added via
devm_gpiochip_add_data() later in adp5585_gpio_probe(), devres will process
the teardown in LIFO order. This means the GPIO chip and its IRQ domain are
removed and freed first.

If an interrupt fires after the GPIO chip is removed but before the notifier
is unregistered, the notifier callback adp5585_gpio_key_event() will attempt
to access the freed domain:

drivers/gpio/gpio-adp5585.c:adp5585_gpio_key_event() {
    ...
	irq = irq_find_mapping(adp5585_gpio->gpio_chip.irq.domain, off);
	if (!irq)
		return NOTIFY_BAD;
    ...
}

Should the notifier registration be moved after devm_gpiochip_add_data() to
ensure a safe teardown sequence?

>  		if (ret)
>  			return ret;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5