media: rc: fix race between unregister and urb/irq callbacks

Some rc device drivers have a race condition between rc_unregister_device()
and irq or urb callbacks. This is because rc_unregister_device() does two
things, it marks the device as unregistered so no new commands can be
issued and then it calls rc_free_device(). This means the driver has no
chance to cancel any pending urb callbacks or interrupts after the device
has been marked as unregistered. Those callbacks may access struct rc_dev
or its members (e.g. struct ir_raw_event_ctrl), which have been freed by
rc_free_device().

This change removes the implicit call to rc_free_device() from
rc_unregister_device(). This means that device drivers can call
rc_unregister_device() in their remove or disconnect function, then cancel
all the urbs and interrupts before explicitly calling rc_free_device().

Note this is an alternative fix for an issue found by Haotian Zhang, see
the Closes: tags.

Reported-by: Haotian Zhang <vulab@iscas.ac.cn>
Closes: https://lore.kernel.org/linux-media/20251114101432.2566-1-vulab@iscas.ac.cn/
Closes: https://lore.kernel.org/linux-media/20251114101418.2548-1-vulab@iscas.ac.cn/
Closes: https://lore.kernel.org/linux-media/20251114101346.2530-1-vulab@iscas.ac.cn/
Closes: https://lore.kernel.org/linux-media/20251114090605.2413-1-vulab@iscas.ac.cn/
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>

drivers/gpu/drm/bridge/sil-sii8620.c

@@ -2221,6 +2221,7 @@ static void sii8620_detach(struct drm_bridge *bridge)
 		return;
 
 	rc_unregister_device(ctx->rc_dev);
+	rc_free_device(ctx->rc_dev);
 }
 
 static int sii8620_is_packing_required(struct sii8620 *ctx,

drivers/hid/hid-picolcd_cir.c

@@ -134,5 +134,6 @@ void picolcd_exit_cir(struct picolcd_data *data)
 
 	data->rc_dev = NULL;
 	rc_unregister_device(rdev);
+	rc_free_device(rdev);
 }
 

drivers/media/cec/core/cec-core.c

