aconnect occasionally causes kernel oopses

[Takashi Iwai]

On Mon, 02 Aug 2021 17:21:17 +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 at 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 at lappiemctopface:~# cat test.sh 
> while true
> do
> 	aconnect 20:0 21:0
> 	aconnect -d 20:0 21:0
> done
> 
> root at 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?


Takashi

---
--- a/sound/core/seq/seq_ports.c
+++ b/sound/core/seq/seq_ports.c
@@ -200,6 +200,17 @@ get_subscriber(struct list_head *p, bool is_src)
 		return list_entry(p, struct snd_seq_subscribers, dest_list);
 }
 
+static void subscriber_get(struct snd_seq_subscribers *subs)
+{
+	atomic_inc(&subs->ref_count);
+}
+
+static void subscriber_put(struct snd_seq_subscribers *subs)
+{
+	if (atomic_dec_and_test(&subs->ref_count))
+		kfree(subs);
+}
+
 /*
  * remove all subscribers on the list
  * this is called from port_delete, for each src and dest list.
@@ -228,8 +239,7 @@ static void clear_subscriber_list(struct snd_seq_client *client,
 			 * we decrease the counter, and when both ports are deleted
 			 * remove the subscriber info
 			 */
-			if (atomic_dec_and_test(&subs->ref_count))
-				kfree(subs);
+			subscriber_put(subs);
 			continue;
 		}
 
@@ -489,6 +499,8 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
 			s = get_subscriber(p, is_src);
 			if (match_subs_info(&subs->info, &s->info))
 				goto __error;
+			if (!subs->ready)
+				goto __error;
 		}
 	}
 
@@ -505,7 +517,7 @@ static int check_and_subscribe_port(struct snd_seq_client *client,
 	else
 		list_add_tail(&subs->dest_list, &grp->list_head);
 	grp->exclusive = exclusive;
-	atomic_inc(&subs->ref_count);
+	subscriber_get(subs);
 	write_unlock_irq(&grp->list_lock);
 	err = 0;
 
@@ -536,6 +548,7 @@ static void delete_and_unsubscribe_port(struct snd_seq_client *client,
 	if (!empty)
 		unsubscribe_port(client, port, grp, &subs->info, ack);
 	up_write(&grp->list_mutex);
+	subscriber_put(subs);
 }
 
 /* connect two ports */
@@ -555,7 +568,7 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
 		return -ENOMEM;
 
 	subs->info = *info;
-	atomic_set(&subs->ref_count, 0);
+	atomic_set(&subs->ref_count, 1);
 	INIT_LIST_HEAD(&subs->src_list);
 	INIT_LIST_HEAD(&subs->dest_list);
 
@@ -572,13 +585,14 @@ int snd_seq_port_connect(struct snd_seq_client *connector,
 	if (err < 0)
 		goto error_dest;
 
+	subs->ready = true;
 	return 0;
 
  error_dest:
 	delete_and_unsubscribe_port(src_client, src_port, subs, true,
 				    connector->number != src_client->number);
  error:
-	kfree(subs);
+	subscriber_put(subs);
 	return err;
 }
 
@@ -597,8 +611,8 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
 	down_write(&src->list_mutex);
 	/* look for the connection */
 	list_for_each_entry(subs, &src->list_head, src_list) {
-		if (match_subs_info(info, &subs->info)) {
-			atomic_dec(&subs->ref_count); /* mark as not ready */
+		if (match_subs_info(info, &subs->info) && subs->ready) {
+			subs->ready = false;
 			err = 0;
 			break;
 		}
@@ -611,7 +625,7 @@ int snd_seq_port_disconnect(struct snd_seq_client *connector,
 				    connector->number != src_client->number);
 	delete_and_unsubscribe_port(dest_client, dest_port, subs, false,
 				    connector->number != dest_client->number);
-	kfree(subs);
+	subscriber_put(subs);
 	return 0;
 }
 
diff --git a/sound/core/seq/seq_ports.h b/sound/core/seq/seq_ports.h
index b1f2c4943174..d9644d5f109e 100644
--- a/sound/core/seq/seq_ports.h
+++ b/sound/core/seq/seq_ports.h
@@ -30,6 +30,7 @@ struct snd_seq_subscribers {
 	struct list_head src_list;	/* link of sources */
 	struct list_head dest_list;	/* link of destinations */
 	atomic_t ref_count;
+	bool ready;
 };
 
 struct snd_seq_port_subs_info {