| Message ID | 20220325092706.17135-1-frederic.danis@collabora.com |
|---|---|
| State | New |
| Headers | show |
| Series | a2dp: Fix crash when SEP codec has not been initialized | expand |
Hi Frédéric, On Fri, Mar 25, 2022 at 12:53 PM Frédéric Danis <frederic.danis@collabora.com> wrote: > > If SEP has not been properly discovered avdtp_get_codec may return NULL > thus causing crashes such as when running AVRCP/TG/VLH/BI-01-C after > AVRCP/TG/RCR/BV-04-C > --- > profiles/audio/a2dp.c | 17 ++++++++++++++--- > 1 file changed, 14 insertions(+), 3 deletions(-) > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > index f761dbe54..7da008071 100644 > --- a/profiles/audio/a2dp.c > +++ b/profiles/audio/a2dp.c > @@ -1995,7 +1995,12 @@ static gboolean get_codec(const GDBusPropertyTable *property, > { > struct a2dp_remote_sep *sep = data; > struct avdtp_service_capability *cap = avdtp_get_codec(sep->sep); > - struct avdtp_media_codec_capability *codec = (void *) cap->data; > + struct avdtp_media_codec_capability *codec; > + > + if (!cap) > + return FALSE; > + > + codec = (void *) cap->data; > > dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, > &codec->media_codec_type); > @@ -2008,10 +2013,16 @@ static gboolean get_capabilities(const GDBusPropertyTable *property, > { > struct a2dp_remote_sep *sep = data; > struct avdtp_service_capability *service = avdtp_get_codec(sep->sep); > - struct avdtp_media_codec_capability *codec = (void *) service->data; > - uint8_t *caps = codec->data; > + struct avdtp_media_codec_capability *codec; > + uint8_t *caps; > DBusMessageIter array; > > + if (!service) > + return FALSE; > + > + codec = (void *) service->data; > + caps = codec->data; > + > dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, > DBUS_TYPE_BYTE_AS_STRING, &array); > > -- > 2.25.1 We should either have a .exist callback or not have the endpoint registered if its codec is not available, I'm leaning toward the latter given that it is useless to have the endpoint if it cannot be used without the codec information.
Hi Frédéric, On Fri, Mar 25, 2022 at 1:06 PM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Frédéric, > > On Fri, Mar 25, 2022 at 12:53 PM Frédéric Danis > <frederic.danis@collabora.com> wrote: > > > > If SEP has not been properly discovered avdtp_get_codec may return NULL > > thus causing crashes such as when running AVRCP/TG/VLH/BI-01-C after > > AVRCP/TG/RCR/BV-04-C > > --- > > profiles/audio/a2dp.c | 17 ++++++++++++++--- > > 1 file changed, 14 insertions(+), 3 deletions(-) > > > > diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c > > index f761dbe54..7da008071 100644 > > --- a/profiles/audio/a2dp.c > > +++ b/profiles/audio/a2dp.c > > @@ -1995,7 +1995,12 @@ static gboolean get_codec(const GDBusPropertyTable *property, > > { > > struct a2dp_remote_sep *sep = data; > > struct avdtp_service_capability *cap = avdtp_get_codec(sep->sep); > > - struct avdtp_media_codec_capability *codec = (void *) cap->data; > > + struct avdtp_media_codec_capability *codec; > > + > > + if (!cap) > > + return FALSE; > > + > > + codec = (void *) cap->data; > > > > dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, > > &codec->media_codec_type); > > @@ -2008,10 +2013,16 @@ static gboolean get_capabilities(const GDBusPropertyTable *property, > > { > > struct a2dp_remote_sep *sep = data; > > struct avdtp_service_capability *service = avdtp_get_codec(sep->sep); > > - struct avdtp_media_codec_capability *codec = (void *) service->data; > > - uint8_t *caps = codec->data; > > + struct avdtp_media_codec_capability *codec; > > + uint8_t *caps; > > DBusMessageIter array; > > > > + if (!service) > > + return FALSE; > > + > > + codec = (void *) service->data; > > + caps = codec->data; > > + > > dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, > > DBUS_TYPE_BYTE_AS_STRING, &array); > > > > -- > > 2.25.1 > > We should either have a .exist callback or not have the endpoint > registered if its codec is not available, I'm leaning toward the > latter given that it is useless to have the endpoint if it cannot be > used without the codec information. In case you missed my response on slack, here is the suggestion change: diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c index c3ac432a7..28654924b 100644 --- a/profiles/audio/a2dp.c +++ b/profiles/audio/a2dp.c @@ -2074,6 +2074,11 @@ static struct a2dp_remote_sep *register_remote_sep(void *data, void *user_data) if (sep) return sep; + if (avdtp_get_codec(rsep)) { + error("Unable to get remote sep codec"); + return NULL; + } + sep = new0(struct a2dp_remote_sep, 1); sep->chan = chan; sep->sep = rsep; > > > > -- > Luiz Augusto von Dentz
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c index f761dbe54..7da008071 100644 --- a/profiles/audio/a2dp.c +++ b/profiles/audio/a2dp.c @@ -1995,7 +1995,12 @@ static gboolean get_codec(const GDBusPropertyTable *property, { struct a2dp_remote_sep *sep = data; struct avdtp_service_capability *cap = avdtp_get_codec(sep->sep); - struct avdtp_media_codec_capability *codec = (void *) cap->data; + struct avdtp_media_codec_capability *codec; + + if (!cap) + return FALSE; + + codec = (void *) cap->data; dbus_message_iter_append_basic(iter, DBUS_TYPE_BYTE, &codec->media_codec_type); @@ -2008,10 +2013,16 @@ static gboolean get_capabilities(const GDBusPropertyTable *property, { struct a2dp_remote_sep *sep = data; struct avdtp_service_capability *service = avdtp_get_codec(sep->sep); - struct avdtp_media_codec_capability *codec = (void *) service->data; - uint8_t *caps = codec->data; + struct avdtp_media_codec_capability *codec; + uint8_t *caps; DBusMessageIter array; + if (!service) + return FALSE; + + codec = (void *) service->data; + caps = codec->data; + dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE_AS_STRING, &array);