Skip to contentSkip to conversation

media: rc: sunxi-cir: unregister rc device on probe failure

PATCH In review 박명훈 3 participants 3 messages May 13 – May 14

Subsystems (4)
Stable branch Media (V4L/DVB) Arm/Allwinner sunXi SoC support RC-core / lirc framework
lore
Share & tools
lore

박명훈

author
#0 lore raw

PATCH 1/1 media: rc: sunxi-cir: unregister rc device on probe failure

From: Myeonghun Pak <[email protected]>

After rc_register_device() succeeds, later probe failures must undo the registration with rc_unregister_device(). The current error path jumps to the allocation cleanup label and only calls rc_free_device(), leaving the rc device registration and resources created by rc_register_device() behind.

Add a registered-device unwind label for the IRQ lookup, IRQ request, and hardware initialization failure paths. Keep rc_free_device() for failures before rc_register_device() succeeds.

Fixes b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
Cc [email protected]
Co-developed-by Ijae Kim <[email protected]>
Signed-off-by Ijae Kim <[email protected]>
Signed-off-by Myeonghun Pak <[email protected]>
---
 drivers/media/rc/sunxi-cir.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/media/rc/sunxi-cir.c +7 -3
index 92ef4e7c6f..cc64a68dfe 100644
@@ -344,22 +344,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	ir->irq = platform_get_irq(pdev, 0);
 	if (ir->irq < 0) {
 		ret = ir->irq;
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 	}
 
 	ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
 	if (ret) {
 		dev_err(dev, "failed request irq\n");
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 	}
 
 	ret = sunxi_ir_hw_init(dev);
 	if (ret)
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 
 	dev_info(dev, "initialized sunXi IR driver\n");
 	return 0;
 
+exit_unregister_dev:
+	rc_unregister_device(ir->rc);
+	return ret;
+
 exit_free_dev:
 	rc_free_device(ir->rc);
Signature · 1 line
2.50.1
maintainer
#1 lore raw

Dne sreda, 13. maj 2026 ob 09:11:03 Srednjeevropski poletni čas je 박명훈 napisal(a):

Quoted 41 lines

From: Myeonghun Pak <[email protected]>

After rc_register_device() succeeds, later probe failures must undo the registration with rc_unregister_device(). The current error path jumps to the allocation cleanup label and only calls rc_free_device(), leaving the rc device registration and resources created by rc_register_device() behind.

Add a registered-device unwind label for the IRQ lookup, IRQ request, and hardware initialization failure paths. Keep rc_free_device() for failures before rc_register_device() succeeds.

Fixes b4e3e59fb59c ("[media] rc: add sunxi-ir driver")
Cc [email protected]
Co-developed-by Ijae Kim <[email protected]>
Signed-off-by Ijae Kim <[email protected]>
Signed-off-by Myeonghun Pak <[email protected]>
---
 drivers/media/rc/sunxi-cir.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)
drivers/media/rc/sunxi-cir.c +2 -2
index 92ef4e7c6f..cc64a68dfe 100644
@@ -344,22 +344,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	ir->irq = platform_get_irq(pdev, 0);
 	if (ir->irq < 0) {
 		ret = ir->irq;
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 	}
 
 	ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
 	if (ret) {
 		dev_err(dev, "failed request irq\n");
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 	}
 
 	ret = sunxi_ir_hw_init(dev);

drivers/media/rc/sunxi-cir.c static int sunxi_ir_probe(struct platform_device *pdev) · line 365

Quoted 10 lines
drivers/media/rc/sunxi-cir.c +4 -1
 	if (ret)
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 
 	dev_info(dev, "initialized sunXi IR driver\n");
 	return 0;
 
+exit_unregister_dev:
+	rc_unregister_device(ir->rc);
+	return ret;

This return should not be here. Device must be freed too.

Best regards,
Jernej

Quoted 5 lines
drivers/media/rc/sunxi-cir.c +1 -0
+
 exit_free_dev:
 	rc_free_device(ir->rc);
