From a099dd706ea6730d4084382a0cff72f80d4bad97 Mon Sep 17 00:00:00 2001 From: yangfeng Date: Fri, 5 Jun 2026 17:38:39 +0800 Subject: [PATCH] fix(bt): Fix the critical issues related to A2DP from AI review report AVDT: - Roll back CCB allocation when cmd/rsp queue creation fails - Free media packet on invalid handle in AVDT_WriteReqOpt - Zero-init timeout failure message before GETCAP callback - Initialize lcid_tbl to 0xFF to avoid mapping to tc_tbl[0] BTA/AVRCP: - Use size_t for AVRC message copy buffer allocation - Allocate before register in BTA_AvEnable - Guard BTA_AvRegister callback when enable never completed A2DP BTC/API: - Default g_a2dp_on_deinit to true before profile init - Add shutdown state check in btc_a2dp_sink_shutdown - Guard A2DP source timer against freed dynamic local param --- components/bt/host/bluedroid/bta/av/bta_av_act.c | 4 ++-- components/bt/host/bluedroid/bta/av/bta_av_api.c | 16 +++++++++------- .../bt/host/bluedroid/bta/av/bta_av_main.c | 4 +++- .../btc/profile/std/a2dp/btc_a2dp_sink.c | 5 +++++ .../btc/profile/std/a2dp/btc_a2dp_source.c | 12 +++++++++++- .../host/bluedroid/btc/profile/std/a2dp/btc_av.c | 2 +- .../bt/host/bluedroid/stack/avdt/avdt_ad.c | 5 +++++ .../bt/host/bluedroid/stack/avdt/avdt_api.c | 1 + .../bt/host/bluedroid/stack/avdt/avdt_ccb.c | 6 ++++++ .../bt/host/bluedroid/stack/avdt/avdt_ccb_act.c | 2 ++ 10 files changed, 45 insertions(+), 12 deletions(-) diff --git a/components/bt/host/bluedroid/bta/av/bta_av_act.c b/components/bt/host/bluedroid/bta/av/bta_av_act.c index 6724bb14d1a..e96a0b44d6b 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_act.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_act.c @@ -254,8 +254,8 @@ static void bta_av_rc_msg_cback(UINT8 handle, UINT8 label, UINT8 opcode, tAVRC_M } /* Create a copy of the message */ - tBTA_AV_RC_MSG *p_buf = - (tBTA_AV_RC_MSG *)osi_malloc((UINT16)(sizeof(tBTA_AV_RC_MSG) + data_len)); + size_t buf_size = sizeof(tBTA_AV_RC_MSG) + data_len; + tBTA_AV_RC_MSG *p_buf = (tBTA_AV_RC_MSG *)osi_malloc(buf_size); if (p_buf != NULL) { p_buf->hdr.event = BTA_AV_AVRC_MSG_EVT; p_buf->handle = handle; diff --git a/components/bt/host/bluedroid/bta/av/bta_av_api.c b/components/bt/host/bluedroid/bta/av/bta_av_api.c index 577a7e98907..5f68104fa1b 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_api.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_api.c @@ -60,16 +60,18 @@ void BTA_AvEnable(tBTA_SEC sec_mask, tBTA_AV_FEAT features, tBTA_AV_CBACK *p_cba { tBTA_AV_API_ENABLE *p_buf; + if ((p_buf = (tBTA_AV_API_ENABLE *) osi_malloc(sizeof(tBTA_AV_API_ENABLE))) == NULL) { + return; + } + /* register with BTA system manager */ bta_sys_register(BTA_ID_AV, &bta_av_reg); - if ((p_buf = (tBTA_AV_API_ENABLE *) osi_malloc(sizeof(tBTA_AV_API_ENABLE))) != NULL) { - p_buf->hdr.event = BTA_AV_API_ENABLE_EVT; - p_buf->p_cback = p_cback; - p_buf->features = features; - p_buf->sec_mask = sec_mask; - bta_sys_sendmsg(p_buf); - } + p_buf->hdr.event = BTA_AV_API_ENABLE_EVT; + p_buf->p_cback = p_cback; + p_buf->features = features; + p_buf->sec_mask = sec_mask; + bta_sys_sendmsg(p_buf); } /******************************************************************************* diff --git a/components/bt/host/bluedroid/bta/av/bta_av_main.c b/components/bt/host/bluedroid/bta/av/bta_av_main.c index 683b9b1763d..bd1469f5414 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_main.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_main.c @@ -748,7 +748,9 @@ static void bta_av_api_register(tBTA_AV_DATA *p_data) } while (0); /* call callback with register event */ - (*bta_av_cb.p_cback)(BTA_AV_REGISTER_EVT, (tBTA_AV *)&av_reg); + if (bta_av_cb.p_cback != NULL) { + (*bta_av_cb.p_cback)(BTA_AV_REGISTER_EVT, (tBTA_AV *)&av_reg); + } } /******************************************************************************* diff --git a/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_sink.c b/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_sink.c index f244903aaa0..b65b8a62b99 100644 --- a/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_sink.c +++ b/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_sink.c @@ -236,6 +236,11 @@ error_exit:; void btc_a2dp_sink_shutdown(void) { + if (btc_a2dp_sink_state != BTC_A2DP_SINK_STATE_ON) { + APPL_TRACE_ERROR("a2dp sink already shutdown"); + return; + } + APPL_TRACE_EVENT("## A2DP SINK STOP MEDIA THREAD ##\n"); // Exit thread diff --git a/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_source.c b/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_source.c index 7e483049019..1ca43b2c65a 100644 --- a/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_source.c +++ b/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_a2dp_source.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2015-2023 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2015-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -412,6 +412,11 @@ static void log_tstamps_us(char *comment) { static UINT64 prev_us = 0; UINT64 now_us = time_now_us(); +#if A2D_DYNAMIC_MEMORY == TRUE + if (a2dp_source_local_param_ptr == NULL) { + return; + } +#endif APPL_TRACE_DEBUG("[%s] ts %08llu, diff : %08llu, queue sz %d", comment, now_us, now_us - prev_us, fixed_queue_length(a2dp_source_local_param.btc_aa_src_cb.TxAaQ)); prev_us = now_us; @@ -1492,6 +1497,11 @@ static void btc_a2dp_source_handle_timer(UNUSED_ATTR void *context) if (btc_a2dp_source_state != BTC_A2DP_SOURCE_STATE_ON || g_a2dp_source_ongoing_deinit){ return; } +#if A2D_DYNAMIC_MEMORY == TRUE + if (a2dp_source_local_param_ptr == NULL) { + return; + } +#endif if (a2dp_source_local_param.btc_aa_src_cb.is_tx_timer == TRUE) { btc_a2dp_source_send_aa_frame(); diff --git a/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_av.c b/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_av.c index 82d9dc266dd..0f25b426fb4 100644 --- a/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_av.c +++ b/components/bt/host/bluedroid/btc/profile/std/a2dp/btc_av.c @@ -39,7 +39,7 @@ bool g_av_with_rc; // global variable to indicate a2dp is initialized bool g_a2dp_on_init; // global variable to indicate a2dp is deinitialized -bool g_a2dp_on_deinit; +bool g_a2dp_on_deinit = true; // global variable to indicate a2dp source deinitialization is ongoing bool g_a2dp_source_ongoing_deinit; // global variable to indicate a2dp sink deinitialization is ongoing diff --git a/components/bt/host/bluedroid/stack/avdt/avdt_ad.c b/components/bt/host/bluedroid/stack/avdt/avdt_ad.c index d6911e1f7cf..fbdaf6dd6ba 100644 --- a/components/bt/host/bluedroid/stack/avdt/avdt_ad.c +++ b/components/bt/host/bluedroid/stack/avdt/avdt_ad.c @@ -110,6 +110,11 @@ void avdt_ad_init(void) tAVDT_TC_TBL *p_tbl = avdt_cb.ad.tc_tbl; memset(&avdt_cb.ad, 0, sizeof(tAVDT_AD)); + /* 0 is a valid tc_tbl index; use invalid marker for unassigned LCIDs */ + for (i = 0; i < MAX_L2CAP_CHANNELS; i++) { + avdt_cb.ad.lcid_tbl[i] = 0xFF; + } + /* make sure the peer_mtu is a valid value */ for (i = 0; i < AVDT_NUM_TC_TBL; i++, p_tbl++) { p_tbl->peer_mtu = L2CAP_DEFAULT_MTU; diff --git a/components/bt/host/bluedroid/stack/avdt/avdt_api.c b/components/bt/host/bluedroid/stack/avdt/avdt_api.c index 5ad577ce978..dd4f5519944 100644 --- a/components/bt/host/bluedroid/stack/avdt/avdt_api.c +++ b/components/bt/host/bluedroid/stack/avdt/avdt_api.c @@ -909,6 +909,7 @@ UINT16 AVDT_WriteReqOpt(UINT8 handle, BT_HDR *p_pkt, UINT32 time_stamp, UINT8 m_ } /* map handle to scb */ if ((p_scb = avdt_scb_by_hdl(handle)) == NULL) { + osi_free(p_pkt); result = AVDT_BAD_HANDLE; } else { evt.apiwrite.p_buf = p_pkt; diff --git a/components/bt/host/bluedroid/stack/avdt/avdt_ccb.c b/components/bt/host/bluedroid/stack/avdt/avdt_ccb.c index bf8943aa54c..a18535d2093 100644 --- a/components/bt/host/bluedroid/stack/avdt/avdt_ccb.c +++ b/components/bt/host/bluedroid/stack/avdt/avdt_ccb.c @@ -383,6 +383,12 @@ tAVDT_CCB *avdt_ccb_alloc(BD_ADDR bd_addr) memcpy(p_ccb->peer_addr, bd_addr, BD_ADDR_LEN); p_ccb->cmd_q = fixed_queue_new(QUEUE_SIZE_MAX); p_ccb->rsp_q = fixed_queue_new(QUEUE_SIZE_MAX); + if (p_ccb->cmd_q == NULL || p_ccb->rsp_q == NULL) { + AVDT_TRACE_ERROR("avdt_ccb_alloc: queue alloc failed"); + avdt_ccb_dealloc(p_ccb, NULL); + p_ccb = NULL; + break; + } p_ccb->timer_entry.param = (UINT32) p_ccb; AVDT_TRACE_DEBUG("avdt_ccb_alloc %d\n", i); break; diff --git a/components/bt/host/bluedroid/stack/avdt/avdt_ccb_act.c b/components/bt/host/bluedroid/stack/avdt/avdt_ccb_act.c index dce084aa7c5..386b5fa40c6 100644 --- a/components/bt/host/bluedroid/stack/avdt/avdt_ccb_act.c +++ b/components/bt/host/bluedroid/stack/avdt/avdt_ccb_act.c @@ -712,6 +712,8 @@ void avdt_ccb_cmd_fail(tAVDT_CCB *p_ccb, tAVDT_CCB_EVT *p_data) UINT8 evt; tAVDT_SCB *p_scb; + memset(&msg, 0, sizeof(msg)); + if (p_ccb->p_curr_cmd != NULL) { if (p_ccb->p_curr_cmd->event < 1 || p_ccb->p_curr_cmd->event > AVDT_SIG_MAX) { osi_free(p_ccb->p_curr_cmd);