mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe
@ 2026-08-27 17:44 Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 1/6] media: cx231xx: keep device state alive until final release Nick Faro via B4 Relay
                   ` (6 more replies)
  0 siblings, 7 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

The cx231xx disconnect path can tear down the USB-owned device state while
open V4L2 or ALSA file handles still reference it. It can also leave nodes
registered when an analog video file is open, and completion handlers can
continue resubmitting URBs while disconnect is trying to kill them.

I encountered this with an Elgato Video Capture V2 (USB 0fd9:0037) while
streaming across a suspend-to-idle/resume cycle. On mainline 7.2.0, resume
re-probed the USB device while the previous cx231xx instance remained partly
registered. The reprobe warned about duplicate I2C-mux sysfs entries. Running
v4l2-ctl --list-devices then logged "refcount_t: saturated; leaking memory"
and triggered a general protection fault in v4l2_open().

The reproducer is:

  1. Start an indefinite capture, for example:

       v4l2-ctl -d /dev/video4 --stream-mmap=3 --stream-poll

  2. Suspend the machine to s2idle while the capture remains open.
  3. Resume, stop the capture process, and inspect the devices with:

       v4l2-ctl --list-devices

  4. Unplug and reconnect the capture device.

The series gives the shared device state the lifetime of its V4L2 and ALSA
users rather than the USB interface. Disconnect now unregisters all public
nodes, stops URB resubmission, and prevents late file operations from issuing
commands to absent hardware. Software-only close and buffer cleanup remain
available so existing file handles can release normally.

The final patch is an independent allocation-size correction found while
reviewing the endpoint-array ownership changed by the first patch.

The patches are:

  1. Keep shared device state alive until its final V4L2 reference.
  2. Stop video and VBI completion handlers resubmitting URBs after
     disconnect.
  3. Reject late V4L2 hardware operations while preserving software cleanup.
  4. Make ALSA work, callbacks, close, and card teardown disconnect-safe.
  5. Always unregister the driver's public nodes during USB disconnect.
  6. Allocate alternate-setting arrays by element count and element size.

Testing performed so far:

  - Built the cx231xx modules at every commit with W=1 against Ubuntu's
    7.2.0-070200-generic headers using GCC 14.
  - Loaded the patched modules with an Elgato Video Capture V2. The video,
    VBI, and media nodes registered, and ordinary video capture worked.
  - git diff --check passes.
  - scripts/checkpatch.pl --strict --max-line-length=80 reports no errors,
    warnings, or checks for any of the six commits.

  - v4l2-compliance 1.26.1 results with a valid NTSC signal:

      v4l2-compliance -d /dev/video4 -s 120
      Total: 76, Succeeded: 76, Failed: 0, Warnings: 28

      v4l2-compliance -V /dev/vbi0 -s 120
      Total: 76, Succeeded: 76, Failed: 0, Warnings: 18

      v4l2-compliance -m /dev/media2
      Grand Total: 148, Succeeded: 148, Failed: 0, Warnings: 42

    The warnings concern the existing audio-control ranges and values not
    aligning to their reported step, unsupported VIDIOC_CREATE_BUFS, and the
    video node accepting invalid pixel formats in TRY_FMT and S_FMT.

  - Repeated the active-capture suspend-to-idle/resume cycle once with a
    userspace V4L2 capture application holding /dev/video4 open, and did 3
    cycles of unplugging the USB while streaming. On resume, the old video and
    VBI nodes were unregistered, the Elgato re-probed and registered one new set
    of nodes, and capture worked after reopening. v4l2-ctl --list-devices
    returned promptly. The kernel log contained no cx231xx warning, duplicate
    I2C-mux entry, refcount error, or oops.

Codex using the gpt-5.6-sol model assisted with all six patches. It was used to
inspect the cx231xx driver and comparable media drivers, propose and revise the
lifetime and teardown changes, split the result into single-purpose commits,
draft changelogs, and assist with style and build validation. The interaction
was an iterative review driven by hunk-by-hunk review, rather than a single
generation prompt.

The submitter reviewed the resulting code commit by commit and is responsible
for the final submission.

---
Nick Faro (6):
      media: cx231xx: keep device state alive until final release
      media: cx231xx: stop resubmitting URBs after disconnect
      media: cx231xx: reject V4L2 operations after disconnect
      media: cx231xx: make ALSA teardown disconnect-safe
      media: cx231xx: always unregister nodes on USB disconnect
      media: cx231xx: size alternate-setting arrays by element count

 drivers/media/usb/cx231xx/cx231xx-417.c   |  44 +++++++++---
 drivers/media/usb/cx231xx/cx231xx-audio.c |  67 +++++++++++++++----
 drivers/media/usb/cx231xx/cx231xx-cards.c | 107 ++++++++++++++++++++++--------
 drivers/media/usb/cx231xx/cx231xx-core.c  |  16 ++++-
 drivers/media/usb/cx231xx/cx231xx-vbi.c   |  11 +++
 drivers/media/usb/cx231xx/cx231xx-video.c |  30 ++++++---
 drivers/media/usb/cx231xx/cx231xx.h       |   1 +
 7 files changed, 214 insertions(+), 62 deletions(-)
---
base-commit: 4900cad020c0580dfb1be27776ff10a4ef110cfa
change-id: 20260827-cx231xx-disconnect-lifetime-772159fee2c2

Best regards,
-- 
Nick Faro <yux50000@hotmail.com>



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/6] media: cx231xx: keep device state alive until final release
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
@ 2026-08-27 17:44 ` Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 2/6] media: cx231xx: stop resubmitting URBs after disconnect Nick Faro via B4 Relay
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

From: Nick Faro <yux50000@hotmail.com>

The USB interface can be unbound while userspace still has a V4L2 or
ALSA device node open. Since struct cx231xx embeds the video devices and
is referenced by ALSA callbacks, interface-managed allocations have a
shorter lifetime than their users.

Allocate the shared device state and endpoint arrays explicitly. Use the
v4l2_device release callback as the single point that unregisters the
V4L2 device and frees its backing storage. Registered video nodes keep
their normal V4L2 references, while the ALSA card takes an additional
reference and is registered only after its endpoint data is ready.

Disconnect public interfaces during teardown, but defer control-handler
and media-device cleanup until the final reference is dropped. Update
probe unwinding to free directly before V4L2 registration and otherwise
drop the initial V4L2 reference.

Fixes: 184a82784d50 ("[media] cx231xx: use devm_ functions to allocate memory")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nick Faro <yux50000@hotmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-417.c   |  1 -
 drivers/media/usb/cx231xx/cx231xx-audio.c | 32 +++++++----
 drivers/media/usb/cx231xx/cx231xx-cards.c | 88 +++++++++++++++++++++++--------
 drivers/media/usb/cx231xx/cx231xx-video.c |  2 -
 4 files changed, 88 insertions(+), 35 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c
