* [PATCH] ALSA: hda: Serialize virtual-master hook registration
@ 2026-10-13 7:54 Cen Zhang
2026-10-13 9:53 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Cen Zhang @ 2026-10-13 7:54 UTC (permalink / raw)
To: perex, tiwai, 2022090917019, hemanth.selam, nfraprado,
songxiebing, sammiee5311, kees, cassiogabrielcontato
Cc: linux-sound, linux-kernel, baijiaju1990, jjzuming, zzzccc427
An HDA virtual-master hook must have its private argument initialized
before a control callback can invoke it. snd_hda_add_vmaster_hook()
installs the trampoline through snd_ctl_add_vmaster_hook(), which stores
master->hook before master->hook_private_data without serialization.
When a codec with a vmaster mute hook binds to an already registered
card, snd_hda_gen_build_controls() publishes and initializes the Master
Playback Switch before installing the hook. A native write that changes
the switch can then run in the following order:
1. The codec bind path publishes and initializes the master control in
__snd_hda_add_vmaster(), then calls snd_hda_add_vmaster_hook().
2. The hook setter stores master->hook, but has not yet stored its
private argument.
3. SNDRV_CTL_IOCTL_ELEM_WRITE takes controls_rwsem and calls
master_put() while the setter remains between those stores.
4. master_put() sees the callback and invokes vmaster_hook() with a
NULL argument. The trampoline dereferences it before the setter
can complete publication.
The write lock held by snd_ctl_elem_write() does not exclude the hook
setter, so this can cause a NULL pointer dereference. Take the same
controls_rwsem for writing in snd_hda_add_vmaster_hook() so the callback
and its argument are installed together with respect to native control
callbacks. Release it when registration returns, before the caller
synchronizes the hook.
KASAN report as below:
Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
CPU: 1 UID: 0 PID: 499 Comm: alsa-vmaster-tr Not tainted 7.2.0-rc6-pmb-bt-functional-v1+ #1 PREEMPT(lazy)
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
RIP: 0010:vmaster_hook+0x35/0xb0
Code: f5 53 48 89 fb 48 83 ec 08 e8 57 f0 9a fc eb 50 e8 50 f0 9a fc 48 8d 7b 08 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75 59 48 8d 7b 10 48 8b 43 08 48 ba 00 00 00 00 00 fc
RSP: 0018:ffff888117647cd0 EFLAGS: 00010212
RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff84e9ff50
RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000008
RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000001
R10: ffffffff89427457 R11: ffff888115ad1d00 R12: ffffffff84e9ff30
R13: ffff888114943ca0 R14: 0000000000000000 R15: 0000000000000001
FS: 00007fe8167b3780(0000) GS:ffff88826b7e3000(0000) knlGS:0000000000000000
CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 0000557c8c2842a8 CR3: 000000011579f005 CR4: 0000000000770ef0
PKRU: 55555554
Call Trace:
<TASK>
master_put+0x20e/0x2b0
snd_ctl_elem_write+0x37f/0x5e0
snd_ctl_ioctl+0x997/0xdd0
? __pfx_snd_ctl_ioctl+0x10/0x10
? srso_alias_return_thunk+0x5/0xfbef5
? selinux_file_ioctl+0xaa/0x280
? __pfx_snd_ctl_ioctl+0x10/0x10
__x64_sys_ioctl+0x163/0x1d0
do_syscall_64+0x115/0x6a0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
RIP: 0033:0x7fe8168c291b
Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
RSP: 002b:00007fffab0cb150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007fe8168c291b
RDX: 00007fffab0cb310 RSI: 00000000c4c85513 RDI: 0000000000000003
RBP: 00007fffab0cb310 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
R13: 00007fffab0cb1c0 R14: 0000000000000001 R15: 0000557c4f71fd10
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
Fixes: ee52e56e7b12 ("ALSA: hda - Fix mute-LED fixed mode")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
diff --git a/sound/hda/common/codec.c b/sound/hda/common/codec.c
index 18838a45e6f72b838eace2d7b21d7a0a3b21017b..029e56ae78727c8cb3c3edbace8b27a5615c2a3c 100644
--- a/sound/hda/common/codec.c
+++ b/sound/hda/common/codec.c
@@ -2024,6 +2024,8 @@ int snd_hda_add_vmaster_hook(struct hda_codec *codec,
{
if (!hook->hook || !hook->sw_kctl)
return 0;
+ /* Serialize hook registration with control callbacks. */
+ guard(rwsem_write)(&codec->card->controls_rwsem);
hook->codec = codec;
snd_ctl_add_vmaster_hook(hook->sw_kctl, vmaster_hook, hook);
return 0;
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] ALSA: hda: Serialize virtual-master hook registration
2026-10-13 7:54 [PATCH] ALSA: hda: Serialize virtual-master hook registration Cen Zhang
@ 2026-10-13 9:53 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-10-13 9:53 UTC (permalink / raw)
To: Cen Zhang
Cc: perex, tiwai, 2022090917019, hemanth.selam, nfraprado,
songxiebing, sammiee5311, kees, cassiogabrielcontato, linux-sound,
linux-kernel, baijiaju1990, jjzuming
On Tue, 13 Oct 2026 09:54:55 +0200,
Cen Zhang wrote:
>
> An HDA virtual-master hook must have its private argument initialized
> before a control callback can invoke it. snd_hda_add_vmaster_hook()
> installs the trampoline through snd_ctl_add_vmaster_hook(), which stores
> master->hook before master->hook_private_data without serialization.
>
> When a codec with a vmaster mute hook binds to an already registered
> card, snd_hda_gen_build_controls() publishes and initializes the Master
> Playback Switch before installing the hook. A native write that changes
> the switch can then run in the following order:
>
> 1. The codec bind path publishes and initializes the master control in
> __snd_hda_add_vmaster(), then calls snd_hda_add_vmaster_hook().
> 2. The hook setter stores master->hook, but has not yet stored its
> private argument.
> 3. SNDRV_CTL_IOCTL_ELEM_WRITE takes controls_rwsem and calls
> master_put() while the setter remains between those stores.
> 4. master_put() sees the callback and invokes vmaster_hook() with a
> NULL argument. The trampoline dereferences it before the setter
> can complete publication.
>
> The write lock held by snd_ctl_elem_write() does not exclude the hook
> setter, so this can cause a NULL pointer dereference. Take the same
> controls_rwsem for writing in snd_hda_add_vmaster_hook() so the callback
> and its argument are installed together with respect to native control
> callbacks. Release it when registration returns, before the caller
> synchronizes the hook.
>
> KASAN report as below:
>
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000001: 0000 [#1] SMP KASAN NOPTI
> KASAN: null-ptr-deref in range [0x0000000000000008-0x000000000000000f]
> CPU: 1 UID: 0 PID: 499 Comm: alsa-vmaster-tr Not tainted 7.2.0-rc6-pmb-bt-functional-v1+ #1 PREEMPT(lazy)
> 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
> RIP: 0010:vmaster_hook+0x35/0xb0
> Code: f5 53 48 89 fb 48 83 ec 08 e8 57 f0 9a fc eb 50 e8 50 f0 9a fc 48 8d 7b 08 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <80> 3c 02 00 75 59 48 8d 7b 10 48 8b 43 08 48 ba 00 00 00 00 00 fc
> RSP: 0018:ffff888117647cd0 EFLAGS: 00010212
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffffff84e9ff50
> RDX: 0000000000000001 RSI: 0000000000000000 RDI: 0000000000000008
> RBP: 0000000000000001 R08: 0000000000000001 R09: 0000000000000001
> R10: ffffffff89427457 R11: ffff888115ad1d00 R12: ffffffff84e9ff30
> R13: ffff888114943ca0 R14: 0000000000000000 R15: 0000000000000001
> FS: 00007fe8167b3780(0000) GS:ffff88826b7e3000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000557c8c2842a8 CR3: 000000011579f005 CR4: 0000000000770ef0
> PKRU: 55555554
> Call Trace:
> <TASK>
> master_put+0x20e/0x2b0
> snd_ctl_elem_write+0x37f/0x5e0
> snd_ctl_ioctl+0x997/0xdd0
> ? __pfx_snd_ctl_ioctl+0x10/0x10
> ? srso_alias_return_thunk+0x5/0xfbef5
> ? selinux_file_ioctl+0xaa/0x280
> ? __pfx_snd_ctl_ioctl+0x10/0x10
> __x64_sys_ioctl+0x163/0x1d0
> do_syscall_64+0x115/0x6a0
> entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7fe8168c291b
> Code: 00 48 89 44 24 18 31 c0 48 8d 44 24 60 c7 04 24 10 00 00 00 48 89 44 24 08 48 8d 44 24 20 48 89 44 24 10 b8 10 00 00 00 0f 05 <89> c2 3d 00 f0 ff ff 77 1c 48 8b 44 24 18 64 48 2b 04 25 28 00 00
> RSP: 002b:00007fffab0cb150 EFLAGS: 00000246 ORIG_RAX: 0000000000000010
> RAX: ffffffffffffffda RBX: 0000000000000003 RCX: 00007fe8168c291b
> RDX: 00007fffab0cb310 RSI: 00000000c4c85513 RDI: 0000000000000003
> RBP: 00007fffab0cb310 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> R13: 00007fffab0cb1c0 R14: 0000000000000001 R15: 0000557c4f71fd10
> </TASK>
> Modules linked in:
> ---[ end trace 0000000000000000 ]---
>
> Fixes: ee52e56e7b12 ("ALSA: hda - Fix mute-LED fixed mode")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
>
> diff --git a/sound/hda/common/codec.c b/sound/hda/common/codec.c
> index 18838a45e6f72b838eace2d7b21d7a0a3b21017b..029e56ae78727c8cb3c3edbace8b27a5615c2a3c 100644
> --- a/sound/hda/common/codec.c
> +++ b/sound/hda/common/codec.c
> @@ -2024,6 +2024,8 @@ int snd_hda_add_vmaster_hook(struct hda_codec *codec,
> {
> if (!hook->hook || !hook->sw_kctl)
> return 0;
> + /* Serialize hook registration with control callbacks. */
> + guard(rwsem_write)(&codec->card->controls_rwsem);
> hook->codec = codec;
> snd_ctl_add_vmaster_hook(hook->sw_kctl, vmaster_hook, hook);
> return 0;
It's not nice to use such a low-level guard here in the upper driver.
So I guess it's better to move the rwsem guard into
snd_ctl_add_vmaster_hook(). But it'd need to pass the card pointer,
too.
Alternatively, you can rearrange the assignment in
snd_ctl_add_vmaster_hook() and put a barrier to assure the
hook_private_data store, too.
thanks,
Takashi
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-10-13 9:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-13 7:54 [PATCH] ALSA: hda: Serialize virtual-master hook registration Cen Zhang
2026-10-13 9:53 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox