snd_seq_event_dup() copies an incoming event into a pool cell and, in
the UMP-enabled build, clears the trailing cell->ump.raw.extra word that
the memcpy() did not cover. The guard deciding whether to clear it
compares the copied size against sizeof(cell->event):
memcpy(&cell->ump, event, size);
if (size < sizeof(cell->event))
cell->ump.raw.extra = 0;
For a legacy (non-UMP) event, size == sizeof(struct snd_seq_event) ==
sizeof(cell->event), so the condition is false and the extra word keeps
stale data. The cell pool is allocated with kvmalloc() (not zeroed) and
cells are reused via a free list, so that word holds uninitialised heap
or leftover event data.
When such a cell is delivered to a UMP client (client->midi_version > 0)
that set SNDRV_SEQ_FILTER_NO_CONVERT -- so the legacy event reaches it
unconverted -- snd_seq_read() reads it out as the larger struct
snd_seq_ump_event and copies the stale word to user space, a 4-byte
kernel heap infoleak to an unprivileged /dev/snd/seq client.
Compare against sizeof(cell->ump) instead, so the trailing word is zeroed
for every event shorter than the UMP cell.
Fixes: 46397622a3fa ("ALSA: seq: Add UMP support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
Link: https://patch.msgid.link/20260623233841.853326-1-sammiee5311@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
snd_seq_fifo_resize() still needs to publish the replacement pool
before it waits for FIFO users. A blocking snd_seq_read() holds
f->use_lock while it sleeps, so concurrent senders must be able to
queue to the new pool and wake that reader instead of failing against a
closing old pool.
However, snd_seq_fifo_event_in() duplicates an event before it takes
f->lock, and snd_seq_read() can dequeue a cell and later call
snd_seq_fifo_cell_putback() if copy_to_user() or
snd_seq_expand_var_event() fails. If resize swaps f->pool and detaches
oldhead in between, either path can relink an old-pool cell after the
snapshot. That stale cell sits outside the drained oldhead list, keeps
oldpool->counter elevated, and can leave snd_seq_pool_delete() waiting
for the retired pool to drain.
Keep the existing swap-before-wait ordering in snd_seq_fifo_resize(),
but reject stale cells before any FIFO relink. Revalidate event-in cells
under f->lock and retry them against the published replacement pool, and
free stale putback cells instead of linking them back into the FIFO.
The buggy scenario involves two paths, with each column showing the
order within that path:
resize path: relink path:
1. Allocate newpool. 1. Take f->use_lock.
2. Swap f->pool to newpool and 2. Duplicate or dequeue an old-pool
detach oldhead. cell before oldpool closes.
3. Mark oldpool closing and 3. Reach a later relink point after
wait for FIFO users. resize published newpool.
4. Free oldhead and delete 4. Relink the old-pool cell after
oldpool. resize detached oldhead.
5. Drop f->use_lock.
The reproducer reports a resize ioctl blocked in the expected pool
teardown path:
signal: resize iteration=98 target_pool=4 exceeded 250ms
(elapsed=251ms)
diagnostic: resize_tid=651 wchan=snd_seq_pool_done
diagnostic: resize_tid=651 stack=
snd_seq_pool_done+0x5b/0x140
snd_seq_pool_delete+0x7a/0x90
snd_seq_fifo_resize+0x193/0x1e0
snd_seq_ioctl_set_client_pool+0x214/0x260
snd_seq_ioctl+0x119/0x540
__x64_sys_ioctl+0xd1/0x120
do_syscall_64+0xbb/0x2f0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
A second run with larger pools hit the same target path:
signal: resize iteration=32 target_pool=64 exceeded 250ms
(elapsed=251ms)
diagnostic: resize_tid=663 wchan=snd_seq_pool_done
diagnostic: resize_tid=663 stack=
snd_seq_pool_done+0x5b/0x140
snd_seq_pool_delete+0x7a/0x90
snd_seq_fifo_resize+0x193/0x1e0
snd_seq_ioctl_set_client_pool+0x214/0x260
snd_seq_ioctl+0x119/0x540
__x64_sys_ioctl+0xd1/0x120
do_syscall_64+0xbb/0x2f0
entry_SYSCALL_64_after_hwframe+0x77/0x7f
Fixes: 2d7d54002e39 ("ALSA: seq: Fix race during FIFO resize")
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Link: https://patch.msgid.link/20260614004801.3507773-2-zzzccc427@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
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>
The error bouncing may fail again, and we have no check for re-bouncing. For avoiding the loop, add the event type check at bouncing, and stop re-bouncing if it's already a bounce error. Link: https://patch.msgid.link/20260612113350.407465-1-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de>
The comment above bounce_error_event() documents that user clients
should receive SNDRV_SEQ_EVENT_BOUNCE with the original event embedded
as variable-length data, while kernel clients should receive
SNDRV_SEQ_EVENT_KERNEL_ERROR with a quoted kernel pointer.
However, the implementation unconditionally uses
SNDRV_SEQ_EVENT_KERNEL_ERROR with data.quote.event set to the raw
struct snd_seq_event pointer for all clients. When a bounce error
event is delivered to a USER_CLIENT via snd_seq_read(), the kernel
heap address in data.quote.event is exposed to userspace through
copy_to_user() in the fixed-length branch.
This is a distinct leak path from the one addressed by commit
705dd6dcbc0e ("ALSA: seq: Clear variable event pointer on read"),
which sanitizes data.ext.ptr in the variable-length branch of
snd_seq_read(). The bounce_error_event() leak uses fixed-length
events that take the else branch where no sanitization occurs.
Differentiate the bounce event by client type. For USER_CLIENT,
send SNDRV_SEQ_EVENT_BOUNCE with SNDRV_SEQ_EVENT_LENGTH_VARIABLE
and data.ext pointing to the original event. The variable-length
path in snd_seq_event_dup() copies the event data into chained
cells, and snd_seq_expand_var_event() copies only the content --
never the pointer -- to userspace. For KERNEL_CLIENT, keep the
existing SNDRV_SEQ_EVENT_KERNEL_ERROR behavior with the quoted
pointer.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: HanQuan <eilaimemedsnaimel@gmail.com>
Link: https://patch.msgid.link/20260612103222.2528305-1-eilaimemedsnaimel@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
So far we've tried to address UAFs in ALSA timer code by applying the
locks at various places, but the fundamental problem is that the timer
object may be released while the belonging timer instance objects are
still present and accessing to it. This patch is a more proper fix to
address that issue, namely, by refcounting and keeping the timer
object.
The basic implementation is to use kref for the refcount of the timer
object, and take/release the reference at assigning/releasing the
instance, as well as at referring from ioctls or ALSA sequencer code.
The reference from ioctl or ALSA sequencer is abstracted with
snd_timeri_timer auto-cleanup.
Note that this change assumes that the code already took the fix
commit da3039e91d1f ("ALSA: timer: Forcibly close timer instances at
closing"); otherwise the refcount may be unbalanced when the timer is
freed while slave instances are still present.
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Link: https://patch.msgid.link/20260609115100.806869-2-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
snd_seq_read() copies a queued variable-length event header to userspace
before expanding the payload. Queued variable-length events use
SNDRV_SEQ_EXT_CHAINED internally, and data.ext.ptr points at the first
extension cell.
The read side strips SNDRV_SEQ_EXT_* bits from data.ext.len before the
copy, but it leaves data.ext.ptr untouched. A userspace sequencer client
can therefore write a direct variable event to itself and read back the
extension-cell kernel address from the returned header.
Clear the temporary header pointer before copy_to_user(). The original
queued event remains unchanged and is still passed to
snd_seq_expand_var_event(), so payload expansion keeps using the
internal chain.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Link: https://patch.msgid.link/20260607004129.61345-1-kylebot@openai.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
snd_seq_expand_var_event_at() clamps the number of bytes to copy to the
remaining variable-event length, but passes the original buffer size to
expand_var_event().
For SNDRV_SEQ_EXT_USRPTR events, expand_var_event() copies exactly the
size argument from userspace. On the final chunk, when the remaining
event data is shorter than the caller's buffer, this can read past the
declared event data and can spuriously fail with -EFAULT if the extra
bytes cross an unmapped page.
Pass the clamped length instead. The chained and kernel-backed paths
already reclamp in dump_var_event(), but the user-pointer path handles
the size directly.
Fixes: ea46f79709b6 ("ALSA: seq: Add snd_seq_expand_var_event_at() helper")
Signed-off-by: HyeongJun An <sammiee5311@gmail.com>
Link: https://patch.msgid.link/20260606040913.230213-1-sammiee5311@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
The dummy sequencer port forwards events by copying an incoming
struct snd_seq_event into a stack temporary, rewriting source and
destination, and dispatching the temporary to subscribers. That legacy
event storage is smaller than struct snd_seq_ump_event.
When a UMP event reaches the dummy client, the copy leaves the UMP flag
set but only provides legacy-sized stack storage. The subscriber
delivery path then uses snd_seq_event_packet_size() and copies a
UMP-sized packet from that stack object, reading past the end of the
temporary.
Use the existing union __snd_seq_event storage and copy the packet size
reported for the incoming event before rewriting the common routing
fields. This preserves the full UMP packet for UMP events while keeping
legacy event handling unchanged.
Fixes: 32cb23a0f911 ("ALSA: seq: dummy: Allow UMP conversion")
Signed-off-by: Kyle Zeng <kylebot@openai.com>
Link: https://patch.msgid.link/20260605080204.32045-1-kylebot@openai.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>