Re: [PATCH 1/2] gpio: mxc: fix irq_high handling

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Missing teardown of the chained IRQ handler in the `mxc_gpio_probe()` error path leads to a Use-After-Free vulnerability.
- [High] Calling `clk_prepare_enable()` during `syscore_ops.suspend` violates atomic context requirements.
- [High] In `mxc_gpio_runtime_resume()`, the chained IRQ handler is hooked before the GPIO clock is enabled, risking unclocked register access.
- [High] In `mxc_gpio_runtime_suspend()`, the GPIO clock is disabled before the chained IRQ handler is unhooked, risking unclocked register access.
- [High] `mx2_gpio_irq_handler` iterates over all GPIO ports and reads their registers unconditionally without checking their runtime PM or clock state.
--

commit 6fc21eff5d34d702cba087494eeb1fa0e1234c1e
Author: Alexander Stein <alexander.stein@ew.tq-group.com>

gpio: mxc: fix irq_high handling

This commit fixes an issue where enable_irq_wake() is incorrectly called
with a negative value when port irq_high is -1 and gpio_idx is >= 16.
It resolves this by initializing irq_high to 0 for the fsl,imx21-gpio
compatible.

> diff --git a/drivers/gpio/gpio-mxc.c b/drivers/gpio/gpio-mxc.c
> index 647b6f4861b74..12f11a6c96653 100644
> --- a/drivers/gpio/gpio-mxc.c
> +++ b/drivers/gpio/gpio-mxc.c
> @@ -469,7 +469,7 @@ static int mxc_gpio_probe(struct platform_device *pdev)
>  		 * the handler is needed only once, but doing it for every port
>  		 * is more robust and easier.
>  		 */
> -		port->irq_high = -1;
> +		port->irq_high = 0;
>  		port->mx_irq_handler = mx2_gpio_irq_handler;

[Severity: High]
This is a pre-existing issue, but does the error path in mxc_gpio_probe()
properly tear down the chained IRQ handler?

If a subsequent initialization step fails, such as gpio_generic_chip_init()
or devm_irq_alloc_descs(), the error path does not appear to call
mxc_update_irq_chained_handler(port, false) to unhook it.

Since the port pointer is managed by devres and gets freed upon returning
the error, could a subsequent parent interrupt cause a use-after-free when
the IRQ core executes the handler and dereferences the freed port pointer?

[Severity: High]
This is a pre-existing issue, but is it safe for mx2_gpio_irq_handler()
to iterate over all GPIO ports without checking their runtime PM or clock
state?

The handler iterates through the global mxc_gpio_ports list and reads the
GPIO_IMR and GPIO_ISR registers for every port.

If a port is suspended and its clock is disabled, wouldn't attempting
to read its registers violate hardware constraints and potentially trigger
a bus fault?

>  	} else
>  		port->mx_irq_handler = mx3_gpio_irq_handler;

[Severity: High]
This is a pre-existing issue, but does mxc_gpio_runtime_resume() risk
accessing unclocked registers?

It calls mxc_update_irq_chained_handler(port, true) to hook the parent IRQ
handler before calling clk_prepare_enable(port->clk).

If an interrupt fires in this window, could the handler execute and
attempt to read device registers while the hardware clock is still
disabled?

[Severity: High]
This is a pre-existing issue, but there appears to be a similar race in
mxc_gpio_runtime_suspend().

The hardware clock is disabled via clk_disable_unprepare(port->clk) before
mxc_update_irq_chained_handler(port, false) unregisters the handler.

If a pending parent interrupt fires between these two calls, could the
active IRQ handler access the hardware registers without an active clock?

[Severity: High]
This is a pre-existing issue, but does mxc_gpio_syscore_suspend() violate
atomic context rules?

It executes in the late stages of system suspend where only one CPU is
online and local interrupts are hard-disabled. By calling
clk_prepare_enable(port->clk), it internally calls clk_prepare() which
takes the global prepare_lock mutex and includes a might_sleep() check.

Could this sleeping or mutex locking in an atomic context cause deadlocks
or trigger warnings?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260522070118.800671-1-alexander.stein@ew.tq-group.com?part=1