@@ -338,8 +338,8 @@ int cec_register_adapter(struct cec_adapter *adap,
 	res = cec_devnode_register(&adap->devnode, adap->owner);
 	if (res) {
 #ifdef CONFIG_MEDIA_CEC_RC
-		/* Note: rc_unregister also calls rc_free */
 		rc_unregister_device(adap->rc);
+		rc_free_device(adap->rc);
 		adap->rc = NULL;
 #endif
 		return res;

drivers/media/common/siano/smsir.c

@@ -92,6 +92,7 @@ int sms_ir_init(struct smscore_device_t *coredev)
 void sms_ir_exit(struct smscore_device_t *coredev)
 {
 	rc_unregister_device(coredev->ir.dev);
+	rc_free_device(coredev->ir.dev);
 
 	pr_debug("\n");
 }

drivers/media/i2c/ir-kbd-i2c.c

@@ -355,6 +355,7 @@ static void ir_work(struct work_struct *work)
 		mutex_unlock(&ir->lock);
 		if (rc == -ENODEV) {
 			rc_unregister_device(ir->rc);
+			rc_free_device(ir->rc);
 			ir->rc = NULL;
 			return;
 		}
@@ -972,6 +973,7 @@ static void ir_remove(struct i2c_client *client)
 	i2c_unregister_device(ir->tx_c);
 
 	rc_unregister_device(ir->rc);
+	rc_free_device(ir->rc);
 }
 
 static const struct i2c_device_id ir_kbd_id[] = {

drivers/media/pci/bt8xx/bttv-input.c

@@ -572,8 +572,9 @@ void bttv_input_fini(struct bttv *btv)
 	if (btv->remote == NULL)
 		return;
 
-	bttv_ir_stop(btv);
 	rc_unregister_device(btv->remote->dev);
+	bttv_ir_stop(btv);
+	rc_free_device(btv->remote->dev);
 	kfree(btv->remote);
 	btv->remote = NULL;
 }

drivers/media/pci/cx23885/cx23885-input.c

@@ -402,6 +402,7 @@ void cx23885_input_fini(struct cx23885_dev *dev)
 	if (dev->kernel_ir == NULL)
 		return;
 	rc_unregister_device(dev->kernel_ir->rc);
+	rc_free_device(dev->kernel_ir->rc);
 	kfree(dev->kernel_ir->phys);
 	kfree(dev->kernel_ir->name);
 	kfree(dev->kernel_ir);

drivers/media/pci/cx88/cx88-input.c

@@ -509,8 +509,9 @@ int cx88_ir_fini(struct cx88_core *core)
 	if (!ir)
 		return 0;
 
-	cx88_ir_stop(core);
 	rc_unregister_device(ir->dev);
+	cx88_ir_stop(core);
+	rc_free_device(ir->dev);
 	kfree(ir);
 
 	/* done */

drivers/media/pci/dm1105/dm1105.c

@@ -763,6 +763,7 @@ static int dm1105_ir_init(struct dm1105_dev *dm1105)
 static void dm1105_ir_exit(struct dm1105_dev *dm1105)
 {
 	rc_unregister_device(dm1105->ir.dev);
+	rc_free_device(dm1105->ir.dev);
 }
 
 static int dm1105_hw_init(struct dm1105_dev *dev)

drivers/media/pci/mantis/mantis_input.c

@@ -72,5 +72,6 @@ EXPORT_SYMBOL_GPL(mantis_input_init);
 void mantis_input_exit(struct mantis_pci *mantis)
 {
 	rc_unregister_device(mantis->rc);
+	rc_free_device(mantis->rc);
 }
 EXPORT_SYMBOL_GPL(mantis_input_exit);

drivers/media/pci/saa7134/saa7134-input.c

@@ -834,6 +834,7 @@ void saa7134_input_fini(struct saa7134_dev *dev)
 		return;
 
 	rc_unregister_device(dev->remote->dev);
+	rc_free_device(dev->remote->dev);
 	kfree(dev->remote);
 	dev->remote = NULL;
 }

drivers/media/pci/smipcie/smipcie-ir.c

@@ -181,5 +181,6 @@ void smi_ir_exit(struct smi_dev *dev)
 
 	rc_unregister_device(rc_dev);
 	smi_ir_stop(ir);
+	rc_free_device(rc_dev);
 	ir->rc_dev = NULL;
 }

drivers/media/pci/ttpci/budget-ci.c

@@ -249,6 +249,7 @@ static void msp430_ir_deinit(struct budget_ci *budget_ci)
 	cancel_work_sync(&budget_ci->ir.msp430_irq_bh_work);
 
 	rc_unregister_device(budget_ci->ir.dev);
+	rc_free_device(budget_ci->ir.dev);
 }
 
 static int ciintf_read_attribute_mem(struct dvb_ca_en50221 *ca, int slot, int address)

drivers/media/rc/ati_remote.c

@@ -921,7 +921,6 @@ static int ati_remote_probe(struct usb_interface *interface,
 	input_free_device(input_dev);
  exit_unregister_device:
 	rc_unregister_device(rc_dev);
-	rc_dev = NULL;
  exit_kill_urbs:
 	usb_kill_urb(ati_remote->irq_urb);
 	usb_kill_urb(ati_remote->out_urb);
@@ -941,18 +940,19 @@ static void ati_remote_disconnect(struct usb_interface *interface)
 	struct ati_remote *ati_remote;
 
 	ati_remote = usb_get_intfdata(interface);
-	usb_set_intfdata(interface, NULL);
 	if (!ati_remote) {
 		dev_warn(&interface->dev, "%s - null device?\n", __func__);
 		return;
 	}
 
+	rc_unregister_device(ati_remote->rdev);
+	usb_set_intfdata(interface, NULL);
 	usb_kill_urb(ati_remote->irq_urb);
 	usb_kill_urb(ati_remote->out_urb);
 	if (ati_remote->idev)
 		input_unregister_device(ati_remote->idev);
-	rc_unregister_device(ati_remote->rdev);
 	ati_remote_free_buffers(ati_remote);
+	rc_free_device(ati_remote->rdev);
 	kfree(ati_remote);
 }
 

