All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ALSA: aloop: Keep the timer source alive until card release
@ 2026-10-19  9:06 Cen Zhang
  2026-10-19 18:06 ` Takashi Iwai
  0 siblings, 1 reply; 2+ messages in thread
From: Cen Zhang @ 2026-10-19  9:06 UTC (permalink / raw)
  To: perex, tiwai; +Cc: linux-sound, linux-kernel, baijiaju1990, jjzuming, zzzccc427

The timer_source string must remain valid while PCM opens can select
and parse it. loopback_set_timer_source() allocates the string with
parent-device devres, which can release it while the card is still
registered.

A successful nonempty proc replacement after snd_card_register() adds
the string allocation after trigger_card_free. Unbinding the platform
device while snd_aloop remains loaded releases devres in reverse order.
An open for an unused cable can run after the string is freed and before
the card-free action disconnects the card:

  Device unbind                      PCM open
  devres_release_all()
    free replacement string
                                     snd_pcm_open()
                                       snd_card_file_add()
                                       loopback_open()
                                         loopback_snd_timer_open()
                                           loopback_parse_timer_id()
    trigger_card_free()
      snd_card_free()
        snd_card_disconnect()

The parser can then read freed memory. KASAN has reported a one-byte
use-after-free read in strchr() from loopback_snd_timer_open(). The
cable_lock mutex serializes explicit replacements with opens, but
implicit devres release does not take it. The card and module references
held by PCM open do not retain this separate devres allocation.

Use kstrdup() and release the current string from card->private_free.
PCM admission already retains the card until each file is removed, and
card disconnect drains proc callbacks before final card release. This
keeps the string alive across admitted PCM opens and preserves the
cable_lock serialization of replacements. Install the callback before
the initial allocation so probe failures also release the string.

Fixes: b072e65aadd6 ("ALSA: aloop: Allocate resources with device-managed APIs")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Assisted-by: LLM

---

diff --git a/sound/drivers/aloop.c b/sound/drivers/aloop.c
index 0aaa64b189daef377f080a63665a9be9036e9d9c..aeb8a57684bd2176449bbfa2abe3383a3420ea0b 100644
--- a/sound/drivers/aloop.c
+++ b/sound/drivers/aloop.c
@@ -1897,13 +1897,10 @@ static int loopback_cable_proc_new(struct loopback *loopback, int cidx)
 static void loopback_set_timer_source(struct loopback *loopback,
 				      const char *value)
 {
-	if (loopback->timer_source) {
-		devm_kfree(loopback->card->dev, loopback->timer_source);
-		loopback->timer_source = NULL;
-	}
+	kfree(loopback->timer_source);
+	loopback->timer_source = NULL;
 	if (value && *value)
-		loopback->timer_source = devm_kstrdup(loopback->card->dev,
-						      value, GFP_KERNEL);
+		loopback->timer_source = kstrdup(value, GFP_KERNEL);
 }
 
 static void print_timer_source_info(struct snd_info_entry *entry,
@@ -1934,6 +1931,13 @@ static int loopback_timer_source_proc_new(struct loopback *loopback)
 				    change_timer_source_info);
 }
 
+static void loopback_private_free(struct snd_card *card)
+{
+	struct loopback *loopback = card->private_data;
+
+	kfree(loopback->timer_source);
+}
+
 static int loopback_probe(struct platform_device *devptr)
 {
 	struct snd_card *card;
@@ -1959,6 +1963,7 @@ static int loopback_probe(struct platform_device *devptr)
 		pcm_substreams[dev] = MAX_PCM_SUBSTREAMS;
 	
 	loopback->card = card;
+	card->private_free = loopback_private_free;
 	loopback_set_timer_source(loopback, timer_source[dev]);
 
 	mutex_init(&loopback->cable_lock);

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

* Re: [PATCH] ALSA: aloop: Keep the timer source alive until card release
  2026-10-19  9:06 [PATCH] ALSA: aloop: Keep the timer source alive until card release Cen Zhang
@ 2026-10-19 18:06 ` Takashi Iwai
  0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-10-19 18:06 UTC (permalink / raw)
  To: Cen Zhang; +Cc: perex, tiwai, linux-sound, linux-kernel, baijiaju1990, jjzuming

On Mon, 19 Oct 2026 11:06:13 +0200,
Cen Zhang wrote:
> 
> The timer_source string must remain valid while PCM opens can select
> and parse it. loopback_set_timer_source() allocates the string with
> parent-device devres, which can release it while the card is still
> registered.
> 
> A successful nonempty proc replacement after snd_card_register() adds
> the string allocation after trigger_card_free. Unbinding the platform
> device while snd_aloop remains loaded releases devres in reverse order.
> An open for an unused cable can run after the string is freed and before
> the card-free action disconnects the card:
> 
>   Device unbind                      PCM open
>   devres_release_all()
>     free replacement string
>                                      snd_pcm_open()
>                                        snd_card_file_add()
>                                        loopback_open()
>                                          loopback_snd_timer_open()
>                                            loopback_parse_timer_id()
>     trigger_card_free()
>       snd_card_free()
>         snd_card_disconnect()
> 
> The parser can then read freed memory. KASAN has reported a one-byte
> use-after-free read in strchr() from loopback_snd_timer_open(). The
> cable_lock mutex serializes explicit replacements with opens, but
> implicit devres release does not take it. The card and module references
> held by PCM open do not retain this separate devres allocation.
> 
> Use kstrdup() and release the current string from card->private_free.
> PCM admission already retains the card until each file is removed, and
> card disconnect drains proc callbacks before final card release. This
> keeps the string alive across admitted PCM opens and preserves the
> cable_lock serialization of replacements. Install the callback before
> the initial allocation so probe failures also release the string.
> 
> Fixes: b072e65aadd6 ("ALSA: aloop: Allocate resources with device-managed APIs")
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> Assisted-by: LLM

Applied now.  Thanks.


Takashi

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

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

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19  9:06 [PATCH] ALSA: aloop: Keep the timer source alive until card release Cen Zhang
2026-10-19 18:06 ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.