[PATCH v2] ALSA: pcm: Serialize and check HW_REFINE ioctls

[Cen Zhang <zzzccc427@gmail.com>]

Hardware refinement must reject disconnected streams before entering
their runtime constraint rules. The native ioctl checks the state at
the outer dispatch, before copying the parameters, while the compat
HW_REFINE32 path only checks that the runtime exists. Both call the
unlocked refinement helper without the buffer access lock used by
HW_PARAMS.

The BUFFER_BYTES rule keeps a substream pointer as its private payload.
A compat request can still enter that rule after component disconnect.
If component free overlaps its execution, the following order permits
a use-after-free read of buffer_bytes_max:

  Component cleanup                 Compat HW_REFINE
  -----------------                 ----------------
  snd_pcm_dev_disconnect()
    Runtime -> DISCONNECTED
                                    Enter compat ioctl
                                    snd_pcm_hw_refine()
                                      Enter BUFFER_BYTES rule
  snd_device_free()
    snd_pcm_free_stream()
      Free substream
                                      Read buffer_bytes_max

Add a wrapper that acquires the existing buffer access lock and checks
the same allowed states and mmap conditions as HW_PARAMS before running
the refinement rules. Share the state checker with snd_pcm_hw_params()
and use the wrapper for native, compat and legacy PCM HW_REFINE ioctls.
This rejects the described entry after disconnect and serializes these
refinement calls with other buffer-locked parameter operations.

Keep the original snd_pcm_hw_refine() helper for existing internal
callers, including snd_pcm_hw_params() which already holds the buffer
lock, to avoid locking that mutex recursively.

