[PATCH v2 0/5] ALSA: yet a few more fixes for AI bug reports

[PATCH v2 0/5] ALSA: yet a few more fixes for AI bug reports

[Takashi Iwai]

some patches in the previous series had obvious issues, and some
needed more considerations, so here are those refreshed ones.
One more fix is added (patch 4) for addressing the potential issues in
the (other) previous series, too.


Takashi

v1: https://lore.kernel.org/20261008192553.300025-1-tiwai@suse.de

===

Takashi Iwai (5):
  ALSA: line6: Fix handling of zero-length capture packets
  ALSA: hda: Disable unsol event handling at error and shutdown paths
  ALSA: hda: Add lock around codec->registered flag manipulations
  ALSA: hda: Stop unsol event and jack polling at hda-ext device
    removal, too
  ALSA: hda: Add NULL check for the driver pointer at unsol event work

 sound/hda/common/bind.c    | 22 +++++++++++++---------
 sound/hda/common/codec.c   |  9 +++++++++
 sound/hda/core/bus.c       |  9 ++++++---
 sound/usb/line6/pcm.c      |  4 +++-
 sound/usb/line6/playback.c | 10 ++--------
 5 files changed, 33 insertions(+), 21 deletions(-)

-- 
2.55.0

[PATCH v2 1/5] ALSA: line6: Fix handling of zero-length capture packets

[Takashi Iwai]

The LINE6 playback engine assumes that line6pcm->prev_fsize=0
indicates that there was no input packet to be processed, and
synthesizes the frames.  But, when a capture stream receives a
zero-length (or a very small size) packet, it sets 0 to prev_fsize,
while playback engine still believes it's for synthesis, hence it
tries to copy the larger data than the actual input data.

As prev_fsize=0 can't be a good indication for "no input", change the
meaning slightly: now prev_fsize=-1 means no input, while prev_fsize=0
means it's a zero-length packet.

As a result, the outgoing packet length can be also zero, so the
corresponding check is dropped, too; otherwise the outgoing URB will
be lost.

Fixes: 1027f476f507 ("staging: line6: sync with upstream")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: drop the zero urb length check code, too

 sound/usb/line6/pcm.c      |  4 +++-
 sound/usb/line6/playback.c | 10 ++--------
 2 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/sound/usb/line6/pcm.c b/sound/usb/line6/pcm.c
index 998869dc4613..e2b99e7a08e0 100644
--- a/sound/usb/line6/pcm.c
+++ b/sound/usb/line6/pcm.c
@@ -217,7 +217,7 @@ static void line6_stream_stop(struct snd_line6_pcm *line6pcm, int direction,
 	if (direction == SNDRV_PCM_STREAM_CAPTURE) {
 		guard(spinlock_irqsave)(&pstr->lock);
 		line6pcm->prev_fbuf = NULL;
-		line6pcm->prev_fsize = 0;
+		line6pcm->prev_fsize = -1;
 	}
 }
 
@@ -538,6 +538,8 @@ int line6_init_pcm(struct usb_line6 *line6,
 	line6pcm->volume_playback[0] = line6pcm->volume_playback[1] = 255;
 	line6pcm->volume_monitor = 255;
 	line6pcm->line6 = line6;
+	line6pcm->prev_fbuf = NULL;
+	line6pcm->prev_fsize = -1;
 
 	spin_lock_init(&line6pcm->out.lock);
 	spin_lock_init(&line6pcm->in.lock);
diff --git a/sound/usb/line6/playback.c b/sound/usb/line6/playback.c
index aa6ddf8746a0..8797f8b1577a 100644
--- a/sound/usb/line6/playback.c
+++ b/sound/usb/line6/playback.c
@@ -171,7 +171,7 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 		    &urb_out->iso_frame_desc[i];
 
 		fsize = line6pcm->prev_fsize;
-		if (fsize == 0) {
+		if (fsize == -1) {
 			int n;
 
 			line6pcm->out.count += frame_increment;
@@ -194,12 +194,6 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 		urb_size += fsize;
 	}
 
-	if (urb_size == 0) {
-		/* can't determine URB size */
-		dev_err(line6pcm->line6->ifcdev, "driver bug: urb_size = 0\n");
-		return -EINVAL;
-	}
-
 	urb_frames = urb_size / bytes_per_frame;
 	urb_out->transfer_buffer =
 	    line6pcm->out.buffer +
@@ -271,7 +265,7 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
 						   bytes_per_frame);
 		}
 		line6pcm->prev_fbuf = NULL;
