Re: [PATCH v5 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] The device-managed event notifier is registered before the gpiochip is added, leading to a Use-After-Free of the IRQ domain during device teardown.
--

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

gpio: adp5585: use devm_blocking_notifier_chain_register()

This commit replaces the manual registration of a blocking notifier and
its associated devm_add_action_or_reset() cleanup with the new
devm_blocking_notifier_chain_register() helper.

> diff --git a/drivers/gpio/gpio-adp5585.c b/drivers/gpio/gpio-adp5585.c
> index 6f10fc6460080..7c04a7e86c8f8 100644
> --- 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
leave a use-after-free window during device unbind?

Because devm_blocking_notifier_chain_register() is called before
devm_gpiochip_add_data() (which happens later in adp5585_gpio_probe),
devres will destroy the gpiochip and its IRQ domain before unregistering
this notifier during device teardown.

If the user unbinds the adp5585-gpio child device via sysfs while the
parent device remains bound, and a hardware interrupt fires on the
parent device during this teardown window, the parent's IRQ handler
executes adp5585_gpio_key_event(). This dereferences the already freed
irq.domain pointer:

adp5585_gpio_key_event() {
	...
	irq = irq_find_mapping(adp5585_gpio->gpio_chip.irq.domain, off);
	...
}

>  		if (ret)
>  			return ret;
>  	}

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