Hi Pauli,
On Fri, Apr 12, 2024 at 3:58 PM Pauli Virtanen <pav@xxxxxx> wrote:
>
> Cancel stream's queued requests before freeing the stream.
>
> As the callbacks may do some cleanup on error, be sure to call them
> before removing the requests.
>
> Fixes:
> =======================================================================
> ERROR: AddressSanitizer: heap-use-after-free on address 0x60d000013430
> READ of size 8 at 0x60d000013430 thread T0
> #0 0x89cb9f in stream_stop_complete src/shared/bap.c:1211
> #1 0x89c997 in bap_req_complete src/shared/bap.c:1192
> #2 0x8a105f in bap_process_queue src/shared/bap.c:1474
> #3 0x93c93f in timeout_callback src/shared/timeout-glib.c:25
> ...
> freed by thread T0 here:
> #1 0x89b744 in bap_stream_free src/shared/bap.c:1105
> #2 0x89bac8 in bap_stream_detach src/shared/bap.c:1122
> #3 0x89dbfc in bap_stream_state_changed src/shared/bap.c:1261
> #4 0x8a2169 in bap_ucast_set_state src/shared/bap.c:1554
> #5 0x89e0d5 in stream_set_state src/shared/bap.c:1291
> #6 0x8a78b6 in bap_ucast_release src/shared/bap.c:1927
> #7 0x8d45bb in bt_bap_stream_release src/shared/bap.c:5516
> #8 0x8ba63f in remove_streams src/shared/bap.c:3538
> #9 0x7f23d0 in queue_foreach src/shared/queue.c:207
> #10 0x8bb875 in bt_bap_remove_pac src/shared/bap.c:3593
> #11 0x47416c in media_endpoint_destroy profiles/audio/media.c:185
> =======================================================================
> ---
> src/shared/bap.c | 27 +++++++++++++++++++++++++++
> 1 file changed, 27 insertions(+)
>
> diff --git a/src/shared/bap.c b/src/shared/bap.c
> index 5fee7b4c5..ccde26431 100644
> --- a/src/shared/bap.c
> +++ b/src/shared/bap.c
> @@ -1105,6 +1105,9 @@ static void bap_stream_free(void *data)
> free(stream);
> }
>
> +static void bap_abort_stream_req(struct bt_bap *bap,
> + struct bt_bap_stream *stream);
Normally we suggest just to move up the function definitions to avoid
forward declarations like the above.
> static void bap_stream_detach(struct bt_bap_stream *stream)
> {
> struct bt_bap_endpoint *ep = stream->ep;
> @@ -1114,6 +1117,8 @@ static void bap_stream_detach(struct bt_bap_stream *stream)
>
> DBG(stream->bap, "stream %p ep %p", stream, ep);
>
> + bap_abort_stream_req(stream->bap, stream);
> +
> queue_remove(stream->bap->streams, stream);
> bap_stream_clear_cfm(stream);
>
> @@ -1477,6 +1482,28 @@ static bool bap_process_queue(void *data)
> return false;
> }
>
> +static bool match_req_stream(const void *data, const void *match_data)
> +{
> + const struct bt_bap_req *pend = data;
> +
> + return pend->stream == match_data;
> +}
> +
> +static void bap_req_abort(void *data)
> +{
> + struct bt_bap_req *req = data;
> + struct bt_bap *bap = req->stream->bap;
> +
> + DBG(bap, "req %p", req);
> + bap_req_complete(req, NULL);
> +}
> +
> +static void bap_abort_stream_req(struct bt_bap *bap,
> + struct bt_bap_stream *stream)
> +{
> + queue_remove_all(bap->reqs, match_req_stream, stream, bap_req_abort);
> +}
> +
> static bool bap_queue_req(struct bt_bap *bap, struct bt_bap_req *req)
> {
> struct bt_bap_req *pend;
> --
> 2.44.0
>
>
--
Luiz Augusto von Dentz