index c695a97e202b..2323584bf818 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1650,7 +1650,6 @@ void cx231xx_417_unregister(struct cx231xx *dev)
 
 	if (video_is_registered(&dev->v4l_device)) {
 		video_unregister_device(&dev->v4l_device);
-		v4l2_ctrl_handler_free(&dev->mpeg_ctrl_handler.hdl);
 	}
 }
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c
index b24ceef497e4..dd819a546af9 100644
--- a/drivers/media/usb/cx231xx/cx231xx-audio.c
+++ b/drivers/media/usb/cx231xx/cx231xx-audio.c
@@ -561,6 +561,15 @@ static const struct snd_pcm_ops snd_cx231xx_pcm_capture = {
 	.pointer = snd_cx231xx_capture_pointer,
 };
 
+static void cx231xx_audio_card_free(struct snd_card *card)
+{
+	struct cx231xx *dev = card->private_data;
+
+	kfree(dev->adev.alt_max_pkt_size);
+	dev->adev.alt_max_pkt_size = NULL;
+	v4l2_device_put(&dev->v4l2_dev);
+}
+
 static int cx231xx_audio_init(struct cx231xx *dev)
 {
 	struct cx231xx_audio *adev = &dev->adev;
@@ -585,6 +594,9 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 			   THIS_MODULE, 0, &card);
 	if (err < 0)
 		return err;
+	v4l2_device_get(&dev->v4l2_dev);
+	card->private_data = dev;
+	card->private_free = cx231xx_audio_card_free;
 
 	spin_lock_init(&adev->slock);
 	err = snd_pcm_new(card, "Cx231xx Audio", 0, 0, 1, &pcm);
@@ -603,11 +615,6 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 
 	INIT_WORK(&dev->wq_trigger, audio_trigger);
 
-	err = snd_card_register(card);
-	if (err < 0)
-		goto err_free_card;
-
-	adev->sndcard = card;
 	adev->udev = dev->udev;
 
 	/* compute alternate max packet sizes for Audio */
@@ -640,7 +647,7 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 
 		if (uif->altsetting[i].desc.bNumEndpoints < isoc_pipe + 1) {
 			err = -ENODEV;
-			goto err_free_pkt_size;
+			goto err_free_card;
 		}
 
 		tmp = le16_to_cpu(uif->altsetting[i].endpoint[isoc_pipe].desc.
@@ -652,10 +659,13 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 			adev->alt_max_pkt_size[i]);
 	}
 
+	err = snd_card_register(card);
+	if (err < 0)
+		goto err_free_card;
+
+	adev->sndcard = card;
 	return 0;
 
-err_free_pkt_size:
-	kfree(adev->alt_max_pkt_size);
 err_free_card:
 	snd_card_free(card);
 
@@ -664,6 +674,8 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 
 static int cx231xx_audio_fini(struct cx231xx *dev)
 {
+	struct snd_card *card;
+
 	if (dev == NULL)
 		return 0;
 
@@ -675,9 +687,9 @@ static int cx231xx_audio_fini(struct cx231xx *dev)
 	}
 
 	if (dev->adev.sndcard) {
-		snd_card_free_when_closed(dev->adev.sndcard);
-		kfree(dev->adev.alt_max_pkt_size);
+		card = dev->adev.sndcard;
 		dev->adev.sndcard = NULL;
+		snd_card_free_when_closed(card);
 	}
 
 	return 0;
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 69b24205bc56..1b1e7bca4c6d 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1339,8 +1339,15 @@ void cx231xx_config_i2c(struct cx231xx *dev)
 static void cx231xx_unregister_media_device(struct cx231xx *dev)
 {
 #ifdef CONFIG_MEDIA_CONTROLLER
-	if (dev->media_dev) {
+	if (dev->media_dev)
 		media_device_unregister(dev->media_dev);
+#endif
+}
+
+static void cx231xx_cleanup_media_device(struct cx231xx *dev)
+{
+#ifdef CONFIG_MEDIA_CONTROLLER
+	if (dev->media_dev) {
 		media_device_cleanup(dev->media_dev);
 		kfree(dev->media_dev);
 		dev->media_dev = NULL;
@@ -1348,6 +1355,27 @@ static void cx231xx_unregister_media_device(struct cx231xx *dev)
 #endif
 }
 
+static void cx231xx_free_device(struct cx231xx *dev)
+{
+	kfree(dev->video_mode.alt_max_pkt_size);
+	kfree(dev->vbi_mode.alt_max_pkt_size);
+	kfree(dev->sliced_cc_mode.alt_max_pkt_size);
+	kfree(dev->ts1_mode.alt_max_pkt_size);
+	kfree(dev);
+}
+
+static void cx231xx_v4l2_release(struct v4l2_device *v4l2_dev)
+{
+	struct cx231xx *dev = container_of(v4l2_dev, struct cx231xx, v4l2_dev);
+
+	v4l2_ctrl_handler_free(&dev->mpeg_ctrl_handler.hdl);
+	v4l2_ctrl_handler_free(&dev->radio_ctrl_handler);
+	v4l2_ctrl_handler_free(&dev->ctrl_handler);
+	v4l2_device_unregister(v4l2_dev);
+	cx231xx_cleanup_media_device(dev);
+	cx231xx_free_device(dev);
+}
+
 /*
  * cx231xx_realease_resources()
  * unregisters the v4l2,i2c and usb devices
@@ -1359,18 +1387,23 @@ void cx231xx_release_resources(struct cx231xx *dev)
 
 	cx231xx_release_analog_resources(dev);
 
+	/* Wait for file operations that started before node removal. */
+	mutex_lock(&dev->lock);
+	v4l2_device_disconnect(&dev->v4l2_dev);
+	mutex_unlock(&dev->lock);
+
 	cx231xx_remove_from_devlist(dev);
 
 	/* Release I2C buses */
 	cx231xx_dev_uninit(dev);
 
-	/* delete v4l2 device */
-	v4l2_device_unregister(&dev->v4l2_dev);
-
 	cx231xx_unregister_media_device(dev);
 
-	/* Mark device as unused */
+	/* Mark the board slot unused before the final put can free dev. */
 	clear_bit(dev->devno, &cx231xx_devused);
+
+	/* Drop the initial reference after all nodes are unregistered. */
+	v4l2_device_put(&dev->v4l2_dev);
 }
 
 static int cx231xx_media_device_init(struct cx231xx *dev,
@@ -1544,7 +1577,6 @@ static void flush_request_modules(struct cx231xx *dev)
 
 static int cx231xx_init_v4l2(struct cx231xx *dev,
 			     struct usb_device *udev,
-			     struct usb_interface *interface,
 			     int isoc_pipe)
 {
 	struct usb_interface *uif;
@@ -1573,8 +1605,8 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		 dev->video_mode.end_point_addr,
 		 dev->video_mode.num_alt);
 
-	dev->video_mode.alt_max_pkt_size = devm_kmalloc_array(&interface->dev, 32,
-							      dev->video_mode.num_alt, GFP_KERNEL);
+	dev->video_mode.alt_max_pkt_size =
+		kmalloc_array(32, dev->video_mode.num_alt, GFP_KERNEL);
 	if (dev->video_mode.alt_max_pkt_size == NULL)
 		return -ENOMEM;
 
@@ -1615,8 +1647,8 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		 dev->vbi_mode.num_alt);
 
 	/* compute alternate max packet sizes for vbi */
-	dev->vbi_mode.alt_max_pkt_size = devm_kmalloc_array(&interface->dev, 32,
-							    dev->vbi_mode.num_alt, GFP_KERNEL);
+	dev->vbi_mode.alt_max_pkt_size =
+		kmalloc_array(32, dev->vbi_mode.num_alt, GFP_KERNEL);
 	if (dev->vbi_mode.alt_max_pkt_size == NULL)
 		return -ENOMEM;
 
@@ -1658,9 +1690,8 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		 "sliced CC EndPoint Addr 0x%x, Alternate settings: %i\n",
 		 dev->sliced_cc_mode.end_point_addr,
 		 dev->sliced_cc_mode.num_alt);
