* [PATCH v2] ALSA: seq: midi: Serialize input teardown with event_input
@ 2026-08-13 15:08 John Keeping
2026-08-13 16:20 ` Takashi Iwai
0 siblings, 1 reply; 2+ messages in thread
From: John Keeping @ 2026-08-13 15:08 UTC (permalink / raw)
To: Takashi Iwai
Cc: John Keeping, Jaroslav Kysela, Zhang Cen, Uwe Kleine-König,
Kees Cook, linux-sound, linux-kernel
snd_midi_input_event() must not be running while a rawmidi substream is
closing, since this can lead to the trigger state becoming out-of-step
through this sequence in snd_rawmidi_input_trigger():
snd_rawmidi_input_trigger(up=0)
snd_midi_input_event()
-> snd_rawmidi_kernel_read()
-> snd_rawmidi_input_trigger(up=1)
-> cancel_work_sync()
which ends with the underlying device being active unexpectedly.
When this is called from close_substream(), further input can re-trigger
the input event leaving it running after rawmidi_release_priv() has set
rfile->rmidi to NULL which leads to:
Unable to handle kernel NULL pointer dereference at virtual address 00000000000000b0
Call trace:
snd_midi_input_event+0x3c/0x134 [snd_seq_midi] (P)
snd_rawmidi_input_event_work+0x1c/0x2c
process_one_work+0x150/0x3a4
worker_thread+0x190/0x318
Apply a similar approach to commit ef7607ab1c8ad ("ALSA: seq: midi:
Serialize output teardown with event_input") which fixed the same issue
in the output direction, but updated to use RCU following Takashi Iwai's
proposed follow-on patch [1].
With this change in place, midisynth_unsubscribe() clears the input file
so snd_midi_input_event() will not re-trigger the stream and will be
quiesced by the cancel_work_sync() in snd_rawmidi_input_trigger().
[1] https://lore.kernel.org/linux-sound/20260813144224.753399-1-tiwai@suse.de/
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
---
Changes in v2:
- Switch to using RCU following Takashi's suggestion
sound/core/seq/seq_midi.c | 37 +++++++++++++++++++++++++++++++------
1 file changed, 31 insertions(+), 6 deletions(-)
diff --git a/sound/core/seq/seq_midi.c b/sound/core/seq/seq_midi.c
index 2eb12199c92f9..28a78c72a5315 100644
--- a/sound/core/seq/seq_midi.c
+++ b/sound/core/seq/seq_midi.c
@@ -42,6 +42,8 @@ struct seq_midisynth {
struct snd_rawmidi *rmidi;
int device;
int subdevice;
+ struct snd_rawmidi_substream __rcu *input_substream;
+ snd_use_lock_t input_use_lock; /* in-flight event_input users */
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 */
@@ -76,6 +78,14 @@ static void snd_midi_input_event(struct snd_rawmidi_substream *substream)
msynth = runtime->private_data;
if (msynth == NULL)
return;
+
+ scoped_guard(rcu) {
+ if (rcu_dereference(msynth->input_substream) != substream)
+ return;
+
+ snd_use_lock_use(&msynth->input_use_lock);
+ }
+
memset(&ev, 0, sizeof(ev));
while (runtime->avail > 0) {
res = snd_rawmidi_kernel_read(substream, buf, sizeof(buf));
@@ -95,6 +105,8 @@ static void snd_midi_input_event(struct snd_rawmidi_substream *substream)
memset(&ev, 0, sizeof(ev));
}
}
+
+ snd_use_lock_free(&msynth->input_use_lock);
}
static int dump_midi(struct snd_rawmidi_substream *substream, const char *buf, int count)
@@ -177,6 +189,7 @@ static int snd_seq_midisynth_new(struct seq_midisynth *msynth,
msynth->card = card;
msynth->device = device;
msynth->subdevice = subdevice;
+ snd_use_lock_init(&msynth->input_use_lock);
spin_lock_init(&msynth->output_lock);
snd_use_lock_init(&msynth->output_use_lock);
return 0;
@@ -188,28 +201,31 @@ static int midisynth_subscribe(void *private_data, struct snd_seq_port_subscribe
int err;
struct seq_midisynth *msynth = private_data;
struct snd_rawmidi_runtime *runtime;
+ 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_INPUT,
- &msynth->input_rfile);
+ &rfile);
if (err < 0) {
pr_debug("ALSA: seq_midi: midi input open failed!!!\n");
return err;
}
- runtime = msynth->input_rfile.input->runtime;
+ runtime = rfile.input->runtime;
memset(¶ms, 0, sizeof(params));
params.avail_min = 1;
params.buffer_size = input_buffer_size;
- err = snd_rawmidi_input_params(msynth->input_rfile.input, ¶ms);
+ err = snd_rawmidi_input_params(rfile.input, ¶ms);
if (err < 0) {
- snd_rawmidi_kernel_release(&msynth->input_rfile);
+ snd_rawmidi_kernel_release(&rfile);
return err;
}
snd_midi_event_reset_encode(msynth->parser);
runtime->event = snd_midi_input_event;
runtime->private_data = msynth;
+ msynth->input_rfile = rfile;
+ rcu_assign_pointer(msynth->input_substream, rfile.input);
snd_rawmidi_kernel_read(msynth->input_rfile.input, NULL, 0);
return 0;
}
@@ -219,10 +235,19 @@ static int midisynth_unsubscribe(void *private_data, struct snd_seq_port_subscri
{
int err;
struct seq_midisynth *msynth = private_data;
+ struct snd_rawmidi_file rfile;
+
+ rcu_assign_pointer(msynth->input_substream, NULL);
+ synchronize_rcu();
+ snd_use_lock_sync(&msynth->input_use_lock);
- if (snd_BUG_ON(!msynth->input_rfile.input))
+ rfile = msynth->input_rfile;
+ msynth->input_rfile = (struct snd_rawmidi_file){};
+
+ if (snd_BUG_ON(!rfile.input))
return -EINVAL;
- err = snd_rawmidi_kernel_release(&msynth->input_rfile);
+
+ err = snd_rawmidi_kernel_release(&rfile);
return err;
}
--
2.55.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH v2] ALSA: seq: midi: Serialize input teardown with event_input
2026-08-13 15:08 [PATCH v2] ALSA: seq: midi: Serialize input teardown with event_input John Keeping
@ 2026-08-13 16:20 ` Takashi Iwai
0 siblings, 0 replies; 2+ messages in thread
From: Takashi Iwai @ 2026-08-13 16:20 UTC (permalink / raw)
To: John Keeping
Cc: Takashi Iwai, Jaroslav Kysela, Zhang Cen, Uwe Kleine-König,
Kees Cook, linux-sound, linux-kernel
On Thu, 13 Aug 2026 17:08:08 +0200,
John Keeping wrote:
>
> snd_midi_input_event() must not be running while a rawmidi substream is
> closing, since this can lead to the trigger state becoming out-of-step
> through this sequence in snd_rawmidi_input_trigger():
>
> snd_rawmidi_input_trigger(up=0)
> snd_midi_input_event()
> -> snd_rawmidi_kernel_read()
> -> snd_rawmidi_input_trigger(up=1)
> -> cancel_work_sync()
>
> which ends with the underlying device being active unexpectedly.
>
> When this is called from close_substream(), further input can re-trigger
> the input event leaving it running after rawmidi_release_priv() has set
> rfile->rmidi to NULL which leads to:
>
> Unable to handle kernel NULL pointer dereference at virtual address 00000000000000b0
> Call trace:
> snd_midi_input_event+0x3c/0x134 [snd_seq_midi] (P)
> snd_rawmidi_input_event_work+0x1c/0x2c
> process_one_work+0x150/0x3a4
> worker_thread+0x190/0x318
>
> Apply a similar approach to commit ef7607ab1c8ad ("ALSA: seq: midi:
> Serialize output teardown with event_input") which fixed the same issue
> in the output direction, but updated to use RCU following Takashi Iwai's
> proposed follow-on patch [1].
>
> With this change in place, midisynth_unsubscribe() clears the input file
> so snd_midi_input_event() will not re-trigger the stream and will be
> quiesced by the cancel_work_sync() in snd_rawmidi_input_trigger().
>
> [1] https://lore.kernel.org/linux-sound/20260813144224.753399-1-tiwai@suse.de/
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: John Keeping <jkeeping@inmusicbrands.com>
> ---
> Changes in v2:
> - Switch to using RCU following Takashi's suggestion
Applied to for-next branch now. Thanks.
Takashi
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-13 16:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-13 15:08 [PATCH v2] ALSA: seq: midi: Serialize input teardown with event_input John Keeping
2026-08-13 16:20 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox