Re: [syzbot] [sound?] possible deadlock in __snd_pcm_lib_xfer (2)

Sebastian Andrzej Siewior
2025-09-04T10:20:59Z

On 2025-08-29 17:06:02 [-0700], syzbot wrote:
> syzbot has bisected this issue to:
>
> commit d2d6422f8bd17c6bb205133e290625a564194496
> Author: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
> Date: Fri Sep 6 10:59:04 2024 +0000
>
> x86: Allow to enable PREEMPT_RT.
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=12db5634580000
> start commit: 07d9df80082b Merge tag 'perf-tools-fixes-for-v6.17-2025-08..
> git tree: upstream
> final oops: https://syzkaller.appspot.com/x/report.txt?x=11db5634580000
> console output: https://syzkaller.appspot.com/x/log.txt?x=16db5634580000
> kernel config: https://syzkaller.appspot.com/x/.config?x=e1e1566c7726877e
> dashboard link: https://syzkaller.appspot.com/bug?extid=10b4363fb0f46527f3f3
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=10307262580000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=17110242580000
>
> Reported-by: syzbot+10b4363fb0f46527f3f3@syzkaller.appspotmail.com
> Fixes: d2d6422f8bd1 ("x86: Allow to enable PREEMPT_RT.")

This is unfortunate. There is nothing that sound did wrong, it is rather
special softirq handling in this case. We don't see this often because
it requires that a timer is cancelled at the time it is running.
The assumption made by sound is that spin_lock_irq() also disables
softirqs. This is not the case on PREEMPT_RT.

The hunk below avoids the splat. Adding local_bh_disable() to
spin_lock_irq() would cure it, too. It would also result in random
synchronisation points across the kernel leading to something less
usable.
The imho best solution would to get rid of softirq_ctrl.lock which has
been proposed
https://lore.kernel.org/all/20250901163811.963326-4-bigeasy@linutronix.de/

Comments?

diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -84,19 +84,24 @@ void snd_pcm_group_init(struct snd_pcm_group *group)
}

/* define group lock helpers */
-#define DEFINE_PCM_GROUP_LOCK(action, mutex_action) \
+#define DEFINE_PCM_GROUP_LOCK(action, bh_lock, bh_unlock, mutex_action) \
static void snd_pcm_group_ ## action(struct snd_pcm_group *group, bool nonatomic) \
{ \
- if (nonatomic) \
+ if (nonatomic) { \
mutex_ ## mutex_action(&group->mutex); \
- else \
- spin_ ## action(&group->lock); \
+ } else { \
+ if (IS_ENABLED(CONFIG_PREEMPT_RT) && bh_lock) \
+ local_bh_disable(); \
+ spin_ ## action(&group->lock); \
+ if (IS_ENABLED(CONFIG_PREEMPT_RT) && bh_unlock) \
+ local_bh_enable(); \
+ } \
}

-DEFINE_PCM_GROUP_LOCK(lock, lock);
-DEFINE_PCM_GROUP_LOCK(unlock, unlock);
-DEFINE_PCM_GROUP_LOCK(lock_irq, lock);
-DEFINE_PCM_GROUP_LOCK(unlock_irq, unlock);
+DEFINE_PCM_GROUP_LOCK(lock, 0, 0, lock);
+DEFINE_PCM_GROUP_LOCK(unlock, 0, 0, unlock);
+DEFINE_PCM_GROUP_LOCK(lock_irq, 1, 0, lock);
+DEFINE_PCM_GROUP_LOCK(unlock_irq, 0, 1, unlock);

/**
* snd_pcm_stream_lock - Lock the PCM stream

Sebastian