[PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Cen Zhang]

The hrtimer record must remain alive while a linked PCM action can
visit its substream. dummy_pcm_close() frees that record before
pcm_release_private() unlinks the substream. runtime->private_data still
points to the freed allocation.

With two configured dummy capture substreams linked and the hrtimer
backend selected, final close drops the group to SETUP. A surviving
peer can then issue PREPARE before the closing substream is unlinked:

  Target close                       Peer PREPARE
  snd_pcm_drop(): group -> SETUP
  do_hw_free()
  dummy_pcm_close(): kfree(dpcm)
                                     snd_pcm_action_nonatomic()
                                       read-lock snd_pcm_link_rwsem
                                     dummy_pcm_prepare()
                                       read dpcm->timer_ops
  pcm_release_private()
    snd_pcm_unlink(): write-lock

PREPARE holds the link read semaphore and the members' buffer mutexes,
but driver close holds neither across the free. Unlink's write lock
comes too late to protect the record. The ops-pointer read can use freed
memory and dispatch through an invalid function pointer. If PREPARE
already obtained the record before close, dummy_hrtimer_prepare() can
instead write period_time after it has been freed. The report below
shows the dispatch-time read and the ensuing fault; it does not show
the later write.

Defer freeing the hrtimer record to runtime->private_free. Runtime
detach follows unlink, which waits for existing linked PREPARE actions
and prevents new ones from visiting the target. Cancel the embedded
timer in the destructor before releasing its storage. Register the
destructor from the backend free callback so the open error path also
releases the record through runtime detach.

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: c631d03c6873 ("ALSA: dummy - Support high-res timer mode")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

---

diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
index 6b0a06e5c87638264e5d22deda1fe4bc028febe7..ae7cef9d74819a68dcf951a21768dccf38c7c4ad 100644
--- a/sound/drivers/dummy.c
+++ b/sound/drivers/dummy.c
@@ -476,13 +476,20 @@ static int dummy_hrtimer_create(struct snd_pcm_substream *substream)
 	return 0;
 }
 
-static void dummy_hrtimer_free(struct snd_pcm_substream *substream)
+static void dummy_hrtimer_runtime_free(struct snd_pcm_runtime *runtime)
 {
-	struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;
+	struct dummy_hrtimer_pcm *dpcm = runtime->private_data;
 
+	hrtimer_cancel(&dpcm->timer);
 	kfree(dpcm);
 }
 