-	dev->sliced_cc_mode.alt_max_pkt_size = devm_kmalloc_array(&interface->dev, 32,
-								  dev->sliced_cc_mode.num_alt,
-								  GFP_KERNEL);
+	dev->sliced_cc_mode.alt_max_pkt_size =
+		kmalloc_array(32, dev->sliced_cc_mode.num_alt, GFP_KERNEL);
 	if (dev->sliced_cc_mode.alt_max_pkt_size == NULL)
 		return -ENOMEM;
 
@@ -1724,7 +1755,7 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	udev = interface_to_usbdev(interface);
 
 	/* allocate memory for our device state and initialize it */
-	dev = devm_kzalloc(&interface->dev, sizeof(*dev), GFP_KERNEL);
+	dev = kzalloc_obj(*dev);
 	if (dev == NULL) {
 		retval = -ENOMEM;
 		goto err_if;
@@ -1818,15 +1849,16 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 		dev_err(d, "v4l2_device_register failed\n");
 		goto err_v4l2;
 	}
+	dev->v4l2_dev.release = cx231xx_v4l2_release;
 
 	/* allocate device struct */
 	retval = cx231xx_init_dev(dev, udev, nr);
 	if (retval)
 		goto err_init;
 
-	retval = cx231xx_init_v4l2(dev, udev, interface, isoc_pipe);
+	retval = cx231xx_init_v4l2(dev, udev, isoc_pipe);
 	if (retval)
-		goto err_init;
+		goto err_video_alt;
 
 	if (dev->current_pcb_config.ts1_source != 0xff) {
 		/* compute alternate max packet sizes for TS1 */
@@ -1854,9 +1886,8 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 			 dev->ts1_mode.end_point_addr,
 			 dev->ts1_mode.num_alt);
 
-		dev->ts1_mode.alt_max_pkt_size = devm_kmalloc_array(&interface->dev, 32,
-								    dev->ts1_mode.num_alt,
-								    GFP_KERNEL);
+		dev->ts1_mode.alt_max_pkt_size =
+			kmalloc_array(32, dev->ts1_mode.num_alt, GFP_KERNEL);
 		if (dev->ts1_mode.alt_max_pkt_size == NULL) {
 			retval = -ENOMEM;
 			goto err_video_alt;
@@ -1900,12 +1931,18 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	if (!retval)
 		retval = media_device_register(dev->media_dev);
 #endif
-	if (retval < 0)
+	if (retval < 0) {
+		dev->state |= DEV_DISCONNECTED;
+		flush_request_modules(dev);
+		cx231xx_close_extension(dev);
+		usb_set_intfdata(interface, NULL);
 		cx231xx_release_resources(dev);
+	}
 	return retval;
 
 err_video_alt:
 	/* cx231xx_uninit_dev: */
+	dev->state |= DEV_DISCONNECTED;
 	cx231xx_close_extension(dev);
 	cx231xx_ir_exit(dev);
 	cx231xx_release_analog_resources(dev);
@@ -1913,13 +1950,20 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	cx231xx_remove_from_devlist(dev);
 	cx231xx_dev_uninit(dev);
 err_init:
-	v4l2_device_unregister(&dev->v4l2_dev);
+	dev->state |= DEV_DISCONNECTED;
+	usb_set_intfdata(interface, NULL);
+	clear_bit(nr, &cx231xx_devused);
+	v4l2_device_disconnect(&dev->v4l2_dev);
+	v4l2_device_put(&dev->v4l2_dev);
+	return retval;
 err_v4l2:
-	cx231xx_unregister_media_device(dev);
+	cx231xx_cleanup_media_device(dev);
 err_media_init:
 	usb_set_intfdata(interface, NULL);
 err_if:
 	clear_bit(nr, &cx231xx_devused);
+	if (dev)
+		cx231xx_free_device(dev);
 	return retval;
 }
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 70aa99fead27..058e8eed2847 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1567,8 +1567,6 @@ void cx231xx_release_analog_resources(struct cx231xx *dev)
 
 		video_unregister_device(&dev->vdev);
 	}
-	v4l2_ctrl_handler_free(&dev->ctrl_handler);
-	v4l2_ctrl_handler_free(&dev->radio_ctrl_handler);
 }
 
 /*

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/6] media: cx231xx: stop resubmitting URBs after disconnect
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 1/6] media: cx231xx: keep device state alive until final release Nick Faro via B4 Relay
@ 2026-08-27 17:44 ` Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 3/6] media: cx231xx: reject V4L2 operations " Nick Faro via B4 Relay
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

From: Nick Faro <yux50000@hotmail.com>

URB completion handlers normally resubmit transfers after processing. A
completion can race with disconnect after the device has been marked gone,
causing the transfer loop to continue while teardown kills and frees URBs.

Return from the video and VBI completion handlers once the device is
disconnected. Continue to kill and free existing transfers during teardown,
but skip endpoint resets and capture commands that require live hardware.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nick Faro <yux50000@hotmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-core.c | 16 ++++++++++++++--
 drivers/media/usb/cx231xx/cx231xx-vbi.c  |  6 ++++++
 2 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-core.c b/drivers/media/usb/cx231xx/cx231xx-core.c
index d8312201694f..462eb4c7564e 100644
--- a/drivers/media/usb/cx231xx/cx231xx-core.c
+++ b/drivers/media/usb/cx231xx/cx231xx-core.c
@@ -785,6 +785,9 @@ static void cx231xx_isoc_irq_callback(struct urb *urb)
 	unsigned long flags;
 	int i;
 
+	if (dev->state & DEV_DISCONNECTED)
+		return;
+
 	switch (urb->status) {
 	case 0:		/* success */
 	case -ETIMEDOUT:	/* NAK */