drivers/media/rc/ene_ir.c

@@ -1090,7 +1090,6 @@ exit_release_hw_io:
 	release_region(dev->hw_io, ENE_IO_SIZE);
 exit_unregister_device:
 	rc_unregister_device(rdev);
-	rdev = NULL;
 exit_free_dev_rdev:
 	rc_free_device(rdev);
 	kfree(dev);
@@ -1110,6 +1109,7 @@ static void ene_remove(struct pnp_dev *pnp_dev)
 	ene_rx_restore_hw_buffer(dev);
 	spin_unlock_irqrestore(&dev->hw_lock, flags);
 
+	rc_free_device(dev->rdev);
 	free_irq(dev->irq, dev);
 	release_region(dev->hw_io, ENE_IO_SIZE);
 	kfree(dev);

drivers/media/rc/fintek-cir.c

@@ -568,6 +568,7 @@ static void fintek_remove(struct pnp_dev *pdev)
 	struct fintek_dev *fintek = pnp_get_drvdata(pdev);
 	unsigned long flags;
 
+	rc_unregister_device(fintek->rdev);
 	spin_lock_irqsave(&fintek->fintek_lock, flags);
 	/* disable CIR */
 	fintek_disable_cir(fintek);
@@ -580,7 +581,7 @@ static void fintek_remove(struct pnp_dev *pdev)
 	free_irq(fintek->cir_irq, fintek);
 	release_region(fintek->cir_addr, fintek->cir_port_len);
 
-	rc_unregister_device(fintek->rdev);
+	rc_free_device(fintek->rdev);
 
 	kfree(fintek);
 }

drivers/media/rc/igorplugusb.c

@@ -242,6 +242,7 @@ static void igorplugusb_disconnect(struct usb_interface *intf)
 	usb_set_intfdata(intf, NULL);
 	usb_unpoison_urb(ir->urb);
 	usb_free_urb(ir->urb);
+	rc_free_device(ir->rc);
 	kfree(ir->buf_in);
 }
 

drivers/media/rc/iguanair.c

@@ -500,6 +500,7 @@ static void iguanair_disconnect(struct usb_interface *intf)
 	usb_set_intfdata(intf, NULL);
 	usb_kill_urb(ir->urb_in);
 	usb_kill_urb(ir->urb_out);
+	rc_free_device(ir->rc);
 	usb_free_urb(ir->urb_in);
 	usb_free_urb(ir->urb_out);
 	usb_free_coherent(ir->udev, MAX_IN_PACKET, ir->buf_in, ir->dma_in);

drivers/media/rc/img-ir/img-ir-hw.c

@@ -1118,9 +1118,10 @@ void img_ir_remove_hw(struct img_ir_priv *priv)
 	struct rc_dev *rdev = hw->rdev;
 	if (!rdev)
 		return;
+	rc_unregister_device(rdev);
 	img_ir_set_decoder(priv, NULL, 0);
 	hw->rdev = NULL;
-	rc_unregister_device(rdev);
+	rc_free_device(rdev);
 #ifdef CONFIG_COMMON_CLK
 	if (!IS_ERR(priv->clk))
 		clk_notifier_unregister(priv->clk, &hw->clk_nb);

drivers/media/rc/img-ir/img-ir-raw.c

@@ -136,6 +136,7 @@ void img_ir_remove_raw(struct img_ir_priv *priv)
 	if (!rdev)
 		return;
 
+	rc_unregister_device(rdev);
 	/* switch off and disable raw (edge) interrupts */
 	spin_lock_irq(&priv->lock);
 	raw->rdev = NULL;
@@ -145,7 +146,7 @@ void img_ir_remove_raw(struct img_ir_priv *priv)
 	img_ir_write(priv, IMG_IR_IRQ_CLEAR, IMG_IR_IRQ_EDGE);
 	spin_unlock_irq(&priv->lock);
 
