From 9517a7b93bee6af9f4b3d7a4bf3fc723cd953ed7 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 - Remove invalid free of inline Cover Art image_descriptor A2DP BTC/API: - Default g_a2dp_on_deinit to true before profile init - Reject source audio send when A2DP is deiniting - Add shutdown state check in btc_a2dp_sink_shutdown - Guard A2DP source timer against freed dynamic local param --- components/bt/host/bluedroid/api/esp_a2dp_api.c | 6 +++++- 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_ca_act.c | 1 - .../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 | 10 ++++++++++ .../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 ++ 12 files changed, 49 insertions(+), 13 deletions(-) diff --git a/components/bt/host/bluedroid/api/esp_a2dp_api.c b/components/bt/host/bluedroid/api/esp_a2dp_api.c index c9a48715e1e..83c04ed731a 100644 --- a/components/bt/host/bluedroid/api/esp_a2dp_api.c +++ b/components/bt/host/bluedroid/api/esp_a2dp_api.c @@ -507,7 +507,11 @@ esp_err_t esp_a2d_source_register_data_callback(esp_a2d_source_data_cb_t callbac esp_err_t esp_a2d_source_audio_data_send(esp_a2d_conn_hdl_t conn_hdl, esp_a2d_audio_buff_t *audio_buf) { - if (esp_bluedroid_get_status() != ESP_BLUEDROID_STATUS_ENABLED || !btc_av_is_started()) { + if (esp_bluedroid_get_status() != ESP_BLUEDROID_STATUS_ENABLED) { + return ESP_ERR_INVALID_STATE; + } + + if (g_a2dp_on_deinit || g_a2dp_source_ongoing_deinit || !btc_av_is_started()) { return ESP_ERR_INVALID_STATE; } 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 13d0901e989..2d8df0599ee 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_act.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_act.c @@ -262,8 +262,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 b95f7d16c66..f65966e5479 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_ca_act.c b/components/bt/host/bluedroid/bta/av/bta_av_ca_act.c index bdb1cd6f851..09000d251d4 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_ca_act.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_ca_act.c @@ -298,7 +298,6 @@ void bta_av_ca_api_get(tBTA_AV_RCB *p_rcb, tBTA_AV_DATA *p_data) GOEPC_RequestAddHeader(p_rcb->cover_art_goep_hdl, COVER_ART_HEADER_ID_IMG_HANDLE, (UINT8 *)image_handle_utf16, BTA_AV_CA_IMG_HDL_UTF16_LEN); if (p_data->api_ca_get.type == BTA_AV_CA_GET_IMAGE) { GOEPC_RequestAddHeader(p_rcb->cover_art_goep_hdl, COVER_ART_HEADER_ID_IMG_DESCRIPTOR, (UINT8 *)p_data->api_ca_get.image_descriptor, p_data->api_ca_get.image_descriptor_len); - osi_free(p_data->api_ca_get.image_descriptor); } /* always request to enable srm */ GOEPC_RequestSetSRM(p_rcb->cover_art_goep_hdl, TRUE, FALSE); 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 d5a4edf46b4..5833ad21420 100644 --- a/components/bt/host/bluedroid/bta/av/bta_av_main.c +++ b/components/bt/host/bluedroid/bta/av/bta_av_main.c @@ -755,7 +755,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 *)®istr); + if (bta_av_cb.p_cback != NULL) { + (*bta_av_cb.p_cback)(BTA_AV_REGISTER_EVT, (tBTA_AV *)®istr); + } } static void bta_av_api_reg_sep(tBTA_AV_DATA *p_data) 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 46b343a542e..2d5b009f12e 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 @@ -230,6 +230,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 a87bd28d82a..8313a65f2ab 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 @@ -406,6 +406,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; @@ -1525,6 +1530,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 cd808898553..3ad23fb1404 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 @@ -42,7 +42,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 81c0fa2e8b9..8d2272d67d9 100644 --- a/components/bt/host/bluedroid/stack/avdt/avdt_api.c +++ b/components/bt/host/bluedroid/stack/avdt/avdt_api.c @@ -940,6 +940,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);