[PATCH] media: cx231xx-audio: gate wq_trigger on an audio-local teardown flag

[Fan Wu]

audio_trigger() is deferred work (dev->wq_trigger) armed from
snd_cx231xx_capture_trigger() on every PCM START/STOP; it dereferences
dev->adev state and may free the URBs via cx231xx_isoc_audio_deinit().
cx231xx_audio_fini() tore down that state (snd_card_free_when_closed,
alt_max_pkt_size) without draining wq_trigger, so work armed before or
racing fini ran against freed state.

Adding cancel_work_sync() alone is insufficient: in capture_trigger() the
DEV_DISCONNECTED test and schedule_work() were not atomic, and
DEV_DISCONNECTED is only set on USB disconnect, but fini also runs on
cx231xx-alsa module unload (cx231xx_unregister_extension()), which never
sets it.  A trigger that passed the check could still queue work after
fini's cancel returned an empty queue.

Add an audio-local teardown gate (dev->adev.teardown): fini raises it under
adev.slock, releases the lock, then calls cancel_work_sync() outside the
spinlock.  Both arm sites perform the teardown check and schedule_work()
inside one adev.slock section, so once the gate is visible no new work can
arm after cancel returns.  Initialize the lock, work and gate at the top of
cx231xx_audio_init(), before any fallible allocation, and clear the
partially-built audio state on its error path, so fini is safe even if a
later step fails.

This issue was found by an in-house static analysis tool.

Fixes: 61b04cb24a12 ("[media] cx231xx-audio: fix some locking issues")
Cc: stable@xxxxxxxxxxxxxxx
Assisted-by: Codex:gpt-5.5
Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
---
 drivers/media/usb/cx231xx/cx231xx-audio.c | 42 +++++++++++++++++++----
 drivers/media/usb/cx231xx/cx231xx.h       |  1 +
 2 files changed, 36 insertions(+), 7 deletions(-)

diff --git a/drivers/media/usb/cx231xx/cx231xx-audio.c b/drivers/media/usb/cx231xx/cx231xx-audio.c
index 9c71b32552df..44ca75b18a5d 100644
--- a/drivers/media/usb/cx231xx/cx231xx-audio.c
+++ b/drivers/media/usb/cx231xx/cx231xx-audio.c
@@ -441,6 +441,7 @@ static int snd_cx231xx_capture_open(struct snd_pcm_substream *substream)
 static int snd_cx231xx_pcm_close(struct snd_pcm_substream *substream)
 {
 	int ret;
+	unsigned long flags;
 	struct cx231xx *dev = snd_pcm_substream_chip(substream);
 
 	dev_dbg(dev->dev, "closing device\n");
@@ -470,7 +471,11 @@ static int snd_cx231xx_pcm_close(struct snd_pcm_substream *substream)
 		dev_dbg(dev->dev, "released lock\n");
 		if (atomic_read(&dev->stream_started) > 0) {
 			atomic_set(&dev->stream_started, 0);
-			schedule_work(&dev->wq_trigger);
+
+			spin_lock_irqsave(&dev->adev.slock, flags);
+			if (!dev->adev.teardown)
+				schedule_work(&dev->wq_trigger);
+			spin_unlock_irqrestore(&dev->adev.slock, flags);
 		}
 	}
 	return 0;
@@ -509,11 +514,14 @@ static int snd_cx231xx_capture_trigger(struct snd_pcm_substream *substream,
 {
 	struct cx231xx *dev = snd_pcm_substream_chip(substream);
 	int retval = 0;
+	unsigned long flags;
 
-	if (dev->state & DEV_DISCONNECTED)
+	spin_lock_irqsave(&dev->adev.slock, flags);
+	if (dev->adev.teardown || (dev->state & DEV_DISCONNECTED)) {
+		spin_unlock_irqrestore(&dev->adev.slock, flags);
 		return -ENODEV;
+	}
 
-	spin_lock(&dev->adev.slock);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
 		atomic_set(&dev->stream_started, 1);
@@ -525,10 +533,10 @@ static int snd_cx231xx_capture_trigger(struct snd_pcm_substream *substream,
 		retval = -EINVAL;
 		break;
 	}
-	spin_unlock(&dev->adev.slock);
 
 	schedule_work(&dev->wq_trigger);
 
+	spin_unlock_irqrestore(&dev->adev.slock, flags);
 	return retval;
 }
 
@@ -576,12 +584,20 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 	dev_dbg(dev->dev,
 		"probing for cx231xx non standard usbaudio\n");
 
+	/*
+	 * Extension init errors are ignored by the cx231xx core, so fini()
+	 * must be safe even if initialization fails part way through.
+	 */
+	spin_lock_init(&adev->slock);
+	INIT_WORK(&dev->wq_trigger, audio_trigger);
+	adev->teardown = false;
+	atomic_set(&dev->stream_started, 0);
+
 	err = snd_card_new(dev->dev, index[devnr], "Cx231xx Audio",
 			   THIS_MODULE, 0, &card);
 	if (err < 0)
 		return err;
 
-	spin_lock_init(&adev->slock);
 	err = snd_pcm_new(card, "Cx231xx Audio", 0, 0, 1, &pcm);
 	if (err < 0)
 		goto err_free_card;
@@ -596,8 +612,6 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 	strscpy(card->shortname, "Cx231xx Audio", sizeof(card->shortname));
 	strscpy(card->longname, "Conexant cx231xx Audio", sizeof(card->longname));
 
-	INIT_WORK(&dev->wq_trigger, audio_trigger);
-
 	err = snd_card_register(card);
 	if (err < 0)
 		goto err_free_card;
@@ -651,14 +665,18 @@ static int cx231xx_audio_init(struct cx231xx *dev)
 
 err_free_pkt_size:
 	kfree(adev->alt_max_pkt_size);
+	adev->alt_max_pkt_size = NULL;
 err_free_card:
 	snd_card_free(card);
+	adev->sndcard = NULL;
 
 	return err;
 }
 
 static int cx231xx_audio_fini(struct cx231xx *dev)
 {
+	unsigned long flags;
+
 	if (dev == NULL)
 		return 0;
 
@@ -669,6 +687,16 @@ static int cx231xx_audio_fini(struct cx231xx *dev)
 		return 0;
 	}
 
+	/*
+	 * Block new trigger work before draining already queued work.
+	 * cancel_work_sync() may sleep, so it must run after dropping slock.
+	 */
+	spin_lock_irqsave(&dev->adev.slock, flags);
+	dev->adev.teardown = true;
+	spin_unlock_irqrestore(&dev->adev.slock, flags);
+
+	cancel_work_sync(&dev->wq_trigger);
+
 	if (dev->adev.sndcard) {
 		snd_card_free_when_closed(dev->adev.sndcard);
 		kfree(dev->adev.alt_max_pkt_size);
diff --git a/drivers/media/usb/cx231xx/cx231xx.h b/drivers/media/usb/cx231xx/cx231xx.h
index 19f5036a78d7..99c6a279944f 100644
--- a/drivers/media/usb/cx231xx/cx231xx.h
+++ b/drivers/media/usb/cx231xx/cx231xx.h
@@ -418,6 +418,7 @@ struct cx231xx_audio {
 	struct snd_card *sndcard;
 
 	int users, shutdown;
+	bool teardown;		/* audio fini in progress; set under slock */
 	/* locks */
 	spinlock_t slock;
 
-- 
2.34.1