Fix leaks in rc core

PATCH Superseded Sean Young 3 participants 12 messages Jul 10 – Jul 12

Subsystems (4)

Media (V4L/DVB) Arm/Amlogic Meson SoC support Arm/Allwinner sunXi SoC support RC-core / lirc framework

lore

Share & tools

lore

Sean Young

author maintainer

2026-07-10 08:53 UTC #0 lore raw

cover Fix leaks in rc core

Sean Young (7):
media: streamzap: Add missing rc_unregister_device()
media: redrat3: Ensure rc device is freed if enable_detector() fails
media: redrat3: Ensure we don't read beyond the end of the packet
media: sunxi-cir: Ensure no more interrupts can occur before free
media: meson-ir-tx: Ensure clock is disabled on unbind
media: meson-ir-tx: Ensure rc_free_device() is called on unbind
media: ir-hix5hd2: Ensure rdev is setup before interrupts are enabled

 drivers/media/rc/ir-hix5hd2.c  |  5 +++--
 drivers/media/rc/meson-ir-tx.c | 10 ++++------
 drivers/media/rc/redrat3.c     | 15 ++++++++++++++-
 drivers/media/rc/streamzap.c   |  1 +
 drivers/media/rc/sunxi-cir.c   |  2 +-
 5 files changed, 23 insertions(+), 10 deletions(-)

Signature · 1 line

2.55.0

Sean Young

author maintainer

2026-07-10 08:53 UTC #1 lore raw

PATCH 1/7 media: streamzap: Add missing rc_unregister_device()

If usb_submit_urb() fails during probe, then the error path is missing a call to rc_unregister_device(), which will leak various things like the input device.

Fixes 42844992664f ("media: rc: streamzap: Error handling in probe")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/streamzap.c | 1 +
 1 file changed, 1 insertion(+)

drivers/media/rc/streamzap.c +1 -0

