Hi, To which kernel version should I apply it? Because: folkert@oensoens:~/linux-5.11.0$ patch -p1 < ~/alsa-patch.diff patching file sound/core/seq/seq_ports.c patching file sound/core/seq/seq_ports.h patching file sound/core/seq/seq_ports.c Hunk #1 succeeded at 526 (offset 12 lines). Hunk #2 succeeded at 538 (offset 12 lines). Hunk #3 succeeded at 547 with fuzz 2 (offset 12 lines). Hunk #4 FAILED at 602. 1 out of 4 hunks FAILED -- saving rejects to file sound/core/seq/seq_ports.c.rej On Tue, Aug 03, 2021 at 08:55:36AM +0200, Takashi Iwai wrote: > On Mon, 02 Aug 2021 21:53:49 +0200, > folkert wrote: > > > > > > [ kernel bug if repeatingly aconnect'ing midi devices ] > > > > > > > > > Does this happen if you do reconnect of kernel sequencer client? > > > > > You can use snd-virmidi as well as snd-dummy. > > > > > I'm asking it because it'll simplify the test a lot, which will be > > > > > almost self-contained. > > > > > > > > Like this? > > > > > > > > root@lappiemctopface:~# aplaymidi -l > > > > Port Client name Port name > > > > 14:0 Midi Through Midi Through Port-0 > > > > 20:0 Virtual Raw MIDI 1-0 VirMIDI 1-0 > > > > 21:0 Virtual Raw MIDI 1-1 VirMIDI 1-1 > > > > 22:0 Virtual Raw MIDI 1-2 VirMIDI 1-2 > > > > 23:0 Virtual Raw MIDI 1-3 VirMIDI 1-3 > > > > 128:0 rtpmidi lappiemctopface Network > > > > 128:1 rtpmidi lappiemctopface metronoom > > > > 128:2 rtpmidi lappiemctopface AppleMidi2IPMidiBridge > > > > 128:3 rtpmidi lappiemctopface oensoens > > > > 130:0 FLUID Synth (11462) Synth input port (11462:0) > > > > > > > > and then: > > > > > > > > root@lappiemctopface:~# cat test.sh > > > > while true > > > > do > > > > aconnect 20:0 21:0 > > > > aconnect -d 20:0 21:0 > > > > done > > > > > > > > root@lappiemctopface:~# for i in `seq 0 3` ; do (./test.sh &) ; done > > > > > > > > This hard locks-up my laptop: it doesn't even respond to capslock (led > > > > on/off) anymore nor the ctrl+prtscr+alt+b combination. > > > > > > Thanks, that's really helpful! > > > I see the possible race now. > > > > > Could you try the quick fix below? > > > > Something may have changed: > > > > folkert@oensoens:~$ aplaymidi -l > > ALSA lib seq_hw.c:466:(snd_seq_hw_open) open /dev/snd/seq failed: Permission denied > > Cannot open sequencer - Permission denied > > > > ??? > > Hrm, that's unexpected. > > Meanwhile, I reconsidered the fix and a better idea came after the > sleep. Below is the new fix patch. Could you give it a try instead > of the previous one? > > > thanks, > > Takashi > > -- 8< -- > From: Takashi Iwai <tiwai@xxxxxxx> > Subject: [PATCH] ALSA: seq: Fix racy deletion of subscriber > > It turned out that the current implementation of the port subscription > is racy. The subscription contains two linked lists, and we have to > add to or delete from both lists. Since both connection and > disconnection procedures perform the same order for those two lists > (i.e. src list, then dest list), when a deletion happens during a > connection procedure, the src list may be deleted before the dest list > addition completes, and this may lead to a use-after-free or an Oops, > even though the access to both lists are protected via mutex. > > The simple workaround for this race is to change the access order for > the disconnection, namely, dest list, then src list. This assures > that the connection has been established when disconnecting, and also > the concurrent deletion can be avoided. > > Reported-by: folkert <folkert@xxxxxxxxxxxxxx> > Cc: <stable@xxxxxxxxxxxxxxx> > Link: https://lore.kernel.org/r/20210801182754.GP890690@xxxxxxxxxxxxxxxxxxxxxxxxxxxxx > Signed-off-by: Takashi Iwai <tiwai@xxxxxxx> > --- > sound/core/seq/seq_ports.c | 39 ++++++++++++++++++++++++++------------ > 1 file changed, 27 insertions(+), 12 deletions(-) > > diff --git a/sound/core/seq/seq_ports.c b/sound/core/seq/seq_ports.c > index b9c2ce2b8d5a..84d78630463e 100644 > --- a/sound/core/seq/seq_ports.c > +++ b/sound/core/seq/seq_ports.c > @@ -514,10 +514,11 @@ static int check_and_subscribe_port(struct snd_seq_client *client, > return err; > } > > -static void delete_and_unsubscribe_port(struct snd_seq_client *client, > - struct snd_seq_client_port *port, > - struct snd_seq_subscribers *subs, > - bool is_src, bool ack) > +/* called with grp->list_mutex held */ > +static void __delete_and_unsubscribe_port(struct snd_seq_client *client, > + struct snd_seq_client_port *port, > + struct snd_seq_subscribers *subs, > + bool is_src, bool ack) > { > struct snd_seq_port_subs_info *grp; > struct list_head *list; > @@ -525,7 +526,6 @@ static void delete_and_unsubscribe_port(struct snd_seq_client *client, > > grp = is_src ? &port->c_src : &port->c_dest; > list = is_src ? &subs->src_list : &subs->dest_list; > - down_write(&grp->list_mutex); > write_lock_irq(&grp->list_lock); > empty = list_empty(list); > if (!empty) > @@ -535,6 +535,18 @@ static void delete_and_unsubscribe_port(struct snd_seq_client *client, > > if (!empty) > unsubscribe_port(client, port, grp, &subs->info, ack); > +} > + > +static void delete_and_unsubscribe_port(struct snd_seq_client *client, > + struct snd_seq_client_port *port, > + struct snd_seq_subscribers *subs, > + bool is_src, bool ack) > +{ > + struct snd_seq_port_subs_info *grp; > + > + grp = is_src ? &port->c_src : &port->c_dest; > + down_write(&grp->list_mutex); > + __delete_and_unsubscribe_port(client, port, subs, is_src, ack); > up_write(&grp->list_mutex); > } > > @@ -590,27 +602,30 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector, > struct snd_seq_client_port *dest_port, > struct snd_seq_port_subscribe *info) > { > - struct snd_seq_port_subs_info *src = &src_port->c_src; > + struct snd_seq_port_subs_info *dest = &dest_port->c_dest; > struct snd_seq_subscribers *subs; > int err = -ENOENT; > > - down_write(&src->list_mutex); > + /* always start from deleting the dest port for avoiding concurrent > + * deletions > + */ > + down_write(&dest->list_mutex); > /* look for the connection */ > - list_for_each_entry(subs, &src->list_head, src_list) { > + list_for_each_entry(subs, &dest->list_head, dest_list) { > if (match_subs_info(info, &subs->info)) { > - atomic_dec(&subs->ref_count); /* mark as not ready */ > + __delete_and_unsubscribe_port(dest_client, dest_port, > + subs, false, > + connector->number != dest_client->number); > err = 0; > break; > } > } > - up_write(&src->list_mutex); > + up_write(&dest->list_mutex); > if (err < 0) > return err; > > delete_and_unsubscribe_port(src_client, src_port, subs, true, > connector->number != src_client->number); > - delete_and_unsubscribe_port(dest_client, dest_port, subs, false, > - connector->number != dest_client->number); > kfree(subs); > return 0; > } > -- > 2.26.2 Folkert van Heusden -- MultiTail is a versatile tool for watching logfiles and output of commands. Filtering, coloring, merging, diff-view, etc. http://www.vanheusden.com/multitail/ ---------------------------------------------------------------------- Phone: +31-6-41278122, PGP-key: 1F28D8AE, www.vanheusden.com
