Skip to content

Commit e9163b0

Browse files
pvVudentz
authored andcommitted
media: look up BAP transports by their associated stream
To look up transports, use BAP stream pointers associated with them, not the path strings stored in the stream user data. This makes it clearer that transports presented to the sound server correspond to the actual streams. The Acquire/etc. of BAP transports are already tied to the associated stream. This fixes use-after-free crashes in pac_clear. They occur because the lifetime of the path string was either that of media transport or media endpoint, which may be shorter than that of the BAP stream. In such case, pac_clear is entered with invalid pointer in stream user data, leading to crash. There are a few code paths for this, e.g. making sound server delay its SetConfiguration response (e.g. gdb breakpoint) to get dbus timeout, then disconnecting: ERROR: AddressSanitizer: heap-use-after-free on address XXXX READ of size 3 at 0x606000031640 thread T0 ... bluez#4 0x559891 in btd_debug src/log.c:117 bluez#5 0x46abfd in pac_clear profiles/audio/media.c:1096 bluez#6 0x79fcaf in bap_stream_clear_cfm src/shared/bap.c:914 bluez#7 0x7a060d in bap_stream_detach src/shared/bap.c:987 bluez#8 0x7a25ea in bap_stream_state_changed src/shared/bap.c:1210 bluez#9 0x7a29cd in stream_set_state src/shared/bap.c:1254 bluez#10 0x7be824 in stream_foreach_detach src/shared/bap.c:3820 bluez#11 0x71d15d in queue_foreach src/shared/queue.c:207 bluez#12 0x7beb98 in bt_bap_detach src/shared/bap.c:3836 bluez#13 0x5228cb in bap_disconnect profiles/audio/bap.c:1342 bluez#14 0x63247c in btd_service_disconnect src/service.c:305 freed by thread T0 here: #0 0x7f16708b9388 in __interceptor_free.part.0 (/lib64/libasan.so.8+0xb9388) ruundii#1 0x7f167071b8cc in g_free (/lib64/libglib-2.0.so.0+0x5b8cc) bluez#2 0x7047b7 in remove_interface gdbus/object.c:660 bluez#3 0x70aef6 in g_dbus_unregister_interface gdbus/object.c:1394 bluez#4 0x47be30 in media_transport_destroy profiles/audio/transport.c:217 bluez#5 0x464ab9 in endpoint_remove_transport profiles/audio/media.c:270 bluez#6 0x464d26 in clear_configuration profiles/audio/media.c:292 bluez#7 0x464e69 in clear_endpoint profiles/audio/media.c:300 bluez#8 0x46516e in endpoint_reply profiles/audio/media.c:325 ... Fixes: 7b1b1a4 ("media: clear the right transport when clearing BAP endpoint")
1 parent f7d0599 commit e9163b0

File tree

1 file changed

+11
-23
lines changed

1 file changed

+11
-23
lines changed

profiles/audio/media.c

+11-23
Original file line numberDiff line numberDiff line change
@@ -978,23 +978,20 @@ struct pac_config_data {
978978
static int transport_cmp(gconstpointer data, gconstpointer user_data)
979979
{
980980
const struct media_transport *transport = data;
981-
const char *path = user_data;
982981

983-
if (g_str_has_prefix(media_transport_get_path((void *)transport), path))
982+
if (media_transport_get_stream((void *)transport) == user_data)
984983
return 0;
985984

986985
return -1;
987986
}
988987

989988
static struct media_transport *find_transport(struct media_endpoint *endpoint,
990-
const char *path)
989+
void *stream)
991990
{
992991
GSList *match;
993992

994-
if (!path)
995-
return NULL;
996-
997-
match = g_slist_find_custom(endpoint->transports, path, transport_cmp);
993+
match = g_slist_find_custom(endpoint->transports, stream,
994+
transport_cmp);
998995
if (match == NULL)
999996
return NULL;
1000997

@@ -1022,11 +1019,9 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
10221019
DBusMessageIter iter;
10231020
const char *path;
10241021

1025-
path = bt_bap_stream_get_user_data(stream);
1022+
DBG("endpoint %p stream %p", endpoint, stream);
10261023

1027-
DBG("endpoint %p path %s", endpoint, path);
1028-
1029-
transport = find_transport(endpoint, path);
1024+
transport = find_transport(endpoint, stream);
10301025
if (!transport) {
10311026
struct bt_bap *bap = bt_bap_stream_get_session(stream);
10321027
struct btd_service *service = bt_bap_get_user_data(bap);
@@ -1046,14 +1041,14 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
10461041
return -EINVAL;
10471042
}
10481043

1044+
path = bt_bap_stream_get_user_data(stream);
1045+
10491046
transport = media_transport_create(device, path, cfg->iov_base,
10501047
cfg->iov_len, endpoint,
10511048
stream);
10521049
if (!transport)
10531050
return -EINVAL;
10541051

1055-
path = media_transport_get_path(transport);
1056-
bt_bap_stream_set_user_data(stream, (void *)path);
10571052
endpoint->transports = g_slist_append(endpoint->transports,
10581053
transport);
10591054
}
@@ -1087,19 +1082,12 @@ static void pac_clear(struct bt_bap_stream *stream, void *user_data)
10871082
{
10881083
struct media_endpoint *endpoint = user_data;
10891084
struct media_transport *transport;
1090-
const char *path;
1091-
1092-
path = bt_bap_stream_get_user_data(stream);
1093-
if (!path)
1094-
return;
10951085

1096-
DBG("endpoint %p path %s", endpoint, path);
1086+
DBG("endpoint %p stream %p", endpoint, stream);
10971087

1098-
transport = find_transport(endpoint, path);
1099-
if (transport) {
1088+
transport = find_transport(endpoint, stream);
1089+
if (transport)
11001090
clear_configuration(endpoint, transport);
1101-
bt_bap_stream_set_user_data(stream, NULL);
1102-
}
11031091
}
11041092

11051093
static struct bt_bap_pac_ops pac_ops = {

0 commit comments

Comments
 (0)