@@ -830,6 +833,9 @@ static void cx231xx_bulk_irq_callback(struct urb *urb)
 	struct cx231xx *dev = container_of(vmode, struct cx231xx, video_mode);
 	unsigned long flags;
 
+	if (dev->state & DEV_DISCONNECTED)
+		return;
+
 	switch (urb->status) {
 	case 0:		/* success */
 	case -ETIMEDOUT:	/* NAK */
@@ -895,7 +901,7 @@ void cx231xx_uninit_isoc(struct cx231xx *dev)
 		dev->video_mode.isoc_ctl.transfer_buffer[i] = NULL;
 	}
 
-	if (broken_pipe) {
+	if (broken_pipe && !(dev->state & DEV_DISCONNECTED)) {
 		cx231xx_isocdbg("Reset endpoint to recover broken pipe.");
 		usb_reset_endpoint(dev->udev, dev->video_mode.end_point_addr);
 	}
@@ -908,6 +914,9 @@ void cx231xx_uninit_isoc(struct cx231xx *dev)
 	dev->video_mode.isoc_ctl.num_bufs = 0;
 	dma_q->p_left_data = NULL;
 
+	if (dev->state & DEV_DISCONNECTED)
+		return;
+
 	if (dev->mode_tv == 0)
 		cx231xx_capture_start(dev, 0, Raw_Video);
 	else
@@ -954,7 +963,7 @@ void cx231xx_uninit_bulk(struct cx231xx *dev)
 		dev->video_mode.bulk_ctl.transfer_buffer[i] = NULL;
 	}
 
-	if (broken_pipe) {
+	if (broken_pipe && !(dev->state & DEV_DISCONNECTED)) {
 		cx231xx_isocdbg("Reset endpoint to recover broken pipe.");
 		usb_reset_endpoint(dev->udev, dev->video_mode.end_point_addr);
 	}
@@ -967,6 +976,9 @@ void cx231xx_uninit_bulk(struct cx231xx *dev)
 	dev->video_mode.bulk_ctl.num_bufs = 0;
 	dma_q->p_left_data = NULL;
 
+	if (dev->state & DEV_DISCONNECTED)
+		return;
+
 	if (dev->mode_tv == 0)
 		cx231xx_capture_start(dev, 0, Raw_Video);
 	else
diff --git a/drivers/media/usb/cx231xx/cx231xx-vbi.c b/drivers/media/usb/cx231xx/cx231xx-vbi.c
index 338e10148465..b51955f1dd30 100644
--- a/drivers/media/usb/cx231xx/cx231xx-vbi.c
+++ b/drivers/media/usb/cx231xx/cx231xx-vbi.c
@@ -260,6 +260,9 @@ static void cx231xx_irq_vbi_callback(struct urb *urb)
 	struct cx231xx *dev = container_of(vmode, struct cx231xx, vbi_mode);
 	unsigned long flags;
 
+	if (dev->state & DEV_DISCONNECTED)
+		return;
+
 	switch (urb->status) {
 	case 0:		/* success */
 	case -ETIMEDOUT:	/* NAK */
@@ -328,6 +331,9 @@ void cx231xx_uninit_vbi_isoc(struct cx231xx *dev)
 	dev->vbi_mode.bulk_ctl.transfer_buffer = NULL;
 	dev->vbi_mode.bulk_ctl.num_bufs = 0;
 
+	if (dev->state & DEV_DISCONNECTED)
+		return;
+
 	cx231xx_capture_start(dev, 0, Vbi);
 }
 EXPORT_SYMBOL_GPL(cx231xx_uninit_vbi_isoc);

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/6] media: cx231xx: reject V4L2 operations after disconnect
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 1/6] media: cx231xx: keep device state alive until final release Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 2/6] media: cx231xx: stop resubmitting URBs after disconnect Nick Faro via B4 Relay
@ 2026-08-27 17:44 ` Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 4/6] media: cx231xx: make ALSA teardown disconnect-safe Nick Faro via B4 Relay
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

From: Nick Faro <yux50000@hotmail.com>

An open V4L2 file can outlive USB disconnect, and VB2 can invoke its
stop-streaming callback while releasing that file. The existing callbacks
continue into subdevice and USB commands even though the hardware is gone.

Reject new analog, VBI and MPEG opens or streams after disconnect. Let
stop and close callbacks return queued buffers and release file state, but
skip operations which require access to the disconnected device.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nick Faro <yux50000@hotmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-417.c   | 41 ++++++++++++++++++++++++-------
 drivers/media/usb/cx231xx/cx231xx-vbi.c   |  5 ++++
 drivers/media/usb/cx231xx/cx231xx-video.c | 18 +++++++++++++-
 3 files changed, 54 insertions(+), 10 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c
index 2323584bf818..3a509e55f61d 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1396,6 +1396,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct cx231xx_dmaqueue *vidq = &dev->video_mode.vidq;
 	int ret = 0;
 
+	if (dev->state & DEV_DISCONNECTED) {
+		return_all_buffers(dev, VB2_BUF_STATE_QUEUED);
+		return -ENODEV;
+	}
+
 	vidq->sequence = 0;
 	dev->mode_tv = 1;
 
@@ -1429,20 +1434,22 @@ static void stop_streaming(struct vb2_queue *vq)
 	struct cx231xx *dev = vb2_get_drv_priv(vq);
 	unsigned long flags;
 
-	call_all(dev, video, s_stream, 0);
-
-	cx231xx_stop_TS1(dev);
+	if (!(dev->state & DEV_DISCONNECTED)) {
+		call_all(dev, video, s_stream, 0);
+		cx231xx_stop_TS1(dev);
+	}
 
 	/* do this before setting alternate! */
 	if (dev->USE_ISO)
 		cx231xx_uninit_isoc(dev);
 	else
 		cx231xx_uninit_bulk(dev);
-	cx231xx_set_mode(dev, CX231XX_SUSPEND);
-
-	cx231xx_api_cmd(dev, CX2341X_ENC_STOP_CAPTURE, 3, 0,
-			CX231xx_END_NOW, CX231xx_MPEG_CAPTURE,
-			CX231xx_RAW_BITS_NONE);
+	if (!(dev->state & DEV_DISCONNECTED)) {
+		cx231xx_set_mode(dev, CX231XX_SUSPEND);
+		cx231xx_api_cmd(dev, CX2341X_ENC_STOP_CAPTURE, 3, 0,
+				CX231xx_END_NOW, CX231xx_MPEG_CAPTURE,
+				CX231xx_RAW_BITS_NONE);
+	}
 
 	spin_lock_irqsave(&dev->video_mode.slock, flags);
 	if (dev->USE_ISO)
@@ -1593,9 +1600,25 @@ static int vidioc_log_status(struct file *file, void *priv)
 	return v4l2_ctrl_log_status(file, priv);
 }
 
+static int cx231xx_mpeg_open(struct file *file)
+{
+	struct cx231xx *dev = video_drvdata(file);
+	int ret;
+
+	if (mutex_lock_interruptible(&dev->lock))
+		return -ERESTARTSYS;
+	if (dev->state & DEV_DISCONNECTED)
+		ret = -ENODEV;
+	else
+		ret = v4l2_fh_open(file);
+	mutex_unlock(&dev->lock);
+
+	return ret;
+}
+
 static const struct v4l2_file_operations mpeg_fops = {
 	.owner	       = THIS_MODULE,
-	.open	       = v4l2_fh_open,
+	.open	       = cx231xx_mpeg_open,
 	.release       = vb2_fop_release,
 	.read	       = vb2_fop_read,
 	.poll          = vb2_fop_poll,
diff --git a/drivers/media/usb/cx231xx/cx231xx-vbi.c b/drivers/media/usb/cx231xx/cx231xx-vbi.c
index b51955f1dd30..6f28b969408f 100644
--- a/drivers/media/usb/cx231xx/cx231xx-vbi.c
+++ b/drivers/media/usb/cx231xx/cx231xx-vbi.c
@@ -220,6 +220,11 @@ static int vbi_start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct cx231xx_dmaqueue *vidq = &dev->vbi_mode.vidq;
 	int ret;
 
+	if (dev->state & DEV_DISCONNECTED) {
+		return_all_buffers(dev, VB2_BUF_STATE_QUEUED);
+		return -ENODEV;
+	}
+
 	vidq->sequence = 0;
 	ret = cx231xx_init_vbi_isoc(dev, CX231XX_NUM_VBI_PACKETS,
 				    CX231XX_NUM_VBI_BUFS,
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 058e8eed2847..154bb4300392 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -767,6 +767,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct cx231xx_dmaqueue *vidq = &dev->video_mode.vidq;
 	int ret = 0;
 
+	if (dev->state & DEV_DISCONNECTED) {
+		return_all_buffers(dev, VB2_BUF_STATE_QUEUED);
+		return -ENODEV;
+	}
+
 	vidq->sequence = 0;
 	dev->mode_tv = 0;
 
@@ -791,7 +796,8 @@ static void stop_streaming(struct vb2_queue *vq)
 {
 	struct cx231xx *dev = vb2_get_drv_priv(vq);
 
-	call_all(dev, video, s_stream, 0);
+	if (!(dev->state & DEV_DISCONNECTED))
+		call_all(dev, video, s_stream, 0);
 	return_all_buffers(dev, VB2_BUF_STATE_ERROR);
 }
 
@@ -1499,6 +1505,10 @@ static int cx231xx_v4l2_open(struct file *filp)
 
 	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;
+	if (dev->state & DEV_DISCONNECTED) {
+		mutex_unlock(&dev->lock);
+		return -ENODEV;
+	}
 
 	ret = v4l2_fh_open(filp);
 	if (ret) {
@@ -1581,6 +1591,12 @@ static int cx231xx_close(struct file *filp)
 
 	_vb2_fop_release(filp, NULL);
 
+	if (dev->state & DEV_DISCONNECTED) {
+		--dev->users;
+		wake_up_interruptible(&dev->open);
+		return 0;
+	}
+
 	if (--dev->users == 0) {
 		/* Save some power by putting tuner to sleep */
 		call_all(dev, tuner, standby);

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 4/6] media: cx231xx: make ALSA teardown disconnect-safe
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
                   ` (2 preceding siblings ...)
  2026-08-27 17:44 ` [PATCH 3/6] media: cx231xx: reject V4L2 operations " Nick Faro via B4 Relay
@ 2026-08-27 17:44 ` Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 5/6] media: cx231xx: always unregister nodes on USB disconnect Nick Faro via B4 Relay
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

From: Nick Faro <yux50000@hotmail.com>

The ALSA card and an open PCM file can outlive the USB interface. Work
items, PCM callbacks and close currently remain able to touch the hardware
after disconnect, and card teardown does not actively disconnect userspace.

Disconnect the ALSA card and stop its stream and transfers. Cancel its
trigger work before deferring the final card release. Make late PCM
callbacks report the disconnected state and let close release software
state without issuing hardware commands.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nick Faro <yux50000@hotmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-audio.c | 31 +++++++++++++++++++++++++++++--
 1 file changed, 29 insertions(+), 2 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c
index dd819a546af9..a10b887edf51 100644
--- a/drivers/media/usb/cx231xx/cx231xx-audio.c
+++ b/drivers/media/usb/cx231xx/cx231xx-audio.c
@@ -448,10 +448,17 @@ static int snd_cx231xx_pcm_close(struct snd_pcm_substream *substream)
 		return -ENODEV;
 	}
 
-	dev_dbg(dev->dev, "closing device\n");
-
 	/* inform hardware to stop streaming */
 	mutex_lock(&dev->lock);
+	dev->adev.capture_pcm_substream = NULL;
+	if (dev->state & DEV_DISCONNECTED) {
+		dev->adev.users--;
+		mutex_unlock(&dev->lock);
+		cancel_work_sync(&dev->wq_trigger);
+		return 0;
+	}
+
+	dev_dbg(dev->dev, "closing device\n");
 	ret = cx231xx_capture_start(dev, 0, Audio);
 
 	/* set alternate setting for audio interface */
