Add a method for getting the audio stream associated with a media
transport.
---
Notes:
v2: make generic and split out to separate patch
profiles/audio/transport.c | 18 ++++++++++++++++++
profiles/audio/transport.h | 1 +
2 files changed, 19 insertions(+)
diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 5e057e2a5..912f404e8 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -116,6 +116,8 @@ struct media_transport {
guint id);
void (*set_state) (struct media_transport *transport,
transport_state_t state);
+ void *(*get_stream)
+ (struct media_transport *transport);
GDestroyNotify destroy;
void *data;
};
@@ -1380,6 +1382,13 @@ static void bap_connecting(struct bt_bap_stream *stream, bool state, int fd,
bap_update_links(transport);
}
+static void *get_stream_bap(struct media_transport *transport)
+{
+ struct bap_transport *bap = transport->data;
+
+ return bap->stream;
+}
+
static void free_bap(void *data)
{
struct bap_transport *bap = data;
@@ -1415,6 +1424,7 @@ static int media_transport_init_bap(struct media_transport *transport,
transport->suspend = suspend_bap;
transport->cancel = cancel_bap;
transport->set_state = set_state_bap;
+ transport->get_stream = get_stream_bap;
transport->destroy = free_bap;
return 0;
@@ -1483,6 +1493,14 @@ const char *media_transport_get_path(struct media_transport *transport)
return transport->path;
}
+void *media_transport_get_stream(struct media_transport *transport)
+{
+ if (transport->get_stream)
+ return transport->get_stream(transport);
+
+ return NULL;
+}
+
void media_transport_update_delay(struct media_transport *transport,
uint16_t delay)
{
diff --git a/profiles/audio/transport.h b/profiles/audio/transport.h
index 102fc3cf1..5ca9b8f9e 100644
--- a/profiles/audio/transport.h
+++ b/profiles/audio/transport.h
@@ -19,6 +19,7 @@ struct media_transport *media_transport_create(struct btd_device *device,
void media_transport_destroy(struct media_transport *transport);
const char *media_transport_get_path(struct media_transport *transport);
+void *media_transport_get_stream(struct media_transport *transport);
struct btd_device *media_transport_get_dev(struct media_transport *transport);
int8_t media_transport_get_volume(struct media_transport *transport);
void media_transport_update_delay(struct media_transport *transport,
--
2.39.1
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
...
#4 0x559891 in btd_debug src/log.c:117
#5 0x46abfd in pac_clear profiles/audio/media.c:1096
#6 0x79fcaf in bap_stream_clear_cfm src/shared/bap.c:914
#7 0x7a060d in bap_stream_detach src/shared/bap.c:987
#8 0x7a25ea in bap_stream_state_changed src/shared/bap.c:1210
#9 0x7a29cd in stream_set_state src/shared/bap.c:1254
#10 0x7be824 in stream_foreach_detach src/shared/bap.c:3820
#11 0x71d15d in queue_foreach src/shared/queue.c:207
#12 0x7beb98 in bt_bap_detach src/shared/bap.c:3836
#13 0x5228cb in bap_disconnect profiles/audio/bap.c:1342
#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)
#1 0x7f167071b8cc in g_free (/lib64/libglib-2.0.so.0+0x5b8cc)
#2 0x7047b7 in remove_interface gdbus/object.c:660
#3 0x70aef6 in g_dbus_unregister_interface gdbus/object.c:1394
#4 0x47be30 in media_transport_destroy profiles/audio/transport.c:217
#5 0x464ab9 in endpoint_remove_transport profiles/audio/media.c:270
#6 0x464d26 in clear_configuration profiles/audio/media.c:292
#7 0x464e69 in clear_endpoint profiles/audio/media.c:300
#8 0x46516e in endpoint_reply profiles/audio/media.c:325
...
Fixes: 7b1b1a499cf3 ("media: clear the right transport when clearing BAP endpoint")
---
Notes:
v2:
* Change find_transport to find them by stream, not path.
To reproduce with pipewire,
gdb -p `pidof wireplumber`
break bluez5-dbus.c:endpoint_set_configuration
c
and then connect with bluetoothctl to a BAP server, and disconnect after
NoReply dbus error is received.
profiles/audio/media.c | 34 +++++++++++-----------------------
1 file changed, 11 insertions(+), 23 deletions(-)
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index 479a11db9..b722278ba 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -978,23 +978,20 @@ struct pac_config_data {
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;
@@ -1022,11 +1019,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);
@@ -1046,14 +1041,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);
}
@@ -1087,19 +1082,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 = {
--
2.39.1
Don't call configuration callback if stream's transport was cleared in
the meantime. The clear callback is called just before the stream is
freed.
Fixes ASAN crash on disconnect while waiting for SetConfiguration DBus
reply:
ERROR: AddressSanitizer: heap-use-after-free on address 0x60b00002eb90
READ of size 8 at 0x60b00002eb90 thread T0
#0 0x7a4892 in bap_stream_config_cfm_cb src/shared/bap.c:3201
#1 0x4688fb in pac_config_cb profiles/audio/media.c:1010
#2 0x462164 in media_endpoint_cancel profiles/audio/media.c:157
#3 0x462243 in media_endpoint_cancel_all profiles/audio/media.c:165
#4 0x46365b in clear_endpoint profiles/audio/media.c:297
#5 0x463a21 in endpoint_reply profiles/audio/media.c:325
...
freed by thread T0 here:
#0 0x7eff644b9388 in __interceptor_free.part.0 (/lib64/libasan.so.8+0xb9388)
#1 0x78d8cc in bap_stream_free src/shared/bap.c:974
#2 0x78dbc8 in bap_stream_detach src/shared/bap.c:991
#3 0x78fa43 in bap_stream_state_changed src/shared/bap.c:1210
#4 0x78fe26 in stream_set_state src/shared/bap.c:1254
#5 0x7ab5ce in stream_foreach_detach src/shared/bap.c:3820
#6 0x70ce06 in queue_foreach src/shared/queue.c:207
#7 0x7ab942 in bt_bap_detach src/shared/bap.c:3836
#8 0x51da7a in bap_disconnect profiles/audio/bap.c:1342
#9 0x626e57 in btd_service_disconnect src/service.c:305
---
Notes:
Reproducer:
Make sound server delay its SetConfiguration response, and disconnect
device before DBus timeout is reached.
profiles/audio/media.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/profiles/audio/media.c b/profiles/audio/media.c
index b722278ba..326e50a09 100644
--- a/profiles/audio/media.c
+++ b/profiles/audio/media.c
@@ -1003,6 +1003,12 @@ static void pac_config_cb(struct media_endpoint *endpoint, void *ret, int size,
{
struct pac_config_data *data = user_data;
gboolean *ret_value = ret;
+ struct media_transport *transport;
+
+ /* If transport was cleared, configuration was cancelled */
+ transport = find_transport(endpoint, data->stream);
+ if (!transport)
+ return;
data->cb(data->stream, ret_value ? 0 : -EINVAL);
}
--
2.39.1
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=722269
---Test result---
Test Summary:
CheckPatch FAIL 1.41 seconds
GitLint FAIL 0.98 seconds
BuildEll PASS 26.48 seconds
BluezMake PASS 754.93 seconds
MakeCheck PASS 11.39 seconds
MakeDistcheck PASS 147.58 seconds
CheckValgrind PASS 241.93 seconds
CheckSmatch PASS 318.98 seconds
bluezmakeextell PASS 96.52 seconds
IncrementalBuild PASS 1841.66 seconds
ScanBuild PASS 962.98 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[BlueZ,v2,2/3] media: look up BAP transports by their associated stream
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#80:
#0 0x7f16708b9388 in __interceptor_free.part.0 (/lib64/libasan.so.8+0xb9388)
/github/workspace/src/src/13142251.patch total: 0 errors, 1 warnings, 78 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13142251.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
[BlueZ,v2,3/3] media: fix ASAN crash in pac_config_cb
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#67:
#0 0x7eff644b9388 in __interceptor_free.part.0 (/lib64/libasan.so.8+0xb9388)
/github/workspace/src/src/13142252.patch total: 0 errors, 1 warnings, 12 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/src/13142252.patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[BlueZ,v2,1/3] audio/transport: add media_transport_get_stream method for transports
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
1: T1 Title exceeds max length (84>80): "[BlueZ,v2,1/3] audio/transport: add media_transport_get_stream method for transports"
[BlueZ,v2,2/3] media: look up BAP transports by their associated stream
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
48: B2 Line has trailing whitespace: " "
53: B2 Line has trailing whitespace: " "
[BlueZ,v2,3/3] media: fix ASAN crash in pac_config_cb
WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
34: B2 Line has trailing whitespace: " "
---
Regards,
Linux Bluetooth
Hello:
This series was applied to bluetooth/bluez.git (master)
by Luiz Augusto von Dentz <[email protected]>:
On Wed, 15 Feb 2023 22:26:00 +0000 you wrote:
> Add a method for getting the audio stream associated with a media
> transport.
> ---
>
> Notes:
> v2: make generic and split out to separate patch
>
> [...]
Here is the summary with links:
- [BlueZ,v2,1/3] audio/transport: add media_transport_get_stream method for transports
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=f7d0599afe9b
- [BlueZ,v2,2/3] media: look up BAP transports by their associated stream
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=e9163b09a3df
- [BlueZ,v2,3/3] media: fix ASAN crash in pac_config_cb
https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=5d347b54714e
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html