[PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Cen Zhang]

A closing PCM substream can remain in its linked group after the
driver close callback has freed its private data. With two configured
dummy capture substreams on the same PCM device, linked with LINK,
PREPARE on the surviving substream can still visit the closing member:

  Final close of A                   PREPARE on linked B
  ----------------                   -------------------
  snd_pcm_drop(): group -> SETUP
  do_hw_free(A)
  dummy_pcm_close(A): free dpcm
                                     Take snd_pcm_link_rwsem for reading
                                     Walk the group, including A
                                     dummy_pcm_prepare(A)
                                       Read freed timer_ops
  pcm_release_private(A)
    snd_pcm_unlink(A)

Unlink at the start of final substream release, before dropping the
stream or releasing driver resources. The link write lock waits for
an existing group PREPARE, and unlink removes the closing member from
later group actions. Keep the existing stop synchronization in
do_hw_free() before the driver resources are freed.

Make unlink independent of the pcm_release callback so OSS substreams
are covered too. Remove the native-only unlink callback and its
registration, while retaining the callback hook for OSS cleanup.

The existing KASAN report below records the freed timer_ops read in
dummy_pcm_prepare().

KASAN report as below:

    BUG: KASAN: slab-use-after-free in dummy_pcm_prepare+0xb7/0xc0
    Read of size 8 at addr ffff888106b35f00 by task alsa_pcm_fixtur/491

    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? dummy_pcm_prepare+0xb7/0xc0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? dummy_pcm_prepare+0xb7/0xc0
     kasan_report+0xe0/0x110
     ? dummy_pcm_prepare+0xb7/0xc0
     dummy_pcm_prepare+0xb7/0xc0
     snd_pcm_do_prepare+0x71/0xb0
     snd_pcm_action_group+0x505/0xb30
     snd_pcm_action_nonatomic+0xab/0x150
     snd_pcm_common_ioctl+0x1bf5/0x3a50
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? do_vfs_ioctl+0x1f5/0x1280
     ? __pfx_do_vfs_ioctl+0x10/0x10
     ? ioctl_has_perm.constprop.0.isra.0+0x34f/0x500
     ? __pfx_snd_pcm_common_ioctl+0x10/0x10
     ? __fput+0x5ba/0xa60
     ? srso_alias_return_thunk+0x5/0xfbef5
     snd_pcm_ioctl+0x76/0xb0
     ? __pfx_snd_pcm_ioctl+0x10/0x10
     __x64_sys_ioctl+0x163/0x1d0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f
    RIP: 0033:0x41ee8d
    Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8 64 48 2b 04 25 28 00 00 00
    RSP: 002b:00007ffcfa72fb60 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
    RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 000000000041ee8d
    RDX: 0000000000000000 RSI: 0000000000004140 RDI: 0000000000000004
    RBP: 00007ffcfa72fbb0 R08: 00007ffcfa72fb98 R09: 0000000000000110
    R10: 0000000000000001 R11: 0000000000000246 R12: 00007ffcfa72fcd8
    R13: 00007ffcfa72fcf0 R14: 00000000004a8f28 R15: 0000000000000001
     </TASK>

    Allocated by task 488:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x610
     dummy_hrtimer_create+0x4a/0x170
     dummy_pcm_open+0xe7/0x730
     snd_pcm_open_substream+0x926/0x1580
     snd_pcm_open+0x28d/0x6d0
     snd_pcm_capture_open+0x82/0xe0
     snd_open+0x1ec/0x3e0
     chrdev_open+0x21f/0x660
     do_dentry_open+0x5c8/0x1300
     vfs_open+0x87/0x390
     path_openat+0x2460/0x3cd0
     do_file_open+0x21e/0x470
     do_sys_openat2+0xf4/0x1b0
     __x64_sys_openat+0x136/0x1e0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 490:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x307/0x580
     dummy_hrtimer_free+0x72/0xe0
     dummy_pcm_close+0x9f/0xd0
     snd_pcm_release_substream.part.0+0x104/0x310
     snd_pcm_release+0x106/0x240
     __fput+0x39f/0xa60
     fput_close_sync+0xff/0x200
     __x64_sys_close+0x8c/0xf0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f
    ==================================================================

Fixes: 1576274d3028 ("[ALSA] Fix Oops of PCM OSS emulation")
Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

---
Changes in v2:
- Replace dummy-specific deferred freeing with Takashi Iwai's PCM core
  release ordering.
- Unlink unconditionally on final substream release before DROP and driver
  close, covering OSS callbacks too; keep the existing sync-stop in
  do_hw_free().
- Update Fixes to the PCM core commit that moved unlink after driver close.

Link to v1: https://lore.kernel.org/r/pm-pmb-9c80f858ccc13b3de84d-v6-d4059ea2910fe5430f5f@gmail.com

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index defbb2977efe9dcd8d48b934c3944d3fe159bcd6..c28d259463a11891893c352216fadcd5d9484f8b 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2792,18 +2792,14 @@ static int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static void pcm_release_private(struct snd_pcm_substream *substream)
-{
-	if (snd_pcm_stream_linked(substream))
-		snd_pcm_unlink(substream);
-}
-
 void snd_pcm_release_substream(struct snd_pcm_substream *substream)
 {
 	substream->ref_count--;
 	if (substream->ref_count > 0)
 		return;
 
+	snd_pcm_unlink(substream);
+
 	snd_pcm_drop(substream);
 	if (substream->hw_opened) {
 		if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
@@ -2887,8 +2883,6 @@ static int snd_pcm_open_file(struct file *file,
 		return -ENOMEM;
 	}
 	pcm_file->substream = substream;
-	if (substream->ref_count == 1)
-		substream->pcm_release = pcm_release_private;
 	file->private_data = pcm_file;
 
 	return 0;

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Takashi Iwai]

On Wed, 14 Oct 2026 12:52:15 +0200,
Cen Zhang wrote:
> 
> A closing PCM substream can remain in its linked group after the
> driver close callback has freed its private data. With two configured
> dummy capture substreams on the same PCM device, linked with LINK,
> PREPARE on the surviving substream can still visit the closing member:
> 
>   Final close of A                   PREPARE on linked B
>   ----------------                   -------------------
>   snd_pcm_drop(): group -> SETUP
>   do_hw_free(A)
>   dummy_pcm_close(A): free dpcm
>                                      Take snd_pcm_link_rwsem for reading
>                                      Walk the group, including A
>                                      dummy_pcm_prepare(A)
>                                        Read freed timer_ops
>   pcm_release_private(A)
>     snd_pcm_unlink(A)
> 
> Unlink at the start of final substream release, before dropping the
> stream or releasing driver resources. The link write lock waits for
> an existing group PREPARE, and unlink removes the closing member from
> later group actions. Keep the existing stop synchronization in
> do_hw_free() before the driver resources are freed.
> 
> Make unlink independent of the pcm_release callback so OSS substreams
> are covered too. Remove the native-only unlink callback and its
> registration, while retaining the callback hook for OSS cleanup.
> 
> The existing KASAN report below records the freed timer_ops read in
> dummy_pcm_prepare().
> 
> KASAN report as below:
> 
>     BUG: KASAN: slab-use-after-free in dummy_pcm_prepare+0xb7/0xc0
>     Read of size 8 at addr ffff888106b35f00 by task alsa_pcm_fixtur/491
> 
>     Call Trace:
>      <TASK>
>      dump_stack_lvl+0x93/0xd0
>      print_report+0xce/0x630
>      ? dummy_pcm_prepare+0xb7/0xc0
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? __virt_addr_valid+0x20d/0x410
>      ? dummy_pcm_prepare+0xb7/0xc0
>      kasan_report+0xe0/0x110
>      ? dummy_pcm_prepare+0xb7/0xc0
>      dummy_pcm_prepare+0xb7/0xc0
>      snd_pcm_do_prepare+0x71/0xb0
>      snd_pcm_action_group+0x505/0xb30
>      snd_pcm_action_nonatomic+0xab/0x150
>      snd_pcm_common_ioctl+0x1bf5/0x3a50
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      ? do_vfs_ioctl+0x1f5/0x1280
>      ? __pfx_do_vfs_ioctl+0x10/0x10
>      ? ioctl_has_perm.constprop.0.isra.0+0x34f/0x500
>      ? __pfx_snd_pcm_common_ioctl+0x10/0x10
>      ? __fput+0x5ba/0xa60
>      ? srso_alias_return_thunk+0x5/0xfbef5
>      snd_pcm_ioctl+0x76/0xb0
>      ? __pfx_snd_pcm_ioctl+0x10/0x10
>      __x64_sys_ioctl+0x163/0x1d0
>      do_syscall_64+0x115/0x6a0
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
>     RIP: 0033:0x41ee8d
>     Code: 04 25 28 00 00 00 48 89 45 c8 31 c0 48 8d 45 10 c7 45 b0 10 00 00 00 48 89 45 b8 48 8d 45 d0 48 89 45 c0 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1a 48 8b 45 c8 64 48 2b 04 25 28 00 00 00
>     RSP: 002b:00007ffcfa72fb60 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
>     RAX: ffffffffffffffda RBX: 0000000000000004 RCX: 000000000041ee8d
>     RDX: 0000000000000000 RSI: 0000000000004140 RDI: 0000000000000004
>     RBP: 00007ffcfa72fbb0 R08: 00007ffcfa72fb98 R09: 0000000000000110
>     R10: 0000000000000001 R11: 0000000000000246 R12: 00007ffcfa72fcd8
>     R13: 00007ffcfa72fcf0 R14: 00000000004a8f28 R15: 0000000000000001
>      </TASK>
> 
>     Allocated by task 488:
>      kasan_save_stack+0x33/0x60
>      kasan_save_track+0x14/0x30
>      __kasan_kmalloc+0xaa/0xb0
>      __kmalloc_cache_noprof+0x251/0x610
>      dummy_hrtimer_create+0x4a/0x170
>      dummy_pcm_open+0xe7/0x730
>      snd_pcm_open_substream+0x926/0x1580
>      snd_pcm_open+0x28d/0x6d0
>      snd_pcm_capture_open+0x82/0xe0
>      snd_open+0x1ec/0x3e0
>      chrdev_open+0x21f/0x660
>      do_dentry_open+0x5c8/0x1300
>      vfs_open+0x87/0x390
>      path_openat+0x2460/0x3cd0
>      do_file_open+0x21e/0x470
>      do_sys_openat2+0xf4/0x1b0
>      __x64_sys_openat+0x136/0x1e0
>      do_syscall_64+0x115/0x6a0
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
> 
>     Freed by task 490:
>      kasan_save_stack+0x33/0x60
>      kasan_save_track+0x14/0x30
>      kasan_save_free_info+0x3b/0x60
>      __kasan_slab_free+0x5f/0x80
>      kfree+0x307/0x580
>      dummy_hrtimer_free+0x72/0xe0
>      dummy_pcm_close+0x9f/0xd0
>      snd_pcm_release_substream.part.0+0x104/0x310
>      snd_pcm_release+0x106/0x240
>      __fput+0x39f/0xa60
>      fput_close_sync+0xff/0x200
>      __x64_sys_close+0x8c/0xf0
>      do_syscall_64+0x115/0x6a0
>      entry_SYSCALL_64_after_hwframe+0x77/0x7f
>     ==================================================================
> 
> Fixes: 1576274d3028 ("[ALSA] Fix Oops of PCM OSS emulation")
> Suggested-by: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

Applied to for-next branch now.

A problem with racy link removal Sashiko suggested is an interesting
case to be revisited, but it's no regression and can be triggered
independently from this change.


Takashi

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Cen Zhang]