-		line6pcm->prev_fsize = 0;
+		line6pcm->prev_fsize = -1;
 	}
 	spin_unlock(&line6pcm->in.lock);
 
-- 
2.55.0

[PATCH v2 2/5] ALSA: hda: Disable unsol event handling at error and shutdown paths

[Takashi Iwai]

When the HD-audio controller driver probe fails, it still leaves the
unsolicited event handling and jackpoll work active, hence if they are
pending, they might fire up later after the resource gets released,
which may lead to a UAF.  A similar problem may be seen at shutdown,
too.

Add the recently added helper to disable unsol events and the cancel
of jackpoll_work at the appropriate places.  We need the workaround in
the error path before driver->ops->remove, too.  As unsol events and
jack polling may have been already enabled during the probe callback,
the driver->ops->remove call is done (more) conditionally now.

Fixes: c3ec8ac82105 ("ASoC: hdac_hda: fix memleak on module unload")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: apply the workaround in the wider error path

 sound/hda/common/bind.c  | 8 ++++++--
 sound/hda/common/codec.c | 1 +
 2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/sound/hda/common/bind.c b/sound/hda/common/bind.c
index 4772ca154a29..d12359970e74 100644
--- a/sound/hda/common/bind.c
+++ b/sound/hda/common/bind.c
@@ -89,6 +89,7 @@ static int hda_codec_driver_probe(struct device *dev)
 	struct hda_codec *codec = dev_to_hda_codec(dev);
 	struct module *owner = dev->driver->owner;
 	struct hda_codec_driver *driver = hda_codec_to_driver(codec);
