github-mirror/linux
1
0
Fork 0
mirror of https://github.com/torvalds/linux.git synced 2026-07-28 01:55:51 +02:00
Code Issues Actions Packages Projects Releases Wiki Activity Graph

ALSA: seq: oss: Serialize readq reset state with q->lock

Browse Source 
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: 1da177e4c3 ("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>
This commit is contained in:
Cen Zhang 2026-06-14 08:48:00 +08:00 committed by Takashi Iwai
parent 9420958c51
commit 49ce92d207
1 changed files with 52 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
sound/core/seq/oss/seq_oss_readq.c
View File

@ -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;
}