* [PATCH v3 1/5] ALSA: hda: Serialize codec probe and remove on the same bus
2026-10-30 8:03 [PATCH v3 0/5] ALSA: hda: Wait for PCM files on codec teardown Cen Zhang
@ 2026-10-30 8:03 ` Cen Zhang
2026-10-30 8:29 ` sashiko-bot
2026-10-30 8:03 ` [PATCH v3 2/5] ALSA: core: Add the card file while looking up a minor Cen Zhang
` (3 subsequent siblings)
4 siblings, 1 reply; 7+ messages in thread
From: Cen Zhang @ 2026-10-30 8:03 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming,
zzzccc427
The probe and remove of different codecs on one HD-audio bus can run at
the same time, for example when two codecs are bound and unbound through
sysfs. A codec probe on a registered card calls snd_card_register(),
which walks card->devices in snd_device_register_all(), while the remove
of another codec frees its PCMs and deletes them from the same list.
The list has no lock of its own.
Add a mutex to struct hdac_bus and hold it in hda_codec_driver_probe()
and hda_codec_driver_remove(). The ASoC ext_ops paths are unchanged.
Fixes: bcd96557bd0a ("ALSA: hda - Build PCMs and controls at codec driver probe")
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
include/sound/hdaudio.h | 1 +
sound/hda/common/bind.c | 7 +++++++
sound/hda/core/bus.c | 1 +
3 files changed, 9 insertions(+)
diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 04fd4794bcee..066d2fef3986 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -367,6 +367,7 @@ struct hdac_bus {
spinlock_t reg_lock;
struct mutex cmd_mutex;
struct mutex lock;
+ struct mutex codec_bind_lock; /* codec driver probe and remove */
/* DRM component interface */
struct drm_audio_component *audio_component;
diff --git a/sound/hda/common/bind.c b/sound/hda/common/bind.c
index 957feace0f58..abbb3b64ec09 100644
--- a/sound/hda/common/bind.c
+++ b/sound/hda/common/bind.c
@@ -104,6 +104,12 @@ static int hda_codec_driver_probe(struct device *dev)
if (WARN_ON(!codec->preset))
return -EINVAL;
+ /*
+ * snd_card_register() below walks the device list of the card, which
+ * the removal of another codec changes when it frees its PCMs.
+ */
+ guard(mutex)(&codec->bus->core.codec_bind_lock);
+
err = snd_hda_codec_set_name(codec, codec->preset->name);
if (err < 0)
goto error;
@@ -171,6 +177,7 @@ static int hda_codec_driver_remove(struct device *dev)
return codec->bus->core.ext_ops->hdev_detach(&codec->core);
}
+ guard(mutex)(&codec->bus->core.codec_bind_lock);
snd_hda_codec_disconnect_pcms(codec);
snd_hda_jack_tbl_disconnect(codec);
snd_refcount_sync(&codec->pcm_ref);
diff --git a/sound/hda/core/bus.c b/sound/hda/core/bus.c
index 02462bd035e7..304110c9a6a7 100644
--- a/sound/hda/core/bus.c
+++ b/sound/hda/core/bus.c
@@ -44,6 +44,7 @@ int snd_hdac_bus_init(struct hdac_bus *bus, struct device *dev,
spin_lock_init(&bus->reg_lock);
mutex_init(&bus->cmd_mutex);
mutex_init(&bus->lock);
+ mutex_init(&bus->codec_bind_lock);
INIT_LIST_HEAD(&bus->hlink_list);
init_waitqueue_head(&bus->rirb_wq);
bus->irq = -1;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH v3 1/5] ALSA: hda: Serialize codec probe and remove on the same bus
2026-10-30 8:03 ` [PATCH v3 1/5] ALSA: hda: Serialize codec probe and remove on the same bus Cen Zhang
@ 2026-10-30 8:29 ` sashiko-bot
0 siblings, 0 replies; 7+ messages in thread
From: sashiko-bot @ 2026-10-30 8:29 UTC (permalink / raw)
To: Cen Zhang; +Cc: linux-sound
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ALSA: hda: unlocked snd_card_register() races with codec bind and unbind
- [Low] ALSA: hda: guard(mutex) mixed with goto cleanup in hda_codec_driver_probe()
--
commit 98315c01897e919f38855a83f81646c5e30c2565
Author: Cen Zhang <zzzccc427@gmail.com>
ALSA: hda: Serialize codec probe and remove on the same bus
This commit adds codec_bind_lock to struct hdac_bus and acquires it in
hda_codec_driver_probe() and hda_codec_driver_remove() to serialize
snd_card_register() against concurrent codec removal modifying card->devices
on the same HD-audio bus.
> diff --git a/sound/hda/common/bind.c b/sound/hda/common/bind.c
> index 957feace0f58e..abbb3b64ec090 100644
> --- a/sound/hda/common/bind.c
> +++ b/sound/hda/common/bind.c
> @@ -104,6 +104,12 @@ static int hda_codec_driver_probe(struct device *dev)
> if (WARN_ON(!codec->preset))
> return -EINVAL;
>
> + /*
> + * snd_card_register() below walks the device list of the card, which
> + * the removal of another codec changes when it frees its PCMs.
> + */
> + guard(mutex)(&codec->bus->core.codec_bind_lock);
[Severity: High]
Can card->devices still be corrupted when snd_card_register() is called
outside hda_codec_driver_probe() while another codec on the same bus is
concurrently bound or unbound through sysfs?
In sound/hda/common/sysfs.c, reconfig_codec() calls device_reprobe(), which
acquires and releases codec_bind_lock inside hda_codec_driver_probe(), and
then calls snd_card_register() without holding codec_bind_lock:
sound/hda/common/sysfs.c:reconfig_codec() {
...
err = device_reprobe(hda_codec_dev(codec));
if (err < 0)
return err;
return snd_card_register(codec->card);
}
Similarly, the controller probe paths in sound/hda/controllers/intel.c
(azx_probe_continue()), sound/hda/controllers/cix-ipbloq.c
(cix_ipbloq_hda_probe_codec()), sound/hda/controllers/tegra.c
(hda_tegra_probe_work()), and sound/hda/controllers/acpi.c
(hda_acpi_probe_work()) register codec devices via azx_codec_configure()
and then call snd_card_register(chip->card) without holding codec_bind_lock
(and in tegra.c and acpi.c without setting bus_probing).
If another codec on the same bus is unbound or bound via sysfs at the same
time, hda_codec_driver_remove() -> snd_hda_codec_cleanup_for_unbind() ->
codec_release_pcms() -> snd_device_free() or hda_codec_driver_probe() ->
snd_hda_codec_build_pcms() -> snd_pcm_new() -> snd_device_new() will mutate
card->devices and free struct snd_device entries while the unlocked
snd_card_register() -> snd_device_register_all() walks card->devices.
> +
> err = snd_hda_codec_set_name(codec, codec->preset->name);
> if (err < 0)
> goto error;
[Severity: Low]
This isn't a bug since guard(mutex) is declared before any jump, but
hda_codec_driver_probe() in sound/hda/common/bind.c still uses goto error,
goto error_module_put, and goto error_module for cleanup:
sound/hda/common/bind.c:hda_codec_driver_probe() {
...
error_module:
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);
error:
snd_hda_codec_cleanup_for_unbind(codec);
codec->preset = NULL;
return err;
}
Per the cleanup.h guidelines on not mixing guard() with goto error handling
in the same function, could the probe body be factored into a helper called
under guard(mutex), or use explicit mutex_lock() and mutex_unlock() instead?
--
Sashiko AI review · https://sashiko.dev/#/patchset/pm-pmb-a4703edca61d34aa679b-v11-d9186d19e8afb48972af-0@gmail.com?part=1
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/5] ALSA: core: Add the card file while looking up a minor
2026-10-30 8:03 [PATCH v3 0/5] ALSA: hda: Wait for PCM files on codec teardown Cen Zhang
2026-10-30 8:03 ` [PATCH v3 1/5] ALSA: hda: Serialize codec probe and remove on the same bus Cen Zhang
@ 2026-10-30 8:03 ` Cen Zhang
2026-10-30 8:03 ` [PATCH v3 3/5] ALSA: pcm: Add the file to the card while looking up the minor Cen Zhang
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Cen Zhang @ 2026-10-30 8:03 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming,
zzzccc427
An open looks up its device with snd_lookup_minor_data(), which takes a
card reference, and adds its file to the card with snd_card_file_add()
only afterwards. The card reference does not keep alive a device that
its driver frees while the card stays registered, as the HD-audio codec
remove does with its PCMs. A remove that waits for the files of the card
can miss an open between the lookup and snd_card_file_add().
Add snd_lookup_minor_data_file() and snd_lookup_oss_minor_data_file(),
which add the file to the card while the minor lock is still held, and
record the device data of the minor with the file. After the minor has
been unregistered, an open either has its file on the card already or
does not find the device.
Add snd_card_file_sync() to wait until the files opened for a device are
gone, and wake its waiters in snd_card_file_remove(). Waiting for the
files of one device instead of all files of the card matters, because
the card files include the control device, which a sound server keeps
open for as long as it runs.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
include/sound/core.h | 7 +++++
sound/core/init.c | 61 +++++++++++++++++++++++++++++++++++++-----
sound/core/sound.c | 34 +++++++++++++++++++++++
sound/core/sound_oss.c | 21 +++++++++++++++
4 files changed, 117 insertions(+), 6 deletions(-)
diff --git a/include/sound/core.h b/include/sound/core.h
index d6584967fd49..4cecf04204f4 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -271,12 +271,16 @@ int snd_register_device(int type, struct snd_card *card, int dev,
void *private_data, struct device *device);
int snd_unregister_device(struct device *dev);
void *snd_lookup_minor_data(unsigned int minor, int type);
+void *snd_lookup_minor_data_file(unsigned int minor, int type,
+ struct file *file);
#ifdef CONFIG_SND_OSSEMUL
int snd_register_oss_device(int type, struct snd_card *card, int dev,
const struct file_operations *f_ops, void *private_data);
int snd_unregister_oss_device(int type, struct snd_card *card, int dev);
void *snd_lookup_oss_minor_data(unsigned int minor, int type);
+void *snd_lookup_oss_minor_data_file(unsigned int minor, int type,
+ struct file *file);
#endif
int snd_minor_info_init(void);
@@ -323,7 +327,10 @@ int snd_card_add_dev_attr(struct snd_card *card,
const struct attribute_group *group);
int snd_component_add(struct snd_card *card, const char *component);
int snd_card_file_add(struct snd_card *card, struct file *file);
+int snd_card_file_add_data(struct snd_card *card, struct file *file,
+ const void *data);
int snd_card_file_remove(struct snd_card *card, struct file *file);
+void snd_card_file_sync(struct snd_card *card, const void *data);
struct snd_card *snd_card_ref(int card);
diff --git a/sound/core/init.c b/sound/core/init.c
index d518cb838777..ee660dc4098b 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -24,6 +24,7 @@
/* monitor files for graceful shutdown (hotplug) */
struct snd_monitor_file {
struct file *file;
+ const void *data; /* device data from the minor lookup, or NULL */
const struct file_operations *disconnected_f_op;
struct list_head shutdown_list; /* still need to shutdown */
struct list_head list; /* link of monitor files */
@@ -1075,17 +1076,18 @@ int snd_component_add(struct snd_card *card, const char *component)
EXPORT_SYMBOL(snd_component_add);
/**
- * snd_card_file_add - add the file to the file list of the card
+ * snd_card_file_add_data - add the file of a device to the file list
* @card: soundcard structure
* @file: file pointer
+ * @data: the device data the file is opened for
*
- * This function adds the file to the file linked-list of the card.
- * This linked-list is used to keep tracking the connection state,
- * and to avoid the release of busy resources by hotplug.
+ * Like snd_card_file_add(), but also records @data for
+ * snd_card_file_sync().
*
* Return: zero or a negative error code.
*/
-int snd_card_file_add(struct snd_card *card, struct file *file)
+int snd_card_file_add_data(struct snd_card *card, struct file *file,
+ const void *data)
{
struct snd_monitor_file *mfile;
@@ -1093,6 +1095,7 @@ int snd_card_file_add(struct snd_card *card, struct file *file)
if (mfile == NULL)
return -ENOMEM;
mfile->file = file;
+ mfile->data = data;
mfile->disconnected_f_op = NULL;
INIT_LIST_HEAD(&mfile->shutdown_list);
guard(spinlock)(&card->files_lock);
@@ -1104,6 +1107,22 @@ int snd_card_file_add(struct snd_card *card, struct file *file)
get_device(&card->card_dev);
return 0;
}
+
+/**
+ * snd_card_file_add - add the file to the file list of the card
+ * @card: soundcard structure
+ * @file: file pointer
+ *
+ * This function adds the file to the file linked-list of the card.
+ * This linked-list is used to keep tracking the connection state,
+ * and to avoid the release of busy resources by hotplug.
+ *
+ * Return: zero or a negative error code.
+ */
+int snd_card_file_add(struct snd_card *card, struct file *file)
+{
+ return snd_card_file_add_data(card, file, NULL);
+}
EXPORT_SYMBOL(snd_card_file_add);
/**
@@ -1135,7 +1154,7 @@ int snd_card_file_remove(struct snd_card *card, struct file *file)
break;
}
}
- if (list_empty(&card->files_list))
+ if (list_empty(&card->files_list) || (found && found->data))
wake_up_all(&card->remove_sleep);
}
if (!found) {
@@ -1148,6 +1167,36 @@ int snd_card_file_remove(struct snd_card *card, struct file *file)
}
EXPORT_SYMBOL(snd_card_file_remove);
+static bool snd_card_has_data_file(struct snd_card *card, const void *data)
+{
+ struct snd_monitor_file *mfile;
+
+ list_for_each_entry(mfile, &card->files_list, list) {
+ if (mfile->data == data)
+ return true;
+ }
+ return false;
+}
+
+/**
+ * snd_card_file_sync - wait until the files opened for a device are closed
+ * @card: soundcard structure
+ * @data: the device data registered for the minors of the device
+ *
+ * Waits until every file added for @data by snd_lookup_minor_data_file()
+ * or snd_lookup_oss_minor_data_file() has been removed from @card. The
+ * minors of the device must already be unregistered, so that no new file
+ * can be added for it.
+ */
+void snd_card_file_sync(struct snd_card *card, const void *data)
+{
+ guard(spinlock_irq)(&card->files_lock);
+ wait_event_lock_irq(card->remove_sleep,
+ !snd_card_has_data_file(card, data),
+ card->files_lock);
+}
+EXPORT_SYMBOL_GPL(snd_card_file_sync);
+
#ifdef CONFIG_PM
/**
* snd_power_ref_and_wait - wait until the card gets powered up
diff --git a/sound/core/sound.c b/sound/core/sound.c
index 24f40763a20c..94334cbc02b5 100644
--- a/sound/core/sound.c
+++ b/sound/core/sound.c
@@ -115,6 +115,40 @@ void *snd_lookup_minor_data(unsigned int minor, int type)
}
EXPORT_SYMBOL(snd_lookup_minor_data);
+/**
+ * snd_lookup_minor_data_file - get user data of a device and add the file
+ * @minor: the minor number
+ * @type: device type (SNDRV_DEVICE_TYPE_XXX)
+ * @file: the file being opened
+ *
+ * Like snd_lookup_minor_data(), but instead of taking a reference to the
+ * card, adds @file to the file list of the card while the minor is still
+ * registered, and records the user data for snd_card_file_sync(). The
+ * caller must call snd_card_file_remove() when it is done with the file.
+ *
+ * Return: The user data pointer if the specified device is found, %NULL if
+ * it is not found, or an error pointer if the file cannot be added.
+ */
+void *snd_lookup_minor_data_file(unsigned int minor, int type,
+ struct file *file)
+{
+ struct snd_minor *mreg;
+ int err;
+
+ if (minor >= ARRAY_SIZE(snd_minors))
+ return NULL;
+ guard(mutex)(&sound_mutex);
+ mreg = snd_minors[minor];
+ if (!mreg || mreg->type != type || !mreg->private_data ||
+ !mreg->card_ptr)
+ return NULL;
+ err = snd_card_file_add_data(mreg->card_ptr, file, mreg->private_data);
+ if (err < 0)
+ return ERR_PTR(err);
+ return mreg->private_data;
+}
+EXPORT_SYMBOL(snd_lookup_minor_data_file);
+
#ifdef CONFIG_MODULES
static struct snd_minor *autoload_device(unsigned int minor)
{
diff --git a/sound/core/sound_oss.c b/sound/core/sound_oss.c
index 7eb49f2cada5..2f0bc481a46d 100644
--- a/sound/core/sound_oss.c
+++ b/sound/core/sound_oss.c
@@ -41,6 +41,27 @@ void *snd_lookup_oss_minor_data(unsigned int minor, int type)
}
EXPORT_SYMBOL(snd_lookup_oss_minor_data);
+/* Like snd_lookup_minor_data_file(), for OSS minors */
+void *snd_lookup_oss_minor_data_file(unsigned int minor, int type,
+ struct file *file)
+{
+ struct snd_minor *mreg;
+ int err;
+
+ if (minor >= ARRAY_SIZE(snd_oss_minors))
+ return NULL;
+ guard(mutex)(&sound_oss_mutex);
+ mreg = snd_oss_minors[minor];
+ if (!mreg || mreg->type != type || !mreg->private_data ||
+ !mreg->card_ptr)
+ return NULL;
+ err = snd_card_file_add_data(mreg->card_ptr, file, mreg->private_data);
+ if (err < 0)
+ return ERR_PTR(err);
+ return mreg->private_data;
+}
+EXPORT_SYMBOL(snd_lookup_oss_minor_data_file);
+
static int snd_oss_kernel_minor(int type, struct snd_card *card, int dev)
{
int minor;
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v3 3/5] ALSA: pcm: Add the file to the card while looking up the minor
2026-10-30 8:03 [PATCH v3 0/5] ALSA: hda: Wait for PCM files on codec teardown Cen Zhang
2026-10-30 8:03 ` [PATCH v3 1/5] ALSA: hda: Serialize codec probe and remove on the same bus Cen Zhang
2026-10-30 8:03 ` [PATCH v3 2/5] ALSA: core: Add the card file while looking up a minor Cen Zhang
@ 2026-10-30 8:03 ` Cen Zhang
2026-10-30 8:03 ` [PATCH v3 4/5] ALSA: pcm: Refuse to open a disconnected PCM Cen Zhang
2026-10-30 8:03 ` [PATCH v3 5/5] ALSA: hda: Wait for the files of the codec PCMs before freeing them Cen Zhang
4 siblings, 0 replies; 7+ messages in thread
From: Cen Zhang @ 2026-10-30 8:03 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming,
zzzccc427
Use snd_lookup_minor_data_file() and snd_lookup_oss_minor_data_file() in
the native and OSS PCM opens, so that the file is on the card before the
PCM minor can be unregistered. The opens then no longer need the extra
card reference from the lookup.
Without that reference, a failed open also no longer reads pcm->card
after it has removed its file, which would race with a remove that waits
for the files before freeing the PCM.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
sound/core/oss/pcm_oss.c | 19 ++++++-------------
sound/core/pcm_native.c | 37 +++++++++++++++----------------------
2 files changed, 21 insertions(+), 35 deletions(-)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index e8df27d09b97..2d99df900f76 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -2497,15 +2497,12 @@ static int snd_pcm_oss_open(struct inode *inode, struct file *file)
nonseekable_open(inode, file);
- pcm = snd_lookup_oss_minor_data(iminor(inode),
- SNDRV_OSS_DEVICE_TYPE_PCM);
- if (pcm == NULL) {
- err = -ENODEV;
- goto __error1;
- }
- err = snd_card_file_add(pcm->card, file);
- if (err < 0)
- goto __error1;
+ pcm = snd_lookup_oss_minor_data_file(iminor(inode),
+ SNDRV_OSS_DEVICE_TYPE_PCM, file);
+ if (IS_ERR(pcm))
+ return PTR_ERR(pcm);
+ if (!pcm)
+ return -ENODEV;
if (!try_module_get(pcm->card->module)) {
err = -ENODEV;
goto __error2;
@@ -2558,16 +2555,12 @@ static int snd_pcm_oss_open(struct inode *inode, struct file *file)
mutex_unlock(&pcm->open_mutex);
if (err < 0)
goto __error;
- snd_card_unref(pcm->card);
return err;
__error:
module_put(pcm->card->module);
__error2:
snd_card_file_remove(pcm->card, file);
- __error1:
- if (pcm)
- snd_card_unref(pcm->card);
return err;
}
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 35152fae74ca..89bfe1f358ee 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2938,45 +2938,39 @@ static int snd_pcm_open_file(struct file *file,
static int snd_pcm_playback_open(struct inode *inode, struct file *file)
{
struct snd_pcm *pcm;
- int err;
nonseekable_open(inode, file);
- pcm = snd_lookup_minor_data(iminor(inode),
- SNDRV_DEVICE_TYPE_PCM_PLAYBACK);
- err = snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_PLAYBACK);
- if (pcm)
- snd_card_unref(pcm->card);
- return err;
+ pcm = snd_lookup_minor_data_file(iminor(inode),
+ SNDRV_DEVICE_TYPE_PCM_PLAYBACK, file);
+ if (IS_ERR(pcm))
+ return PTR_ERR(pcm);
+ if (!pcm)
+ return -ENODEV;
+ return snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_PLAYBACK);
}
static int snd_pcm_capture_open(struct inode *inode, struct file *file)
{
struct snd_pcm *pcm;
- int err;
nonseekable_open(inode, file);
- pcm = snd_lookup_minor_data(iminor(inode),
- SNDRV_DEVICE_TYPE_PCM_CAPTURE);
- err = snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_CAPTURE);
- if (pcm)
- snd_card_unref(pcm->card);
- return err;
+ pcm = snd_lookup_minor_data_file(iminor(inode),
+ SNDRV_DEVICE_TYPE_PCM_CAPTURE, file);
+ if (IS_ERR(pcm))
+ return PTR_ERR(pcm);
+ if (!pcm)
+ return -ENODEV;
+ return snd_pcm_open(file, pcm, SNDRV_PCM_STREAM_CAPTURE);
}
+/* called with the file added to the card by snd_lookup_minor_data_file() */
static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
{
int err;
wait_queue_entry_t wait;
- if (pcm == NULL) {
- err = -ENODEV;
- goto __error1;
- }
- err = snd_card_file_add(pcm->card, file);
- if (err < 0)
- goto __error1;
if (!try_module_get(pcm->card->module)) {
err = -ENODEV;
goto __error2;
@@ -3018,7 +3012,6 @@ static int snd_pcm_open(struct file *file, struct snd_pcm *pcm, int stream)
module_put(pcm->card->module);
__error2:
snd_card_file_remove(pcm->card, file);
- __error1:
return err;
}
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v3 4/5] ALSA: pcm: Refuse to open a disconnected PCM
2026-10-30 8:03 [PATCH v3 0/5] ALSA: hda: Wait for PCM files on codec teardown Cen Zhang
` (2 preceding siblings ...)
2026-10-30 8:03 ` [PATCH v3 3/5] ALSA: pcm: Add the file to the card while looking up the minor Cen Zhang
@ 2026-10-30 8:03 ` Cen Zhang
2026-10-30 8:03 ` [PATCH v3 5/5] ALSA: hda: Wait for the files of the codec PCMs before freeing them Cen Zhang
4 siblings, 0 replies; 7+ messages in thread
From: Cen Zhang @ 2026-10-30 8:03 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming,
zzzccc427
An open that found the PCM before snd_pcm_dev_disconnect() can still
attach a substream after it, because nothing in the open path checks
whether the PCM was disconnected. When only the PCM is disconnected, as
on an HD-audio codec unbind, the open then succeeds on a device that is
going away, and its file delays the remove until it is closed.
Mark the PCM as disconnected under open_mutex in snd_pcm_dev_disconnect()
and make snd_pcm_attach_substream() fail with -ENODEV for it.
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
include/sound/pcm.h | 1 +
sound/core/pcm.c | 3 +++
2 files changed, 4 insertions(+)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 1cf511f8d880..04f58c2ec5c6 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -548,6 +548,7 @@ struct snd_pcm {
bool internal; /* pcm is for internal use only */
bool nonatomic; /* whole PCM operations are in non-atomic context */
bool no_device_suspend; /* don't invoke device PM suspend */
+ bool disconnected; /* disconnected, no new open */
#if IS_ENABLED(CONFIG_SND_PCM_OSS)
struct snd_pcm_oss oss;
#endif
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 064df4cd3b50..567eeff23095 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -897,6 +897,8 @@ int snd_pcm_attach_substream(struct snd_pcm *pcm, int stream,
stream != SNDRV_PCM_STREAM_CAPTURE))
return -EINVAL;
*rsubstream = NULL;
+ if (pcm->disconnected)
+ return -ENODEV;
pstr = &pcm->streams[stream];
if (pstr->substream == NULL || pstr->substream_count == 0)
return -ENODEV;
@@ -1101,6 +1103,7 @@ static int snd_pcm_dev_disconnect(struct snd_device *device)
guard(mutex)(®ister_mutex);
scoped_guard(mutex, &pcm->open_mutex) {
+ pcm->disconnected = true;
wake_up(&pcm->open_wait);
list_del_init(&pcm->list);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread* [PATCH v3 5/5] ALSA: hda: Wait for the files of the codec PCMs before freeing them
2026-10-30 8:03 [PATCH v3 0/5] ALSA: hda: Wait for PCM files on codec teardown Cen Zhang
` (3 preceding siblings ...)
2026-10-30 8:03 ` [PATCH v3 4/5] ALSA: pcm: Refuse to open a disconnected PCM Cen Zhang
@ 2026-10-30 8:03 ` Cen Zhang
4 siblings, 0 replies; 7+ messages in thread
From: Cen Zhang @ 2026-10-30 8:03 UTC (permalink / raw)
To: perex, tiwai
Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming,
zzzccc427
The codec remove waits for codec->pcm_ref, which only counts the streams
opened through the HD-audio PCM ops. A file uses the PCM before that:
snd_pcm_open() takes pcm->open_mutex, and an OSS open walks the setup
list of the PCM, before a substream is opened. snd_pcm_release() also
still takes pcm->open_mutex after the close callback has dropped the
reference. When a codec is unbound through sysfs while the card stays
registered, the remove can free the PCMs under such a file:
PCM open codec unbind
-------- ------------
snd_lookup_minor_data()
snd_card_file_add()
snd_hda_codec_disconnect_pcms()
snd_refcount_sync(&codec->pcm_ref)
snd_hda_codec_cleanup_for_unbind()
snd_pcm_free()
mutex_lock(&pcm->open_mutex)
Opening the PCM while the codec is unbound and bound in a loop triggers
it:
BUG: KASAN: slab-use-after-free in mutex_spin_on_owner+0x94/0x2b0
Read of size 8 at addr ffff888005d52a58 by task hdatest/90
Call Trace:
__mutex_lock+0x16a4/0x24a0
snd_pcm_open+0x230/0x600
snd_pcm_playback_open+0x5e/0xb0
Freed by task 94:
kfree+0x2e5/0x510
snd_pcm_free+0x9d/0x150
__snd_device_free+0x121/0x370
snd_hda_codec_cleanup_for_unbind+0x219/0x920
hda_codec_driver_remove+0x1a7/0x2b0
unbind_store+0xd1/0xf0
An open can also get past the wait and call into the codec driver after
the remove has cleared its state, which ends in a NULL pointer
dereference in playback_pcm_open().
After disconnecting the PCMs, wait until the files opened on them are
closed. Do the same on the probe error path, which frees PCMs that a
late probe may already have registered.
Fixes: bcd96557bd0a ("ALSA: hda - Build PCMs and controls at codec driver probe")
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
sound/hda/common/bind.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/sound/hda/common/bind.c b/sound/hda/common/bind.c
index abbb3b64ec09..514f6d581b33 100644
--- a/sound/hda/common/bind.c
+++ b/sound/hda/common/bind.c
@@ -84,6 +84,20 @@ int snd_hda_codec_set_name(struct hda_codec *codec, const char *name)
}
EXPORT_SYMBOL_GPL(snd_hda_codec_set_name);
+/*
+ * A file opened on a codec PCM can use the PCM until it is closed, also
+ * after the PCM was disconnected. Wait for those files.
+ */
+static void hda_codec_sync_pcm_files(struct hda_codec *codec)
+{
+ struct hda_pcm *cpcm;
+
+ list_for_each_entry(cpcm, &codec->pcm_list_head, list) {
+ if (cpcm->pcm)
+ snd_card_file_sync(codec->card, cpcm->pcm);
+ }
+}
+
static int hda_codec_driver_probe(struct device *dev)
{
struct hda_codec *codec = dev_to_hda_codec(dev);
@@ -151,6 +165,8 @@ static int hda_codec_driver_probe(struct device *dev)
error_module:
snd_hdac_device_disable_unsol(&codec->core);
cancel_delayed_work_sync(&codec->jackpoll_work);
+ snd_hda_codec_disconnect_pcms(codec);
+ hda_codec_sync_pcm_files(codec);
if (probed && driver->ops->remove)
driver->ops->remove(codec);
error_module_put:
@@ -180,6 +196,7 @@ static int hda_codec_driver_remove(struct device *dev)
guard(mutex)(&codec->bus->core.codec_bind_lock);
snd_hda_codec_disconnect_pcms(codec);
snd_hda_jack_tbl_disconnect(codec);
+ hda_codec_sync_pcm_files(codec);
snd_refcount_sync(&codec->pcm_ref);
snd_power_sync_ref(codec->bus->card);
--
2.43.0
^ permalink raw reply related [flat|nested] 7+ messages in thread