[PATCH v4 01/15] media: streamzap: Add missing rc_unregister_device()

[PATCH v4 01/15] media: streamzap: Add missing rc_unregister_device()

[Sean Young]

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 <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/streamzap.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/rc/streamzap.c b/drivers/media/rc/streamzap.c
index 307985d74fe8..41195ad82734 100644
--- a/drivers/media/rc/streamzap.c
+++ b/drivers/media/rc/streamzap.c
@@ -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:
-- 
2.55.0

[PATCH v4 02/15] media: redrat3: Ensure rc device is freed if enable_detector() fails

[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 <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/redrat3.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 3f828a564e19..2b639fe59923 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -979,6 +979,7 @@ static int redrat3_dev_probe(struct usb_interface *intf,
 	struct usb_endpoint_descriptor *ep_narrow = NULL;
 	struct usb_endpoint_descriptor *ep_wide = NULL;
 	struct usb_endpoint_descriptor *ep_out = NULL;
+	struct rc_dev *rc = NULL;
 	u8 addr, attrs;
 	int pipe, i;
 	int retval = -ENOMEM;
@@ -1102,26 +1103,31 @@ static int redrat3_dev_probe(struct usb_interface *intf,
 	if (retval)
 		goto redrat_free;
 
-	rr3->rc = redrat3_init_rc_dev(rr3);
-	if (!rr3->rc) {
+	rc = redrat3_init_rc_dev(rr3);
+	if (!rc) {
 		retval = -ENOMEM;
 		goto led_free;
 	}
 
+	rr3->rc = rc;
+
 	/* 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(rc);
 led_free:
 	led_classdev_unregister(&rr3->led);
 redrat_free:
 	redrat3_delete(rr3, rr3->udev);
+	rc_free_device(rc);
 
 no_endpoints:
 	return retval;
-- 
2.55.0

[PATCH v4 03/15] media: redrat3: Ensure we don't read beyond the end of the packet

[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 <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/redrat3.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 2b639fe59923..f86efcb74e6e 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -358,8 +358,24 @@ 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);
+
+	/*
+	 * Note we are not checking if we are reading beyond the end of the
+	 * packet which was sent, and reading stale data. If the device
+	 * sends a packet which is short then we get garbage IR, but no
+	 * out of bounds read.
+	 */
+	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 */
-- 
2.55.0

[PATCH v4 04/15] media: redrat3: Ensure all urbs are suspended

[Sean Young]

Ensure that the learn urb is stopped before suspend.

Fixes: c49fcdde38cb ("[media] redrat3: enable carrier reports using wideband receiver")
Signed-off-by: Sean Young <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/redrat3.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index f86efcb74e6e..994d4864520c 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -1170,6 +1170,7 @@ static int redrat3_dev_suspend(struct usb_interface *intf, pm_message_t message)
 	usb_kill_urb(rr3->narrow_urb);
 	usb_kill_urb(rr3->wide_urb);
 	usb_kill_urb(rr3->flash_urb);
+	usb_kill_urb(rr3->learn_urb);
 	return 0;
 }
 
-- 
2.55.0

[PATCH v4 05/15] media: redrat3: Error path leaves device in transmitting state

[Sean Young]

If the allocation fails, transmitting is left as true and the transmitter
cannot be used any more.

Fixes: fac59136bc22 ("[media] RedRat3: Return directly after a failed kcalloc() in redrat3_transmit_ir()")"
Signed-off-by: Sean Young <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/redrat3.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/media/rc/redrat3.c b/drivers/media/rc/redrat3.c
index 994d4864520c..bc75f68ca535 100644
--- a/drivers/media/rc/redrat3.c
+++ b/drivers/media/rc/redrat3.c
@@ -781,9 +781,6 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf,
 	if (count > RR3_MAX_SIG_SIZE - RR3_TX_TRAILER_LEN)
 		return -EINVAL;
 
-	/* rr3 will disable rc detector on transmit */
-	rr3->transmitting = true;
-
 	sample_lens = kzalloc_objs(*sample_lens, RR3_DRIVER_MAXLENS);
 	if (!sample_lens)
 		return -ENOMEM;
@@ -794,6 +791,9 @@ static int redrat3_transmit_ir(struct rc_dev *rcdev, unsigned *txbuf,
 		goto out;
 	}
 
+	/* rr3 will disable rc detector on transmit */
+	rr3->transmitting = true;
+
 	for (i = 0; i < count; i++) {
 		cur_sample_len = redrat3_us_to_len(txbuf[i]);
 		if (cur_sample_len > 0xffff) {
-- 
2.55.0

Re: [PATCH v4 05/15] media: redrat3: Error path leaves device in transmitting state

[Markus Elfring]

> If the allocation fails, transmitting is left as true and the transmitter
> cannot be used any more.

See also once more:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n94


…
> ---
>  drivers/media/rc/redrat3.c | 6 +++---…

Some contributors would appreciate patch version descriptions.
https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n310

Regards,
Markus

Re: [PATCH v4 05/15] media: redrat3: Error path leaves device in transmitting state

[Sean Young]

On Mon, Jul 27, 2026 at 05:57:23PM +0200, Markus Elfring wrote:
> > If the allocation fails, transmitting is left as true and the transmitter
> > cannot be used any more.
> 
> See also once more:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n94

I've tried to re-write the message in imperative mood, but doesn't look
any better. It can be improved though, how about:

In the allocation failure error path, the transmitting boolean is not
cleared and the transmitter cannot be used until the device is unplugged
or the module reloaded.

Is that better?

(I don't think we need a v5 just for a change of the commit message before
commit.)

> …
> > ---
> >  drivers/media/rc/redrat3.c | 6 +++---…
> 
> Some contributors would appreciate patch version descriptions.
> https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n310

This patch has not changed since earlier versions.


Sean

Re: [PATCH v4 05/15] media: redrat3: Error path leaves device in transmitting state

[Markus Elfring]

>>> If the allocation fails, transmitting is left as true and the transmitter
>>> cannot be used any more.
>>
>> See also once more:
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n94
> 
> I've tried to re-write the message in imperative mood, but doesn't look
> any better. It can be improved though, how about:
> 
> In the allocation failure error path, the transmitting boolean is not
> cleared and the transmitter cannot be used until the device is unplugged
> or the module reloaded.
> 
> Is that better?

Not really according to known wording requirements.

* Which allocation would have failed here?

* How do you think about to add a wording like the following to the introduction paragraph?

  Thus move an assignment statement.


> (I don't think we need a v5 just for a change of the commit message before
> commit.)
> 
>> …
>>> ---
>>>  drivers/media/rc/redrat3.c | 6 +++---…
>>
>> Some contributors would appreciate patch version descriptions.
>> https://lore.kernel.org/all/?q=%22This+looks+like+a+new+version+of+a+previously+submitted+patch%22
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?h=v7.2-rc5#n310
> 
> This patch has not changed since earlier versions.
Can the running patch review trigger further extensions for the changelog (in the cover letter)?
https://lore.kernel.org/linux-media/cover.1785158244.git.sean@mess.org/

Regards,
Markus

[PATCH v4 06/15] media: sunxi-cir: Ensure no more interrupts can occur before free

[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 <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/sunxi-cir.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/rc/sunxi-cir.c b/drivers/media/rc/sunxi-cir.c
index 28e840a7e5b8..af1ee08ffdbe 100644
--- 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);
 }
 
 static void sunxi_ir_shutdown(struct platform_device *pdev)
-- 
2.55.0

[PATCH v4 07/15] media: meson-ir-tx: Ensure clock is disabled on unbind

[Sean Young]

clk_prepare_enable() needs a call to clk_disable_unprepare() on
driver unbind. Make it devm managed.

Fixes: 49be1c78d575 ("media: rc: introduce Meson IR TX driver")
Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/meson-ir-tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
index fded2c256f2a..e7bb107e6a84 100644
--- a/drivers/media/rc/meson-ir-tx.c
+++ b/drivers/media/rc/meson-ir-tx.c
@@ -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;
-- 
2.55.0

[PATCH v4 08/15] media: meson-ir-tx: Ensure rc_free_device() is called on unbind

[Sean Young]

Make rc_dev devm managed.

Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/meson-ir-tx.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
index e7bb107e6a84..abb107d19e8c 100644
--- a/drivers/media/rc/meson-ir-tx.c
+++ b/drivers/media/rc/meson-ir-tx.c
@@ -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;
 }
-- 
2.55.0

Re: [PATCH v4 08/15] media: meson-ir-tx: Ensure rc_free_device() is called on unbind

[Neil Armstrong]

On 7/27/26 15:18, Sean Young wrote:
> Make rc_dev devm managed.
> 
> Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
> Signed-off-by: Sean Young <sean@mess.org>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: stable@vger.kernel.org
> ---
>   drivers/media/rc/meson-ir-tx.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
> index e7bb107e6a84..abb107d19e8c 100644
> --- a/drivers/media/rc/meson-ir-tx.c
> +++ b/drivers/media/rc/meson-ir-tx.c
> @@ -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;
>   }

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

[PATCH v4 09/15] media: meson-ir-tx: Ensure probe error is propagated

[Sean Young]

devm_clk_get_enabled() may return -EPROBE_DEFER which needs to be
propagated else the probe will not be deferred, it will fail instead.

Fixes: 49be1c78d575 ("media: rc: introduce Meson IR TX driver")
Signed-off-by: Sean Young <sean@mess.org>
Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/meson-ir-tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
index abb107d19e8c..174d5135e1bb 100644
--- a/drivers/media/rc/meson-ir-tx.c
+++ b/drivers/media/rc/meson-ir-tx.c
@@ -290,7 +290,7 @@ static int meson_irtx_mod_clock_probe(struct meson_irtx *ir,
 
 	clock = devm_clk_get_enabled(ir->dev, "xtal");
 	if (IS_ERR(clock))
-		return -ENODEV;
+		return PTR_ERR(clock);
 
 	*clk_nr = IRB_MOD_XTAL3_CLK;
 	ir->clk_rate = clk_get_rate(clock) / 3;
@@ -324,7 +324,7 @@ static int meson_irtx_probe(struct platform_device *pdev)
 
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0)
-		return -ENODEV;
+		return irq;
 
 	ir->dev = dev;
 	ir->carrier = MIRTX_DEFAULT_CARRIER;
-- 
2.55.0

Re: [PATCH v4 09/15] media: meson-ir-tx: Ensure probe error is propagated

[Neil Armstrong]

On 7/27/26 15:18, Sean Young wrote:
> devm_clk_get_enabled() may return -EPROBE_DEFER which needs to be
> propagated else the probe will not be deferred, it will fail instead.
> 
> Fixes: 49be1c78d575 ("media: rc: introduce Meson IR TX driver")
> Signed-off-by: Sean Young <sean@mess.org>
> Reviewed-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: stable@vger.kernel.org
> ---
>   drivers/media/rc/meson-ir-tx.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/rc/meson-ir-tx.c b/drivers/media/rc/meson-ir-tx.c
> index abb107d19e8c..174d5135e1bb 100644
> --- a/drivers/media/rc/meson-ir-tx.c
> +++ b/drivers/media/rc/meson-ir-tx.c
> @@ -290,7 +290,7 @@ static int meson_irtx_mod_clock_probe(struct meson_irtx *ir,
>   
>   	clock = devm_clk_get_enabled(ir->dev, "xtal");
>   	if (IS_ERR(clock))
> -		return -ENODEV;
> +		return PTR_ERR(clock);
>   
>   	*clk_nr = IRB_MOD_XTAL3_CLK;
>   	ir->clk_rate = clk_get_rate(clock) / 3;
> @@ -324,7 +324,7 @@ static int meson_irtx_probe(struct platform_device *pdev)
>   
>   	irq = platform_get_irq(pdev, 0);
>   	if (irq < 0)
> -		return -ENODEV;
> +		return irq;
>   
>   	ir->dev = dev;
>   	ir->carrier = MIRTX_DEFAULT_CARRIER;

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil

[PATCH v4 10/15] media: ir-hix5hd2: Ensure rdev is setup before interrupts are enabled

[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 <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/ir-hix5hd2.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/ir-hix5hd2.c b/drivers/media/rc/ir-hix5hd2.c
index 1b061e4a3dcf..aa3de4d57a58 100644
--- a/drivers/media/rc/ir-hix5hd2.c
+++ b/drivers/media/rc/ir-hix5hd2.c
@@ -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;
-- 
2.55.0

[PATCH v4 11/15] media: rc: Use after free in ir_raw_event_handle()

[Sean Young]

If rc_unregister_device() is called while IR is being processed, then
ir_raw_event_handle() could call wake_up_process(dev->raw->thread)
after kthread_stop(dev->raw->thread).

Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
Signed-off-by: Sean Young <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/rc-ir-raw.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index ba24c2f22d39..96849faafde3 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -637,6 +637,7 @@ int ir_raw_event_register(struct rc_dev *dev)
 	if (IS_ERR(thread))
 		return PTR_ERR(thread);
 
+	get_task_struct(thread);
 	dev->raw->thread = thread;
 
 	mutex_lock(&ir_raw_handler_lock);
@@ -648,8 +649,13 @@ int ir_raw_event_register(struct rc_dev *dev)
 
 void ir_raw_event_free(struct rc_dev *dev)
 {
-	kfree(dev->raw);
-	dev->raw = NULL;
+	if (dev->raw) {
+		timer_delete_sync(&dev->raw->edge_handle);
+		if (dev->raw->thread)
+			put_task_struct(dev->raw->thread);
+		kfree(dev->raw);
+		dev->raw = NULL;
+	}
 }
 
 void ir_raw_event_unregister(struct rc_dev *dev)
-- 
2.55.0

Re: [PATCH v4 11/15] media: rc: Use after free in ir_raw_event_handle()

[Hans Verkuil]

On 27/07/2026 15:18, Sean Young wrote:
> If rc_unregister_device() is called while IR is being processed, then
> ir_raw_event_handle() could call wake_up_process(dev->raw->thread)
> after kthread_stop(dev->raw->thread).
> 
> Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
> Signed-off-by: Sean Young <sean@mess.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/media/rc/rc-ir-raw.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index ba24c2f22d39..96849faafde3 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -637,6 +637,7 @@ int ir_raw_event_register(struct rc_dev *dev)
>  	if (IS_ERR(thread))
>  		return PTR_ERR(thread);
>  
> +	get_task_struct(thread);
>  	dev->raw->thread = thread;
>  
>  	mutex_lock(&ir_raw_handler_lock);
> @@ -648,8 +649,13 @@ int ir_raw_event_register(struct rc_dev *dev)
>  
>  void ir_raw_event_free(struct rc_dev *dev)
>  {
> -	kfree(dev->raw);
> -	dev->raw = NULL;
> +	if (dev->raw) {
> +		timer_delete_sync(&dev->raw->edge_handle);
> +		if (dev->raw->thread)
> +			put_task_struct(dev->raw->thread);
> +		kfree(dev->raw);
> +		dev->raw = NULL;
> +	}
>  }
>  
>  void ir_raw_event_unregister(struct rc_dev *dev)

I think this patch could do with some comments in the code explaining why
you need get/put_task_struct.

It's the first time I've seen these functions being used, and I'm not sure
why they solve the issue or whether this is the right approach.

Regards,

	Hans

[PATCH v4 12/15] media: rc: Fix use after free in bpf progs

[Sean Young]

Since commit dccc0c3ddf8f ("media: rc: fix race between unregister and
urb/irq callbacks"), rcdev->raw is no longer set to NULL after device
unregister. raw->progs could point to stale data.

Fixes: dccc0c3ddf8f ("media: rc: fix race between unregister and urb/irq callbacks")
Signed-off-by: Sean Young <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/bpf-lirc.c  | 18 +++++++++++++-----
 drivers/media/rc/rc-ir-raw.c | 11 +++--------
 2 files changed, 16 insertions(+), 13 deletions(-)

diff --git a/drivers/media/rc/bpf-lirc.c b/drivers/media/rc/bpf-lirc.c
index 2f7564f26445..14ab611e7445 100644
--- a/drivers/media/rc/bpf-lirc.c
+++ b/drivers/media/rc/bpf-lirc.c
@@ -148,12 +148,13 @@ static int lirc_bpf_attach(struct rc_dev *rcdev, struct bpf_prog *prog)
 	if (ret)
 		return ret;
 
-	raw = rcdev->raw;
-	if (!raw) {
+	if (!rcdev->registered) {
 		ret = -ENODEV;
 		goto unlock;
 	}
 
+	raw = rcdev->raw;
+
 	old_array = lirc_rcu_dereference(raw->progs);
 	if (old_array && bpf_prog_array_length(old_array) >= BPF_MAX_PROGS) {
 		ret = -E2BIG;
@@ -186,12 +187,13 @@ static int lirc_bpf_detach(struct rc_dev *rcdev, struct bpf_prog *prog)
 	if (ret)
 		return ret;
 
-	raw = rcdev->raw;
-	if (!raw) {
+	if (!rcdev->registered) {
 		ret = -ENODEV;
 		goto unlock;
 	}
 
+	raw = rcdev->raw;
+
 	old_array = lirc_rcu_dereference(raw->progs);
 	ret = bpf_prog_array_copy(old_array, prog, NULL, 0, &new_array);
 	/*
@@ -235,7 +237,8 @@ void lirc_bpf_free(struct rc_dev *rcdev)
 	struct bpf_prog_array_item *item;
 	struct bpf_prog_array *array;
 
-	array = lirc_rcu_dereference(rcdev->raw->progs);
+	array = rcu_replace_pointer(rcdev->raw->progs, NULL,
+				    lockdep_is_held(&ir_raw_handler_lock));
 	if (!array)
 		return;
 
@@ -316,6 +319,11 @@ int lirc_prog_query(const union bpf_attr *attr, union bpf_attr __user *uattr)
 	if (ret)
 		goto put;
 
+	if (!rcdev->registered) {
+		ret = -ENODEV;
+		goto unlock;
+	}
+
 	progs = lirc_rcu_dereference(rcdev->raw->progs);
 	cnt = progs ? bpf_prog_array_length(progs) : 0;
 
diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index 96849faafde3..b32f3cff9401 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -651,8 +651,11 @@ void ir_raw_event_free(struct rc_dev *dev)
 {
 	if (dev->raw) {
 		timer_delete_sync(&dev->raw->edge_handle);
+		mutex_lock(&ir_raw_handler_lock);
 		if (dev->raw->thread)
 			put_task_struct(dev->raw->thread);
+		lirc_bpf_free(dev);
+		mutex_unlock(&ir_raw_handler_lock);
 		kfree(dev->raw);
 		dev->raw = NULL;
 	}
@@ -662,9 +665,6 @@ void ir_raw_event_unregister(struct rc_dev *dev)
 {
 	struct ir_raw_handler *handler;
 
-	if (!dev || !dev->raw)
-		return;
-
 	kthread_stop(dev->raw->thread);
 	timer_delete_sync(&dev->raw->edge_handle);
 
@@ -677,11 +677,6 @@ void ir_raw_event_unregister(struct rc_dev *dev)
 
 	lirc_bpf_free(dev);
 
-	/*
-	 * A user can be calling bpf(BPF_PROG_{QUERY|ATTACH|DETACH}), so
-	 * ensure that the raw member is null on unlock; this is how
-	 * "device gone" is checked.
-	 */
 	mutex_unlock(&ir_raw_handler_lock);
 }
 
-- 
2.55.0

[PATCH v4 13/15] media: cx88: Specify rc type at rc_allocate_type()

[Sean Young]

Changing the type of the rc device after allocation is ugly and
this will also break in a future commit. Ensure that the allocation
happens with the correct type in the first place.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/pci/cx88/cx88-input.c | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/drivers/media/pci/cx88/cx88-input.c b/drivers/media/pci/cx88/cx88-input.c
index 5d9ce4f9af01..c96d289e5ab7 100644
--- a/drivers/media/pci/cx88/cx88-input.c
+++ b/drivers/media/pci/cx88/cx88-input.c
@@ -259,7 +259,7 @@ static void cx88_ir_close(struct rc_dev *rc)
 int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 {
 	struct cx88_IR *ir;
-	struct rc_dev *dev;
+	struct rc_dev *dev = NULL;
 	char *ir_codes = NULL;
 	u64 rc_proto = RC_PROTO_BIT_OTHER;
 	int err = -ENOMEM;
@@ -268,11 +268,8 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 				 */
 
 	ir = kzalloc_obj(*ir);
-	dev = rc_allocate_device(RC_DRIVER_IR_RAW);
-	if (!ir || !dev)
-		goto err_out_free;
-
-	ir->dev = dev;
+	if (!ir)
+		return -ENOMEM;
 
 	/* detect & configure */
 	switch (core->boardnr) {
@@ -439,6 +436,13 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 		goto err_out_free;
 	}
 
+	dev = rc_allocate_device(ir->sampling ?
+				 RC_DRIVER_IR_RAW : RC_DRIVER_SCANCODE);
+	if (!dev)
+		goto err_out_free;
+
+	ir->dev = dev;
+
 	/*
 	 * The usage of mask_keycode were very convenient, due to several
 	 * reasons. Among others, the scancode tables were using the scancode
@@ -477,12 +481,10 @@ int cx88_ir_init(struct cx88_core *core, struct pci_dev *pci)
 	dev->close = cx88_ir_close;
 	dev->scancode_mask = hardware_mask;
 
-	if (ir->sampling) {
+	if (ir->sampling)
 		dev->timeout = MS_TO_US(10); /* 10 ms */
-	} else {
-		dev->driver_type = RC_DRIVER_SCANCODE;
+	else
 		dev->allowed_protocols = rc_proto;
-	}
 
 	ir->core = core;
 	core->ir = ir;
-- 
2.55.0

[PATCH v4 14/15] media: saa7134: Specify rc type at rc_allocate_type()

[Sean Young]

Changing the type of the rc device after allocation is ugly and
this will also break in a future commit. Ensure that the allocation
happens with the correct type in the first place.

Signed-off-by: Sean Young <sean@mess.org>
---
 drivers/media/pci/saa7134/saa7134-input.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/pci/saa7134/saa7134-input.c b/drivers/media/pci/saa7134/saa7134-input.c
index 7f6680de3156..81620e629449 100644
--- a/drivers/media/pci/saa7134/saa7134-input.c
+++ b/drivers/media/pci/saa7134/saa7134-input.c
@@ -769,7 +769,8 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	}
 
 	ir = kzalloc_obj(*ir);
-	rc = rc_allocate_device(RC_DRIVER_SCANCODE);
+	rc = rc_allocate_device(raw_decode ?
+				RC_DRIVER_IR_RAW : RC_DRIVER_SCANCODE);
 	if (!ir || !rc) {
 		err = -ENOMEM;
 		goto err_out_free;
@@ -792,11 +793,6 @@ int saa7134_input_init1(struct saa7134_dev *dev)
 	rc->priv = dev;
 	rc->open = saa7134_ir_open;
 	rc->close = saa7134_ir_close;
-	if (raw_decode) {
-		rc->driver_type = RC_DRIVER_IR_RAW;
-		rc->allowed_protocols = RC_PROTO_BIT_ALL_IR_DECODER;
-	}
-
 	rc->device_name = saa7134_boards[dev->board].name;
 	rc->input_phys = ir->phys;
 	rc->input_id.bustype = BUS_PCI;
-- 
2.55.0

[PATCH v4 15/15] media: rc: Fix race condition during rc_register_device()

[Sean Young]

The correct sequence for rc core is so:

     rc_allocation_device()

     /*
      * setup hardware and now calls to ir_raw_event_store etc are
      * permitted, as well as rc_keydown. There is no rc device in sysfs
      * or lirc chardev.
      */
     rc_register_device()

     /*
      * After rc_register_device(), the rc device can be used now from lirc
      * chardev, sysfs and IR is now decoded and reported.
      */

     rc_unregister_device()
     /*
      * User space can no longer access /dev/lirc or the rc sysfs
      * attributes. They will get -ENODEV if they still have a file
      * descriptor open.
      * Calls to ir_raw_event_handle() etc or rc_keydown() are permitted
      * but they must stop before the call to rc_free_device(), so
      * this is the time to stop the hardware.
      */
     rc_free_device()

This means that during rc_register_device(), we can get calls to
ir_raw_event_{store,handle,overflow,store_with_filter}. Ensure this
is done race-free.

Fixes: a3572c34da8d ("V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core")
Signed-off-by: Sean Young <sean@mess.org>
Cc: stable@vger.kernel.org
---
 drivers/media/rc/rc-ir-raw.c | 20 +-------------------
 drivers/media/rc/rc-main.c   | 19 +++++++++++--------
 2 files changed, 12 insertions(+), 27 deletions(-)

diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
index b32f3cff9401..bebb45ffa971 100644
--- a/drivers/media/rc/rc-ir-raw.c
+++ b/drivers/media/rc/rc-ir-raw.c
@@ -71,9 +71,6 @@ static int ir_raw_event_thread(void *data)
  */
 int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev)
 {
-	if (!dev->raw)
-		return -EINVAL;
-
 	dev_dbg(&dev->dev, "sample: (%05dus %s)\n",
 		ev->duration, TO_STR(ev->pulse));
 
@@ -102,9 +99,6 @@ int ir_raw_event_store_edge(struct rc_dev *dev, bool pulse)
 	ktime_t			now;
 	struct ir_raw_event	ev = {};
 
-	if (!dev->raw)
-		return -EINVAL;
-
 	now = ktime_get();
 	ev.duration = ktime_to_us(ktime_sub(now, dev->raw->last_event));
 	ev.pulse = !pulse;
@@ -129,9 +123,6 @@ int ir_raw_event_store_with_timeout(struct rc_dev *dev, struct ir_raw_event *ev)
 	ktime_t		now;
 	int		rc = 0;
 
-	if (!dev->raw)
-		return -EINVAL;
-
 	now = ktime_get();
 
 	spin_lock(&dev->raw->edge_spinlock);
@@ -166,9 +157,6 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store_with_timeout);
  */
 int ir_raw_event_store_with_filter(struct rc_dev *dev, struct ir_raw_event *ev)
 {
-	if (!dev->raw)
-		return -EINVAL;
-
 	/* Ignore spaces in idle mode */
 	if (dev->idle && !ev->pulse)
 		return 0;
@@ -200,9 +188,6 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter);
  */
 void ir_raw_event_set_idle(struct rc_dev *dev, bool idle)
 {
-	if (!dev->raw)
-		return;
-
 	dev_dbg(&dev->dev, "%s idle mode\n", idle ? "enter" : "leave");
 
 	if (idle) {
@@ -226,7 +211,7 @@ EXPORT_SYMBOL_GPL(ir_raw_event_set_idle);
  */
 void ir_raw_event_handle(struct rc_dev *dev)
 {
-	if (!dev->raw || !dev->raw->thread)
+	if (!dev->raw->thread)
 		return;
 
 	wake_up_process(dev->raw->thread);
@@ -612,9 +597,6 @@ EXPORT_SYMBOL(ir_raw_encode_carrier);
  */
 int ir_raw_event_prepare(struct rc_dev *dev)
 {
-	if (!dev)
-		return -EINVAL;
-
 	dev->raw = kzalloc_obj(*dev->raw);
 	if (!dev->raw)
 		return -ENOMEM;
diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
index dda3479ea3ad..d93e98189c1a 100644
--- a/drivers/media/rc/rc-main.c
+++ b/drivers/media/rc/rc-main.c
@@ -1701,14 +1701,24 @@ static const struct device_type rc_dev_type = {
 struct rc_dev *rc_allocate_device(enum rc_driver_type type)
 {
 	struct rc_dev *dev;
+	int ret;
 
 	dev = kzalloc_obj(*dev);
 	if (!dev)
 		return NULL;
 
+	if (type == RC_DRIVER_IR_RAW) {
+		ret = ir_raw_event_prepare(dev);
+		if (ret < 0) {
+			kfree(dev);
+			return NULL;
+		}
+	}
+
 	if (type != RC_DRIVER_IR_RAW_TX) {
 		dev->input_dev = input_allocate_device();
 		if (!dev->input_dev) {
+			ir_raw_event_free(dev);
 			kfree(dev);
 			return NULL;
 		}
@@ -1724,6 +1734,7 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type type)
 		spin_lock_init(&dev->rc_map.lock);
 		spin_lock_init(&dev->keylock);
 	}
+
 	mutex_init(&dev->lock);
 
 	dev->dev.type = &rc_dev_type;
@@ -1917,12 +1928,6 @@ int rc_register_device(struct rc_dev *dev)
 		dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
 	dev->sysfs_groups[attr++] = NULL;
 
-	if (dev->driver_type == RC_DRIVER_IR_RAW) {
-		rc = ir_raw_event_prepare(dev);
-		if (rc < 0)
-			goto out_minor;
-	}
-
 	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
 		rc = rc_prepare_rx_device(dev);
 		if (rc)
@@ -1979,8 +1984,6 @@ int rc_register_device(struct rc_dev *dev)
 out_rx_free:
 	ir_free_table(&dev->rc_map);
 out_raw:
-	ir_raw_event_free(dev);
-out_minor:
 	ida_free(&rc_ida, minor);
 	return rc;
 }
-- 
2.55.0

Re: [PATCH v4 15/15] media: rc: Fix race condition during rc_register_device()

[Hans Verkuil]

On 27/07/2026 15:18, Sean Young wrote:
> The correct sequence for rc core is so:
> 
>      rc_allocation_device()
> 
>      /*
>       * setup hardware and now calls to ir_raw_event_store etc are
>       * permitted, as well as rc_keydown. There is no rc device in sysfs
>       * or lirc chardev.
>       */
>      rc_register_device()
> 
>      /*
>       * After rc_register_device(), the rc device can be used now from lirc
>       * chardev, sysfs and IR is now decoded and reported.
>       */
> 
>      rc_unregister_device()
>      /*
>       * User space can no longer access /dev/lirc or the rc sysfs
>       * attributes. They will get -ENODEV if they still have a file
>       * descriptor open.
>       * Calls to ir_raw_event_handle() etc or rc_keydown() are permitted
>       * but they must stop before the call to rc_free_device(), so
>       * this is the time to stop the hardware.
>       */
>      rc_free_device()
> 
> This means that during rc_register_device(), we can get calls to
> ir_raw_event_{store,handle,overflow,store_with_filter}. Ensure this
> is done race-free.
> 
> Fixes: a3572c34da8d ("V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core")
> Signed-off-by: Sean Young <sean@mess.org>
> Cc: stable@vger.kernel.org
> ---
>  drivers/media/rc/rc-ir-raw.c | 20 +-------------------
>  drivers/media/rc/rc-main.c   | 19 +++++++++++--------
>  2 files changed, 12 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> index b32f3cff9401..bebb45ffa971 100644
> --- a/drivers/media/rc/rc-ir-raw.c
> +++ b/drivers/media/rc/rc-ir-raw.c
> @@ -71,9 +71,6 @@ static int ir_raw_event_thread(void *data)
>   */
>  int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev)
>  {
> -	if (!dev->raw)
> -		return -EINVAL;
> -
>  	dev_dbg(&dev->dev, "sample: (%05dus %s)\n",
>  		ev->duration, TO_STR(ev->pulse));
>  
> @@ -102,9 +99,6 @@ int ir_raw_event_store_edge(struct rc_dev *dev, bool pulse)
>  	ktime_t			now;
>  	struct ir_raw_event	ev = {};
>  
> -	if (!dev->raw)
> -		return -EINVAL;
> -
>  	now = ktime_get();
>  	ev.duration = ktime_to_us(ktime_sub(now, dev->raw->last_event));
>  	ev.pulse = !pulse;
> @@ -129,9 +123,6 @@ int ir_raw_event_store_with_timeout(struct rc_dev *dev, struct ir_raw_event *ev)
>  	ktime_t		now;
>  	int		rc = 0;
>  
> -	if (!dev->raw)
> -		return -EINVAL;
> -
>  	now = ktime_get();
>  
>  	spin_lock(&dev->raw->edge_spinlock);
> @@ -166,9 +157,6 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store_with_timeout);
>   */
>  int ir_raw_event_store_with_filter(struct rc_dev *dev, struct ir_raw_event *ev)
>  {
> -	if (!dev->raw)
> -		return -EINVAL;
> -
>  	/* Ignore spaces in idle mode */
>  	if (dev->idle && !ev->pulse)
>  		return 0;
> @@ -200,9 +188,6 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter);
>   */
>  void ir_raw_event_set_idle(struct rc_dev *dev, bool idle)
>  {
> -	if (!dev->raw)
> -		return;
> -
>  	dev_dbg(&dev->dev, "%s idle mode\n", idle ? "enter" : "leave");
>  
>  	if (idle) {
> @@ -226,7 +211,7 @@ EXPORT_SYMBOL_GPL(ir_raw_event_set_idle);
>   */
>  void ir_raw_event_handle(struct rc_dev *dev)
>  {
> -	if (!dev->raw || !dev->raw->thread)
> +	if (!dev->raw->thread)
>  		return;
>  
>  	wake_up_process(dev->raw->thread);
> @@ -612,9 +597,6 @@ EXPORT_SYMBOL(ir_raw_encode_carrier);
>   */
>  int ir_raw_event_prepare(struct rc_dev *dev)
>  {
> -	if (!dev)
> -		return -EINVAL;
> -
>  	dev->raw = kzalloc_obj(*dev->raw);
>  	if (!dev->raw)
>  		return -ENOMEM;
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index dda3479ea3ad..d93e98189c1a 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1701,14 +1701,24 @@ static const struct device_type rc_dev_type = {
>  struct rc_dev *rc_allocate_device(enum rc_driver_type type)
>  {
>  	struct rc_dev *dev;
> +	int ret;
>  
>  	dev = kzalloc_obj(*dev);
>  	if (!dev)
>  		return NULL;
>  
> +	if (type == RC_DRIVER_IR_RAW) {
> +		ret = ir_raw_event_prepare(dev);
> +		if (ret < 0) {
> +			kfree(dev);
> +			return NULL;
> +		}
> +	}
> +
>  	if (type != RC_DRIVER_IR_RAW_TX) {
>  		dev->input_dev = input_allocate_device();
>  		if (!dev->input_dev) {
> +			ir_raw_event_free(dev);
>  			kfree(dev);
>  			return NULL;
>  		}
> @@ -1724,6 +1734,7 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type type)
>  		spin_lock_init(&dev->rc_map.lock);
>  		spin_lock_init(&dev->keylock);
>  	}
> +
>  	mutex_init(&dev->lock);
>  
>  	dev->dev.type = &rc_dev_type;
> @@ -1917,12 +1928,6 @@ int rc_register_device(struct rc_dev *dev)
>  		dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
>  	dev->sysfs_groups[attr++] = NULL;
>  
> -	if (dev->driver_type == RC_DRIVER_IR_RAW) {
> -		rc = ir_raw_event_prepare(dev);
> -		if (rc < 0)
> -			goto out_minor;
> -	}
> -
>  	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
>  		rc = rc_prepare_rx_device(dev);
>  		if (rc)

I don't see what the race condition is you are trying to solve. AFAICT the sysfs
files aren't created until the device_add() function that is right after this line.

I'm probably missing something, but that would mean that the commit log needs to
be improved.

Regards,

	Hans

> @@ -1979,8 +1984,6 @@ int rc_register_device(struct rc_dev *dev)
>  out_rx_free:
>  	ir_free_table(&dev->rc_map);
>  out_raw:
> -	ir_raw_event_free(dev);
> -out_minor:
>  	ida_free(&rc_ida, minor);
>  	return rc;
>  }

Re: [PATCH v4 15/15] media: rc: Fix race condition during rc_register_device()

[Sean Young]

On Tue, Jul 28, 2026 at 11:35:36AM +0200, Hans Verkuil wrote:
> On 27/07/2026 15:18, Sean Young wrote:
> > The correct sequence for rc core is so:
> > 
> >      rc_allocation_device()
> > 
> >      /*
> >       * setup hardware and now calls to ir_raw_event_store etc are
> >       * permitted, as well as rc_keydown. There is no rc device in sysfs
> >       * or lirc chardev.
> >       */
> >      rc_register_device()
> > 
> >      /*
> >       * After rc_register_device(), the rc device can be used now from lirc
> >       * chardev, sysfs and IR is now decoded and reported.
> >       */
> > 
> >      rc_unregister_device()
> >      /*
> >       * User space can no longer access /dev/lirc or the rc sysfs
> >       * attributes. They will get -ENODEV if they still have a file
> >       * descriptor open.
> >       * Calls to ir_raw_event_handle() etc or rc_keydown() are permitted
> >       * but they must stop before the call to rc_free_device(), so
> >       * this is the time to stop the hardware.
> >       */
> >      rc_free_device()
> > 
> > This means that during rc_register_device(), we can get calls to
> > ir_raw_event_{store,handle,overflow,store_with_filter}. Ensure this
> > is done race-free.
> > 
> > Fixes: a3572c34da8d ("V4L/DVB: ir-core: Add logic to decode IR protocols at the IR core")
> > Signed-off-by: Sean Young <sean@mess.org>
> > Cc: stable@vger.kernel.org
> > ---
> >  drivers/media/rc/rc-ir-raw.c | 20 +-------------------
> >  drivers/media/rc/rc-main.c   | 19 +++++++++++--------
> >  2 files changed, 12 insertions(+), 27 deletions(-)
> > 
> > diff --git a/drivers/media/rc/rc-ir-raw.c b/drivers/media/rc/rc-ir-raw.c
> > index b32f3cff9401..bebb45ffa971 100644
> > --- a/drivers/media/rc/rc-ir-raw.c
> > +++ b/drivers/media/rc/rc-ir-raw.c
> > @@ -71,9 +71,6 @@ static int ir_raw_event_thread(void *data)
> >   */
> >  int ir_raw_event_store(struct rc_dev *dev, struct ir_raw_event *ev)
> >  {
> > -	if (!dev->raw)
> > -		return -EINVAL;
> > -
> >  	dev_dbg(&dev->dev, "sample: (%05dus %s)\n",
> >  		ev->duration, TO_STR(ev->pulse));
> >  
> > @@ -102,9 +99,6 @@ int ir_raw_event_store_edge(struct rc_dev *dev, bool pulse)
> >  	ktime_t			now;
> >  	struct ir_raw_event	ev = {};
> >  
> > -	if (!dev->raw)
> > -		return -EINVAL;
> > -
> >  	now = ktime_get();
> >  	ev.duration = ktime_to_us(ktime_sub(now, dev->raw->last_event));
> >  	ev.pulse = !pulse;
> > @@ -129,9 +123,6 @@ int ir_raw_event_store_with_timeout(struct rc_dev *dev, struct ir_raw_event *ev)
> >  	ktime_t		now;
> >  	int		rc = 0;
> >  
> > -	if (!dev->raw)
> > -		return -EINVAL;
> > -
> >  	now = ktime_get();
> >  
> >  	spin_lock(&dev->raw->edge_spinlock);
> > @@ -166,9 +157,6 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store_with_timeout);
> >   */
> >  int ir_raw_event_store_with_filter(struct rc_dev *dev, struct ir_raw_event *ev)
> >  {
> > -	if (!dev->raw)
> > -		return -EINVAL;
> > -
> >  	/* Ignore spaces in idle mode */
> >  	if (dev->idle && !ev->pulse)
> >  		return 0;
> > @@ -200,9 +188,6 @@ EXPORT_SYMBOL_GPL(ir_raw_event_store_with_filter);
> >   */
> >  void ir_raw_event_set_idle(struct rc_dev *dev, bool idle)
> >  {
> > -	if (!dev->raw)
> > -		return;
> > -
> >  	dev_dbg(&dev->dev, "%s idle mode\n", idle ? "enter" : "leave");
> >  
> >  	if (idle) {
> > @@ -226,7 +211,7 @@ EXPORT_SYMBOL_GPL(ir_raw_event_set_idle);
> >   */
> >  void ir_raw_event_handle(struct rc_dev *dev)
> >  {
> > -	if (!dev->raw || !dev->raw->thread)
> > +	if (!dev->raw->thread)
> >  		return;
> >  
> >  	wake_up_process(dev->raw->thread);
> > @@ -612,9 +597,6 @@ EXPORT_SYMBOL(ir_raw_encode_carrier);
> >   */
> >  int ir_raw_event_prepare(struct rc_dev *dev)
> >  {
> > -	if (!dev)
> > -		return -EINVAL;
> > -
> >  	dev->raw = kzalloc_obj(*dev->raw);
> >  	if (!dev->raw)
> >  		return -ENOMEM;
> > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> > index dda3479ea3ad..d93e98189c1a 100644
> > --- a/drivers/media/rc/rc-main.c
> > +++ b/drivers/media/rc/rc-main.c
> > @@ -1701,14 +1701,24 @@ static const struct device_type rc_dev_type = {
> >  struct rc_dev *rc_allocate_device(enum rc_driver_type type)
> >  {
> >  	struct rc_dev *dev;
> > +	int ret;
> >  
> >  	dev = kzalloc_obj(*dev);
> >  	if (!dev)
> >  		return NULL;
> >  
> > +	if (type == RC_DRIVER_IR_RAW) {
> > +		ret = ir_raw_event_prepare(dev);
> > +		if (ret < 0) {
> > +			kfree(dev);
> > +			return NULL;
> > +		}
> > +	}
> > +
> >  	if (type != RC_DRIVER_IR_RAW_TX) {
> >  		dev->input_dev = input_allocate_device();
> >  		if (!dev->input_dev) {
> > +			ir_raw_event_free(dev);
> >  			kfree(dev);
> >  			return NULL;
> >  		}
> > @@ -1724,6 +1734,7 @@ struct rc_dev *rc_allocate_device(enum rc_driver_type type)
> >  		spin_lock_init(&dev->rc_map.lock);
> >  		spin_lock_init(&dev->keylock);
> >  	}
> > +
> >  	mutex_init(&dev->lock);
> >  
> >  	dev->dev.type = &rc_dev_type;
> > @@ -1917,12 +1928,6 @@ int rc_register_device(struct rc_dev *dev)
> >  		dev->sysfs_groups[attr++] = &rc_dev_wakeup_filter_attr_grp;
> >  	dev->sysfs_groups[attr++] = NULL;
> >  
> > -	if (dev->driver_type == RC_DRIVER_IR_RAW) {
> > -		rc = ir_raw_event_prepare(dev);
> > -		if (rc < 0)
> > -			goto out_minor;
> > -	}
> > -
> >  	if (dev->driver_type != RC_DRIVER_IR_RAW_TX) {
> >  		rc = rc_prepare_rx_device(dev);
> >  		if (rc)
> 
> I don't see what the race condition is you are trying to solve. AFAICT the sysfs
> files aren't created until the device_add() function that is right after this line.
> 
> I'm probably missing something, but that would mean that the commit log needs to
> be improved.

You're right, the commit message is terrible. I've reworded it for v5.


Sean

Re: [PATCH v4 00/15] media: Fix leaks in rc core

[Markus Elfring]

…
>   media: rc: Fix use after free in bpf progs
>   media: cx88: Specify rc type at rc_allocate_type()
>   media: saa7134: Specify rc type at rc_allocate_type()
>   media: rc: Fix race condition during rc_register_device()
…

How do you think about to reorder desirable adjustments for affected modules?

Regards,
Markus

Re: [PATCH v4 00/15] media: Fix leaks in rc core

[Sean Young]

On Wed, Jul 29, 2026 at 08:18:40AM +0200, Markus Elfring wrote:
> …
> >   media: rc: Fix use after free in bpf progs
> >   media: cx88: Specify rc type at rc_allocate_type()
> >   media: saa7134: Specify rc type at rc_allocate_type()
> >   media: rc: Fix race condition during rc_register_device()
> …
> 
> How do you think about to reorder desirable adjustments for affected modules?

I'm not entirely sure what you mean. Do you mean just sort the
commits so that media: rc: .. sits next to each other?


Sean

Re: [v4 00/15] media: Fix leaks in rc core

[Markus Elfring]

>> …
>>>   media: rc: Fix use after free in bpf progs
>>>   media: cx88: Specify rc type at rc_allocate_type()
>>>   media: saa7134: Specify rc type at rc_allocate_type()
>>>   media: rc: Fix race condition during rc_register_device()
>> …
>>
>> How do you think about to reorder desirable adjustments for affected modules?
> 
> I'm not entirely sure what you mean. Do you mean just sort the
> commits so that media: rc: .. sits next to each other?
Yes (for example).

Regards,
Markus