From: Cen Zhang Date: Sun, 14 Jun 2026 00:48:00 +0000 (+0800) Subject: ALSA: seq: oss: Serialize readq reset state with q->lock X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=49ce92d207820f588b0406add82f053decfbe5d9;p=thirdparty%2Flinux.git ALSA: seq: oss: Serialize readq reset state with q->lock snd_seq_oss_readq_clear() resets qlen, head, and tail without q->lock even though the normal reader and producer paths serialize the same ring state under that spinlock. A reset can therefore race snd_seq_oss_readq_free() or snd_seq_oss_readq_put_event() and leave stale records in the queue, drop freshly queued ones, or report the wrong readiness after wakeup. KCSAN reports a data race between snd_seq_oss_readq_clear() and snd_seq_oss_readq_free(). Take q->lock while clearing the ring and resetting input_time. Factor the enqueue logic into a caller-locked helper so snd_seq_oss_readq_put_timestamp() updates its suppression state under the same lock instead of racing the reset path. The buggy scenario involves two paths, with each column showing the order within that path: reset path: locked readq updater: 1. snd_seq_oss_reset() or 1. A reader or callback producer release reaches takes q->lock on the same queue. snd_seq_oss_readq_clear(). 2. snd_seq_oss_readq_clear() 2. The updater tests or modifies resets qlen, head, tail, qlen, head, and tail. and input_time. 3. snd_seq_oss_readq_clear() 3. The updater completes its wakes sleepers on read-modify-write sequence. q->midi_sleep. 4. Without q->lock, the reset 4. The resulting ring state drives can overlap the locked later reads and readiness. update. KCSAN reports: BUG: KCSAN: data-race in snd_seq_oss_readq_clear / snd_seq_oss_readq_free write to 0xffff8881069fe608 of 4 bytes by task 120516 on cpu 0: snd_seq_oss_readq_free+0x6c/0x80 snd_seq_oss_read+0xcb/0x250 odev_read+0x38/0x60 vfs_read+0xff/0x600 ksys_read+0xb4/0x140 __x64_sys_read+0x46/0x60 do_syscall_64+0xbb/0x2f0 entry_SYSCALL_64_after_hwframe+0x77/0x7f read to 0xffff8881069fe608 of 4 bytes by task 120517 on cpu 1: snd_seq_oss_readq_clear+0x1f/0x90 snd_seq_oss_reset+0xa7/0xf0 snd_seq_oss_ioctl+0x6f6/0x7e0 odev_ioctl+0x56/0xc0 __x64_sys_ioctl+0xd1/0x120 do_syscall_64+0xbb/0x2f0 entry_SYSCALL_64_after_hwframe+0x77/0x7f value changed: 0x00000001 -> 0x00000000 Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Cen Zhang Link: https://patch.msgid.link/20260614004801.3507773-1-zzzccc427@gmail.com Signed-off-by: Takashi Iwai --- diff --git a/sound/core/seq/oss/seq_oss_readq.c b/sound/core/seq/oss/seq_oss_readq.c index 6b474615ced8..d5dc76501ada 100644 --- a/sound/core/seq/oss/seq_oss_readq.c +++ b/sound/core/seq/oss/seq_oss_readq.c @@ -64,13 +64,17 @@ snd_seq_oss_readq_delete(struct seq_oss_readq *q) void snd_seq_oss_readq_clear(struct seq_oss_readq *q) { - if (q->qlen) { - q->qlen = 0; - q->head = q->tail = 0; + scoped_guard(spinlock_irqsave, &q->lock) { + if (q->qlen) { + q->qlen = 0; + q->head = 0; + q->tail = 0; + } + q->input_time = (unsigned long)-1; } + /* if someone sleeping, wake'em up */ wake_up(&q->midi_sleep); - q->input_time = (unsigned long)-1; } /* @@ -127,11 +131,11 @@ int snd_seq_oss_readq_sysex(struct seq_oss_readq *q, int dev, /* * copy an event to input queue: * return zero if enqueued + * caller must hold lock */ -int -snd_seq_oss_readq_put_event(struct seq_oss_readq *q, union evrec *ev) +static int snd_seq_oss_readq_put_event_locked(struct seq_oss_readq *q, + union evrec *ev) { - guard(spinlock_irqsave)(&q->lock); if (q->qlen >= q->maxlen - 1) return -ENOMEM; @@ -139,12 +143,27 @@ snd_seq_oss_readq_put_event(struct seq_oss_readq *q, union evrec *ev) q->tail = (q->tail + 1) % q->maxlen; q->qlen++; - /* wake up sleeper */ - wake_up(&q->midi_sleep); - return 0; } +/* + * copy an event to input queue: + * return zero if enqueued + */ +int +snd_seq_oss_readq_put_event(struct seq_oss_readq *q, union evrec *ev) +{ + int rc; + + scoped_guard(spinlock_irqsave, &q->lock) { + rc = snd_seq_oss_readq_put_event_locked(q, ev); + if (!rc) + wake_up(&q->midi_sleep); + } + + return rc; +} + /* * pop queue @@ -200,23 +219,31 @@ snd_seq_oss_readq_poll(struct seq_oss_readq *q, struct file *file, poll_table *w int snd_seq_oss_readq_put_timestamp(struct seq_oss_readq *q, unsigned long curt, int seq_mode) { - if (curt != q->input_time) { - union evrec rec; - memset(&rec, 0, sizeof(rec)); - switch (seq_mode) { - case SNDRV_SEQ_OSS_MODE_SYNTH: - rec.echo = (curt << 8) | SEQ_WAIT; - snd_seq_oss_readq_put_event(q, &rec); - break; - case SNDRV_SEQ_OSS_MODE_MUSIC: - rec.t.code = EV_TIMING; - rec.t.cmd = TMR_WAIT_ABS; - rec.t.time = curt; - snd_seq_oss_readq_put_event(q, &rec); - break; + int queued = 0; + + scoped_guard(spinlock_irqsave, &q->lock) { + if (curt != q->input_time) { + union evrec rec; + + memset(&rec, 0, sizeof(rec)); + switch (seq_mode) { + case SNDRV_SEQ_OSS_MODE_SYNTH: + rec.echo = (curt << 8) | SEQ_WAIT; + queued = !snd_seq_oss_readq_put_event_locked(q, &rec); + break; + case SNDRV_SEQ_OSS_MODE_MUSIC: + rec.t.code = EV_TIMING; + rec.t.cmd = TMR_WAIT_ABS; + rec.t.time = curt; + queued = !snd_seq_oss_readq_put_event_locked(q, &rec); + break; + } + q->input_time = curt; } - q->input_time = curt; } + if (queued) + wake_up(&q->midi_sleep); + return 0; }