sound/core/pcm_timer.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-)
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
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
© 2016 - 2026 Red Hat, Inc.