+static void dummy_hrtimer_free(struct snd_pcm_substream *substream)
+{
+	/* Linked PREPARE can still use the data until unlink. */
+	substream->runtime->private_free = dummy_hrtimer_runtime_free;
+}
+
 static const struct dummy_timer_ops dummy_hrtimer_ops = {
 	.create =	dummy_hrtimer_create,
 	.free =		dummy_hrtimer_free,

Re: [PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Takashi Iwai]

On Wed, 14 Oct 2026 07:25:31 +0200,
Cen Zhang wrote:
> 
> The hrtimer record must remain alive while a linked PCM action can
> visit its substream. dummy_pcm_close() frees that record before
> pcm_release_private() unlinks the substream. runtime->private_data still
> points to the freed allocation.
> 
> With two configured dummy capture substreams linked and the hrtimer
> backend selected, final close drops the group to SETUP. A surviving
> peer can then issue PREPARE before the closing substream is unlinked:
> 
>   Target close                       Peer PREPARE
>   snd_pcm_drop(): group -> SETUP
>   do_hw_free()
>   dummy_pcm_close(): kfree(dpcm)
>                                      snd_pcm_action_nonatomic()
>                                        read-lock snd_pcm_link_rwsem
>                                      dummy_pcm_prepare()
>                                        read dpcm->timer_ops
>   pcm_release_private()
>     snd_pcm_unlink(): write-lock
> 
> PREPARE holds the link read semaphore and the members' buffer mutexes,
> but driver close holds neither across the free. Unlink's write lock
> comes too late to protect the record. The ops-pointer read can use freed
> memory and dispatch through an invalid function pointer. If PREPARE
> already obtained the record before close, dummy_hrtimer_prepare() can
> instead write period_time after it has been freed. The report below
> shows the dispatch-time read and the ensuing fault; it does not show
> the later write.
> 
> Defer freeing the hrtimer record to runtime->private_free. Runtime
> detach follows unlink, which waits for existing linked PREPARE actions
> and prevents new ones from visiting the target. Cancel the embedded
> timer in the destructor before releasing its storage. Register the
> destructor from the backend free callback so the open error path also
> releases the record through runtime detach.
> 
> 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: c631d03c6873 ("ALSA: dummy - Support high-res timer mode")
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> 
> ---
> 
> diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> index 6b0a06e5c87638264e5d22deda1fe4bc028febe7..ae7cef9d74819a68dcf951a21768dccf38c7c4ad 100644
> --- a/sound/drivers/dummy.c
> +++ b/sound/drivers/dummy.c
> @@ -476,13 +476,20 @@ static int dummy_hrtimer_create(struct snd_pcm_substream *substream)
>  	return 0;
>  }
>  
> -static void dummy_hrtimer_free(struct snd_pcm_substream *substream)
> +static void dummy_hrtimer_runtime_free(struct snd_pcm_runtime *runtime)
>  {
> -	struct dummy_hrtimer_pcm *dpcm = substream->runtime->private_data;
> +	struct dummy_hrtimer_pcm *dpcm = runtime->private_data;
>  
> +	hrtimer_cancel(&dpcm->timer);
>  	kfree(dpcm);
>  }
>  
> +static void dummy_hrtimer_free(struct snd_pcm_substream *substream)
> +{
> +	/* Linked PREPARE can still use the data until unlink. */
> +	substream->runtime->private_free = dummy_hrtimer_runtime_free;
> +}
> +
>  static const struct dummy_timer_ops dummy_hrtimer_ops = {
>  	.create =	dummy_hrtimer_create,
>  	.free =		dummy_hrtimer_free,

Hmm, it's not clear how this happens.  It looks like a typical
obscuration by AI reports...

Basically dummy_hrtimer_sync_stop should handled the hrtimer cancel,
but maybe the problem is rather its lack at releasing?  If so, the
patch below should cover instead.


thanks,

Takashi

-- 8< --
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2301,6 +2301,8 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
 	result = snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
 	/* runtime->control->appl_ptr = runtime->status->hw_ptr; */
 
+	snd_pcm_sync_stop(substream, true);
+
 	return result;
 }
 

Re: [PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Cen Zhang]

Hi Takashi,

>
> Hmm, it's not clear how this happens.  It looks like a typical
> obscuration by AI reports...
>
> Basically dummy_hrtimer_sync_stop should handled the hrtimer cancel,
> but maybe the problem is rather its lack at releasing?  If so, the
> patch below should cover instead.
>
>
> thanks,
>
> Takashi

Thank you for looking at this. My commit message did not explain the
scenario clearly. Sorry for the confusion.

The observed failure involves a PREPARE ioctl on a surviving linked
substream. The test opens two dummy capture substreams, A and B, with
the hrtimer backend selected, configures both with HW_PARAMS, and links
them with LINK. It then performs the final close of A while issuing
PREPARE on B. It does not issue START.

On the current for-next source, the close path is:

  snd_pcm_release_substream(A)
    snd_pcm_drop(A)
    do_hw_free(A)
      snd_pcm_sync_stop(A, true)
    dummy_pcm_close(A)
      dummy_hrtimer_free(A)
        kfree(dpcm)
    pcm_release_private(A)
      snd_pcm_unlink(A)
    snd_pcm_detach_substream(A)

