| Message ID | 20200312135409.Bluez.v1.1.I24708f815f78397d51e263f5c68fc47ec0b76acd@changeid (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Delegated to: | Luiz Von Dentz |
| Headers | show |
| Series | [Bluez,v1] a2dp: Fix race when connecting and being connected at the same time | expand |
Hi Archie, On Wed, Mar 11, 2020 at 10:55 PM Archie Pusaka <apusaka@google.com> wrote: > > From: Archie Pusaka <apusaka@chromium.org> > > There is a possibility where BlueZ initiate an A2DP connection just > around the same time as the peripheral also initiate it. > > One scenario is the peripheral initiate the connection first, so > confirm_cb() on /profiles/audio/a2dp.c is called. However, while we > are waiting for the authentication step, BlueZ initiate a connection > to the peripheral, therefore a2dp_sink_connect() is called, which > from there a2dp_avdtp_get() is called. > > If this happens: When calling confirm_cb(), chan for the > corresponding device is created. > > Then when calling a2dp_avdtp_get(), chan will be found as it is > created in confirm_cb(), and the value of chan->io is not NULL. > However, a NULL is supplied instead to create a new session and > assigned to chan->session. > > Then when calling connect_cb(), chan->session will NOT be NULL, as > it is assigned in a2dp_avdtp_get(). Nevertheless, chan->session is > always assigned a new value. > > These cause failure in connection. > > Therefore, fixing this by supplying the value of chan->io inside > a2dp_avdtp_get() (it's going to be NULL on the normal case so it is > fine), and check whether chan->session already assigned inside > connect_cb(). > --- > > profiles/audio/a2dp.c | 11 +++++++---- > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > index e8262cdfe..a5590b24c 100644 > --- a/profiles/audio/a2dp.c > +++ b/profiles/audio/a2dp.c > @@ -2118,7 +2118,7 @@ struct avdtp *a2dp_avdtp_get(struct btd_device *device) > return NULL; > > found: > - chan->session = avdtp_new(NULL, device, server->seps); > + chan->session = avdtp_new(chan->io, device, server->seps); > if (!chan->session) { > channel_remove(chan); > return NULL; > @@ -2136,10 +2136,13 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) > goto fail; > } > > - chan->session = avdtp_new(chan->io, chan->device, chan->server->seps); > if (!chan->session) { > - error("Unable to create AVDTP session"); > - goto fail; > + chan->session = avdtp_new(chan->io, chan->device, > + chan->server->seps); > + if (!chan->session) { > + error("Unable to create AVDTP session"); > + goto fail; > + } > } > > g_io_channel_unref(chan->io); > -- > 2.25.1.481.gfbce0eb801-goog Applied, thanks.
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c index e8262cdfe..a5590b24c 100644 --- a/profiles/audio/a2dp.c +++ b/profiles/audio/a2dp.c @@ -2118,7 +2118,7 @@ struct avdtp *a2dp_avdtp_get(struct btd_device *device) return NULL; found: - chan->session = avdtp_new(NULL, device, server->seps); + chan->session = avdtp_new(chan->io, device, server->seps); if (!chan->session) { channel_remove(chan); return NULL; @@ -2136,10 +2136,13 @@ static void connect_cb(GIOChannel *io, GError *err, gpointer user_data) goto fail; } - chan->session = avdtp_new(chan->io, chan->device, chan->server->seps); if (!chan->session) { - error("Unable to create AVDTP session"); - goto fail; + chan->session = avdtp_new(chan->io, chan->device, + chan->server->seps); + if (!chan->session) { + error("Unable to create AVDTP session"); + goto fail; + } } g_io_channel_unref(chan->io);
From: Archie Pusaka <apusaka@chromium.org> There is a possibility where BlueZ initiate an A2DP connection just around the same time as the peripheral also initiate it. One scenario is the peripheral initiate the connection first, so confirm_cb() on /profiles/audio/a2dp.c is called. However, while we are waiting for the authentication step, BlueZ initiate a connection to the peripheral, therefore a2dp_sink_connect() is called, which from there a2dp_avdtp_get() is called. If this happens: When calling confirm_cb(), chan for the corresponding device is created. Then when calling a2dp_avdtp_get(), chan will be found as it is created in confirm_cb(), and the value of chan->io is not NULL. However, a NULL is supplied instead to create a new session and assigned to chan->session. Then when calling connect_cb(), chan->session will NOT be NULL, as it is assigned in a2dp_avdtp_get(). Nevertheless, chan->session is always assigned a new value. These cause failure in connection. Therefore, fixing this by supplying the value of chan->io inside a2dp_avdtp_get() (it's going to be NULL on the normal case so it is fine), and check whether chan->session already assigned inside connect_cb(). --- profiles/audio/a2dp.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)