mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 0/2] ALSA: Fix OSS teardown lifetime handling
@ 2026-10-18 14:07 Cen Zhang
  2026-10-18 14:07 ` [PATCH v3 1/2] ALSA: pcm: Protect the unregister notifier traversal Cen Zhang
  2026-10-18 14:07 ` [PATCH v3 2/2] ALSA: oss: Keep emulation modules loaded until object teardown Cen Zhang
  0 siblings, 2 replies; 4+ messages in thread
From: Cen Zhang @ 2026-10-18 14:07 UTC (permalink / raw)
  To: perex, tiwai
  Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming, zzzccc427

OSS module unload can race recursive proc entry registration and
notifier callbacks, leaving references to freed entries or unloaded
module code.

Keep the OSS PCM and mixer modules loaded until their objects are
torn down, and serialize notifier traversal and callback withdrawal.

---
Changes in v3:
- Split the PCM core unregister traversal lock into patch 1 and the OSS
  module lifecycle changes into patch 2, as requested by Takashi.
- Rename the PCM OSS boolean module_ref to registered; its lifetime still
  spans DISCONNECT until final unregister.
- Rebase onto current sound for-next; no additional code changes beyond the
  flag rename.

Changes in v2:
- Follow Takashi's suggestion to pin OSS modules until object teardown.
- Track successful PCM pins across disconnect and unregistered cleanup.
- Protect final PCM notifier traversal and mixer callback withdrawal across
  the last module reference drop.
- Keep card enumeration in the OSS module and prevent duplicate mixer
  REGISTER notifications from replacing a disconnected adapter.
- Leave partial HDA codec teardown for a separate fix.

Link to v1: https://lore.kernel.org/r/pm-pmb-26d66390a25fec9fad6c-v2-743c9b89c0f0d761eda1@gmail.com
Link to v2: https://lore.kernel.org/r/pm-pmb-26d66390a25fec9fad6c-v4-a7d8a5d3db28c5bca90a@gmail.com

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

* [PATCH v3 1/2] ALSA: pcm: Protect the unregister notifier traversal
  2026-10-18 14:07 [PATCH v3 0/2] ALSA: Fix OSS teardown lifetime handling Cen Zhang
@ 2026-10-18 14:07 ` Cen Zhang
  2026-10-19  8:13   ` Takashi Iwai
  2026-10-18 14:07 ` [PATCH v3 2/2] ALSA: oss: Keep emulation modules loaded until object teardown Cen Zhang
  1 sibling, 1 reply; 4+ messages in thread
From: Cen Zhang @ 2026-10-18 14:07 UTC (permalink / raw)
  To: perex, tiwai
  Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming, zzzccc427

snd_pcm_free() traverses snd_pcm_notify_list and calls n_unregister
without register_mutex, although snd_pcm_notify() takes that mutex to
add and remove notifiers. PCM disconnect releases the mutex before the
free callback runs, so it does not protect this later traversal.

With no open OSS file, normal snd-pcm-oss removal can overlap PCM card
teardown, for example when unbinding snd-dummy:

  PCM card teardown                     snd-pcm-oss unload
  -----------------                     ------------------
  snd_pcm_dev_disconnect()
    remove PCM under register_mutex
    unlock register_mutex
  snd_pcm_free()
    fetch notifier
                                        snd_pcm_notify(..., 1)
                                          unlink notifier under mutex
                                        free module storage
    read notifier->n_unregister
    invoke callback

The free path can then read the notifier from released module storage
or call its unloaded code.

Hold register_mutex across the unregister traversal and its callbacks.
Notifier removal must wait until both have finished. This also protects
the traversal if a callback releases the last module reference: module
exit cannot unlink its notifier until the free path releases the mutex.
Drop the mutex before private_free and the remaining stream cleanup.

Fixes: c461482c8072 ("[ALSA] Unregister device files at disconnection")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 sound/core/pcm.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index c133f23418fe..35ece386d1a2 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -853,8 +853,10 @@ static int snd_pcm_free(struct snd_pcm *pcm)
 {
 	if (!pcm)
 		return 0;
-	if (!pcm->internal)
-		pcm_call_notify(pcm, n_unregister);
+	if (!pcm->internal) {
+		scoped_guard(mutex, &register_mutex)
+			pcm_call_notify(pcm, n_unregister);
+	}
 	if (pcm->private_free)
 		pcm->private_free(pcm);
 	snd_pcm_lib_preallocate_free_for_all(pcm);

base-commit: f6e8d5ebde5ceb7d3c0eef6e9e4d82862b503ade
-- 
2.43.0



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

* [PATCH v3 2/2] ALSA: oss: Keep emulation modules loaded until object teardown
  2026-10-18 14:07 [PATCH v3 0/2] ALSA: Fix OSS teardown lifetime handling Cen Zhang
  2026-10-18 14:07 ` [PATCH v3 1/2] ALSA: pcm: Protect the unregister notifier traversal Cen Zhang