So the close path already synchronizes the timer before freeing dpcm.
The gap is between driver close and unlink. snd_pcm_drop() leaves the
linked group in SETUP. The release path calls do_hw_free() directly;
unlike the HW_FREE ioctl wrapper, that helper does not change the
runtime state to OPEN. A therefore remains linked and in a state that
snd_pcm_pre_prepare() accepts until the later unlink.

The relevant ordering is:

  Target A final close              PREPARE on linked peer B
  --------------------              ------------------------
  snd_pcm_drop(): group -> SETUP
  do_hw_free(): synchronize timer
  dummy_pcm_close(): free dpcm
                                    Take link read lock
                                    Walk group, including A
                                    Accept A's SETUP state
                                    dummy_pcm_prepare(A)
                                      Read freed timer_ops
  pcm_release_private()
    snd_pcm_unlink(): link write lock

PREPARE holds the link read lock and the participating runtime buffer
mutexes. Driver close does not hold those locks across the free. Timer
cancellation waits for timer callbacks, but does not wait for this
separate PREPARE action. The later unlink is what prevents another
group action from visiting A.

>
> -- 8< --
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -2301,6 +2301,8 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
>         result = snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
>         /* runtime->control->appl_ptr = runtime->status->hw_ptr; */
>
> +       snd_pcm_sync_stop(substream, true);
> +
>         return result;
>  }
>

Synchronizing a pending hrtimer callback before release is sensible.
For the linked PREPARE ordering above, though, I think an additional
lifetime step may still be needed. PREPARE on B can start after the
extra sync_stop() has completed and after A's close has freed dpcm,
while A is still linked. It can then reach A's prepare callback as
shown above. Waiting for timer callbacks alone does not exclude that
later group action.

Best regards,
Cen Zhang

Re: [PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Takashi Iwai]

On Wed, 14 Oct 2026 10:35:24 +0200,
Cen Zhang wrote:
> 
> Hi Takashi,
> 
> >
> > Hmm, it's not clear how this happens.  It looks like a typical
> > obscuration by AI reports...
> >
> > Basically dummy_hrtimer_sync_stop should handled the hrtimer cancel,
> > but maybe the problem is rather its lack at releasing?  If so, the
> > patch below should cover instead.
> >
> >
> > thanks,
> >
> > Takashi
> 
> Thank you for looking at this. My commit message did not explain the
> scenario clearly. Sorry for the confusion.
> 
> The observed failure involves a PREPARE ioctl on a surviving linked
> substream. The test opens two dummy capture substreams, A and B, with
> the hrtimer backend selected, configures both with HW_PARAMS, and links
> them with LINK. It then performs the final close of A while issuing
> PREPARE on B. It does not issue START.
> 
> On the current for-next source, the close path is:
> 
>   snd_pcm_release_substream(A)
>     snd_pcm_drop(A)
>     do_hw_free(A)
>       snd_pcm_sync_stop(A, true)
>     dummy_pcm_close(A)
>       dummy_hrtimer_free(A)
>         kfree(dpcm)
>     pcm_release_private(A)
>       snd_pcm_unlink(A)
>     snd_pcm_detach_substream(A)
> 
> So the close path already synchronizes the timer before freeing dpcm.
> The gap is between driver close and unlink. snd_pcm_drop() leaves the
> linked group in SETUP. The release path calls do_hw_free() directly;
> unlike the HW_FREE ioctl wrapper, that helper does not change the
> runtime state to OPEN. A therefore remains linked and in a state that
> snd_pcm_pre_prepare() accepts until the later unlink.
> 
> The relevant ordering is:
> 
>   Target A final close              PREPARE on linked peer B
>   --------------------              ------------------------
>   snd_pcm_drop(): group -> SETUP
>   do_hw_free(): synchronize timer
>   dummy_pcm_close(): free dpcm
>                                     Take link read lock
>                                     Walk group, including A
>                                     Accept A's SETUP state
>                                     dummy_pcm_prepare(A)
>                                       Read freed timer_ops
>   pcm_release_private()
>     snd_pcm_unlink(): link write lock
> 
> PREPARE holds the link read lock and the participating runtime buffer
> mutexes. Driver close does not hold those locks across the free. Timer
> cancellation waits for timer callbacks, but does not wait for this
> separate PREPARE action. The later unlink is what prevents another
> group action from visiting A.
> 
> >
> > -- 8< --
> > --- a/sound/core/pcm_native.c
> > +++ b/sound/core/pcm_native.c
> > @@ -2301,6 +2301,8 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream)
> >         result = snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP);
> >         /* runtime->control->appl_ptr = runtime->status->hw_ptr; */
> >
> > +       snd_pcm_sync_stop(substream, true);
> > +
> >         return result;
> >  }
> >
> 
> Synchronizing a pending hrtimer callback before release is sensible.
> For the linked PREPARE ordering above, though, I think an additional
> lifetime step may still be needed. PREPARE on B can start after the
> extra sync_stop() has completed and after A's close has freed dpcm,
> while A is still linked. It can then reach A's prepare callback as
> shown above. Waiting for timer callbacks alone does not exclude that
> later group action.

