* [PATCH] ALSA: pcm: set timer->private_data before registering the PCM timer
@ 2026-09-13 13:44 Nguyen Ngoc Thang
2026-09-13 16:39 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-13 13:44 UTC (permalink / raw)
To: Jaroslav Kysela, Takashi Iwai
Cc: Nguyen Ngoc Thang, linux-sound, linux-kernel,
syzbot+19da64013c46df87f971
snd_pcm_timer_init() calls snd_device_register() to link the new
struct snd_timer into the global timer list while it still carries
hw.c_resolution = snd_pcm_timer_resolution (and hw.start/hw.stop),
and only afterwards sets timer->private_data = substream.
Once the timer is on the list under register_mutex, a concurrent
reader can already reach it through the same mutex and invoke these
callbacks. /proc/asound/timers does this via c_resolution(), and
snd_timer_open()+snd_timer_start() reach start()/stop() the same way.
All three dereference timer->private_data, which for this brief
window is NULL, giving a NULL-pointer dereference:
substream = timer->private_data;
return substream->runtime ? ... // substream is NULL
Move the private_data/private_free assignment before
snd_device_register() so the timer is never visible on the list
without its private_data set. On the snd_device_register() failure
path, private_free() (snd_pcm_timer_free()) can now run, but it only
does substream->timer = NULL, which is already NULL at that point
since substream->timer is set to the new timer just once, after a
successful registration -- so the failure path stays safe.
Reported-by: syzbot+19da64013c46df87f971@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=19da64013c46df87f971
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
sound/core/pcm_timer.c | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/sound/core/pcm_timer.c b/sound/core/pcm_timer.c
index ab0e5bd70f8f..18bedd66435d 100644
--- a/sound/core/pcm_timer.c
+++ b/sound/core/pcm_timer.c
@@ -111,12 +111,15 @@ void snd_pcm_timer_init(struct snd_pcm_substream *substream)
snd_pcm_direction_name(substream->stream),
tid.card, tid.device, tid.subdevice);
timer->hw = snd_pcm_timer;
+ /* Set before registering: a concurrent reader can invoke our hw
+ * callbacks as soon as the timer is on the global list.
+ */
+ timer->private_data = substream;
+ timer->private_free = snd_pcm_timer_free;
if (snd_device_register(timer->card, timer) < 0) {
snd_device_free(timer->card, timer);
return;
}
- timer->private_data = substream;
- timer->private_free = snd_pcm_timer_free;
substream->timer = timer;
}
--
2.43.0
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH] ALSA: pcm: set timer->private_data before registering the PCM timer
2026-09-13 13:44 [PATCH] ALSA: pcm: set timer->private_data before registering the PCM timer Nguyen Ngoc Thang
@ 2026-09-13 16:39 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-09-13 16:39 UTC (permalink / raw)
To: Nguyen Ngoc Thang
Cc: Jaroslav Kysela, Takashi Iwai, linux-sound, linux-kernel,
syzbot+19da64013c46df87f971
On Sun, 13 Sep 2026 15:44:46 +0200,
Nguyen Ngoc Thang wrote:
>
> snd_pcm_timer_init() calls snd_device_register() to link the new
> struct snd_timer into the global timer list while it still carries
> hw.c_resolution = snd_pcm_timer_resolution (and hw.start/hw.stop),
> and only afterwards sets timer->private_data = substream.
>
> Once the timer is on the list under register_mutex, a concurrent
> reader can already reach it through the same mutex and invoke these
> callbacks. /proc/asound/timers does this via c_resolution(), and
> snd_timer_open()+snd_timer_start() reach start()/stop() the same way.
> All three dereference timer->private_data, which for this brief
> window is NULL, giving a NULL-pointer dereference:
>
> substream = timer->private_data;
> return substream->runtime ? ... // substream is NULL
>
> Move the private_data/private_free assignment before
> snd_device_register() so the timer is never visible on the list
> without its private_data set. On the snd_device_register() failure
> path, private_free() (snd_pcm_timer_free()) can now run, but it only
> does substream->timer = NULL, which is already NULL at that point
> since substream->timer is set to the new timer just once, after a
> successful registration -- so the failure path stays safe.
>
> Reported-by: syzbot+19da64013c46df87f971@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=19da64013c46df87f971
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
Applied now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-09-13 16:39 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-13 13:44 [PATCH] ALSA: pcm: set timer->private_data before registering the PCM timer Nguyen Ngoc Thang
2026-09-13 16:39 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®