@@ -485,6 +492,9 @@ static int snd_cx231xx_prepare(struct snd_pcm_substream *substream)
 {
 	struct cx231xx *dev = snd_pcm_substream_chip(substream);
 
+	if (dev->state & DEV_DISCONNECTED)
+		return -ENODEV;
+
 	dev->adev.hwptr_done_capture = 0;
 	dev->adev.capture_transfer_done = 0;
 
@@ -495,6 +505,14 @@ static void audio_trigger(struct work_struct *work)
 {
 	struct cx231xx *dev = container_of(work, struct cx231xx, wq_trigger);
 
+	if (dev->state & DEV_DISCONNECTED) {
+		if (dev->USE_ISO)
+			cx231xx_isoc_audio_deinit(dev);
+		else
+			cx231xx_bulk_audio_deinit(dev);
+		return;
+	}
+
 	if (atomic_read(&dev->stream_started)) {
 		dev_dbg(dev->dev, "starting capture");
 		if (is_fw_load(dev) == 0)
@@ -545,6 +563,8 @@ static snd_pcm_uframes_t snd_cx231xx_capture_pointer(struct snd_pcm_substream
 	snd_pcm_uframes_t hwptr_done;
 
 	dev = snd_pcm_substream_chip(substream);
+	if (dev->state & DEV_DISCONNECTED)
+		return SNDRV_PCM_POS_XRUN;
 
 	spin_lock_irqsave(&dev->adev.slock, flags);
 	hwptr_done = dev->adev.hwptr_done_capture;
@@ -689,6 +709,13 @@ static int cx231xx_audio_fini(struct cx231xx *dev)
 	if (dev->adev.sndcard) {
 		card = dev->adev.sndcard;
 		dev->adev.sndcard = NULL;
+		snd_card_disconnect(card);
+		atomic_set(&dev->stream_started, 0);
+		cancel_work_sync(&dev->wq_trigger);
+		if (dev->USE_ISO)
+			cx231xx_isoc_audio_deinit(dev);
+		else
+			cx231xx_bulk_audio_deinit(dev);
 		snd_card_free_when_closed(card);
 	}
 

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 5/6] media: cx231xx: always unregister nodes on USB disconnect
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
                   ` (3 preceding siblings ...)
  2026-08-27 17:44 ` [PATCH 4/6] media: cx231xx: make ALSA teardown disconnect-safe Nick Faro via B4 Relay
@ 2026-08-27 17:44 ` Nick Faro via B4 Relay
  2026-08-27 17:44 ` [PATCH 6/6] media: cx231xx: size alternate-setting arrays by element count Nick Faro via B4 Relay
  2026-09-08  7:20 ` [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Hans Verkuil
  6 siblings, 0 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

From: Nick Faro <yux50000@hotmail.com>

The disconnect path skips cx231xx_release_resources() whenever an analog
video file is open. No close path performs the deferred teardown, so the
V4L2 nodes and I2C adapters remain registered and a later probe collides
with the stale instance. The users counter also does not cover MPEG, ALSA
or other framework references.

Always unregister every device node and subsystem during disconnect. Use
the VB2 unregister helper for queue-backed nodes so active streams are
stopped, and serialize disconnection against in-flight file operations.
The v4l2_device references now keep the backing memory alive until old
file handles close.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nick Faro <yux50000@hotmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-417.c   |  2 +-
 drivers/media/usb/cx231xx/cx231xx-cards.c | 11 +++++------
 drivers/media/usb/cx231xx/cx231xx-video.c | 10 +++++-----
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 4 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-417.c b/drivers/media/usb/cx231xx/cx231xx-417.c
index 3a509e55f61d..9bda68666b05 100644
--- a/drivers/media/usb/cx231xx/cx231xx-417.c
+++ b/drivers/media/usb/cx231xx/cx231xx-417.c
@@ -1672,7 +1672,7 @@ void cx231xx_417_unregister(struct cx231xx *dev)
 	dprintk(3, "%s()\n", __func__);
 
 	if (video_is_registered(&dev->v4l_device)) {
-		video_unregister_device(&dev->v4l_device);
+		vb2_video_unregister_device(&dev->v4l_device);
 	}
 }
 
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 1b1e7bca4c6d..8e23de4cddf8 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1946,7 +1946,6 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 	cx231xx_close_extension(dev);
 	cx231xx_ir_exit(dev);
 	cx231xx_release_analog_resources(dev);
-	cx231xx_417_unregister(dev);
 	cx231xx_remove_from_devlist(dev);
 	cx231xx_dev_uninit(dev);
 err_init:
@@ -1970,7 +1969,8 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 /*
  * cx231xx_usb_disconnect()
  * called when the device gets disconnected
- * video device will be unregistered on v4l2_close in case it is still open
+ * Device nodes are unregistered immediately. Their backing memory remains
+ * alive until the last open file descriptor is closed.
  */
 static void cx231xx_usb_disconnect(struct usb_interface *interface)
 {
@@ -1997,7 +1997,7 @@ static void cx231xx_usb_disconnect(struct usb_interface *interface)
 
 	if (dev->users) {
 		dev_warn(dev->dev,
-			 "device %s is open! Deregistration and memory deallocation are deferred on close.\n",
+			 "device %s is open; disconnecting it now\n",
 			 video_device_node_name(&dev->vdev));
 
 		/* Even having users, it is safe to remove the RC i2c driver */
@@ -2007,17 +2007,16 @@ static void cx231xx_usb_disconnect(struct usb_interface *interface)
 			cx231xx_uninit_isoc(dev);
 		else
 			cx231xx_uninit_bulk(dev);
+		cx231xx_uninit_vbi_isoc(dev);
 		wake_up_interruptible(&dev->wait_frame);
 		wake_up_interruptible(&dev->wait_stream);
-	} else {
 	}
 
 	cx231xx_close_extension(dev);
 
 	mutex_unlock(&dev->lock);
 
-	if (!dev->users)
-		cx231xx_release_resources(dev);
+	cx231xx_release_resources(dev);
 }
 
 static struct usb_driver cx231xx_usb_driver = {
diff --git a/drivers/media/usb/cx231xx/cx231xx-video.c b/drivers/media/usb/cx231xx/cx231xx-video.c
index 154bb4300392..c8c460da2b9a 100644
--- a/drivers/media/usb/cx231xx/cx231xx-video.c
+++ b/drivers/media/usb/cx231xx/cx231xx-video.c
@@ -1561,21 +1561,21 @@ void cx231xx_release_analog_resources(struct cx231xx *dev)
 
 	/*FIXME: I2C IR should be disconnected */
 
+	if (dev->board.has_417)
+		cx231xx_417_unregister(dev);
+
 	if (video_is_registered(&dev->radio_dev))
 		video_unregister_device(&dev->radio_dev);
 	if (video_is_registered(&dev->vbi_dev)) {
 		dev_info(dev->dev, "V4L2 device %s deregistered\n",
 			video_device_node_name(&dev->vbi_dev));
-		video_unregister_device(&dev->vbi_dev);
+		vb2_video_unregister_device(&dev->vbi_dev);
 	}
 	if (video_is_registered(&dev->vdev)) {
 		dev_info(dev->dev, "V4L2 device %s deregistered\n",
 			video_device_node_name(&dev->vdev));
 
-		if (dev->board.has_417)
-			cx231xx_417_unregister(dev);
-
-		video_unregister_device(&dev->vdev);
+		vb2_video_unregister_device(&dev->vdev);
 	}
 }
 
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 19f5036a78d7..8fb1eb9d600e 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -873,6 +873,7 @@ void cx231xx_stop_TS1(struct cx231xx *dev);
 void cx231xx_start_TS1(struct cx231xx *dev);
 void cx231xx_uninit_isoc(struct cx231xx *dev);
 void cx231xx_uninit_bulk(struct cx231xx *dev);
+void cx231xx_uninit_vbi_isoc(struct cx231xx *dev);
 int cx231xx_set_mode(struct cx231xx *dev, enum cx231xx_mode set_mode);
 int cx231xx_unmute_audio(struct cx231xx *dev);
 int cx231xx_ep5_bulkout(struct cx231xx *dev, u8 *firmware, u16 size);

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 6/6] media: cx231xx: size alternate-setting arrays by element count
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
                   ` (4 preceding siblings ...)
  2026-08-27 17:44 ` [PATCH 5/6] media: cx231xx: always unregister nodes on USB disconnect Nick Faro via B4 Relay
@ 2026-08-27 17:44 ` Nick Faro via B4 Relay
  2026-09-08  7:20 ` [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Hans Verkuil
  6 siblings, 0 replies; 8+ messages in thread
From: Nick Faro via B4 Relay @ 2026-08-27 17:44 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel, Nick Faro

From: Nick Faro <yux50000@hotmail.com>

The arguments to kmalloc_array() are the number of elements followed by
the size of each element. The driver instead passes 32 and the number of
alternate settings, apparently confusing the width of the stored value
with its size in bytes.

Allocate one correctly sized element for each alternate setting in the
video, VBI, sliced-caption, transport-stream and audio arrays.

Fixes: e0d3bafd0258 ("V4L/DVB (10954): Add cx231xx USB driver")
Assisted-by: Codex:gpt-5.6-sol
Signed-off-by: Nick Faro <yux50000@hotmail.com>
---
 drivers/media/usb/cx231xx/cx231xx-audio.c |  4 +++-
 drivers/media/usb/cx231xx/cx231xx-cards.c | 16 ++++++++++++----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c
index a10b887edf51..eaddad793900 100644
--- a/drivers/media/usb/cx231xx/cx231xx-audio.c
+++ b/drivers/media/usb/cx231xx/cx231xx-audio.c
@@ -656,7 +656,9 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 	dev_info(dev->dev,
 		"audio EndPoint Addr 0x%x, Alternate settings: %i\n",
 		adev->end_point_addr, adev->num_alt);
-	adev->alt_max_pkt_size = kmalloc_array(32, adev->num_alt, GFP_KERNEL);
+	adev->alt_max_pkt_size = kmalloc_array(adev->num_alt,
+					       sizeof(*adev->alt_max_pkt_size),
+					       GFP_KERNEL);
 	if (!adev->alt_max_pkt_size) {
 		err = -ENOMEM;
 		goto err_free_card;
diff --git a/drivers/media/usb/cx231xx/cx231xx-cards.c b/drivers/media/usb/cx231xx/cx231xx-cards.c
index 8e23de4cddf8..3c4bbf724d63 100644
--- a/drivers/media/usb/cx231xx/cx231xx-cards.c
+++ b/drivers/media/usb/cx231xx/cx231xx-cards.c
@@ -1606,7 +1606,9 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		 dev->video_mode.num_alt);
 
 	dev->video_mode.alt_max_pkt_size =
-		kmalloc_array(32, dev->video_mode.num_alt, GFP_KERNEL);
+		kmalloc_array(dev->video_mode.num_alt,
+			      sizeof(*dev->video_mode.alt_max_pkt_size),
+			      GFP_KERNEL);
 	if (dev->video_mode.alt_max_pkt_size == NULL)
 		return -ENOMEM;
 
@@ -1648,7 +1650,9 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 
 	/* compute alternate max packet sizes for vbi */
 	dev->vbi_mode.alt_max_pkt_size =
-		kmalloc_array(32, dev->vbi_mode.num_alt, GFP_KERNEL);
+		kmalloc_array(dev->vbi_mode.num_alt,
+			      sizeof(*dev->vbi_mode.alt_max_pkt_size),
+			      GFP_KERNEL);
 	if (dev->vbi_mode.alt_max_pkt_size == NULL)
 		return -ENOMEM;
 
@@ -1691,7 +1695,9 @@ static int cx231xx_init_v4l2(struct cx231xx *dev,
 		 dev->sliced_cc_mode.end_point_addr,
 		 dev->sliced_cc_mode.num_alt);
 	dev->sliced_cc_mode.alt_max_pkt_size =
-		kmalloc_array(32, dev->sliced_cc_mode.num_alt, GFP_KERNEL);
+		kmalloc_array(dev->sliced_cc_mode.num_alt,
+			      sizeof(*dev->sliced_cc_mode.alt_max_pkt_size),
+			      GFP_KERNEL);
 	if (dev->sliced_cc_mode.alt_max_pkt_size == NULL)
 		return -ENOMEM;
 
@@ -1887,7 +1893,9 @@ static int cx231xx_usb_probe(struct usb_interface *interface,
 			 dev->ts1_mode.num_alt);
 
 		dev->ts1_mode.alt_max_pkt_size =
-			kmalloc_array(32, dev->ts1_mode.num_alt, GFP_KERNEL);
+			kmalloc_array(dev->ts1_mode.num_alt,
+				      sizeof(*dev->ts1_mode.alt_max_pkt_size),
+				      GFP_KERNEL);
 		if (dev->ts1_mode.alt_max_pkt_size == NULL) {
 			retval = -ENOMEM;
 			goto err_video_alt;

-- 
2.43.0



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe
  2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
                   ` (5 preceding siblings ...)
  2026-08-27 17:44 ` [PATCH 6/6] media: cx231xx: size alternate-setting arrays by element count Nick Faro via B4 Relay
@ 2026-09-08  7:20 ` Hans Verkuil
  6 siblings, 0 replies; 8+ messages in thread
From: Hans Verkuil @ 2026-09-08  7:20 UTC (permalink / raw)
  To: yux50000, Mauro Carvalho Chehab, Hans Verkuil; +Cc: linux-media, linux-kernel

Hi Nick,

Thank you for this series: cleaning up this driver w.r.t. disconnect safety
was always on my todo list, but I never got around to it.

I have some high-level comments below, and I hope you can make a v2 of this series
based on that.

On 27/08/2026 19:44, Nick Faro via B4 Relay wrote:
> The cx231xx disconnect path can tear down the USB-owned device state while
> open V4L2 or ALSA file handles still reference it. It can also leave nodes
> registered when an analog video file is open, and completion handlers can
> continue resubmitting URBs while disconnect is trying to kill them.
> 
> I encountered this with an Elgato Video Capture V2 (USB 0fd9:0037) while
> streaming across a suspend-to-idle/resume cycle. On mainline 7.2.0, resume
> re-probed the USB device while the previous cx231xx instance remained partly
> registered. The reprobe warned about duplicate I2C-mux sysfs entries. Running
> v4l2-ctl --list-devices then logged "refcount_t: saturated; leaking memory"
> and triggered a general protection fault in v4l2_open().
> 
> The reproducer is:
> 
>   1. Start an indefinite capture, for example:
> 
>        v4l2-ctl -d /dev/video4 --stream-mmap=3 --stream-poll
> 
>   2. Suspend the machine to s2idle while the capture remains open.
>   3. Resume, stop the capture process, and inspect the devices with:
> 
>        v4l2-ctl --list-devices
> 
>   4. Unplug and reconnect the capture device.
> 
> The series gives the shared device state the lifetime of its V4L2 and ALSA
> users rather than the USB interface. Disconnect now unregisters all public
> nodes, stops URB resubmission, and prevents late file operations from issuing
> commands to absent hardware. Software-only close and buffer cleanup remain
> available so existing file handles can release normally.
> 
> The final patch is an independent allocation-size correction found while
> reviewing the endpoint-array ownership changed by the first patch.
> 
> The patches are:
> 
>   1. Keep shared device state alive until its final V4L2 reference.

Nice, this is the main change that needed to be done.

>   2. Stop video and VBI completion handlers resubmitting URBs after
>      disconnect.
>   3. Reject late V4L2 hardware operations while preserving software cleanup.

I am not convinced these two patches are needed, since I think this is
actually covered by patch 5. Generally having to add all these disconnect
checks indicates that you are just papering over a symptom, not the cause.

>   4. Make ALSA work, callbacks, close, and card teardown disconnect-safe.

Looks OK.

>   5. Always unregister the driver's public nodes during USB disconnect.

So this is an important one and should actually come first in this series.
Using vb2_video_unregister_device ensures that 1) new device opens (or any
file operation other than close() are blocked, and 2) that any ongoing streaming
is stopped.

With this in place, then together with patch 1 and 4 I do not think there is
a need for patches 2 and 3.

>   6. Allocate alternate-setting arrays by element count and element size.

Nice catch. You can drop the Fixes tag from this patch: it wasn't a bug before,
it was just allocating more memory than was needed.

I'm not sure there is a need for the Fixes tags in general for this series, but
I'll wait for a v2 and I'll see if I keep them or not. These changes are fairly
major, and I'm not sure how well they apply to older kernels. I didn't do this
for the similar em28xx modifications.

Since you have tested this as well with actual hardware, please add a 'Tested-by'
tag as well. It's good to know that this was actually verified with real HW.

I'll mark this series as 'Changes Requested'.

Regards,

	Hans

> 
> Testing performed so far:
> 
>   - Built the cx231xx modules at every commit with W=1 against Ubuntu's
>     7.2.0-070200-generic headers using GCC 14.
>   - Loaded the patched modules with an Elgato Video Capture V2. The video,
>     VBI, and media nodes registered, and ordinary video capture worked.
>   - git diff --check passes.
>   - scripts/checkpatch.pl --strict --max-line-length=80 reports no errors,
>     warnings, or checks for any of the six commits.
> 
>   - v4l2-compliance 1.26.1 results with a valid NTSC signal:
> 
>       v4l2-compliance -d /dev/video4 -s 120
>       Total: 76, Succeeded: 76, Failed: 0, Warnings: 28
> 
>       v4l2-compliance -V /dev/vbi0 -s 120
>       Total: 76, Succeeded: 76, Failed: 0, Warnings: 18
> 
>       v4l2-compliance -m /dev/media2
>       Grand Total: 148, Succeeded: 148, Failed: 0, Warnings: 42
> 
>     The warnings concern the existing audio-control ranges and values not
>     aligning to their reported step, unsupported VIDIOC_CREATE_BUFS, and the
>     video node accepting invalid pixel formats in TRY_FMT and S_FMT.
> 
>   - Repeated the active-capture suspend-to-idle/resume cycle once with a
>     userspace V4L2 capture application holding /dev/video4 open, and did 3
>     cycles of unplugging the USB while streaming. On resume, the old video and
>     VBI nodes were unregistered, the Elgato re-probed and registered one new set
>     of nodes, and capture worked after reopening. v4l2-ctl --list-devices
>     returned promptly. The kernel log contained no cx231xx warning, duplicate
>     I2C-mux entry, refcount error, or oops.
> 
> Codex using the gpt-5.6-sol model assisted with all six patches. It was used to
> inspect the cx231xx driver and comparable media drivers, propose and revise the
> lifetime and teardown changes, split the result into single-purpose commits,
> draft changelogs, and assist with style and build validation. The interaction
> was an iterative review driven by hunk-by-hunk review, rather than a single
> generation prompt.
> 
> The submitter reviewed the resulting code commit by commit and is responsible
> for the final submission.
> 
> ---
> Nick Faro (6):
>       media: cx231xx: keep device state alive until final release
>       media: cx231xx: stop resubmitting URBs after disconnect
>       media: cx231xx: reject V4L2 operations after disconnect
>       media: cx231xx: make ALSA teardown disconnect-safe
>       media: cx231xx: always unregister nodes on USB disconnect
>       media: cx231xx: size alternate-setting arrays by element count
> 
>  drivers/media/usb/cx231xx/cx231xx-417.c   |  44 +++++++++---
>  drivers/media/usb/cx231xx/cx231xx-audio.c |  67 +++++++++++++++----
>  drivers/media/usb/cx231xx/cx231xx-cards.c | 107 ++++++++++++++++++++++--------
>  drivers/media/usb/cx231xx/cx231xx-core.c  |  16 ++++-
>  drivers/media/usb/cx231xx/cx231xx-vbi.c   |  11 +++
>  drivers/media/usb/cx231xx/cx231xx-video.c |  30 ++++++---
>  drivers/media/usb/cx231xx/cx231xx.h       |   1 +
>  7 files changed, 214 insertions(+), 62 deletions(-)
> ---
> base-commit: 4900cad020c0580dfb1be27776ff10a4ef110cfa
> change-id: 20260827-cx231xx-disconnect-lifetime-772159fee2c2
> 
> Best regards,


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-09-08  7:20 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-27 17:44 [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Nick Faro via B4 Relay
2026-08-27 17:44 ` [PATCH 1/6] media: cx231xx: keep device state alive until final release Nick Faro via B4 Relay
2026-08-27 17:44 ` [PATCH 2/6] media: cx231xx: stop resubmitting URBs after disconnect Nick Faro via B4 Relay
2026-08-27 17:44 ` [PATCH 3/6] media: cx231xx: reject V4L2 operations " Nick Faro via B4 Relay
2026-08-27 17:44 ` [PATCH 4/6] media: cx231xx: make ALSA teardown disconnect-safe Nick Faro via B4 Relay
2026-08-27 17:44 ` [PATCH 5/6] media: cx231xx: always unregister nodes on USB disconnect Nick Faro via B4 Relay
2026-08-27 17:44 ` [PATCH 6/6] media: cx231xx: size alternate-setting arrays by element count Nick Faro via B4 Relay
2026-09-08  7:20 ` [PATCH 0/6] media: cx231xx: make USB disconnect teardown safe Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®