That's not what I didn't understand: do both A and B use the same
device?  The release shouldn't happen until all detached, so the
prepare and the release can't happen concurrently for the same device.

If A and B are for different devices, they don't interfere with each
other in the case of dummy driver.


Takashi

Re: [PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Cen Zhang]

Hi Takashi,

>
> That's not what I didn't understand: do both A and B use the same
> device?  The release shouldn't happen until all detached, so the
> prepare and the release can't happen concurrently for the same device.
>
> If A and B are for different devices, they don't interfere with each
> other in the case of dummy driver.
>

Yes, A and B use the same PCM device. The test opens
/dev/snd/pcmC0D0c twice, and SNDRV_PCM_IOCTL_INFO reports:

  A: card=0, device=0, subdevice=0, capture
  B: card=0, device=0, subdevice=1, capture

They are separate substreams with separate runtimes. With the hrtimer
backend selected, dummy_hrtimer_create() allocates a separate dpcm for
each runtime. The test configures both with HW_PARAMS and explicitly
links them with SNDRV_PCM_IOCTL_LINK.

Sorry, I should have been clearer about what is being released. The
PCM device remains alive throughout. The object being freed is A's
runtime->private_data, from A's driver close callback.

snd_pcm_release_substream() checks A's own ref_count. When A's last
file reference is closed, it calls A's driver close before unlinking
A, even while B is still open:

  Final close of A                  PREPARE ioctl on B
  ----------------                  ------------------
  snd_pcm_drop(): group -> SETUP
  do_hw_free(A)
  dummy_pcm_close(A)
    kfree(A's dpcm)
                                    snd_pcm_action_nonatomic(B)
                                      Take link read lock
                                      Walk group: B and A
                                      snd_pcm_do_prepare(A)
                                        dummy_pcm_prepare(A)
                                          Read freed timer_ops
  pcm_release_private(A)
    snd_pcm_unlink(A)
  snd_pcm_detach_substream(A)

B reaches A through the PCM core's linked-group iteration:
snd_pcm_action_group() invokes the prepare action for each member.
A is still linked and in SETUP at that point, which the prepare state
check accepts. The two dummy runtimes do not share their private data;
the group action explicitly invokes A's callback with A's substream.

The final close holds pcm->open_mutex, but the PREPARE path does not
take that mutex. PREPARE holds snd_pcm_link_rwsem for reading and the
members' buffer mutexes; driver close does not hold those locks across
the free. The link write lock is taken at the later unlink.

That is the overlap I meant: A's own backend data can be freed while
A is still reachable by a PREPARE action issued through B.

Best regards,
Cen Zhang

Re: [PATCH] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Takashi Iwai]

