From: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To: Takashi Iwai <tiwai@suse.de>
Cc: syzbot <syzbot+10b4363fb0f46527f3f3@syzkaller.appspotmail.com>,
tglx@linutronix.de, bp@alien8.de, dave.hansen@linux.intel.com,
hpa@zytor.com, linux-kernel@vger.kernel.org,
linux-sound@vger.kernel.org, mingo@redhat.com, perex@perex.cz,
syzkaller-bugs@googlegroups.com, tiwai@suse.com, x86@kernel.org
Subject: [PATCH] ALSA: pcm: Disable bottom softirqs as part of spin_lock_irq() on PREEMPT_RT
Date: Mon, 15 Sep 2025 17:28:51 +0200 [thread overview]
Message-ID: <20250915152851.-ATGMKhp@linutronix.de> (raw)
In-Reply-To: <878qiutsdu.wl-tiwai@suse.de>
snd_pcm_group_lock_irq() acquires a spinlock_t and disables interrupts
via spin_lock_irq(). This also implicitly disables the handling of
softirqs such as TIMER_SOFTIRQ.
On PREEMPT_RT softirqs are preemptible and spin_lock_irq() does not
disable them. That means a timer can be invoked during spin_lock_irq()
on the same CPU. Due to synchronisations reasons local_bh_disable() has
a per-CPU lock named softirq_ctrl.lock which synchronizes individual
softirq against each other.
syz-bot managed to trigger a lockdep report where softirq_ctrl.lock is
acquired in hrtimer_cancel() in addition to hrtimer_run_softirq(). This
is a possible deadlock.
The softirq_ctrl.lock can not be made part of spin_lock_irq() as this
would lead to too much synchronisation against individual threads on the
system. To avoid the possible deadlock, softirqs must be manually
disabled before the lock is acquired.
Disable softirqs before the lock is acquired on PREEMPT_RT.
Reported-by: syzbot+10b4363fb0f46527f3f3@syzkaller.appspotmail.com
Fixes: d2d6422f8bd1 ("x86: Allow to enable PREEMPT_RT.")
Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
I don't see a way around this given the report. I don't see how to
address this within the softirq. Taking this lock as part of every
spin_lock_irq() would be a mess and while testing I didn't even manage
to boot the machine. So I probably missed a detail (but then I would
only know how mad it really is).
This can be an intermediate solution until
https://lore.kernel.org/all/20250901163811.963326-4-bigeasy@linutronix.de/
gets merged and the !PREEMPT_RT_NEEDS_BH_LOCK case the default (i.e. not
a config option anymore).
sound/core/pcm_native.c | 21 +++++++++++++--------
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index 1eab940fa2e5a..68bee40c9adaf 100644
--- 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, false, false, lock);
+DEFINE_PCM_GROUP_LOCK(unlock, false, false, unlock);
+DEFINE_PCM_GROUP_LOCK(lock_irq, true, false, lock);
+DEFINE_PCM_GROUP_LOCK(unlock_irq, false, true, unlock);
/**
* snd_pcm_stream_lock - Lock the PCM stream
--
2.51.0
next prev parent reply other threads:[~2025-09-15 15:28 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-08-29 18:38 [syzbot] [sound?] possible deadlock in __snd_pcm_lib_xfer (2) syzbot
2025-08-30 0:06 ` syzbot
2025-09-04 10:20 ` Sebastian Andrzej Siewior
2025-09-04 10:38 ` Takashi Iwai
2025-09-15 15:28 ` Sebastian Andrzej Siewior [this message]
2025-09-16 9:43 ` [PATCH] ALSA: pcm: Disable bottom softirqs as part of spin_lock_irq() on PREEMPT_RT Takashi Iwai
2025-09-16 12:23 ` Sebastian Andrzej Siewior
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20250915152851.-ATGMKhp@linutronix.de \
--to=bigeasy@linutronix.de \
--cc=bp@alien8.de \
--cc=dave.hansen@linux.intel.com \
--cc=hpa@zytor.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sound@vger.kernel.org \
--cc=mingo@redhat.com \
--cc=perex@perex.cz \
--cc=syzbot+10b4363fb0f46527f3f3@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tglx@linutronix.de \
--cc=tiwai@suse.com \
--cc=tiwai@suse.de \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox