Commit 60a1969f authored by Zhang Cen's avatar Zhang Cen Committed by Takashi Iwai
Browse files

ALSA: seq: Serialize UMP output teardown with event_input



seq_ump_process_event() borrows client->out_rfile.output without
synchronizing with the first-open and last-close transition in
seq_ump_client_open() and seq_ump_client_close().

The last output unuse can therefore drop opened[STR_OUT] to zero and
release the rawmidi file while an in-flight event_input callback is still
inside snd_rawmidi_kernel_write(). That leaves the rawmidi substream
runtime exposed to teardown before the write path has taken its own
buffer reference.

Add a per-client rwlock for the event_input-visible output file. Publish
a newly opened output file under the write side, and hold the read side
from the output lookup through snd_rawmidi_kernel_write(). The last
output close copies and clears the visible output file under the write
side, then drops the lock and releases the saved rawmidi file. Use
IRQ-safe rwlock guards because event_input can also be reached from
atomic sequencer delivery.

The buggy scenario involves two paths, with each column showing the
order within that path:

path A label: event_input path         path B label: last unuse path
1. seq_ump_process_event() reads       1. seq_ump_client_close()
   client->out_rfile.output.              drops opened[STR_OUT] to zero.
2. snd_rawmidi_kernel_write1()         2. snd_rawmidi_kernel_release()
   has not yet pinned runtime.            closes the output file.
3. The writer continues using          3. close_substream() frees
   the borrowed substream.                substream->runtime.

This keeps the output substream and runtime alive for the full
event_input write while keeping rawmidi release outside the rwlock.

KASAN reproduced this as a slab-use-after-free in
snd_rawmidi_kernel_write1(), with allocation through
seq_ump_use()/snd_seq_port_connect() and free through
seq_ump_unuse()/snd_seq_port_disconnect().

Suggested-by: default avatarTakashi Iwai <tiwai@suse.de>

Validation reproduced this kernel report:
KASAN slab-use-after-free in snd_rawmidi_kernel_write1+0x9d/0x400
RIP: 0033:0x7f5528af837f
Read of size 8
Call trace:
  dump_stack_lvl+0x73/0xb0 (?:?)
  print_report+0xd1/0x650 (?:?)
  srso_alias_return_thunk+0x5/0xfbef5 (?:?)
  __virt_addr_valid+0x1a7/0x340 (?:?)
  kasan_complete_mode_report_info+0x64/0x200 (?:?)
  kasan_report+0xf7/0x130 (?:?)
  snd_rawmidi_kernel_write1+0x9d/0x400 (?:?)
  __asan_load8+0x82/0xb0 (?:?)
  update_stack_state+0x1ef/0x2d0 (?:?)
  snd_rawmidi_kernel_write+0x1a/0x20 (?:?)
  seq_ump_process_event+0xd4/0x120 (sound/core/seq/seq_ump_client.c:82)
  __snd_seq_deliver_single_event+0x8a/0xe0 (?:?)
  snd_seq_deliver_from_ump+0x2b2/0xd60 (?:?)
  lock_acquire+0x14e/0x2e0 (?:?)
  find_held_lock+0x31/0x90 (?:?)
  snd_seq_port_use_ptr+0xa6/0xe0 (?:?)
  __kasan_check_write+0x18/0x20 (?:?)
  do_raw_read_unlock+0x32/0xa0 (?:?)
  _raw_read_unlock+0x26/0x50 (?:?)
  snd_seq_deliver_single_event+0x45c/0x4b0 (?:?)
  snd_seq_deliver_event+0x10d/0x1b0 (?:?)
  snd_seq_client_enqueue_event+0x192/0x240 (?:?)
  snd_seq_write+0x2cd/0x450 (?:?)
  apparmor_file_permission+0x20/0x30 (?:?)
  security_file_permission+0x51/0x60 (?:?)
  vfs_write+0x1ce/0x850 (?:?)
  __fget_files+0x12b/0x220 (?:?)
  lock_release+0xc8/0x2a0 (?:?)
  __rcu_read_unlock+0x74/0x2d0 (?:?)
  __fget_files+0x135/0x220 (?:?)
  ksys_write+0x15a/0x180 (?:?)
  rcu_is_watching+0x24/0x60 (?:?)
  __x64_sys_write+0x46/0x60 (?:?)
  x64_sys_call+0x7d/0x20d0 (?:?)
  do_syscall_64+0xc1/0x360 (arch/x86/entry/syscall_64.c:87)
  entry_SYSCALL_64_after_hwframe+0x77/0x7f (?:?)