On Wed, 14 Oct 2026 11:33:54 +0200,
Cen Zhang wrote:
> 
> Hi Takashi,
> 
> >
> > That's not what I didn't understand: do both A and B use the same
> > device?  The release shouldn't happen until all detached, so the
> > prepare and the release can't happen concurrently for the same device.
> >
> > If A and B are for different devices, they don't interfere with each
> > other in the case of dummy driver.
> >
> 
> Yes, A and B use the same PCM device. The test opens
> /dev/snd/pcmC0D0c twice, and SNDRV_PCM_IOCTL_INFO reports:
> 
>   A: card=0, device=0, subdevice=0, capture
>   B: card=0, device=0, subdevice=1, capture
> 
> They are separate substreams with separate runtimes. With the hrtimer
> backend selected, dummy_hrtimer_create() allocates a separate dpcm for
> each runtime. The test configures both with HW_PARAMS and explicitly
> links them with SNDRV_PCM_IOCTL_LINK.
> 
> Sorry, I should have been clearer about what is being released. The
> PCM device remains alive throughout. The object being freed is A's
> runtime->private_data, from A's driver close callback.
> 
> snd_pcm_release_substream() checks A's own ref_count. When A's last
> file reference is closed, it calls A's driver close before unlinking
> A, even while B is still open:
> 
>   Final close of A                  PREPARE ioctl on B
>   ----------------                  ------------------
>   snd_pcm_drop(): group -> SETUP
>   do_hw_free(A)
>   dummy_pcm_close(A)
>     kfree(A's dpcm)
>                                     snd_pcm_action_nonatomic(B)
>                                       Take link read lock
>                                       Walk group: B and A
>                                       snd_pcm_do_prepare(A)
>                                         dummy_pcm_prepare(A)
>                                           Read freed timer_ops
>   pcm_release_private(A)
>     snd_pcm_unlink(A)
>   snd_pcm_detach_substream(A)
> 
> B reaches A through the PCM core's linked-group iteration:
> snd_pcm_action_group() invokes the prepare action for each member.
> A is still linked and in SETUP at that point, which the prepare state
> check accepts. The two dummy runtimes do not share their private data;
> the group action explicitly invokes A's callback with A's substream.
> 
> The final close holds pcm->open_mutex, but the PREPARE path does not
> take that mutex. PREPARE holds snd_pcm_link_rwsem for reading and the
> members' buffer mutexes; driver close does not hold those locks across
> the free. The link write lock is taken at the later unlink.
> 
> That is the overlap I meant: A's own backend data can be freed while
> A is still reachable by a PREPARE action issued through B.

OK, it's then about unlinking in addition to the sync-stop.

I believe that we should rearrange the call order of unlink; it should
be rather called at the very beginning and unconditionally.

So the revised patch is something like below.

(BTW, this will cover also a case where the link may remain when
linked to an OSS substream.)


Takashi

-- 8< --
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -2792,19 +2792,17 @@ 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);
+	snd_pcm_sync_stop(substream, true);
+
 	if (substream->hw_opened) {
 		if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
 			do_hw_free(substream);
@@ -2887,8 +2885,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] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Takashi Iwai]

