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 <zzzccc427@gmail.com>
Link: https://patch.msgid.link/20260614004801.3507773-1-zzzccc427@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>

diff --git a/sound/core/seq/oss/seq_oss_readq.c b/sound/core/seq/oss/seq_oss_readq.c
index 6b474615ced89304dc2d32ea9f2166f8bf6811fe..d5dc76501ada8184c73b816b2787d667a460dc1d 100644 (file)
--- 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;
 }