]> git.ipfire.org Git - thirdparty/linux.git/commitdiff
ALSA: seq: midi: Serialize output teardown with event_input
authorZhang Cen <rollkingzzc@gmail.com>
Wed, 27 May 2026 06:29:48 +0000 (14:29 +0800)
committerTakashi Iwai <tiwai@suse.de>
Wed, 27 May 2026 10:34:29 +0000 (12:34 +0200)
event_process_midi() borrows msynth->output_rfile.output and then
passes the substream to dump_midi() and snd_rawmidi_kernel_write()
without synchronizing with the output open/close transition.
midisynth_use() also publishes output_rfile before
snd_rawmidi_output_params() has finished.

The last midisynth_unuse() can therefore release the same rawmidi file
and free substream->runtime before snd_rawmidi_kernel_write1() takes
its runtime buffer reference. That leaves the event_input path using a
stale substream or runtime and can end in a NULL-deref or use-after-free.

Fix this with two pieces of synchronization. Keep a short IRQ-safe
spinlock only for publishing or clearing output_rfile and for pairing
the output snapshot with an snd_use_lock_t reference. Once
event_process_midi() has taken that in-flight reference, it drops the
spinlock before calling snd_seq_dump_var_event(), dump_midi(), or
snd_rawmidi_kernel_write(). midisynth_unuse() now detaches the visible
rawmidi file under the same spinlock, waits for the in-flight writers
to drain, and only then drains and releases the saved file.
midisynth_use() likewise opens into a local snd_rawmidi_file and
publishes it only after snd_rawmidi_output_params() succeeds.

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

event_input path:                     last unuse path:
1. event_process_midi() snapshots    1. midisynth_unuse() starts
   output_rfile.output.                 tearing down output_rfile.
2. dump_midi() reaches               2. snd_rawmidi_kernel_release()
   snd_rawmidi_kernel_write()           closes the output file.
   before runtime is pinned.         3. close_substream() frees
3. The callback keeps using             substream->runtime.
   the borrowed substream.

Validation reproduced this kernel report:
KASAN null-ptr-deref in snd_rawmidi_kernel_write1+0x56/0x360
RIP: 0033:0x7fde7dd0837f
RIP: 0010:snd_rawmidi_kernel_write1+0x56/0x360

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: Codex:gpt-5.5
Signed-off-by: Zhang Cen <rollkingzzc@gmail.com>
Link: https://patch.msgid.link/20260527062948.3614025-1-rollkingzzc@gmail.com
Signed-off-by: Takashi Iwai <tiwai@suse.de>
sound/core/seq/seq_midi.c

index ca3f5fc309927b1aab92212cf61bc026763da3b8..2eb12199c92f90b674b62ed6093fee04d67fca43 100644 (file)
@@ -24,6 +24,7 @@ Possible options for midisynth module:
 #include <sound/seq_device.h>
 #include <sound/seq_midi_event.h>
 #include <sound/initval.h>
+#include "seq_lock.h"
 
 MODULE_AUTHOR("Frank van de Pol <fvdpol@coil.demon.nl>, Jaroslav Kysela <perex@perex.cz>");
 MODULE_DESCRIPTION("Advanced Linux Sound Architecture sequencer MIDI synth.");