Hi Takashi,

>
> Applied to for-next branch now.
>
> A problem with racy link removal Sashiko suggested is an interesting
> case to be revisited, but it's no regression and can be triggered
> independently from this change.
>
>
> Takashi

I saw that. The drain/unlink case looks like a real concurrency issue.
I'll try to
address it with a separate patch.

Best regards,
Cen Zhang

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Takashi Iwai]

On Thu, 15 Oct 2026 04:07:10 +0200,
Cen Zhang wrote:
> 
> Hi Takashi,
> 
> >
> > Applied to for-next branch now.
> >
> > A problem with racy link removal Sashiko suggested is an interesting
> > case to be revisited, but it's no regression and can be triggered
> > independently from this change.
> >
> >
> > Takashi
> 
> I saw that. The drain/unlink case looks like a real concurrency issue.
> I'll try to
> address it with a separate patch.

The patch I submitted yesterday is such an attempt:
  https://lore.kernel.org/20261014141212.316223-1-tiwai@suse.de

Unfortunately it couldn't be reviewed by Sashiko, as it's based on my
latest tree.  And now I noticed that there is still a potential race,
and fixed now.  Will resubmit v2 patch.


thanks,

Takashi

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Cen Zhang]

Hi Takashi,

>
> The patch I submitted yesterday is such an attempt:
>   https://lore.kernel.org/20261014141212.316223-1-tiwai@suse.de
>
> Unfortunately it couldn't be reviewed by Sashiko, as it's based on my
> latest tree.  And now I noticed that there is still a potential race,
> and fixed now.  Will resubmit v2 patch.
>
>
> thanks,
>
> Takashi