KASAN report as below:

    BUG: KASAN: slab-use-after-free in snd_pcm_hw_rule_buffer_bytes_max+0x276/0x2a0
    Read of size 8 at addr ffff888111b36870 by task pmbd_alsa_reade/486

    CPU: 2 UID: 0 PID: 486 Comm: pmbd_alsa_reade Tainted: G           O        7.2.0-rc6-pmb-bt-functional-v1+ #1 PREEMPT(lazy)
    Tainted: [O]=OOT_MODULE
    Hardware name: QEMU Ubuntu 24.04 PC v2 (i440FX + PIIX, arch_caps fix, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
    Call Trace:
     <TASK>
     dump_stack_lvl+0x93/0xd0
     print_report+0xce/0x630
     ? snd_pcm_hw_rule_buffer_bytes_max+0x276/0x2a0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? __virt_addr_valid+0x20d/0x410
     ? snd_pcm_hw_rule_buffer_bytes_max+0x276/0x2a0
     kasan_report+0xe0/0x110
     ? snd_pcm_hw_rule_buffer_bytes_max+0x276/0x2a0
     snd_pcm_hw_rule_buffer_bytes_max+0x276/0x2a0
     ? snd_pcm_hw_rule_div+0x16b/0x200
     ? __pfx_snd_pcm_hw_rule_buffer_bytes_max+0x10/0x10
     ? constrain_params_by_rules+0x17a/0xfe0
     constrain_params_by_rules+0x506/0xfe0
     ? __pfx_constrain_params_by_rules+0x10/0x10
     ? __ia32_compat_sys_ioctl+0x289/0x2f0
     ? do_int80_emulation+0x14a/0x5e0
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? lock_release+0xc8/0x280
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? snd_interval_refine+0x284/0x470
     snd_pcm_hw_refine+0x768/0xcc0
     ? __pfx_snd_pcm_hw_refine+0x10/0x10
     ? _copy_from_user+0x51/0xa0
     snd_pcm_ioctl_hw_params_compat+0x138/0x280
     snd_pcm_ioctl_compat+0x4c5/0x7c0
     ? __pfx_snd_pcm_ioctl_compat+0x10/0x10
     ? srso_alias_return_thunk+0x5/0xfbef5
     ? selinux_file_ioctl+0xaa/0x280
     ? ksys_write+0x16b/0x200
     ? __pfx_snd_pcm_ioctl_compat+0x10/0x10
     __ia32_compat_sys_ioctl+0x289/0x2f0
     ? srso_alias_return_thunk+0x5/0xfbef5
     do_int80_emulation+0x14a/0x5e0
     asm_int80_emulation+0x1a/0x20
    RIP: 0023:0x8049084
    Code: 80 b8 04 00 00 00 bb 01 00 00 00 b9 43 a0 04 08 ba 2b 00 00 00 cd 80 b8 36 00 00 00 89 f3 b9 10 41 5c c2 ba b8 b0 04 08 cd 80 <85> c0 78 6c b8 04 00 00 00 bb 01 00 00 00 b9 27 a0 04 08 ba 1c 00
    RSP: 002b:00000000ffd35e40 EFLAGS: 00000246 ORIG_RAX: 0000000000000036
    RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00000000c25c4110
    RDX: 000000000804b0b8 RSI: 0000000000000003 RDI: 0000000000000004
    RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000000 R11: 0000000000000000 R12: 0000000000000000
    R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
     </TASK>

    Allocated by task 486:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     __kasan_kmalloc+0xaa/0xb0
     __kmalloc_cache_noprof+0x251/0x610
     snd_pcm_new_stream+0x98f/0x1350
     _snd_pcm_new+0x1d1/0x310
     snd_pcm_new+0x43/0x60
     0xffffffffc0401115
     do_one_initcall+0xf3/0x4e0
     do_init_module+0x281/0x8e0
     load_module+0x658c/0x7cc0
     init_module_from_file+0x15b/0x180
     idempotent_init_module+0x22d/0x770
     __x64_sys_finit_module+0xd2/0x160
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    Freed by task 494:
     kasan_save_stack+0x33/0x60
     kasan_save_track+0x14/0x30
     kasan_save_free_info+0x3b/0x60
     __kasan_slab_free+0x5f/0x80
     kfree+0x307/0x580
     snd_pcm_free_stream+0xcf/0x290
     snd_pcm_free+0xb1/0xf0
     __snd_device_free+0x1a1/0x490
     snd_device_free+0xcf/0x190
     case_owner_ioctl+0xb8/0x160 [pmbd_alsa_fixture]
     __x64_sys_ioctl+0x163/0x1d0
     do_syscall_64+0x115/0x6a0
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

    The buggy address belongs to the object at ffff888111b36800
     which belongs to the cache kmalloc-1k of size 1024
    The buggy address is located 112 bytes inside of
     freed 1024-byte region [ffff888111b36800, ffff888111b36c00)

    The buggy address belongs to the physical page:
    page: refcount:0 mapcount:0 mapping:0000000000000000 index:0xffff888111b35800 pfn:0x111b30
    head: order:3 mapcount:0 entire_mapcount:0 nr_pages_mapped:0 pincount:0
    flags: 0x200000000000240(workingset|head|node=0|zone=2)
    page_type: f5(slab)
    raw: 0200000000000240 ffff888100042dc0 ffffea0004218610 ffffea00042c6810
    raw: ffff888111b35800 000000000010000f 00000000f5000000 0000000000000000
    head: 0200000000000240 ffff888100042dc0 ffffea0004218610 ffffea00042c6810
    head: ffff888111b35800 000000000010000f 00000000f5000000 0000000000000000
    head: 0200000000000003 fffffffffffffe01 00000000ffffffff 00000000ffffffff
    head: 0000000000000000 0000000000000000 00000000ffffffff 0000000000000000
    page dumped because: kasan: bad access detected

    Memory state around the buggy address:
     ffff888111b36700: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
     ffff888111b36780: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
    >ffff888111b36800: fa fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
                                                                 ^
     ffff888111b36880: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
     ffff888111b36900: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
    ==================================================================

Suggested-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Assisted-by: LLM

---
Changes in v2:
- Use the buffer-lock and state-check wrapper for all three PCM HW_REFINE
  ioctl paths, as Takashi suggested.
- Share the HW_PARAMS state check and keep the original helper for existing
  locked callers.

Link to v1: https://lore.kernel.org/r/pm-pmb-b3f7b3445bb11aaa9a7b-v2-a514c1399f30f407d626@gmail.com

diff --git a/sound/core/pcm_compat.c b/sound/core/pcm_compat.c
index 55ecf87586c4ddc553a82997ddaf30a69ab6627a..e48f4b7b692f6e5da453c1072489a81e7104050c 100644
--- a/sound/core/pcm_compat.c
+++ b/sound/core/pcm_compat.c
@@ -252,7 +252,7 @@ static int snd_pcm_ioctl_hw_params_compat(struct snd_pcm_substream *substream,
 		return -EFAULT;
 
 	if (refine) {
-		err = snd_pcm_hw_refine(substream, data);
+		err = snd_pcm_hw_refine_with_lock(substream, data);
 		if (err < 0)
 			return err;
 		err = fixup_unreferenced_params(substream, data);
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index defbb2977efe9dcd8d48b934c3944d3fe159bcd6..20d5c7c7f56364a7d469559a9497e13a2aae9703 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -580,29 +580,6 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream,
 }
 EXPORT_SYMBOL(snd_pcm_hw_refine);
 
-static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream,
-				  struct snd_pcm_hw_params __user * _params)
-{
-	int err;
-	struct snd_pcm_hw_params *params __free(kfree) =
-		memdup_user(_params, sizeof(*params));
-
-	if (IS_ERR(params))
-		return PTR_ERR(params);
-
-	err = snd_pcm_hw_refine(substream, params);
-	if (err < 0)
-		return err;
-
-	err = fixup_unreferenced_params(substream, params);
-	if (err < 0)
-		return err;
-
-	if (copy_to_user(_params, params, sizeof(*params)))
-		return -EFAULT;
-	return 0;
-}
-
 static int period_to_usecs(struct snd_pcm_runtime *runtime)
 {
 	int usecs;
@@ -799,6 +776,22 @@ static void pcm_qos_req_update(struct snd_pcm_substream *substream, int usecs)
 					    usecs);
 }
 