index 307985d74fe8..41195ad82734 100644
@@ -365,6 +365,7 @@ static int streamzap_probe(struct usb_interface *intf,
 
 	return 0;
 rc_submit_fail:
+	rc_unregister_device(sz->rdev);
 	rc_free_device(sz->rdev);
 	usb_set_intfdata(intf, NULL);
 rc_dev_fail:

Signature · 1 line

2.55.0

Sean Young

author maintainer

2026-07-10 08:53 UTC #2 lore raw

PATCH 2/7 media: redrat3: Ensure rc device is freed if enable_detector() fails

In reply to Sean Young

This particular error path does not free the rc device at all, with its priv pointer still pointing at freed memory.

Fixes 2154be651b90 ("[media] redrat3: new rc-core IR transceiver device driver")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/redrat3.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

drivers/media/rc/redrat3.c +4 -1

index 3f828a564e19..d2a805dbd3f3 100644
@@ -1111,13 +1111,16 @@ static int redrat3_dev_probe(struct usb_interface *intf,
 	/* might be all we need to do? */
 	retval = redrat3_enable_detector(rr3);
 	if (retval < 0)
-		goto led_free;
+		goto rc_free;
 
 	/* we can register the device now, as it is ready */
 	usb_set_intfdata(intf, rr3);
 
 	return 0;
 
+rc_free:
+	rc_unregister_device(rr3->rc);
+	rc_free_device(rr3->rc);
 led_free:
 	led_classdev_unregister(&rr3->led);
 redrat_free:

Signature · 1 line

2.55.0

Sean Young

author maintainer

2026-07-10 08:53 UTC #3 lore raw

PATCH 3/7 media: redrat3: Ensure we don't read beyond the end of the packet

In reply to Sean Young

The length and offset is provided by the usb device, so it should be validated.

Fixes 2154be651b90 ("[media] redrat3: new rc-core IR transceiver device driver")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/redrat3.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

drivers/media/rc/redrat3.c +10 -0

index d2a805dbd3f3..424f8b9e4d17 100644
@@ -358,8 +358,18 @@ static void redrat3_process_ir_data(struct redrat3_dev *rr3)
 
 	/* process each rr3 encoded byte into an int */
 	sig_size = be16_to_cpu(rr3->irdata.sig_size);
+
+	if (sig_size > RR3_MAX_SIG_SIZE) {
+		dev_err(dev, "length %u is incorrect\n", sig_size);
+		return;
+	}
+
 	for (i = 0; i < sig_size; i++) {
 		offset = rr3->irdata.sigdata[i];
+		if (offset >= RR3_DRIVER_MAXLENS) {
+			dev_err(dev, "offset %u is incorrect\n", offset);
+			return;
+		}
 		val = get_unaligned_be16(&rr3->irdata.lens[offset]);
 
 		/* we should always get pulse/space/pulse/space samples */

Signature · 1 line

2.55.0

Sean Young

author maintainer

2026-07-10 08:53 UTC #4 lore raw

PATCH 4/7 media: sunxi-cir: Ensure no more interrupts can occur before free

In reply to Sean Young

Only call rc_free_device() once the hardware has been stopped.

Fixes dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/sunxi-cir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/media/rc/sunxi-cir.c +1 -1

index 28e840a7e5b8..af1ee08ffdbe 100644
@@ -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);
 }
 
 static void sunxi_ir_shutdown(struct platform_device *pdev)

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-10 09:03 UTC #5 lore raw

Re: [PATCH 4/7] media: sunxi-cir: Ensure no more interrupts can occur before free

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

Pre-existing issues:
- [High] The patch fails to fix the IRQ Use-After-Free race condition and exacerbates a bus fault vulnerability by disabling hardware clocks without synchronizing the devm-managed interrupt.
- [High] The interrupt is requested before the hardware is initialized in the probe function, leading to a possible bus fault on spurious interrupts and a Use-After-Free in the error path. --

--- Patch [4]: [PATCH 4/7] media: sunxi-cir: Ensure no more interrupts can occur before free --- commit 8ac4bdb72847c70ea6f26a6012ec7552bc70de07
Author: Sean Young <[email protected]>

media: sunxi-cir: Ensure no more interrupts can occur before free

This commit reorders the device removal sequence to call rc_free_device() only after the hardware has been stopped via sunxi_ir_hw_exit(). The stated intent is to prevent interrupts from occurring after the device is freed.

drivers/media/rc/sunxi-cir.c static void sunxi_ir_remove(struct platform_device *pdev) · line 378

Quoted 11 lines

drivers/media/rc/sunxi-cir.c +1 -1

index 28e840a7e5b8c..af1ee08ffdbec 100644
@@ -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 is a pre-existing issue, but does this sequence completely prevent interrupts from occurring before the free?

Because the interrupt is managed by devm_request_irq(), it remains active until devres cleanup runs after sunxi_ir_remove() completes. Since sunxi_ir_hw_exit() only disables clocks, if a pending or spurious interrupt fires afterward, sunxi_ir_irq() will access unclocked memory-mapped registers and trigger a bus fault.

Additionally, if the interrupt handler runs concurrently on another processor:

sunxi_ir_irq()

    ir_raw_event_overflow(ir->rc);

could it access ir->rc after it is freed by rc_free_device() here, since there is no synchronize_irq() or disable_irq() in sunxi_ir_remove()?

[Severity: High]
This is also a pre-existing issue, but does sunxi_ir_probe() suffer from a similar race condition?

In sunxi_ir_probe(), devm_request_irq() is called to unmask the interrupt before sunxi_ir_hw_init() enables the clock. If a pending interrupt from a previous boot or a spurious interrupt fires during this window, sunxi_ir_irq() will execute before the clock is enabled, potentially causing a bus fault.

Also, if sunxi_ir_hw_init() fails, the error path eventually calls rc_free_device() without explicitly disabling the active interrupt, which leaves a window for a use-after-free on ir->rc.

Quoted 1 line

drivers/media/rc/sunxi-cir.c +0 -0
 }

Signature · 1 line

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

Sean Young

author maintainer

2026-07-10 08:53 UTC #6 lore raw

PATCH 5/7 media: meson-ir-tx: Ensure clock is disabled on unbind

In reply to Sean Young