-	rc_unregister_device(rdev);
+	rc_free_device(rdev);
 
 	timer_delete_sync(&raw->timer);
 }

drivers/media/rc/imon.c

@@ -2541,9 +2541,10 @@ static void imon_disconnect(struct usb_interface *interface)
 
 	if (ifnum == 0) {
 		ictx->dev_present_intf0 = false;
+		rc_unregister_device(ictx->rdev);
 		usb_kill_urb(ictx->rx_urb_intf0);
 		input_unregister_device(ictx->idev);
-		rc_unregister_device(ictx->rdev);
+		rc_free_device(ictx->rdev);
 		if (ictx->display_supported) {
 			if (ictx->display_type == IMON_DISPLAY_TYPE_LCD)
 				usb_deregister_dev(interface, &imon_lcd_class);

drivers/media/rc/ir-hix5hd2.c

@@ -331,7 +331,6 @@ static int hix5hd2_ir_probe(struct platform_device *pdev)
 
 regerr:
 	rc_unregister_device(rdev);
-	rdev = NULL;
 clkerr:
 	clk_disable_unprepare(priv->clock);
 err:
@@ -346,6 +345,7 @@ static void hix5hd2_ir_remove(struct platform_device *pdev)
 
 	clk_disable_unprepare(priv->clock);
 	rc_unregister_device(priv->rdev);
+	rc_free_device(priv->rdev);
 }
 
 #ifdef CONFIG_PM_SLEEP

drivers/media/rc/ir_toy.c

@@ -536,6 +536,7 @@ static void irtoy_disconnect(struct usb_interface *intf)
 	usb_free_urb(ir->urb_out);
 	usb_kill_urb(ir->urb_in);
 	usb_free_urb(ir->urb_in);
+	rc_free_device(ir->rc);
 	kfree(ir->in);
 	kfree(ir->out);
 	kfree(ir);

drivers/media/rc/ite-cir.c

@@ -1414,7 +1414,6 @@ exit_release_cir_addr:
 	release_region(itdev->cir_addr, itdev->params->io_region_size);
 exit_unregister_device:
 	rc_unregister_device(rdev);
-	rdev = NULL;
 exit_free_dev_rdev:
 	rc_free_device(rdev);
 	kfree(itdev);
@@ -1439,6 +1438,7 @@ static void ite_remove(struct pnp_dev *pdev)
 	release_region(dev->cir_addr, dev->params->io_region_size);
 
 	rc_unregister_device(dev->rdev);
+	rc_free_device(dev->rdev);
 
 	kfree(dev);
 }

drivers/media/rc/mceusb.c

@@ -1850,6 +1850,7 @@ static void mceusb_dev_disconnect(struct usb_interface *intf)
 	usb_free_urb(ir->urb_in);
 	usb_free_coherent(dev, ir->len_in, ir->buf_in, ir->dma_in);
 	usb_put_dev(dev);
+	rc_free_device(ir->rc);
 
 	kfree(ir);
 }

drivers/media/rc/rc-ir-raw.c

@@ -648,9 +648,6 @@ int ir_raw_event_register(struct rc_dev *dev)
 
 void ir_raw_event_free(struct rc_dev *dev)
 {
-	if (!dev)
-		return;
-
 	kfree(dev->raw);
 	dev->raw = NULL;
 }
@@ -674,8 +671,6 @@ void ir_raw_event_unregister(struct rc_dev *dev)
 
 	lirc_bpf_free(dev);
 
-	ir_raw_event_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

drivers/media/rc/rc-loopback.c

@@ -263,6 +263,7 @@ static int __init loop_init(void)
 static void __exit loop_exit(void)
 {
 	rc_unregister_device(loopdev.dev);
+	rc_free_device(loopdev.dev);
 }
 
 module_init(loop_init);

drivers/media/rc/rc-main.c

@@ -1611,6 +1611,7 @@ static void rc_dev_release(struct device *device)
 {
 	struct rc_dev *dev = to_rc_dev(device);
 
+	ir_raw_event_free(dev);
 	kfree(dev);
 }
 
