Skip to content

Commit

Permalink
media: look up BAP transports by their associated stream
Browse files Browse the repository at this point in the history
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.

This fixes use-after-free crashes in pac_clear.  They occur because the
lifetime of the path string is 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,
which crashes.  There are a few code paths for this. One is 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)
    bluez#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")
  • Loading branch information
pv committed Feb 15, 2023
1 parent 125c696 commit 62e2ac4
Showing 1 changed file with 11 additions and 22 deletions.
33 changes: 11 additions & 22 deletions profiles/audio/media.c
Original file line number Diff line number Diff line change
Expand Up @@ -980,21 +980,19 @@ static int transport_cmp(gconstpointer data, gconstpointer user_data)
const struct media_transport *transport = data;
const char *path = user_data;

if (g_str_has_prefix(media_transport_get_path((void *)transport), path))
if (media_transport_get_stream((void *)transport) == user_data)
return 0;

return -1;
}

static struct media_transport *find_transport(struct media_endpoint *endpoint,
const char *path)
void *stream)
{
GSList *match;

if (!path)
return NULL;

match = g_slist_find_custom(endpoint->transports, path, transport_cmp);
match = g_slist_find_custom(endpoint->transports, stream,
transport_cmp);
if (match == NULL)
return NULL;

Expand Down Expand Up @@ -1022,11 +1020,9 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
DBusMessageIter iter;
const char *path;

path = bt_bap_stream_get_user_data(stream);
DBG("endpoint %p stream %p", endpoint, stream);

DBG("endpoint %p path %s", endpoint, path);

transport = find_transport(endpoint, path);
transport = find_transport(endpoint, stream);
if (!transport) {
struct bt_bap *bap = bt_bap_stream_get_session(stream);
struct btd_service *service = bt_bap_get_user_data(bap);
Expand All @@ -1046,14 +1042,14 @@ static int pac_config(struct bt_bap_stream *stream, struct iovec *cfg,
return -EINVAL;
}

path = bt_bap_stream_get_user_data(stream);

transport = media_transport_create(device, path, cfg->iov_base,
cfg->iov_len, endpoint,
stream);
if (!transport)
return -EINVAL;

path = media_transport_get_path(transport);
bt_bap_stream_set_user_data(stream, (void *)path);
endpoint->transports = g_slist_append(endpoint->transports,
transport);
}
Expand Down Expand Up @@ -1087,19 +1083,12 @@ static void pac_clear(struct bt_bap_stream *stream, void *user_data)
{
struct media_endpoint *endpoint = user_data;
struct media_transport *transport;
const char *path;

path = bt_bap_stream_get_user_data(stream);
if (!path)
return;

DBG("endpoint %p path %s", endpoint, path);
DBG("endpoint %p stream %p", endpoint, stream);

transport = find_transport(endpoint, path);
if (transport) {
transport = find_transport(endpoint, stream);
if (transport)
clear_configuration(endpoint, transport);
bt_bap_stream_set_user_data(stream, NULL);
}
}

static struct bt_bap_pac_ops pac_ops = {
Expand Down

0 comments on commit 62e2ac4

Please sign in to comment.