Fixes: 81fd444a ("ALSA: seq: Bind UMP device")
Signed-off-by: default avatarZhang Cen <rollkingzzc@gmail.com>
Link: https://patch.msgid.link/20260520103249.3048345-1-rollkingzzc@gmail.com


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent a69b677e
Loading
Loading
Loading
Loading
+18 −4
Original line number Diff line number Diff line
@@ -37,6 +37,7 @@ struct seq_ump_client {
	struct snd_ump_endpoint *ump;	/* assigned endpoint */
	int seq_client;			/* sequencer client id */
	int opened[2];			/* current opens for each direction */
	rwlock_t output_lock;		/* protects out_rfile output access */
	struct snd_rawmidi_file out_rfile; /* rawmidi for output */
	struct seq_ump_input_buffer input; /* input parser context */
	void *ump_info[SNDRV_UMP_MAX_BLOCKS + 1]; /* shadow of seq client ump_info */
@@ -88,6 +89,7 @@ static int seq_ump_process_event(struct snd_seq_event *ev, int direct,
	unsigned char type;
	int len;

	guard(read_lock_irqsave)(&client->output_lock);
	substream = client->out_rfile.output;
	if (!substream)
		return -ENODEV;
@@ -106,6 +108,7 @@ static int seq_ump_process_event(struct snd_seq_event *ev, int direct,
static int seq_ump_client_open(struct seq_ump_client *client, int dir)
{
	struct snd_ump_endpoint *ump = client->ump;
	struct snd_rawmidi_file rfile = {};
	int err;

	guard(mutex)(&ump->open_mutex);
@@ -113,9 +116,11 @@ static int seq_ump_client_open(struct seq_ump_client *client, int dir)
		err = snd_rawmidi_kernel_open(&ump->core, 0,
					      SNDRV_RAWMIDI_LFLG_OUTPUT |
					      SNDRV_RAWMIDI_LFLG_APPEND,
					      &client->out_rfile);
					      &rfile);
		if (err < 0)
			return err;
		scoped_guard(write_lock_irqsave, &client->output_lock)
			client->out_rfile = rfile;
	}
	client->opened[dir]++;
	return 0;
@@ -125,11 +130,19 @@ static int seq_ump_client_open(struct seq_ump_client *client, int dir)
static int seq_ump_client_close(struct seq_ump_client *client, int dir)
{
	struct snd_ump_endpoint *ump = client->ump;
	struct snd_rawmidi_file rfile = {};

	guard(mutex)(&ump->open_mutex);
	if (!--client->opened[dir])
		if (dir == STR_OUT)
			snd_rawmidi_kernel_release(&client->out_rfile);
	if (!--client->opened[dir]) {
		if (dir == STR_OUT) {
			scoped_guard(write_lock_irqsave, &client->output_lock) {
				rfile = client->out_rfile;
				client->out_rfile = (struct snd_rawmidi_file){};
			}
			if (rfile.rmidi)
				snd_rawmidi_kernel_release(&rfile);
		}
	}
	return 0;
}

@@ -467,6 +480,7 @@ static int snd_seq_ump_probe(struct snd_seq_device *dev)

	INIT_WORK(&client->group_notify_work, handle_group_notify);
	client->ump = ump;
	rwlock_init(&client->output_lock);

	client->seq_client =
		snd_seq_create_kernel_client(card, ump->core.device,