@@ -1773,7 +1774,6 @@ struct rc_dev *devm_rc_allocate_device(struct device *dev,
 	}
 
 	rc->dev.parent = dev;
-	rc->managed_alloc = true;
 	*dr = rc;
 	devres_add(dev, dr);
 
@@ -2042,11 +2042,7 @@ void rc_unregister_device(struct rc_dev *dev)
 	device_del(&dev->dev);
 
 	ida_free(&rc_ida, dev->minor);
-
-	if (!dev->managed_alloc)
-		rc_free_device(dev);
 }
-
 EXPORT_SYMBOL_GPL(rc_unregister_device);
 
 /*

drivers/media/rc/redrat3.c

@@ -1131,11 +1131,13 @@ static void redrat3_dev_disconnect(struct usb_interface *intf)
 {
 	struct usb_device *udev = interface_to_usbdev(intf);
 	struct redrat3_dev *rr3 = usb_get_intfdata(intf);
+	struct rc_dev *rc = rr3->rc;
 
 	usb_set_intfdata(intf, NULL);
-	rc_unregister_device(rr3->rc);
+	rc_unregister_device(rc);
 	led_classdev_unregister(&rr3->led);
 	redrat3_delete(rr3, udev);
+	rc_free_device(rc);
 }
 
 static int redrat3_dev_suspend(struct usb_interface *intf, pm_message_t message)

drivers/media/rc/st_rc.c

@@ -203,6 +203,7 @@ static void st_rc_remove(struct platform_device *pdev)
 	device_init_wakeup(&pdev->dev, false);
 	clk_disable_unprepare(rc_dev->sys_clock);
 	rc_unregister_device(rc_dev->rdev);
+	rc_free_device(rc_dev->rdev);
 }
 
 static int st_rc_open(struct rc_dev *rdev)
@@ -334,7 +335,6 @@ static int st_rc_probe(struct platform_device *pdev)
 	return ret;
 rcerr:
 	rc_unregister_device(rdev);
-	rdev = NULL;
 clkerr:
 	clk_disable_unprepare(rc_dev->sys_clock);
 err:

drivers/media/rc/streamzap.c

@@ -388,15 +388,16 @@ static void streamzap_disconnect(struct usb_interface *interface)
 	struct streamzap_ir *sz = usb_get_intfdata(interface);
 	struct usb_device *usbdev = interface_to_usbdev(interface);
 
-	usb_set_intfdata(interface, NULL);
-
 	if (!sz)
 		return;
 
-	usb_kill_urb(sz->urb_in);
 	rc_unregister_device(sz->rdev);
+	usb_set_intfdata(interface, NULL);
+
+	usb_kill_urb(sz->urb_in);
 	usb_free_urb(sz->urb_in);
 	usb_free_coherent(usbdev, sz->buf_in_len, sz->buf_in, sz->dma_in);
+	rc_free_device(sz->rdev);
 
 	kfree(sz);
 }

drivers/media/rc/sunxi-cir.c

@@ -371,6 +371,7 @@ 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);
 }
 

drivers/media/rc/ttusbir.c

@@ -333,7 +333,6 @@ static int ttusbir_probe(struct usb_interface *intf,
 	return 0;
 out3:
 	rc_unregister_device(rc);
-	rc = NULL;
 out2:
 	led_classdev_unregister(&tt->led);
 out:
@@ -373,6 +372,7 @@ static void ttusbir_disconnect(struct usb_interface *intf)
 	}
 	usb_kill_urb(tt->bulk_urb);
 	usb_free_urb(tt->bulk_urb);
+	rc_free_device(tt->rc);
 	usb_set_intfdata(intf, NULL);
 	kfree(tt);
 }

drivers/media/rc/winbond-cir.c

@@ -1132,7 +1132,6 @@ exit_release_wbase:
 	release_region(data->wbase, WAKEUP_IOMEM_LEN);
 exit_unregister_device:
 	rc_unregister_device(data->dev);
-	data->dev = NULL;
 exit_free_rc:
 	rc_free_device(data->dev);
 exit_unregister_led:
@@ -1163,6 +1162,7 @@ wbcir_remove(struct pnp_dev *device)
 	wbcir_set_bits(data->wbase + WBCIR_REG_WCEIR_EV_EN, 0x00, 0x07);
 
 	rc_unregister_device(data->dev);
+	rc_free_device(data->dev);
 
 	led_classdev_unregister(&data->led);
 

drivers/media/rc/xbox_remote.c

@@ -277,14 +277,15 @@ static void xbox_remote_disconnect(struct usb_interface *interface)
 	struct xbox_remote *xbox_remote;
 
 	xbox_remote = usb_get_intfdata(interface);
-	usb_set_intfdata(interface, NULL);
 	if (!xbox_remote) {
 		dev_warn(&interface->dev, "%s - null device?\n", __func__);
 		return;
 	}
 
-	usb_kill_urb(xbox_remote->irq_urb);
 	rc_unregister_device(xbox_remote->rdev);
+	usb_set_intfdata(interface, NULL);
+	usb_kill_urb(xbox_remote->irq_urb);
+	rc_free_device(xbox_remote->rdev);
 	usb_free_urb(xbox_remote->irq_urb);
 	kfree(xbox_remote);
 }

drivers/media/usb/au0828/au0828-input.c

@@ -358,6 +358,7 @@ void au0828_rc_unregister(struct au0828_dev *dev)
 		return;
 
 	rc_unregister_device(ir->rc);
+	rc_free_device(ir->rc);
 
 	/* done */
 	kfree(ir);