Thanks for the update. I’ll take a look at the v2.

Best regards,
Cen Zhang

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Cen Zhang]

Hi Takashi,

Takashi Iwai <tiwai@suse.de> 于2026年10月15日周四 15:18写道：
>
> The patch I submitted yesterday is such an attempt:
>   https://lore.kernel.org/20261014141212.316223-1-tiwai@suse.de
>
> Unfortunately it couldn't be reviewed by Sashiko, as it's based on my
> latest tree.  And now I noticed that there is still a potential race,
> and fixed now.  Will resubmit v2 patch.
>
>
> thanks,
>
> Takashi

After read the v2 patch. I also have a minor concern: is there still a
lifetime window between snd_refcount_put() and snd_refcount_sync()?


Suppose drain(A) has pinned B's runtime, and close(B) has completed
unlink and drop. Could the following happen on two CPUs?

  drain(A)                         close(B)
  --------                         --------
  finish_wait(&rt->sleep, &wait);
  snd_refcount_put(&rt->refcnt):
    decrement count to zero
                                   snd_refcount_sync(&rt->refcnt);
                                     /* sees zero and returns */
                                   ...
                                   snd_pcm_detach_substream(B);
                                     kfree(rt);
    wake_up(&rt->refcnt.waiter);

Here rt is B's runtime. Since wait_event() can return on its initial
condition check, could the final wake_up() access the embedded
waitqueue after rt has been freed? A holds its own stream lock at
this point, while B has already been unlinked.

Best regards,
Cen Zhang

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Takashi Iwai]

On Thu, 15 Oct 2026 17:35:30 +0200,
Cen Zhang wrote:
> 
> Hi Takashi,
> 
> Takashi Iwai <tiwai@suse.de> 于2026年10月15日周四 15:18写道：
> >
> > The patch I submitted yesterday is such an attempt:
> >   https://lore.kernel.org/20261014141212.316223-1-tiwai@suse.de
> >
> > Unfortunately it couldn't be reviewed by Sashiko, as it's based on my
> > latest tree.  And now I noticed that there is still a potential race,
> > and fixed now.  Will resubmit v2 patch.
> >
> >
> > thanks,
> >
> > Takashi
> 
> After read the v2 patch. I also have a minor concern: is there still a
> lifetime window between snd_refcount_put() and snd_refcount_sync()?
> 
> 
> Suppose drain(A) has pinned B's runtime, and close(B) has completed
> unlink and drop. Could the following happen on two CPUs?
> 
>   drain(A)                         close(B)
>   --------                         --------
>   finish_wait(&rt->sleep, &wait);
>   snd_refcount_put(&rt->refcnt):
>     decrement count to zero
>                                    snd_refcount_sync(&rt->refcnt);
>                                      /* sees zero and returns */
>                                    ...
>                                    snd_pcm_detach_substream(B);
>                                      kfree(rt);
>     wake_up(&rt->refcnt.waiter);
> 
> Here rt is B's runtime. Since wait_event() can return on its initial
> condition check, could the final wake_up() access the embedded
> waitqueue after rt has been freed? A holds its own stream lock at
> this point, while B has already been unlinked.

Yes, and it's covered by another patch:
  https://lore.kernel.org/20261015112256.3902-1-tiwai@suse.de


thanks,

Takashi

Re: [PATCH v2] ALSA: pcm: Unlink substreams before releasing them

[Cen Zhang]

Hi Takashi,

>
> Yes, and it's covered by another patch:
>   https://lore.kernel.org/20261015112256.3902-1-tiwai@suse.de
>
>
> thanks,
>
> Takashi

Sorry for missing that. That patch looks good to me. Feel free to add:

Reviewed-by: Cen Zhang <zzzccc427@gmail.com>

Best regards,
Cen Zhang