+	bool probed = false;
 	int err;
 
 	if (codec->bus->core.ext_ops) {
@@ -122,7 +123,8 @@ static int hda_codec_driver_probe(struct device *dev)
 
 	err = driver->ops->probe(codec, codec->preset);
 	if (err < 0)
-		goto error_module_put;
+		goto error_module;
+	probed = true;
 	err = snd_hda_codec_build_pcms(codec);
 	if (err < 0)
 		goto error_module;
@@ -141,7 +143,9 @@ static int hda_codec_driver_probe(struct device *dev)
 	return 0;
 
  error_module:
-	if (driver->ops->remove)
+	snd_hdac_device_disable_unsol(&codec->core);
+	cancel_delayed_work_sync(&codec->jackpoll_work);
+	if (probed && driver->ops->remove)
 		driver->ops->remove(codec);
  error_module_put:
 	module_put(owner);
diff --git a/sound/hda/common/codec.c b/sound/hda/common/codec.c
index c99fe61c29db..94da4f3a21ed 100644
--- a/sound/hda/common/codec.c
+++ b/sound/hda/common/codec.c
@@ -3039,6 +3039,7 @@ void snd_hda_codec_shutdown(struct hda_codec *codec)
 
 	codec->jackpoll_interval = 0; /* don't poll any longer */
 	cancel_delayed_work_sync(&codec->jackpoll_work);
+	snd_hdac_device_disable_unsol(&codec->core);
 	list_for_each_entry(cpcm, &codec->pcm_list_head, list)
 		snd_pcm_suspend_all(cpcm->pcm);
 
-- 
2.55.0

[PATCH v2 3/5] ALSA: hda: Add lock around codec->registered flag manipulations

[Takashi Iwai]

Currently the places setting or clearing codec->registered flag have
no locking, while its reference in the unsol handling work
snd_hdac_bus_process_unsol_events() takes the bus->reg_lock lock.
Let's add the same bus->reg_lock at the places where codec->registered
is set and cleared for avoiding the races.

Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: fix build error

 sound/hda/common/codec.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/sound/hda/common/codec.c b/sound/hda/common/codec.c
index 94da4f3a21ed..18838a45e6f7 100644
--- a/sound/hda/common/codec.c
+++ b/sound/hda/common/codec.c
@@ -757,11 +757,15 @@ static void codec_release_pcms(struct hda_codec *codec)
  */
 void snd_hda_codec_cleanup_for_unbind(struct hda_codec *codec)
 {
+	struct hda_bus *bus = codec->bus;
+
 	if (codec->core.registered) {
 		/* pm_runtime_put() is called in snd_hdac_device_exit() */
 		pm_runtime_get_noresume(hda_codec_dev(codec));
 		pm_runtime_disable(hda_codec_dev(codec));
+		spin_lock_irq(&bus->core.reg_lock);
 		codec->core.registered = false;
+		spin_unlock_irq(&bus->core.reg_lock);
 	}
 
 	snd_hda_codec_disconnect_pcms(codec);
@@ -808,6 +812,8 @@ void snd_hda_codec_display_power(struct hda_codec *codec, bool enable)
  */
 void snd_hda_codec_register(struct hda_codec *codec)
 {
+	struct hda_bus *bus = codec->bus;
+
 	if (codec->core.registered)
 		return;
 	if (device_is_registered(hda_codec_dev(codec))) {
@@ -815,7 +821,9 @@ void snd_hda_codec_register(struct hda_codec *codec)
 		pm_runtime_enable(hda_codec_dev(codec));
 		/* it was powered up in snd_hda_codec_new(), now all done */
 		snd_hda_power_down(codec);
+		spin_lock_irq(&bus->core.reg_lock);
 		codec->core.registered = true;
+		spin_unlock_irq(&bus->core.reg_lock);
 	}
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_register);
-- 
2.55.0

[PATCH v2 4/5] ALSA: hda: Stop unsol event and jack polling at hda-ext device removal, too

[Takashi Iwai]

In the recent commit f3607b6952ca ("ALSA: hda: Stop unsol events and
jack polling before codec unbind"), we applied the workaround to stop
pending works at the hda codec driver removal, but it was applied
after the bypass for hda-ext ops hdev_detach.  Basically the same
issue is seen for hda-ext stuff, so it's safer to apply the workaround
at the beginning of hda_codec_driver_remove().

Along with it, the clearance of unsol_disabled flag is moved to the
beginning of hda_codec_driver_probe(), too; otherwise hda-ext
hdev_attach() might miss the unsol event handling when the code gets
re-bound.

Fixes: f3607b6952ca ("ALSA: hda: Stop unsol events and jack polling before codec unbind")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/hda/common/bind.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/sound/hda/common/bind.c b/sound/hda/common/bind.c
index d12359970e74..957feace0f58 100644
--- a/sound/hda/common/bind.c
+++ b/sound/hda/common/bind.c
@@ -92,6 +92,9 @@ static int hda_codec_driver_probe(struct device *dev)
 	bool probed = false;
 	int err;
 
+	/* unsol events are still blocked until registered */
+	codec->core.unsol_disabled = false;
+
 	if (codec->bus->core.ext_ops) {
 		if (WARN_ON(!codec->bus->core.ext_ops->hdev_attach))
 			return -EINVAL;
@@ -101,9 +104,6 @@ static int hda_codec_driver_probe(struct device *dev)
 	if (WARN_ON(!codec->preset))
 		return -EINVAL;
 
-	/* unsol events are still blocked until registered */
-	codec->core.unsol_disabled = false;
-
 	err = snd_hda_codec_set_name(codec, codec->preset->name);
 	if (err < 0)
 		goto error;
@@ -161,16 +161,16 @@ static int hda_codec_driver_remove(struct device *dev)
 	struct hda_codec *codec = dev_to_hda_codec(dev);
 	struct hda_codec_driver *driver = hda_codec_to_driver(codec);
 
+	/* stop asynchronous jack handling before freeing driver resources */
+	snd_hdac_device_disable_unsol(&codec->core);
+	cancel_delayed_work_sync(&codec->jackpoll_work);
+
 	if (codec->bus->core.ext_ops) {
 		if (WARN_ON(!codec->bus->core.ext_ops->hdev_detach))
 			return -EINVAL;
 		return codec->bus->core.ext_ops->hdev_detach(&codec->core);
 	}
 
-	/* stop asynchronous jack handling before freeing driver resources */
-	snd_hdac_device_disable_unsol(&codec->core);
-	cancel_delayed_work_sync(&codec->jackpoll_work);
-
 	snd_hda_codec_disconnect_pcms(codec);
 	snd_hda_jack_tbl_disconnect(codec);
 	snd_refcount_sync(&codec->pcm_ref);
-- 
2.55.0

[PATCH v2 5/5] ALSA: hda: Add NULL check for the driver pointer at unsol event work

[Takashi Iwai]

snd_hdac_bus_process_unsol_events() assumes that the codec driver is
always set when it's accessible from the codec table.  It's true in
general, but in a sheer timing, it might have been already NULLified
at the driver unbinding.

Although the codec removal should have been done properly by the
previous fixes, just for more safety, let's add a NULL check before
dereferencing and calling the driver's unsol_event callback.

Fixes: e7255c00b10e ("ALSA: hda: Skip event processing for unregistered codecs")
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
v1->v2: fix bogus NULL check to a more correct one, mention it's just to be sure

 sound/hda/core/bus.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/sound/hda/core/bus.c b/sound/hda/core/bus.c
index 8d4ed9834aaa..21af4b7c870d 100644
--- a/sound/hda/core/bus.c
+++ b/sound/hda/core/bus.c
@@ -182,10 +182,13 @@ static void snd_hdac_bus_process_unsol_events(struct work_struct *work)
 		codec = bus->caddr_tbl[caddr & 0x0f];
 		if (!codec || !codec->registered || codec->unsol_disabled)
 			continue;
-		spin_unlock_irq(&bus->reg_lock);
+		if (!codec->dev.driver)
+			continue;
 		drv = drv_to_hdac_driver(codec->dev.driver);
-		if (drv->unsol_event)
-			drv->unsol_event(codec, res);
+		if (!drv->unsol_event)
+			continue;
+		spin_unlock_irq(&bus->reg_lock);
+		drv->unsol_event(codec, res);
 		spin_lock_irq(&bus->reg_lock);
 	}
 	spin_unlock_irq(&bus->reg_lock);
-- 
2.55.0

Re: [PATCH v2 1/5] ALSA: line6: Fix handling of zero-length capture packets

[Takashi Iwai]

On Fri, 09 Oct 2026 15:33:35 +0200,
Takashi Iwai wrote:
> 
> The LINE6 playback engine assumes that line6pcm->prev_fsize=0
> indicates that there was no input packet to be processed, and
> synthesizes the frames.  But, when a capture stream receives a
> zero-length (or a very small size) packet, it sets 0 to prev_fsize,
> while playback engine still believes it's for synthesis, hence it
> tries to copy the larger data than the actual input data.
> 
> As prev_fsize=0 can't be a good indication for "no input", change the
> meaning slightly: now prev_fsize=-1 means no input, while prev_fsize=0
> means it's a zero-length packet.
> 
> As a result, the outgoing packet length can be also zero, so the
> corresponding check is dropped, too; otherwise the outgoing URB will
> be lost.
> 
> Fixes: 1027f476f507 ("staging: line6: sync with upstream")
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
> v1->v2: drop the zero urb length check code, too

Hm, still this one looks tricky and can't cover the full cases, as it
seems.  So I dropped this from the series for reconsideration for now.


Takashi