On Tue, 03 Aug 2021 09:40:50 +0200, folkert wrote: > > 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 Did you scratch the previous patch? AFAIK, the latest patch should be applicable to 5.11 and older cleanly. Takashi > > 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 >