+static int hw_params_state_check(struct snd_pcm_substream *substream)
+{
+	guard(pcm_stream_lock_irq)(substream);
+	switch (substream->runtime->state) {
+	case SNDRV_PCM_STATE_OPEN:
+	case SNDRV_PCM_STATE_SETUP:
+	case SNDRV_PCM_STATE_PREPARED:
+		if (!is_oss_stream(substream) &&
+		    atomic_read(&substream->mmap_count))
+			return -EBADFD;
+		return 0;
+	default:
+		return -EBADFD;
+	}
+}
+
 static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 			     struct snd_pcm_hw_params *params)
 {
@@ -813,20 +806,7 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream,
 	err = snd_pcm_buffer_access_lock(runtime);
 	if (err < 0)
 		return err;
-	scoped_guard(pcm_stream_lock_irq, substream) {
-		switch (runtime->state) {
-		case SNDRV_PCM_STATE_OPEN:
-		case SNDRV_PCM_STATE_SETUP:
-		case SNDRV_PCM_STATE_PREPARED:
-			if (!is_oss_stream(substream) &&
-			    atomic_read(&substream->mmap_count))
-				err = -EBADFD;
-			break;
-		default:
-			err = -EBADFD;
-			break;
-		}
-	}
+	err = hw_params_state_check(substream);
 	if (err)
 		goto unlock;
 
@@ -947,6 +927,57 @@ static int snd_pcm_hw_params_user(struct snd_pcm_substream *substream,
 	return err;
 }
 
+/*
+ * call snd_pcm_hw_refine() after buffer lock and sanity check;
+ * this one should be used for ioctls and other external paths instead of
+ * direct call of snd_pcm_hw_refine()
+ */
+static int snd_pcm_hw_refine_with_lock(struct snd_pcm_substream *substream,
+				       struct snd_pcm_hw_params *params)
+{
+	struct snd_pcm_runtime *runtime;
+	int err;
+
+	if (PCM_RUNTIME_CHECK(substream))
+		return -ENXIO;
+	runtime = substream->runtime;
+	err = snd_pcm_buffer_access_lock(runtime);
+	if (err < 0)
+		return err;
+	err = hw_params_state_check(substream);
+	if (err)
+		goto unlock;
+
+	err = snd_pcm_hw_refine(substream, params);
+
+ unlock:
+	snd_pcm_buffer_access_unlock(runtime);
+	return err;
+}
+
+static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream,
+				  struct snd_pcm_hw_params __user * _params)
+{
+	int err;
+	struct snd_pcm_hw_params *params __free(kfree) =
+		memdup_user(_params, sizeof(*params));
+
+	if (IS_ERR(params))
+		return PTR_ERR(params);
+
+	err = snd_pcm_hw_refine_with_lock(substream, params);
+	if (err < 0)
+		return err;
+
+	err = fixup_unreferenced_params(substream, params);
+	if (err < 0)
+		return err;
+
+	if (copy_to_user(_params, params, sizeof(*params)))
+		return -EFAULT;
+	return 0;
+}
+
 static int do_hw_free(struct snd_pcm_substream *substream)
 {
 	int result = 0;
@@ -4203,7 +4234,7 @@ static int snd_pcm_hw_refine_old_user(struct snd_pcm_substream *substream,
 	if (IS_ERR(oparams))
 		return PTR_ERR(oparams);
 	snd_pcm_hw_convert_from_old_params(params, oparams);
-	err = snd_pcm_hw_refine(substream, params);
+	err = snd_pcm_hw_refine_with_lock(substream, params);
 	if (err < 0)
 		return err;