drivers/media/usb/dvb-usb-v2/dvb_usb_core.c

@@ -187,6 +187,7 @@ static int dvb_usbv2_remote_exit(struct dvb_usb_device *d)
 	if (d->rc_dev) {
 		cancel_delayed_work_sync(&d->rc_query_work);
 		rc_unregister_device(d->rc_dev);
+		rc_free_device(d->rc_dev);
 		d->rc_dev = NULL;
 	}
 

drivers/media/usb/dvb-usb/dvb-usb-remote.c

@@ -347,10 +347,12 @@ int dvb_usb_remote_exit(struct dvb_usb_device *d)
 {
 	if (d->state & DVB_USB_STATE_REMOTE) {
 		cancel_delayed_work_sync(&d->rc_query_work);
-		if (d->props.rc.mode == DVB_RC_LEGACY)
+		if (d->props.rc.mode == DVB_RC_LEGACY) {
 			input_unregister_device(d->input_dev);
-		else
+		} else {
 			rc_unregister_device(d->rc_dev);
+			rc_free_device(d->rc_dev);
+		}
 	}
 	d->state &= ~DVB_USB_STATE_REMOTE;
 	return 0;

drivers/media/usb/em28xx/em28xx-input.c

@@ -853,6 +853,7 @@ static int em28xx_ir_fini(struct em28xx *dev)
 		goto ref_put;
 
 	rc_unregister_device(ir->rc);
+	rc_free_device(ir->rc);
 
 	kfree(ir->i2c_client);
 

drivers/staging/media/av7110/av7110_ir.c

@@ -151,6 +151,7 @@ int av7110_ir_init(struct av7110 *av7110)
 void av7110_ir_exit(struct av7110 *av7110)
 {
 	rc_unregister_device(av7110->ir.rcdev);
+	rc_free_device(av7110->ir.rcdev);
 }
 
 //MODULE_AUTHOR("Holger Waechtler <holger@convergence.de>, Oliver Endriss <o.endriss@gmx.de>");

include/media/rc-core.h

@@ -81,7 +81,6 @@ struct lirc_fh {
 /**
  * struct rc_dev - represents a remote control device
  * @dev: driver model's view of this device
- * @managed_alloc: devm_rc_allocate_device was used to create rc_dev
  * @registered: set to true by rc_register_device(), false by
  *	rc_unregister_device
  * @idle: used to keep track of RX state
@@ -156,7 +155,6 @@ struct lirc_fh {
  */
 struct rc_dev {
 	struct device			dev;
-	bool				managed_alloc;
 	bool				registered;
 	bool				idle;
 	bool				encode_wakeup;