@ 2026-10-18 14:07 ` Cen Zhang
  1 sibling, 0 replies; 4+ messages in thread
From: Cen Zhang @ 2026-10-18 14:07 UTC (permalink / raw)
  To: perex, tiwai
  Cc: linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming, zzzccc427

OSS proc entries and notifier callbacks can remain in use without an
open OSS device file. Normal snd-pcm-oss removal can therefore delete
an existing PCM's OSS proc entry while late HDA codec binding repeats
snd_card_register() on the same live card:

  OSS PCM module unload                 Card registration
  ---------------------                 -----------------
  snd_info_free_entry(entry)
    proc_remove(entry->p)
    clear entry->p
                                        snd_info_card_register()
                                          republish linked entry
    unlink and free entry

The new proc node retains a pointer to freed storage. Mixer OSS removal
can likewise release an adapter while a card notification still uses it.

Hold a snd-pcm-oss module reference for each PCM registered with its
notifier and a snd-mixer-oss reference for each card adapter. Release
them at final unregister or FREE, after proc and adapter cleanup. Normal
module unload can no longer initiate removal while these objects exist.

Track PCM OSS registration with a separate bool registered. Unlike
oss.reg, this state also covers unmapped PCMs and remains set across
DISCONNECT until final unregister. BUILD-state and creation-error cleanup
must not drop a reference when registration never acquired one. The PCM
notifier's register_mutex serialization keeps the final put ahead of
notifier removal and module teardown until dispatch has finished.

For mixer OSS, serialize callback publication, dispatch and withdrawal
in the core so module exit waits for callbacks, including one dropping
the last reference, to return. Keep card enumeration in the OSS module,
with card references released after dispatch drops the mutex. Ignore
duplicate REGISTER notifications so a delayed registration cannot
replace an adapter retained across DISCONNECT and leak its module pin.

Partial HDA codec teardown can still remove an individual PCM while the
card remains registered; its object-lifetime handling is separate.

Fixes: 2471b6c80a70 ("ALSA: info: Register proc entries recursively, too")
Suggested-by: Takashi Iwai <tiwai@suse.de>
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
 include/sound/core.h       |  3 ++-
 include/sound/pcm_oss.h    |  1 +
 sound/core/init.c          | 29 +++++++++++++++++++++--------
 sound/core/oss/mixer_oss.c | 36 ++++++++++++++++++++----------------
 sound/core/oss/pcm_oss.c   | 12 ++++++++++++
 5 files changed, 56 insertions(+), 25 deletions(-)

diff --git a/include/sound/core.h b/include/sound/core.h
index 657b12e1e95b..d6584967fd49 100644
--- a/include/sound/core.h
+++ b/include/sound/core.h
@@ -301,7 +301,8 @@ int snd_card_locked(int card);
 #define SND_MIXER_OSS_NOTIFY_REGISTER	0
 #define SND_MIXER_OSS_NOTIFY_DISCONNECT	1
 #define SND_MIXER_OSS_NOTIFY_FREE	2
-extern int (*snd_mixer_oss_notify_callback)(struct snd_card *card, int cmd);
+void snd_mixer_oss_set_notify(int (*callback)(struct snd_card *card, int cmd));
+void snd_mixer_oss_notify(struct snd_card *card, int cmd);
 #endif
 
 int snd_card_new(struct device *parent, int idx, const char *xid,
diff --git a/include/sound/pcm_oss.h b/include/sound/pcm_oss.h
index c96f23c7755b..43f765300031 100644
--- a/include/sound/pcm_oss.h
+++ b/include/sound/pcm_oss.h
@@ -70,6 +70,7 @@ struct snd_pcm_oss_stream {
 struct snd_pcm_oss {
 	int reg;
 	unsigned int reg_mask;
+	bool registered;
 };
 
 #endif /* __SOUND_PCM_OSS_H */
diff --git a/sound/core/init.c b/sound/core/init.c
index 9deb5e920d0d..d518cb838777 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -82,8 +82,24 @@ static int module_slot_match(struct module *module, int idx)
 }
 
 #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
-int (*snd_mixer_oss_notify_callback)(struct snd_card *card, int free_flag);
-EXPORT_SYMBOL(snd_mixer_oss_notify_callback);
+static DEFINE_MUTEX(mixer_oss_notify_mutex);
+static int (*mixer_oss_notify_callback)(struct snd_card *card, int cmd);
+
+void snd_mixer_oss_notify(struct snd_card *card, int cmd)
+{
+	guard(mutex)(&mixer_oss_notify_mutex);
+	if (mixer_oss_notify_callback)
+		mixer_oss_notify_callback(card, cmd);
+}
+EXPORT_SYMBOL(snd_mixer_oss_notify);
+
+void snd_mixer_oss_set_notify(int (*callback)(struct snd_card *, int))
+{
+	/* Wait for callbacks, including a final module_put(), to return. */
+	guard(mutex)(&mixer_oss_notify_mutex);
+	mixer_oss_notify_callback = callback;
+}
+EXPORT_SYMBOL(snd_mixer_oss_set_notify);
 #endif
 
 static int check_empty_slot(struct module *module, int slot)
@@ -508,8 +524,7 @@ void snd_card_disconnect(struct snd_card *card)
 	/* at this point, they cannot respond to any calls except release() */
 
 #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
-	if (snd_mixer_oss_notify_callback)
-		snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_DISCONNECT);
+	snd_mixer_oss_notify(card, SND_MIXER_OSS_NOTIFY_DISCONNECT);
 #endif
 
 	/* notify all devices that we are disconnected */
@@ -567,8 +582,7 @@ static int snd_card_do_free(struct snd_card *card)
 
 	card->releasing = true;
 #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
-	if (snd_mixer_oss_notify_callback)
-		snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_FREE);
+	snd_mixer_oss_notify(card, SND_MIXER_OSS_NOTIFY_FREE);
 #endif
 	snd_device_free_all(card);
 	kfree(card->components);
@@ -912,8 +926,7 @@ int snd_card_register(struct snd_card *card)
 		return err;
 
 #if IS_ENABLED(CONFIG_SND_MIXER_OSS)
-	if (snd_mixer_oss_notify_callback)
-		snd_mixer_oss_notify_callback(card, SND_MIXER_OSS_NOTIFY_REGISTER);
+	snd_mixer_oss_notify(card, SND_MIXER_OSS_NOTIFY_REGISTER);
 #endif
 	return 0;
 }
diff --git a/sound/core/oss/mixer_oss.c b/sound/core/oss/mixer_oss.c
index f1fa42e634fc..a51bc68473e6 100644
--- a/sound/core/oss/mixer_oss.c
+++ b/sound/core/oss/mixer_oss.c
@@ -1309,13 +1309,22 @@ static int snd_mixer_oss_free1(void *private)
 static int snd_mixer_oss_notify_handler(struct snd_card *card, int cmd)
 {
 	struct snd_mixer_oss *mixer;
+	int err;
 
 	if (cmd == SND_MIXER_OSS_NOTIFY_REGISTER) {
-		int idx, err;
+		int idx;
+
+		/* Module loading and card registration can both notify us. */
+		if (card->mixer_oss)
+			return 0;
 
 		mixer = kzalloc_objs(*mixer, 2);
 		if (mixer == NULL)
 			return -ENOMEM;
+		if (!try_module_get(THIS_MODULE)) {
+			kfree(mixer);
+			return -ENODEV;
+		}
 		mutex_init(&mixer->reg_mutex);
 		err = snd_register_oss_device(SNDRV_OSS_DEVICE_TYPE_MIXER,
 					      card, 0,
@@ -1325,6 +1334,7 @@ static int snd_mixer_oss_notify_handler(struct snd_card *card, int cmd)
 				"unable to register OSS mixer device %i:%i\n",
 				card->number, 0);
 			kfree(mixer);
+			module_put(THIS_MODULE);
 			return err;
 		}
 		mixer->oss_dev_alloc = 1;
@@ -1358,7 +1368,10 @@ static int snd_mixer_oss_notify_handler(struct snd_card *card, int cmd)
 		if (cmd == SND_MIXER_OSS_NOTIFY_DISCONNECT)
 			return 0;
 		snd_mixer_oss_proc_done(mixer);
-		return snd_mixer_oss_free1(mixer);
+		err = snd_mixer_oss_free1(mixer);
+		if (!err)
+			module_put(THIS_MODULE);
+		return err;
 	}
 	return 0;
 }
@@ -1367,12 +1380,12 @@ static int __init alsa_mixer_oss_init(void)
 {
 	struct snd_card *card;
 	int idx;
-	
-	snd_mixer_oss_notify_callback = snd_mixer_oss_notify_handler;
+
+	snd_mixer_oss_set_notify(snd_mixer_oss_notify_handler);
 	for (idx = 0; idx < SNDRV_CARDS; idx++) {
 		card = snd_card_ref(idx);
 		if (card) {
-			snd_mixer_oss_notify_handler(card, SND_MIXER_OSS_NOTIFY_REGISTER);
+			snd_mixer_oss_notify(card, SND_MIXER_OSS_NOTIFY_REGISTER);
 			snd_card_unref(card);
 		}
 	}
@@ -1381,17 +1394,8 @@ static int __init alsa_mixer_oss_init(void)
 
 static void __exit alsa_mixer_oss_exit(void)
 {
-	struct snd_card *card;
-	int idx;
-
-	snd_mixer_oss_notify_callback = NULL;
-	for (idx = 0; idx < SNDRV_CARDS; idx++) {
-		card = snd_card_ref(idx);
-		if (card) {
-			snd_mixer_oss_notify_handler(card, SND_MIXER_OSS_NOTIFY_FREE);
-			snd_card_unref(card);
-		}
-	}
+	/* Live adapters hold module references and prevent normal unload. */
+	snd_mixer_oss_set_notify(NULL);
 }
 
 module_init(alsa_mixer_oss_init)
diff --git a/sound/core/oss/pcm_oss.c b/sound/core/oss/pcm_oss.c
index bd19328dca9e..e8df27d09b97 100644
--- a/sound/core/oss/pcm_oss.c
+++ b/sound/core/oss/pcm_oss.c
@@ -3151,6 +3151,12 @@ static void register_oss_dsp(struct snd_pcm *pcm, int index)
 
 static int snd_pcm_oss_register_minor(struct snd_pcm *pcm)
 {
+	if (pcm->oss.registered)
+		return 0;
+	if (!try_module_get(THIS_MODULE))
+		return -ENODEV;
+	pcm->oss.registered = true;
+
 	pcm->oss.reg = 0;
 	if (dsp_map[pcm->card->number] == (int)pcm->device) {
 		char name[128];
@@ -3205,8 +3211,14 @@ static int snd_pcm_oss_disconnect_minor(struct snd_pcm *pcm)
 
 static int snd_pcm_oss_unregister_minor(struct snd_pcm *pcm)
 {
+	/* BUILD-state and creation-error cleanup need not have registered us. */
+	if (!pcm->oss.registered)
+		return 0;
+
 	snd_pcm_oss_disconnect_minor(pcm);
 	snd_pcm_oss_proc_done(pcm);
+	pcm->oss.registered = false;
+	module_put(THIS_MODULE);
 	return 0;
 }
 
-- 
2.43.0


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

* Re: [PATCH v3 1/2] ALSA: pcm: Protect the unregister notifier traversal
  2026-10-18 14:07 ` [PATCH v3 1/2] ALSA: pcm: Protect the unregister notifier traversal Cen Zhang
