[PATCH BlueZ v2] bap: don't pass in stream's own metadata to enable()

[PATCH BlueZ v2] bap: don't pass in stream's own metadata to enable()

[Pauli Virtanen]

Stream owned metadata pointers may be invalidated in bt_bap_stream
operations.  Callers should make copies and not rely on details of their
invalidation semantics.

Fixes:

ERROR: AddressSanitizer: heap-use-after-free
READ of size 8 at 0x7b86a76f5d18 thread T0
    #0 0x000000836745 in util_iov_dup src/shared/util.c:353
    #1 0x0000008ea96b in bap_stream_metadata src/shared/bap.c:1991
    #2 0x0000008ebfbe in bap_ucast_enable src/shared/bap.c:2072
    #3 0x0000009226e7 in bt_bap_stream_enable src/shared/bap.c:6392
    #4 0x00000044037d in transport_bap_resume profiles/audio/transport.c:1981
freed by thread T0 here:
    #0 0x7f66a92e5bcb in free.part.0 (/lib64/libasan.so.8+0xe5bcb)
    #1 0x000000837002 in util_iov_free src/shared/util.c:392
    #2 0x0000008ea94e in bap_stream_metadata src/shared/bap.c:1990
    #3 0x0000008ebfbe in bap_ucast_enable src/shared/bap.c:2072
---

Notes:
    The other option is to do like in v1 and specify the semantics.  In that
    case, it's best to be the same as in the other bt_bap functions, which
    use util_iov_memcmp.

 profiles/audio/transport.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
index 9bf3b47ee..62abd83d7 100644
--- a/profiles/audio/transport.c
+++ b/profiles/audio/transport.c
@@ -1977,9 +1977,10 @@ static guint transport_bap_resume(struct media_transport *transport,
 		return bap->resume_id;
 	}
 
-	meta = bt_bap_stream_get_metadata(bap->stream);
+	meta = util_iov_dup(bt_bap_stream_get_metadata(bap->stream), 1);
 	id = bt_bap_stream_enable(bap->stream, bap->linked, meta,
 					bap_enable_complete, owner);
+	util_iov_free(meta, 1);
 	if (!id)
 		return 0;
 
-- 
2.49.0

RE: [BlueZ,v2] bap: don't pass in stream's own metadata to enable()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1261 bytes --]

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=974152

---Test result---

Test Summary:
CheckPatch                    PENDING   0.24 seconds
GitLint                       PENDING   0.23 seconds
BuildEll                      PASS      20.15 seconds
BluezMake                     PASS      2698.56 seconds
MakeCheck                     PASS      20.22 seconds
MakeDistcheck                 PASS      203.25 seconds
CheckValgrind                 PASS      278.09 seconds
CheckSmatch                   PASS      306.96 seconds
bluezmakeextell               PASS      128.53 seconds
IncrementalBuild              PENDING   0.26 seconds
ScanBuild                     PASS      921.31 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ v2] bap: don't pass in stream's own metadata to enable()

[Luiz Augusto von Dentz]

Hi Pauli,

On Fri, Jun 20, 2025 at 4:59 AM Pauli Virtanen <pav@iki.fi> wrote:
>
> Stream owned metadata pointers may be invalidated in bt_bap_stream
> operations.  Callers should make copies and not rely on details of their
> invalidation semantics.
>
> Fixes:
>
> ERROR: AddressSanitizer: heap-use-after-free
> READ of size 8 at 0x7b86a76f5d18 thread T0
>     #0 0x000000836745 in util_iov_dup src/shared/util.c:353
>     #1 0x0000008ea96b in bap_stream_metadata src/shared/bap.c:1991
>     #2 0x0000008ebfbe in bap_ucast_enable src/shared/bap.c:2072
>     #3 0x0000009226e7 in bt_bap_stream_enable src/shared/bap.c:6392
>     #4 0x00000044037d in transport_bap_resume profiles/audio/transport.c:1981
> freed by thread T0 here:
>     #0 0x7f66a92e5bcb in free.part.0 (/lib64/libasan.so.8+0xe5bcb)
>     #1 0x000000837002 in util_iov_free src/shared/util.c:392
>     #2 0x0000008ea94e in bap_stream_metadata src/shared/bap.c:1990
>     #3 0x0000008ebfbe in bap_ucast_enable src/shared/bap.c:2072
> ---
>
> Notes:
>     The other option is to do like in v1 and specify the semantics.  In that
>     case, it's best to be the same as in the other bt_bap functions, which
>     use util_iov_memcmp.
>
>  profiles/audio/transport.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/profiles/audio/transport.c b/profiles/audio/transport.c
> index 9bf3b47ee..62abd83d7 100644
> --- a/profiles/audio/transport.c
> +++ b/profiles/audio/transport.c
> @@ -1977,9 +1977,10 @@ static guint transport_bap_resume(struct media_transport *transport,
>                 return bap->resume_id;
>         }
>
> -       meta = bt_bap_stream_get_metadata(bap->stream);
> +       meta = util_iov_dup(bt_bap_stream_get_metadata(bap->stream), 1);
>         id = bt_bap_stream_enable(bap->stream, bap->linked, meta,
>                                         bap_enable_complete, owner);
> +       util_iov_free(meta, 1);

Oh, that is what was causing the problem, well in this case the
bt_bap_stream_get_metadata returns the stream->meta but
bt_bap_stream_enable would already be using it anyway, so I think we
can just remove this entirely and just pass NULL as metadata, perhaps
we can remove the argument as well and just leave it up to
bt_ba_stream_set_metadata to update it.

>         if (!id)
>                 return 0;
>
> --
> 2.49.0
>
>


-- 
Luiz Augusto von Dentz