| Message ID | tencent_AC7946DBB367A364897F43B2045A37625705@qq.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ALSA: timer: fix deadlock in _snd_pcm_stream_lock_irqsave | expand |
On Thu, 21 Mar 2024 03:22:24 +0100, Edward Adam Davis wrote: > > [Syzbot reported] > swapper/2/0 just changed the state of lock: > ffff88802a304110 (&group->lock#2){..-.}-{2:2}, at: _snd_pcm_stream_lock_irqsave+0xa0/0xd0 sound/core/pcm_native.c:170 > but this lock took another, SOFTIRQ-unsafe lock in the past: > (&timer->lock){+.+.}-{2:2} > > > and interrupts could create inverse lock ordering between them. > > > other info that might help us debug this: > Possible interrupt unsafe locking scenario: > > CPU0 CPU1 > ---- ---- > lock(&timer->lock); > local_irq_disable(); > lock(&group->lock#2); > lock(&timer->lock); > <Interrupt> > lock(&group->lock#2); > > *** DEADLOCK *** > [Fix] > Ensure that the context interrupt state is the same before and after using the > timer->lock. > > Fixes: beb45974dd49 ("ALSA: timer: Use guard() for locking") > Reported-and-tested-by: syzbot+18840ef96e57b83b7fea@syzkaller.appspotmail.com > Signed-off-by: Edward Adam Davis <eadavis@qq.com> This was already fixed in Linus tree commit 587d67fd929ad89801bcc429675bda90d53f6592. thanks, Takashi
diff --git a/sound/core/timer.c b/sound/core/timer.c index 15b07d09c4b7..c501faa30040 100644 --- a/sound/core/timer.c +++ b/sound/core/timer.c @@ -409,7 +409,7 @@ static void snd_timer_close_locked(struct snd_timer_instance *timeri, struct snd_timer *timer = timeri->timer; if (timer) { - guard(spinlock)(&timer->lock); + guard(spinlock_irqsave)(&timer->lock); timeri->flags |= SNDRV_TIMER_IFLG_DEAD; }
[Syzbot reported] swapper/2/0 just changed the state of lock: ffff88802a304110 (&group->lock#2){..-.}-{2:2}, at: _snd_pcm_stream_lock_irqsave+0xa0/0xd0 sound/core/pcm_native.c:170 but this lock took another, SOFTIRQ-unsafe lock in the past: (&timer->lock){+.+.}-{2:2} and interrupts could create inverse lock ordering between them. other info that might help us debug this: Possible interrupt unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&timer->lock); local_irq_disable(); lock(&group->lock#2); lock(&timer->lock); <Interrupt> lock(&group->lock#2); *** DEADLOCK *** [Fix] Ensure that the context interrupt state is the same before and after using the timer->lock. Fixes: beb45974dd49 ("ALSA: timer: Use guard() for locking") Reported-and-tested-by: syzbot+18840ef96e57b83b7fea@syzkaller.appspotmail.com Signed-off-by: Edward Adam Davis <eadavis@qq.com> --- sound/core/timer.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)