Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Marijn Suijten <marijns95@gmail.com>
To:     linux-bluetooth@vger.kernel.org
Cc:     luiz.dentz@gmail.com, pali@kernel.org,
        Marijn Suijten <marijns95@gmail.com>
Subject: [PATCH BlueZ] audio/a2dp: a2dp_channel should have a refcount on avdtp session
Date:   Sun, 25 Oct 2020 17:27:30 +0100
Message-Id: <20201025162730.47247-1-marijns95@gmail.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

a2dp_channel keeps a reference to an avdtp session without incrementing
its refcount.  Not only does this appear wrong, it causes unexpected
disconnections when the remote SEP responds with rejections.

During testing with an audio application disconnections are observed
when a codec config change through MediaEndpoint1.SetConfiguration
fails.  As soon as BlueZ receives this failure from the peer the
corresponding a2dp_setup object is cleaned up which holds the last
refcount to an avdtp session, in turn starting the disconnect process.
An eventual open sink/source and transport have already closed by that
time and released their refcounts.

Adding refcounting semantics around a2dp_channel resolves the
disconnections and allows future calls on MediaEndpoint1 to safely
access the sesion stored within this channel.
---
 profiles/audio/a2dp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index cc4866b5b..0eac0135f 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1507,6 +1507,9 @@ static void channel_free(void *data)
 
 	avdtp_remove_state_cb(chan->state_id);
 
+	if (chan->session)
+		avdtp_unref(chan->session);
+
 	queue_destroy(chan->seps, remove_remote_sep);
 	free(chan->last_used);
 	g_free(chan);
@@ -2065,7 +2068,7 @@ static void avdtp_state_cb(struct btd_device *dev, struct avdtp *session,
 		break;
 	case AVDTP_SESSION_STATE_CONNECTED:
 		if (!chan->session)
-			chan->session = session;
+			chan->session = avdtp_ref(session);
 		load_remote_seps(chan);
 		break;
 	}
@@ -2145,6 +2148,7 @@ found:
 		channel_remove(chan);
 		return NULL;
 	}
+	avdtp_ref(chan->session);
 
 	return avdtp_ref(chan->session);
 }
@@ -2165,6 +2169,7 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data)
 			error("Unable to create AVDTP session");
 			goto fail;
 		}
+		avdtp_ref(chan->session);
 	}
 
 	g_io_channel_unref(chan->io);
-- 
2.29.1

Marijn Suijten