@@ -42,6 +43,8 @@ struct seq_midisynth {
        int device;
        int subdevice;
        struct snd_rawmidi_file input_rfile;
+       spinlock_t output_lock;         /* protects output_rfile publication */
+       snd_use_lock_t output_use_lock; /* in-flight event_input users */
        struct snd_rawmidi_file output_rfile;
        int seq_client;
        int seq_port;
@@ -125,31 +128,42 @@ static int event_process_midi(struct snd_seq_event *ev, int direct,
        struct seq_midisynth *msynth = private_data;
        unsigned char msg[10];  /* buffer for constructing midi messages */
        struct snd_rawmidi_substream *substream;
+       int err = 0;
        int len;
 
        if (snd_BUG_ON(!msynth))
                return -EINVAL;
-       substream = msynth->output_rfile.output;
-       if (substream == NULL)
-               return -ENODEV;
+
+       scoped_guard(spinlock_irqsave, &msynth->output_lock) {
+               substream = msynth->output_rfile.output;
+               if (!substream)
+                       return -ENODEV;
+               snd_use_lock_use(&msynth->output_use_lock);
+       }
+
        if (ev->type == SNDRV_SEQ_EVENT_SYSEX) {        /* special case, to save space */
                if ((ev->flags & SNDRV_SEQ_EVENT_LENGTH_MASK) != SNDRV_SEQ_EVENT_LENGTH_VARIABLE) {
                        /* invalid event */
                        pr_debug("ALSA: seq_midi: invalid sysex event flags = 0x%x\n", ev->flags);
-                       return 0;
+                       goto out;
                }
                snd_seq_dump_var_event(ev, __dump_midi, substream);
                snd_midi_event_reset_decode(msynth->parser);
        } else {
-               if (msynth->parser == NULL)
-                       return -EIO;
+               if (!msynth->parser) {
+                       err = -EIO;
+                       goto out;
+               }
                len = snd_midi_event_decode(msynth->parser, msg, sizeof(msg), ev);
                if (len < 0)
-                       return 0;
+                       goto out;
                if (dump_midi(substream, msg, len) < 0)
                        snd_midi_event_reset_decode(msynth->parser);
        }
-       return 0;
+
+out:
+       snd_use_lock_free(&msynth->output_use_lock);
+       return err;
 }
 
 
@@ -163,6 +177,8 @@ static int snd_seq_midisynth_new(struct seq_midisynth *msynth,
        msynth->card = card;
        msynth->device = device;
        msynth->subdevice = subdevice;
+       spin_lock_init(&msynth->output_lock);
+       snd_use_lock_init(&msynth->output_use_lock);
        return 0;
 }
 
@@ -215,12 +231,13 @@ static int midisynth_use(void *private_data, struct snd_seq_port_subscribe *info
 {
        int err;
        struct seq_midisynth *msynth = private_data;
+       struct snd_rawmidi_file rfile = {};
        struct snd_rawmidi_params params;
 
        /* open midi port */
        err = snd_rawmidi_kernel_open(msynth->rmidi, msynth->subdevice,
                                      SNDRV_RAWMIDI_LFLG_OUTPUT,
-                                     &msynth->output_rfile);
+                                     &rfile);
        if (err < 0) {
                pr_debug("ALSA: seq_midi: midi output open failed!!!\n");
                return err;
@@ -229,12 +246,14 @@ static int midisynth_use(void *private_data, struct snd_seq_port_subscribe *info
        params.avail_min = 1;
        params.buffer_size = output_buffer_size;
        params.no_active_sensing = 1;
-       err = snd_rawmidi_output_params(msynth->output_rfile.output, &params);
+       err = snd_rawmidi_output_params(rfile.output, &params);
        if (err < 0) {
-               snd_rawmidi_kernel_release(&msynth->output_rfile);
+               snd_rawmidi_kernel_release(&rfile);
                return err;
        }
        snd_midi_event_reset_decode(msynth->parser);
+       scoped_guard(spinlock_irqsave, &msynth->output_lock)
+               msynth->output_rfile = rfile;
        return 0;
 }
 
@@ -242,11 +261,19 @@ static int midisynth_use(void *private_data, struct snd_seq_port_subscribe *info
 static int midisynth_unuse(void *private_data, struct snd_seq_port_subscribe *info)
 {
        struct seq_midisynth *msynth = private_data;
+       struct snd_rawmidi_file rfile = {};
 
-       if (snd_BUG_ON(!msynth->output_rfile.output))
+       scoped_guard(spinlock_irqsave, &msynth->output_lock) {
+               rfile = msynth->output_rfile;
+               msynth->output_rfile = (struct snd_rawmidi_file){};
+       }
+
+       if (snd_BUG_ON(!rfile.output))
                return -EINVAL;
-       snd_rawmidi_drain_output(msynth->output_rfile.output);
-       return snd_rawmidi_kernel_release(&msynth->output_rfile);
+
+       snd_use_lock_sync(&msynth->output_use_lock);
+       snd_rawmidi_drain_output(rfile.output);
+       return snd_rawmidi_kernel_release(&rfile);
 }
 
 /* delete given midi synth port */