On Wed, 14 Oct 2026 11:54:37 +0200,
Takashi Iwai wrote:
> 
> On Wed, 14 Oct 2026 11:33:54 +0200,
> Cen Zhang wrote:
> > 
> > Hi Takashi,
> > 
> > >
> > > That's not what I didn't understand: do both A and B use the same
> > > device?  The release shouldn't happen until all detached, so the
> > > prepare and the release can't happen concurrently for the same device.
> > >
> > > If A and B are for different devices, they don't interfere with each
> > > other in the case of dummy driver.
> > >
> > 
> > Yes, A and B use the same PCM device. The test opens
> > /dev/snd/pcmC0D0c twice, and SNDRV_PCM_IOCTL_INFO reports:
> > 
> >   A: card=0, device=0, subdevice=0, capture
> >   B: card=0, device=0, subdevice=1, capture
> > 
> > They are separate substreams with separate runtimes. With the hrtimer
> > backend selected, dummy_hrtimer_create() allocates a separate dpcm for
> > each runtime. The test configures both with HW_PARAMS and explicitly
> > links them with SNDRV_PCM_IOCTL_LINK.
> > 
> > Sorry, I should have been clearer about what is being released. The
> > PCM device remains alive throughout. The object being freed is A's
> > runtime->private_data, from A's driver close callback.
> > 
> > snd_pcm_release_substream() checks A's own ref_count. When A's last
> > file reference is closed, it calls A's driver close before unlinking
> > A, even while B is still open:
> > 
> >   Final close of A                  PREPARE ioctl on B
> >   ----------------                  ------------------
> >   snd_pcm_drop(): group -> SETUP
> >   do_hw_free(A)
> >   dummy_pcm_close(A)
> >     kfree(A's dpcm)
> >                                     snd_pcm_action_nonatomic(B)
> >                                       Take link read lock
> >                                       Walk group: B and A
> >                                       snd_pcm_do_prepare(A)
> >                                         dummy_pcm_prepare(A)
> >                                           Read freed timer_ops
> >   pcm_release_private(A)
> >     snd_pcm_unlink(A)
> >   snd_pcm_detach_substream(A)
> > 
> > B reaches A through the PCM core's linked-group iteration:
> > snd_pcm_action_group() invokes the prepare action for each member.
> > A is still linked and in SETUP at that point, which the prepare state
> > check accepts. The two dummy runtimes do not share their private data;
> > the group action explicitly invokes A's callback with A's substream.
> > 
> > The final close holds pcm->open_mutex, but the PREPARE path does not
> > take that mutex. PREPARE holds snd_pcm_link_rwsem for reading and the
> > members' buffer mutexes; driver close does not hold those locks across
> > the free. The link write lock is taken at the later unlink.
> > 
> > That is the overlap I meant: A's own backend data can be freed while
> > A is still reachable by a PREPARE action issued through B.
> 
> OK, it's then about unlinking in addition to the sync-stop.
> 
> I believe that we should rearrange the call order of unlink; it should
> be rather called at the very beginning and unconditionally.
> 
> So the revised patch is something like below.

... and after re-reading the code, I noticed that the
snd_pcm_sync_stop() call is redundant.  After snd_pcm_drop(), the code
continues as:

	if (substream->hw_opened) {
		if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
			do_hw_free(substream);
		substream->ops->close(substream);
		substream->hw_opened = 0;
	}

and do_hw_free() does call snd_pcm_sync_stop()

static int do_hw_free(struct snd_pcm_substream *substream)
{
	int result = 0;

	snd_pcm_sync_stop(substream, true);
	if (substream->ops->hw_free)
	.....

That said, the only missing piece is the rearrangement of unlinking
call order.  The hrtimer was a red herring, after all, and rather the
access to a PCM substream that is being released via the PCM link was
the real problem.

Please verify whether the fix below works for your case.


thanks,

Takashi


-- 8< --
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index a845f242a163..4fea3f6df682 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] ALSA: dummy: Defer hrtimer data freeing until runtime detach

[Cen Zhang]

Hi Takashi,

Takashi Iwai <tiwai@suse.de> 于2026年10月14日周三 18:07写道：
>
> On Wed, 14 Oct 2026 11:54:37 +0200,
> Takashi Iwai wrote:
> >
> > On Wed, 14 Oct 2026 11:33:54 +0200,
> > Cen Zhang wrote:
> > >
> > > Hi Takashi,
> > >
> > > >
> > > > That's not what I didn't understand: do both A and B use the same
> > > > device?  The release shouldn't happen until all detached, so the
> > > > prepare and the release can't happen concurrently for the same device.
> > > >
> > > > If A and B are for different devices, they don't interfere with each
> > > > other in the case of dummy driver.
> > > >
> > >
> > > Yes, A and B use the same PCM device. The test opens
> > > /dev/snd/pcmC0D0c twice, and SNDRV_PCM_IOCTL_INFO reports:
> > >
> > >   A: card=0, device=0, subdevice=0, capture
> > >   B: card=0, device=0, subdevice=1, capture
> > >
> > > They are separate substreams with separate runtimes. With the hrtimer
> > > backend selected, dummy_hrtimer_create() allocates a separate dpcm for
> > > each runtime. The test configures both with HW_PARAMS and explicitly
> > > links them with SNDRV_PCM_IOCTL_LINK.
> > >
> > > Sorry, I should have been clearer about what is being released. The
> > > PCM device remains alive throughout. The object being freed is A's
> > > runtime->private_data, from A's driver close callback.
> > >
> > > snd_pcm_release_substream() checks A's own ref_count. When A's last
> > > file reference is closed, it calls A's driver close before unlinking
> > > A, even while B is still open:
> > >
> > >   Final close of A                  PREPARE ioctl on B
> > >   ----------------                  ------------------
> > >   snd_pcm_drop(): group -> SETUP
> > >   do_hw_free(A)
> > >   dummy_pcm_close(A)
> > >     kfree(A's dpcm)
> > >                                     snd_pcm_action_nonatomic(B)
> > >                                       Take link read lock
> > >                                       Walk group: B and A
> > >                                       snd_pcm_do_prepare(A)
> > >                                         dummy_pcm_prepare(A)
> > >                                           Read freed timer_ops
> > >   pcm_release_private(A)
> > >     snd_pcm_unlink(A)
> > >   snd_pcm_detach_substream(A)
> > >
> > > B reaches A through the PCM core's linked-group iteration:
> > > snd_pcm_action_group() invokes the prepare action for each member.
> > > A is still linked and in SETUP at that point, which the prepare state
> > > check accepts. The two dummy runtimes do not share their private data;
> > > the group action explicitly invokes A's callback with A's substream.
> > >
> > > The final close holds pcm->open_mutex, but the PREPARE path does not
> > > take that mutex. PREPARE holds snd_pcm_link_rwsem for reading and the
> > > members' buffer mutexes; driver close does not hold those locks across
> > > the free. The link write lock is taken at the later unlink.
> > >
> > > That is the overlap I meant: A's own backend data can be freed while
> > > A is still reachable by a PREPARE action issued through B.
> >
> > OK, it's then about unlinking in addition to the sync-stop.
> >
> > I believe that we should rearrange the call order of unlink; it should
> > be rather called at the very beginning and unconditionally.
> >
> > So the revised patch is something like below.
>
> ... and after re-reading the code, I noticed that the
> snd_pcm_sync_stop() call is redundant.  After snd_pcm_drop(), the code
> continues as:
>
>         if (substream->hw_opened) {
>                 if (substream->runtime->state != SNDRV_PCM_STATE_OPEN)
>                         do_hw_free(substream);
>                 substream->ops->close(substream);
>                 substream->hw_opened = 0;
>         }
>
> and do_hw_free() does call snd_pcm_sync_stop()
>
> static int do_hw_free(struct snd_pcm_substream *substream)
> {
>         int result = 0;
>
>         snd_pcm_sync_stop(substream, true);
>         if (substream->ops->hw_free)
>         .....
>
> That said, the only missing piece is the rearrangement of unlinking
> call order.  The hrtimer was a red herring, after all, and rather the
> access to a PCM substream that is being released via the PCM link was
> the real problem.
>
> Please verify whether the fix below works for your case.
>
>
> thanks,
>
> Takashi
>

This approach looks right and addresses this case. Thank you for your
patience and help.

>
> -- 8< --
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index a845f242a163..4fea3f6df682 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;

I'll rework the patch based on this approach.

Best regards,
Cen Zhang