On 10/25/25 08:54, Sasha Levin wrote:
From: Takashi Iwai tiwai@suse.de
[ Upstream commit 1f9fc89cbbe8a7a8648ea2f827f7d8590e62e52c ]
snd_seq_fifo_poll_wait() evaluates f->cells without locking after poll_wait(), and KCSAN doesn't like it as it appears to be a data-race. Although this doesn't matter much in practice as the value is volatile, it's still better to address it for the mind piece.
Wrap it with f->lock spinlock for avoiding the potential data race.
Reported-by: syzbot+c3dbc239259940ededba@syzkaller.appspotmail.com Link: https://syzkaller.appspot.com/bug?extid=c3dbc239259940ededba Signed-off-by: Takashi Iwai tiwai@suse.de Signed-off-by: Sasha Levin sashal@kernel.org
LLM Generated explanations, may be completely bogus:
YES
- The only runtime change wraps the `f->cells` check in `snd_seq_fifo_poll_wait()` with `guard(spinlock_irq)(&f->lock)` (`sound/core/seq/seq_fifo.c:213`), aligning this reader with every writer of `f->cells`, each of which already holds `f->lock` via `scoped_guard(spinlock_irqsave)` or explicit `spin_lock_irqsave` (`sound/core/seq/seq_fifo.c:125`, `sound/core/seq/seq_fifo.c:183`). That removes the unlocked load which KCSAN flagged as a real data race on the non-atomic `int` counter.
- This race is user-visible: if `snd_seq_fifo_poll_wait()` races with a concurrent producer/consumer, the poll mask built in `snd_seq_poll()` (`sound/core/seq/seq_clientmgr.c:1092-1106`) can sporadically omit `EPOLLIN`, leaving sequencer clients to sleep despite queued events. On weakly ordered architectures that behavior is not just theoretical; racing non-atomic accesses are undefined in the kernel memory model and trigger syzbot reports.
- The fix is minimal, self-contained, and mirrors existing guard usage in this file, so it has negligible regression risk: the lock is already part of the FIFO hot path, RAII unlock occurs immediately on return, and there are no new dependencies or API changes.
- Because the bug allows incorrect poll readiness and trips KCSAN, it meets stable criteria (user-visible correctness plus sanitizer warning) and applies cleanly to older trees that already contain the guard helpers used elsewhere in this file.
Suggested next step: run the targeted ALSA sequencer poll tests (or reproducer from the linked syzbot report) on the backport branch to confirm the warning disappears.
sound/core/seq/seq_fifo.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/sound/core/seq/seq_fifo.c b/sound/core/seq/seq_fifo.c index 3a10b081f129c..7dc2bd94cefc3 100644 --- a/sound/core/seq/seq_fifo.c +++ b/sound/core/seq/seq_fifo.c @@ -213,6 +213,7 @@ int snd_seq_fifo_poll_wait(struct snd_seq_fifo *f, struct file *file, poll_table *wait) { poll_wait(file, &f->input_sleep, wait);
- guard(spinlock_irq)(&f->lock); return (f->cells > 0); }
With CONFIG_WERROR enabled, 5.15.y fails to build for me now, and it seems to be due to this patch introducing a new warning. This is with Debian bookworm and its default gcc (12.2), building for amd64. I didn't try building 6.12.y or 6.17.y yet, but this warning does not happen on 6.1.y, 6.6.y, or 6.18-rc4.
In file included from ./include/linux/irqflags.h:16, from ./include/linux/rcupdate.h:26, from ./include/linux/rculist.h:11, from ./include/linux/pid.h:5, from ./include/linux/sched.h:14, from ./include/linux/ratelimit.h:6, from ./include/linux/dev_printk.h:16, from ./include/linux/device.h:15, from ./include/sound/core.h:10, from sound/core/seq/seq_fifo.c:7: sound/core/seq/seq_fifo.c: In function ‘snd_seq_fifo_poll_wait’: ./include/linux/cleanup.h:86:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement] 86 | class_##_name##_t var __cleanup(class_##_name##_destructor) = \ | ^~~~~~ ./include/linux/cleanup.h:109:9: note: in expansion of macro ‘CLASS’ 109 | CLASS(_name, __UNIQUE_ID(guard)) | ^~~~~ sound/core/seq/seq_fifo.c:221:9: note: in expansion of macro ‘guard’ 221 | guard(spinlock_irq)(&f->lock); | ^~~~~ CC net/core/sock.o cc1: all warnings being treated as errors make[3]: *** [scripts/Makefile.build:289: sound/core/seq/seq_fifo.o] Error 1