clk_prepare_enabled() needs a call to clk_disable_prepare() on driver unbind. Make it devm managed.

Fixes 49be1c78d575 ("media: rc: introduce Meson IR TX driver")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/meson-ir-tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

drivers/media/rc/meson-ir-tx.c +2 -2

index fded2c256f2a..e7bb107e6a84 100644
@@ -288,8 +288,8 @@ static int meson_irtx_mod_clock_probe(struct meson_irtx *ir,
 	if (!np)
 		return -ENODEV;
 
-	clock = devm_clk_get(ir->dev, "xtal");
-	if (IS_ERR(clock) || clk_prepare_enable(clock))
+	clock = devm_clk_get_enabled(ir->dev, "xtal");
+	if (IS_ERR(clock))
 		return -ENODEV;
 
 	*clk_nr = IRB_MOD_XTAL3_CLK;

Signature · 1 line

2.55.0

[email protected]

AI review

2026-07-10 09:06 UTC #7 lore raw

Re: [PATCH 5/7] media: meson-ir-tx: Ensure clock is disabled on unbind

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

New issues:
- [Low] The commit message refers to non-existent clock API functions.

Pre-existing issues:
- [High] The driver masks -EPROBE_DEFER and other error codes by unconditionally returning -ENODEV when devm_clk_get_enabled() or platform_get_irq() fails, breaking probe deferral.
- [High] The rc_dev structure allocated during probe is permanently leaked on driver unbind due to the use of rc_allocate_device() without ensuring a corresponding rc_free_device() call. --

--- Patch [5]: [PATCH 5/7] media: meson-ir-tx: Ensure clock is disabled on unbind --- commit e95895078113495fea5b981fafe002c5ff1f56c1
Author: Sean Young <[email protected]>

media: meson-ir-tx: Ensure clock is disabled on unbind

This patch changes the clock acquisition in the meson-ir-tx driver to use the devres managed devm_clk_get_enabled(). This ensures the clock is automatically disabled and unprepared when the driver unbinds.

Quoted 2 lines

clk_prepare_enabled() needs a call to clk_disable_prepare() on driver unbind. Make it devm managed.

[Severity: Low]
This isn't a bug, but there are typos in the function names mentioned here. Should they be clk_prepare_enable() and clk_disable_unprepare() instead?

drivers/media/rc/meson-ir-tx.c static int meson_irtx_mod_clock_probe(struct meson_irtx *ir, · line 293

Quoted 13 lines

drivers/media/rc/meson-ir-tx.c +2 -2

index fded2c256f2a6..e7bb107e6a843 100644
@@ -288,8 +288,8 @@ static int meson_irtx_mod_clock_probe(struct meson_irtx *ir,
 	if (!np)
 		return -ENODEV;
 
-	clock = devm_clk_get(ir->dev, "xtal");
-	if (IS_ERR(clock) || clk_prepare_enable(clock))
+	clock = devm_clk_get_enabled(ir->dev, "xtal");
+	if (IS_ERR(clock))
 		return -ENODEV;

[Severity: High]
This is a pre-existing issue, but does this incorrectly mask -EPROBE_DEFER?

If devm_clk_get_enabled() returns -EPROBE_DEFER, returning a hardcoded -ENODEV will permanently fail the probe instead of allowing it to defer. Would it be better to return PTR_ERR(clock) here?

The same issue appears to happen with platform_get_irq() in meson_irtx_probe():

drivers/media/rc/meson-ir-tx.c:meson_irtx_probe() {
    ...
    irq = platform_get_irq(pdev, 0);
    if (irq < 0)
        return -ENODEV;
    ...
}

Would it be better to propagate these error codes directly?

[Severity: High]
This is another pre-existing issue, but does this driver permanently leak the rc_dev structure on unbind?

The driver allocates the structure manually in meson_irtx_probe():

drivers/media/rc/meson-ir-tx.c:meson_irtx_probe() {
    ...
    rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
    ...
    ret = devm_rc_register_device(dev, rc);
    ...
}

Since devm_rc_register_device() only ties rc_unregister_device() to the devres cleanup, and there is no matching rc_free_device() call or use of devm_rc_allocate_device(), does the initial reference count remain un-dropped when the driver is unbound?

Signature · 6 lines

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

_______________________________________________
linux-amlogic mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-amlogic

Martin Blumenstingl

reviewer Reviewed-by on v1 5/7

2026-07-12 13:26 UTC #8 lore raw

Re: [PATCH 5/7] media: meson-ir-tx: Ensure clock is disabled on unbind

In reply to Sean Young

Quoted 6 lines · Sean Young

clk_prepare_enabled() needs a call to clk_disable_prepare() on driver unbind. Make it devm managed.
Fixes 49be1c78d575 ("media: rc: introduce Meson IR TX driver")
Signed-off-by Sean Young <[email protected]>

Reviewed-by Martin Blumenstingl <[email protected]>

The additional error that sashiko reported will need to be fixed too. Let's do that in a separate patch though to keep things simple.

Sean Young

author maintainer

2026-07-10 08:53 UTC #9 lore raw

PATCH 6/7 media: meson-ir-tx: Ensure rc_free_device() is called on unbind

In reply to Sean Young

Make rc_dev devm managed.

Fixes dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/meson-ir-tx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

drivers/media/rc/meson-ir-tx.c +2 -4

index e7bb107e6a84..abb107d19e8c 100644
@@ -345,7 +345,7 @@ static int meson_irtx_probe(struct platform_device *pdev)
 	if (ret)
 		return dev_err_probe(dev, ret, "irq request failed\n");
 
-	rc = rc_allocate_device(RC_DRIVER_IR_RAW_TX);
+	rc = devm_rc_allocate_device(dev, RC_DRIVER_IR_RAW_TX);
 	if (!rc)
 		return -ENOMEM;

@@ -358,10 +358,8 @@ static int meson_irtx_probe(struct platform_device *pdev)
 	rc->s_tx_duty_cycle = meson_irtx_set_duty_cycle;
 
 	ret = devm_rc_register_device(dev, rc);
-	if (ret < 0) {
-		rc_free_device(rc);
+	if (ret < 0)
 		return dev_err_probe(dev, ret, "rc_dev registration failed\n");
-	}
 
 	return 0;
 }

Signature · 1 line

2.55.0

Martin Blumenstingl

reviewer Reviewed-by on v1 6/7

2026-07-12 13:23 UTC #10 lore raw

Re: [PATCH 6/7] media: meson-ir-tx: Ensure rc_free_device() is called on unbind

Quoted 5 lines · Sean Young

Make rc_dev devm managed.
Fixes dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
Signed-off-by Sean Young <[email protected]>

Reviewed-by Martin Blumenstingl <[email protected]>

Sean Young

author maintainer

2026-07-10 08:53 UTC #11 lore raw

PATCH 7/7 media: ir-hix5hd2: Ensure rdev is setup before interrupts are enabled

In reply to Sean Young

Once the interrupt handler is enabled, priv->rdev can be used. Ensure it is setup correctly so there is no race condition.

Fixes a84fcdaa9058 ("[media] rc: Introduce hix5hd2 IR transmitter driver")

Signed-off-by Sean Young <[email protected]>

---
 drivers/media/rc/ir-hix5hd2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

drivers/media/rc/ir-hix5hd2.c +3 -2

index 1b061e4a3dcf..aa3de4d57a58 100644
@@ -316,6 +316,9 @@ static int hix5hd2_ir_probe(struct platform_device *pdev)
 	if (ret < 0)
 		goto clkerr;
 
+	priv->rdev = rdev;
+	priv->dev = dev;
+
 	if (devm_request_irq(dev, priv->irq, hix5hd2_ir_rx_interrupt,
 			     0, pdev->name, priv) < 0) {
 		dev_err(dev, "IRQ %d register failed\n", priv->irq);

@@ -323,8 +326,6 @@ static int hix5hd2_ir_probe(struct platform_device *pdev)
 		goto regerr;
 	}
 
-	priv->rdev = rdev;
-	priv->dev = dev;
 	platform_set_drvdata(pdev, priv);
 
 	return ret;

Signature · 1 line

2.55.0