* Re: [PATCH v2 06/11] media: sunxi-cir: Ensure no more interrupts can occur before free
2026-07-13 21:31 ` [PATCH v2 06/11] media: sunxi-cir: Ensure no more interrupts can occur before free Sean Young
@ 2026-07-16 21:34 ` sashiko-bot
0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-07-16 21:34 UTC (permalink / raw)
To: Sean Young; +Cc: media-ci, linux-sunxi
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] The teardown sequence fails to synchronize with or disable the active devm-managed IRQ handler, leaving a race window that guarantees either a fatal bus error (unclocked register access) or a Use-After-Free.
- [High] Device interfaces and the IRQ handler are registered before hardware initialization in probe(), allowing userspace or interrupts to access unclocked registers and cause a bus fault.
- [High] The suspend callback disables clocks without synchronizing or disabling the device interrupt, allowing an unclocked MMIO access race.
--
--- Patch [6]: [PATCH v2 06/11] media: sunxi-cir: Ensure no more interrupts can occur before free ---
commit 2912f57e54d0f869a5daead9e5ddf88c0db3d980
Author: Sean Young <sean@mess.org>
media: sunxi-cir: Ensure no more interrupts can occur before free
This commit attempts to fix a race condition during module removal by moving
the call to rc_free_device() after sunxi_ir_hw_exit().
> diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
> --- a/drivers/media/rc/sunxi-cir.c
> +++ b/drivers/media/rc/sunxi-cir.c
> @@ -374,8 +374,8 @@ static void sunxi_ir_remove(struct platform_device *pdev)
> struct sunxi_ir *ir = platform_get_drvdata(pdev);
>
> rc_unregister_device(ir->rc);
> - rc_free_device(ir->rc);
> sunxi_ir_hw_exit(&pdev->dev);
> + rc_free_device(ir->rc);
[Severity: High]
This isn't a bug introduced by this patch, but does the teardown sequence
leave a race window with the devm-managed IRQ handler?
The commit message mentions ensuring no more interrupts can occur before
free, but sunxi_ir_hw_exit() only disables the clocks and asserts reset:
sunxi_ir_hw_exit() {
clk_disable_unprepare(ir->clk);
clk_disable_unprepare(ir->apb_clk);
reset_control_assert(ir->rst);
}
Because the IRQ is managed by devm_request_irq(), it remains active until
after sunxi_ir_remove() returns. If an interrupt fires concurrently after
the clocks are disabled, sunxi_ir_irq() will attempt to access unclocked
MMIO registers:
sunxi_ir_irq() {
...
status = readl(ir->base + SUNXI_IR_RXSTA_REG);
Could this lead to a synchronous external abort? Should there be an
explicit disable_irq() or similar synchronization before disabling the
clocks?
Additionally, this same pattern appears to exist in sunxi_ir_suspend(),
which also calls sunxi_ir_hw_exit() while the IRQ is still globally active.
[Severity: High]
This is also a pre-existing issue, but is there a similar unclocked access
risk during driver initialization?
In sunxi_ir_probe(), rc_register_device() and devm_request_irq() are called
before the hardware clocks are enabled via sunxi_ir_hw_init():
sunxi_ir_probe() {
...
ret = rc_register_device(ir->rc);
...
ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
...
ret = sunxi_ir_hw_init(dev);
}
If userspace immediately issues a LIRC_SET_REC_TIMEOUT ioctl after the
device is registered, sunxi_ir_set_timeout() will write to SUNXI_IR_CIR_REG
before the clocks are enabled. Similarly, a pending interrupt could cause
sunxi_ir_irq() to run before sunxi_ir_hw_init().
Should the hardware initialization be moved before exposing the device and
requesting the IRQ?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1783977550.git.sean@mess.org?part=6
^ permalink raw reply [flat|nested] 2+ messages in thread