@ 2026-10-19  8:13   ` Takashi Iwai
  0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2026-10-19  8:13 UTC (permalink / raw)
  To: Cen Zhang
  Cc: perex, tiwai, linux-sound, linux-kernel, tiwai, baijiaju1990, jjzuming

On Sun, 18 Oct 2026 16:07:26 +0200,
Cen Zhang wrote:
> 
> snd_pcm_free() traverses snd_pcm_notify_list and calls n_unregister
> without register_mutex, although snd_pcm_notify() takes that mutex to
> add and remove notifiers. PCM disconnect releases the mutex before the
> free callback runs, so it does not protect this later traversal.
> 
> With no open OSS file, normal snd-pcm-oss removal can overlap PCM card
> teardown, for example when unbinding snd-dummy:
> 
>   PCM card teardown                     snd-pcm-oss unload
>   -----------------                     ------------------
>   snd_pcm_dev_disconnect()
>     remove PCM under register_mutex
>     unlock register_mutex
>   snd_pcm_free()
>     fetch notifier
>                                         snd_pcm_notify(..., 1)
>                                           unlink notifier under mutex
>                                         free module storage
>     read notifier->n_unregister
>     invoke callback
> 
> The free path can then read the notifier from released module storage
> or call its unloaded code.
> 
> Hold register_mutex across the unregister traversal and its callbacks.
> Notifier removal must wait until both have finished. This also protects
> the traversal if a callback releases the last module reference: module
> exit cannot unlink its notifier until the free path releases the mutex.
> Drop the mutex before private_free and the remaining stream cleanup.
> 
> Fixes: c461482c8072 ("[ALSA] Unregister device files at disconnection")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

Applied both patches to for-next branch now.  Thanks.


Takashi

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

end of thread, other threads:[~2026-10-19  8:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-18 14:07 [PATCH v3 0/2] ALSA: Fix OSS teardown lifetime handling Cen Zhang
2026-10-18 14:07 ` [PATCH v3 1/2] ALSA: pcm: Protect the unregister notifier traversal Cen Zhang
2026-10-19  8:13   ` Takashi Iwai
2026-10-18 14:07 ` [PATCH v3 2/2] ALSA: oss: Keep emulation modules loaded until object teardown Cen Zhang

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®