AI review
#2 lore raw
In reply to 박명훈
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider: - [High] The newly added error path leaks the `rc_dev` structure. - [High] Device registration and IRQ…

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The newly added error path leaks the rc_dev structure.
- [High] Device registration and IRQ request occur before hardware clocks are enabled, creating a race condition that can panic the kernel.
- [High] Mixing devm-managed IRQ with manual error path cleanup creates a Use-After-Free vulnerability.
- [High] Unvalidated clock-frequency device tree property allows a divide-by-zero kernel panic.
--

commit 230e9a371e882d2d3972a39ec6ca89c53afbeaf9
Author: Myeonghun Pak <[email protected]>

media: rc: sunxi-cir: unregister rc device on probe failure

This commit updates the error handling in sunxi_ir_probe() to ensure the RC device is unregistered if a failure occurs after rc_register_device() succeeds.

While reviewing this commit, I noticed a few potential issues, including some existing concerns in the probe function.

Quoted 10 lines
drivers/media/rc/sunxi-cir.c +1 -1
index cb4c56bf0752a..f40f3ea0ac8e5 100644
@@ -344,22 +344,26 @@ static int sunxi_ir_probe(struct platform_device *pdev)
 	ir->irq = platform_get_irq(pdev, 0);
 	if (ir->irq < 0) {
 		ret = ir->irq;
-		goto exit_free_dev;
+		goto exit_unregister_dev;

drivers/media/rc/sunxi-cir.c static int sunxi_ir_probe(struct platform_device *pdev) · line 356

Quoted 10 lines
drivers/media/rc/sunxi-cir.c +1 -1
 	}
 
 	ret = devm_request_irq(dev, ir->irq, sunxi_ir_irq, 0, SUNXI_IR_DEV, ir);
 	if (ret) {
 		dev_err(dev, "failed request irq\n");
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 	}
 
 	ret = sunxi_ir_hw_init(dev);

Is there a race condition since rc_register_device() and devm_request_irq() are called before sunxi_ir_hw_init() enables the hardware clocks?

If an early interrupt fires, or if userspace interacts with the device via sysfs or lirc (like setting the timeout), could the driver attempt to read or write memory-mapped registers while the clocks are still disabled?

Also, if sunxi_ir_hw_init() fails, the code jumps to exit_unregister_dev. Because the IRQ is managed by devm_request_irq(), it remains active until the probe function completely returns. If the interrupt fires concurrently while rc_unregister_device() is tearing down the device, could the handler access freed resources?

drivers/media/rc/sunxi-cir.c static int sunxi_ir_probe(struct platform_device *pdev) · line 366

Quoted 11 lines
drivers/media/rc/sunxi-cir.c +5 -1
 	if (ret)
-		goto exit_free_dev;
+		goto exit_unregister_dev;
 
 	dev_info(dev, "initialized sunXi IR driver\n");
 	return 0;
 
+exit_unregister_dev:
+	rc_unregister_device(ir->rc);
+	return ret;
+

Does this error path leak the rc_dev structure?

rc_allocate_device() initializes the structure with a reference count of 1. While rc_unregister_device() removes the device from the system, it does not drop this initial reference count.

Would it be necessary to fall through or explicitly call rc_free_device() here to fully free the memory?

drivers/media/rc/sunxi-cir.c static int sunxi_ir_probe(struct platform_device *pdev) · line 369

Quoted 3 lines
drivers/media/rc/sunxi-cir.c +0 -0
 exit_free_dev:
 	rc_free_device(ir->rc);

Additionally, earlier in sunxi_ir_probe(), the driver reads the clock-frequency property and calculates the rx_resolution:

drivers/media/rc/sunxi-cir.c:sunxi_ir_probe() {
        ...
        of_property_read_u32(dn, "clock-frequency", &b_clk_freq);
        ...
        ir->rc->rx_resolution = (USEC_PER_SEC / (b_clk_freq / 64));
        ...
}

Could this cause a divide-by-zero issue? If the device tree specifies a clock frequency less than 64 (such as 0), b_clk_freq / 64 would evaluate to 0.

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