From abfd19366216e568c4ea0bdfbc5330563cf830f4 Mon Sep 17 00:00:00 2001 From: hejiaxin Date: Thu, 23 Jul 2026 19:14:56 +0800 Subject: [PATCH] fix(bt_pbap): Fix some bugs in bluedroid PBAP - Add sdp_seq to avoid p_ccb being free during sdp - Changed some BTA_Pba functions to return non-void value - Improve error catching and report - Refactor bta_pba_client_response to avoid UAF problem - Rearrange btc_pba_client init flag to avoid some disturbing bug --- .../bt/host/bluedroid/api/esp_pbac_api.c | 3 + .../bta/include/bta/bta_pba_client_api.h | 10 +- .../bluedroid/bta/pba/bta_pba_client_act.c | 246 ++++++++++-------- .../bluedroid/bta/pba/bta_pba_client_api.c | 42 ++- .../bluedroid/bta/pba/bta_pba_client_main.c | 34 +-- .../bluedroid/bta/pba/bta_pba_client_sdp.c | 63 ++++- .../bta/pba/include/bta_pba_client_int.h | 2 + .../bt/host/bluedroid/bta/sys/bta_sys_main.c | 1 + .../btc/profile/std/pba/btc_pba_client.c | 132 ++++++++-- 9 files changed, 358 insertions(+), 175 deletions(-) diff --git a/components/bt/host/bluedroid/api/esp_pbac_api.c b/components/bt/host/bluedroid/api/esp_pbac_api.c index 3d6273c2d6a..34ef3b93ae7 100644 --- a/components/bt/host/bluedroid/api/esp_pbac_api.c +++ b/components/bt/host/bluedroid/api/esp_pbac_api.c @@ -208,6 +208,9 @@ esp_err_t esp_pbac_pull_vcard_listing(esp_pbac_conn_hdl_t handle, const char *na args.pull_vcard_listing.handle = handle; args.pull_vcard_listing.name = (char *)name; if (app_param != NULL) { + if (app_param->include_search_value && app_param->search_value == NULL) { + return ESP_ERR_INVALID_ARG; + } args.pull_vcard_listing.include_app_param = true; memcpy(&args.pull_vcard_listing.app_param, app_param, sizeof(esp_pbac_pull_vcard_listing_app_param_t)); } diff --git a/components/bt/host/bluedroid/bta/include/bta/bta_pba_client_api.h b/components/bt/host/bluedroid/bta/include/bta/bta_pba_client_api.h index 404f8ffd81f..df9e5a4d339 100644 --- a/components/bt/host/bluedroid/bta/include/bta/bta_pba_client_api.h +++ b/components/bt/host/bluedroid/bta/include/bta/bta_pba_client_api.h @@ -82,11 +82,11 @@ void BTA_PbaClientEnable(tBTA_PBA_CLIENT_CBACK *p_cback); void BTA_PbaClientDisable(void); void BTA_PbaClientRegister(const char *server_name); void BTA_PbaClientDeregister(void); -void BTA_PbaClientOpen(BD_ADDR bd_addr, tBTA_SEC sec_mask, UINT32 supported_feat, UINT16 mtu); +tBTA_STATUS BTA_PbaClientOpen(BD_ADDR bd_addr, tBTA_SEC sec_mask, UINT32 supported_feat, UINT16 mtu); void BTA_PbaClientClose(UINT16 handle); -void BTA_PbaClientPullPhoneBook(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len); -void BTA_PbaClientSetPhoneBook(UINT16 handle, UINT8 flags, char *name); -void BTA_PbaClientPullvCardListing(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len); -void BTA_PbaClientPullvCardEntry(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len); +tBTA_STATUS BTA_PbaClientPullPhoneBook(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len); +tBTA_STATUS BTA_PbaClientSetPhoneBook(UINT16 handle, UINT8 flags, char *name); +tBTA_STATUS BTA_PbaClientPullvCardListing(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len); +tBTA_STATUS BTA_PbaClientPullvCardEntry(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len); #endif diff --git a/components/bt/host/bluedroid/bta/pba/bta_pba_client_act.c b/components/bt/host/bluedroid/bta/pba/bta_pba_client_act.c index af220475342..7eeb2159b3b 100644 --- a/components/bt/host/bluedroid/bta/pba/bta_pba_client_act.c +++ b/components/bt/host/bluedroid/bta/pba/bta_pba_client_act.c @@ -27,11 +27,15 @@ static const char *type_pull_vcard_entry = "x-bt/vcard"; static void free_ccb(tBTA_PBA_CLIENT_CCB *p_ccb) { - /* free sdp db */ + UINT8 sdp_seq; + + /* free sdp db (also invalidates in-flight SDP via sdp_seq++) */ bta_pba_client_free_db(p_ccb); - /* clear all field, set allocated to 0 */ + /* Preserve SDP generation across memset so a reused slot cannot accept a stale callback */ + sdp_seq = p_ccb->sdp_seq; memset(p_ccb, 0, sizeof(tBTA_PBA_CLIENT_CCB)); + p_ccb->sdp_seq = sdp_seq; } static void close_goepc_and_report(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_ERR reason) @@ -156,6 +160,105 @@ static void report_error_data_event(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_ bta_pba_client_cb.p_cback(event, (tBTA_PBA_CLIENT *)&response); } +/******************************************************************************* +** +** Function bta_pba_client_validate_get_headers +** +** Description Walk GET response headers and verify Body / End-of-Body / +** App-Param lengths before any data is reported to BTC. +** Callers must re-run OBEX_ParseResponse before reporting so +** the header iterator is reset. +** +** Returns TRUE if all relevant headers are valid +** +*******************************************************************************/ +static BOOLEAN bta_pba_client_validate_get_headers(BT_HDR *pkt, tOBEX_PARSE_INFO *info, + tBTA_PBA_CLIENT_ERR *reason) +{ + UINT8 *header; + UINT8 *pkt_data = (UINT8 *)(pkt + 1) + pkt->offset; + UINT8 *pkt_end = pkt_data + pkt->len; + + while ((header = OBEX_GetNextHeader(pkt, info)) != NULL) { + switch (*header) { + case OBEX_HEADER_ID_BODY: + case OBEX_HEADER_ID_END_OF_BODY: + case OBEX_HEADER_ID_APP_PARAM: { + UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); + if (hi_len < 3) { + *reason = BTA_PBA_CLIENT_BAD_REQUEST; + return FALSE; + } + break; + } + default: + break; + } + } + return TRUE; +} + +/******************************************************************************* +** +** Function bta_pba_client_report_get_headers +** +** Description Report Body / App-Param from a GET response that has already +** been validated. Intermediate body chunks use pkt = NULL; +** the last report transfers pkt ownership to BTC when there is +** any body or app-param data. For a non-final empty response, +** frees pkt locally. For a final empty response, reports with +** pkt ownership transferred. +** +** Returns void +** +*******************************************************************************/ +static void bta_pba_client_report_get_headers(tBTA_PBA_CLIENT_CCB *p_ccb, BT_HDR *pkt, + tOBEX_PARSE_INFO *info, BOOLEAN final) +{ + UINT8 *header = NULL; + UINT8 *body_data = NULL; + UINT16 body_data_len = 0; + UINT8 *app_param = NULL; + UINT16 app_param_len = 0; + UINT8 *pkt_data = (UINT8 *)(pkt + 1) + pkt->offset; + UINT8 *pkt_end = pkt_data + pkt->len; + + while ((header = OBEX_GetNextHeader(pkt, info)) != NULL) { + switch (*header) { + case OBEX_HEADER_ID_BODY: + case OBEX_HEADER_ID_END_OF_BODY: { + /* Length already validated in bta_pba_client_validate_get_headers */ + UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); + if (body_data == NULL) { + body_data = header + 3; + body_data_len = hi_len - 3; + } else { + report_data_event(p_ccb, body_data, body_data_len, NULL, 0, FALSE, NULL); + body_data = header + 3; + body_data_len = hi_len - 3; + } + break; + } + case OBEX_HEADER_ID_APP_PARAM: { + UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); + app_param = header + 3; + app_param_len = hi_len - 3; + break; + } + default: + break; + } + } + + if (body_data != NULL || app_param != NULL) { + report_data_event(p_ccb, body_data, body_data_len, app_param, app_param_len, final, pkt); + } else if (final) { + report_data_event(p_ccb, NULL, 0, NULL, 0, TRUE, pkt); + } else { + osi_free(pkt); + } +} + static void ascii_to_utf16(const UINT8 *src, UINT16 len, UINT8 *dst) { UINT16 pos = 0; @@ -205,7 +308,9 @@ void bta_pba_client_api_register(tBTA_PBA_CLIENT_DATA *p_data) /* create SDP records */ bta_pba_client_create_record(p_data->api_register.name); - bta_pba_client_cb.p_cback(BTA_PBA_CLIENT_REGISTER_EVT, NULL); + if (bta_pba_client_cb.p_cback) { + bta_pba_client_cb.p_cback(BTA_PBA_CLIENT_REGISTER_EVT, NULL); + } } void bta_pba_client_api_deregister(tBTA_PBA_CLIENT_DATA *p_data) @@ -213,7 +318,9 @@ void bta_pba_client_api_deregister(tBTA_PBA_CLIENT_DATA *p_data) /* delete SDP records */ bta_pba_client_del_record(); - bta_pba_client_cb.p_cback(BTA_PBA_CLIENT_DEREGISTER_EVT, NULL); + if (bta_pba_client_cb.p_cback) { + bta_pba_client_cb.p_cback(BTA_PBA_CLIENT_DEREGISTER_EVT, NULL); + } } void bta_pba_client_api_open(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data) @@ -370,6 +477,8 @@ static void goep_event_callback(UINT16 handle, UINT8 event, tGOEPC_MSG *p_msg) else { p_data->goep_response.hdr.event = BTA_PBA_CLIENT_RESPONSE_EVT; } + } else { + osi_free(p_msg->response.pkt); } break; case GOEPC_MTU_CHANGED_EVT: @@ -468,60 +577,16 @@ void bta_pba_client_response(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p } if (p_data->goep_response.opcode == OBEX_OPCODE_GET_FINAL && (info.response_code == OBEX_RESPONSE_CODE_CONTINUE || info.response_code == (OBEX_RESPONSE_CODE_CONTINUE | OBEX_FINAL_BIT_MASK))) { - UINT8 *header = NULL; - UINT8 *body_data = NULL; - UINT16 body_data_len = 0; - UINT8 *app_param = NULL; - UINT16 app_param_len = 0; - UINT8 *pkt_data = (UINT8 *)(p_data->goep_response.pkt + 1) + p_data->goep_response.pkt->offset; - UINT8 *pkt_end = pkt_data + p_data->goep_response.pkt->len; - while((header = OBEX_GetNextHeader(p_data->goep_response.pkt, &info)) != NULL) { - switch (*header) - { - case OBEX_HEADER_ID_BODY: - case OBEX_HEADER_ID_END_OF_BODY: - { - UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); - if (hi_len < 3) { - reason = BTA_PBA_CLIENT_BAD_REQUEST; - goto error; - } - if (body_data == NULL) { - /* first body header */ - body_data = header + 3; /* skip opcode, length */ - body_data_len = hi_len - 3; - } - else { - /* another body header found */ - report_data_event(p_ccb, body_data, body_data_len, NULL, 0, FALSE, NULL); - body_data = header + 3; /* skip opcode, length */ - body_data_len = hi_len - 3; - } - break; - } - case OBEX_HEADER_ID_APP_PARAM: - { - UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); - if (hi_len < 3) { - reason = BTA_PBA_CLIENT_BAD_REQUEST; - goto error; - } - app_param = header + 3; - app_param_len = hi_len - 3; - break; - } - default: - break; - } + /* Validate all headers before reporting so we never free pkt while BTC + * still holds pointers into it from an earlier report_data_event. */ + if (!bta_pba_client_validate_get_headers(p_data->goep_response.pkt, &info, &reason)) { + goto error; } - if (body_data != NULL || app_param != NULL) { - /* report body data and app param, dont free packet here */ - report_data_event(p_ccb, body_data, body_data_len, app_param, app_param_len, FALSE, p_data->goep_response.pkt); - } - else { - /* not any body data or app param */ - osi_free(p_data->goep_response.pkt); + if (OBEX_ParseResponse(p_data->goep_response.pkt, p_data->goep_response.opcode, &info) != OBEX_SUCCESS) { + goto error; } + bta_pba_client_report_get_headers(p_ccb, p_data->goep_response.pkt, &info, FALSE); + p_data->goep_response.pkt = NULL; /* if SRM not enable, we need to send a empty get request */ if (!p_data->goep_response.srm_en || p_data->goep_response.srm_wait) { @@ -607,65 +672,22 @@ void bta_pba_client_response_final(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_D else if (p_data->goep_response.opcode == OBEX_OPCODE_GET_FINAL) { /* check response code is success */ if (info.response_code == (OBEX_RESPONSE_CODE_OK | OBEX_FINAL_BIT_MASK)) { - UINT8 *body_data = NULL; - UINT16 body_data_len = 0; - UINT8 *app_param = NULL; - UINT16 app_param_len = 0; - UINT8 *pkt_data = (UINT8 *)(p_data->goep_response.pkt + 1) + p_data->goep_response.pkt->offset; - UINT8 *pkt_end = pkt_data + p_data->goep_response.pkt->len; - while((header = OBEX_GetNextHeader(p_data->goep_response.pkt, &info)) != NULL) { - switch (*header) - { - /* actually, BODY should not in this final response */ - case OBEX_HEADER_ID_BODY: - case OBEX_HEADER_ID_END_OF_BODY: - { - UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); - if (hi_len < 3) { - reason = BTA_PBA_CLIENT_BAD_REQUEST; - goto error; - } - if (body_data == NULL) { - /* first body header */ - body_data = header + 3; /* skip opcode, length */ - body_data_len = hi_len - 3; - } - else { - /* another body header found */ - report_data_event(p_ccb, body_data, body_data_len, NULL, 0, FALSE, NULL); - body_data = header + 3; /* skip opcode, length */ - body_data_len = hi_len - 3; - } - break; - } - case OBEX_HEADER_ID_APP_PARAM: - { - UINT16 hi_len = OBEX_GetHeaderLength(header, pkt_end); - if (hi_len < 3) { - reason = BTA_PBA_CLIENT_BAD_REQUEST; - goto error; - } - app_param = header + 3; - app_param_len = hi_len - 3; - break; - } - default: - break; - } + /* Validate before reporting to avoid UAF if a later header is bad */ + if (!bta_pba_client_validate_get_headers(p_data->goep_response.pkt, &info, &reason)) { + goto error; } - if (body_data != NULL || app_param != NULL) { - /* report body data and app param, dont free packet here */ - report_data_event(p_ccb, body_data, body_data_len, app_param, app_param_len, TRUE, p_data->goep_response.pkt); - /* done, return */ - return; + if (OBEX_ParseResponse(p_data->goep_response.pkt, p_data->goep_response.opcode, &info) != OBEX_SUCCESS) { + goto error; } - report_data_event(p_ccb, NULL, 0, NULL, 0, TRUE, p_data->goep_response.pkt); + bta_pba_client_report_get_headers(p_ccb, p_data->goep_response.pkt, &info, TRUE); + p_data->goep_response.pkt = NULL; return; } /* unexpected response code or body data not found */ reason = calculate_response_error(info.response_code); report_error_data_event(p_ccb, reason); osi_free(p_data->goep_response.pkt); + p_data->goep_response.pkt = NULL; /* state machine is good, don't goto error */ } @@ -754,4 +776,14 @@ void bta_pba_client_free_response(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DA close_goepc_and_report(p_ccb, BTA_PBA_CLIENT_GOEP_ERROR); } +void bta_pba_client_free_req(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data) +{ + if (p_data->api_req.name != NULL) { + osi_free(p_data->api_req.name); + } + if (p_data->api_req.app_param != NULL) { + osi_free(p_data->api_req.app_param); + } +} + #endif diff --git a/components/bt/host/bluedroid/bta/pba/bta_pba_client_api.c b/components/bt/host/bluedroid/bta/pba/bta_pba_client_api.c index 8169a7d6de4..1771dd04062 100644 --- a/components/bt/host/bluedroid/bta/pba/bta_pba_client_api.c +++ b/components/bt/host/bluedroid/bta/pba/bta_pba_client_api.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2024-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -32,10 +32,9 @@ void BTA_PbaClientEnable(tBTA_PBA_CLIENT_CBACK *p_cback) return; } - /* register with BTA system manager */ - bta_sys_register(BTA_ID_PBC, &bta_pba_client_reg); - if ((p_buf = (tBTA_PBA_CLIENT_API_ENABLE *)osi_malloc(sizeof(tBTA_PBA_CLIENT_API_ENABLE))) != NULL) { + /* register with BTA system manager */ + bta_sys_register(BTA_ID_PBC, &bta_pba_client_reg); p_buf->hdr.event = BTA_PBA_CLIENT_API_ENABLE_EVT; p_buf->p_cback = p_cback; bta_sys_sendmsg(p_buf); @@ -73,11 +72,13 @@ void BTA_PbaClientDeregister(void) } } -void BTA_PbaClientOpen(BD_ADDR bd_addr, tBTA_SEC sec_mask, UINT32 supported_feat, UINT16 mtu) +tBTA_STATUS BTA_PbaClientOpen(BD_ADDR bd_addr, tBTA_SEC sec_mask, UINT32 supported_feat, UINT16 mtu) { tBTA_PBA_CLIENT_API_OPEN *p_buf; - if ((p_buf = (tBTA_PBA_CLIENT_API_OPEN *)osi_malloc(sizeof(tBTA_PBA_CLIENT_API_OPEN))) != NULL) { + if ((p_buf = (tBTA_PBA_CLIENT_API_OPEN *)osi_malloc(sizeof(tBTA_PBA_CLIENT_API_OPEN))) == NULL) { + return BTA_FAILURE; + } else { p_buf->hdr.event = BTA_PBA_CLIENT_API_OPEN_EVT; p_buf->sec_mask = sec_mask; p_buf->supported_feat = supported_feat; @@ -85,6 +86,7 @@ void BTA_PbaClientOpen(BD_ADDR bd_addr, tBTA_SEC sec_mask, UINT32 supported_feat bdcpy(p_buf->bd_addr, bd_addr); bta_sys_sendmsg(p_buf); } + return BTA_SUCCESS; } void BTA_PbaClientClose(UINT16 handle) @@ -98,11 +100,13 @@ void BTA_PbaClientClose(UINT16 handle) } } -void BTA_PbaClientPullPhoneBook(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len) +tBTA_STATUS BTA_PbaClientPullPhoneBook(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len) { tBTA_PBA_CLIENT_API_REQ *p_buf; - if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) != NULL) { + if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) == NULL) { + return BTA_FAILURE; + } else { p_buf->hdr.event = BTA_PBA_CLIENT_API_REQ_EVT; p_buf->hdr.layer_specific = handle; p_buf->operation = BTA_PBA_CLIENT_OP_PULL_PHONE_BOOK; @@ -111,13 +115,16 @@ void BTA_PbaClientPullPhoneBook(UINT16 handle, char *name, UINT8 *app_param, UIN p_buf->app_param_len = app_param_len; bta_sys_sendmsg(p_buf); } + return BTA_SUCCESS; } -void BTA_PbaClientSetPhoneBook(UINT16 handle, UINT8 flags, char *name) +tBTA_STATUS BTA_PbaClientSetPhoneBook(UINT16 handle, UINT8 flags, char *name) { tBTA_PBA_CLIENT_API_REQ *p_buf; - if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) != NULL) { + if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) == NULL) { + return BTA_FAILURE; + } else { p_buf->hdr.event = BTA_PBA_CLIENT_API_REQ_EVT; p_buf->hdr.layer_specific = handle; p_buf->operation = BTA_PBA_CLIENT_OP_SET_PHONE_BOOK; @@ -127,13 +134,16 @@ void BTA_PbaClientSetPhoneBook(UINT16 handle, UINT8 flags, char *name) p_buf->app_param_len = 0; bta_sys_sendmsg(p_buf); } + return BTA_SUCCESS; } -void BTA_PbaClientPullvCardListing(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len) +tBTA_STATUS BTA_PbaClientPullvCardListing(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len) { tBTA_PBA_CLIENT_API_REQ *p_buf; - if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) != NULL) { + if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) == NULL) { + return BTA_FAILURE; + } else { p_buf->hdr.event = BTA_PBA_CLIENT_API_REQ_EVT; p_buf->hdr.layer_specific = handle; p_buf->operation = BTA_PBA_CLIENT_OP_PULL_VCARD_LISTING; @@ -142,13 +152,16 @@ void BTA_PbaClientPullvCardListing(UINT16 handle, char *name, UINT8 *app_param, p_buf->app_param_len = app_param_len; bta_sys_sendmsg(p_buf); } + return BTA_SUCCESS; } -void BTA_PbaClientPullvCardEntry(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len) +tBTA_STATUS BTA_PbaClientPullvCardEntry(UINT16 handle, char *name, UINT8 *app_param, UINT16 app_param_len) { tBTA_PBA_CLIENT_API_REQ *p_buf; - if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) != NULL) { + if ((p_buf = (tBTA_PBA_CLIENT_API_REQ *) osi_malloc(sizeof(tBTA_PBA_CLIENT_API_REQ))) == NULL) { + return BTA_FAILURE; + } else { p_buf->hdr.event = BTA_PBA_CLIENT_API_REQ_EVT; p_buf->hdr.layer_specific = handle; p_buf->operation = BTA_PBA_CLIENT_OP_PULL_VCARD_ENTRY; @@ -157,6 +170,7 @@ void BTA_PbaClientPullvCardEntry(UINT16 handle, char *name, UINT8 *app_param, UI p_buf->app_param_len = app_param_len; bta_sys_sendmsg(p_buf); } + return BTA_SUCCESS; } #endif diff --git a/components/bt/host/bluedroid/bta/pba/bta_pba_client_main.c b/components/bt/host/bluedroid/bta/pba/bta_pba_client_main.c index e6e7ba1de4e..12d3b7cd5c3 100644 --- a/components/bt/host/bluedroid/bta/pba/bta_pba_client_main.c +++ b/components/bt/host/bluedroid/bta/pba/bta_pba_client_main.c @@ -39,6 +39,7 @@ enum { BTA_PBA_CLIENT_GOEP_DISCONNECT, BTA_PBA_CLIENT_FORCE_DISCONNECT, BTA_PBA_CLIENT_FREE_RESPONSE, + BTA_PBA_CLIENT_FREE_REQ, BTA_PBA_CLIENT_NUM_ACTIONS }; @@ -49,18 +50,19 @@ typedef void (*tBTA_PBA_CLIENT_ACTION)(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIE /* action functions table, indexed with action enum */ const tBTA_PBA_CLIENT_ACTION bta_pba_client_action[] = { - /* BTA_PBA_CLIENT_API_OPEN */ bta_pba_client_api_open, - /* BTA_PBA_CLIENT_API_CLOSE */ bta_pba_client_api_close, - /* BTA_PBA_CLIENT_API_REQ */ bta_pba_client_api_req, - /* BTA_PBA_CLIENT_DO_CONNECT */ bta_pba_client_do_connect, - /* BTA_PBA_CLIENT_AUTHENTICATE */ bta_pba_client_authenticate, - /* BTA_PBA_CLIENT_CONNECT */ bta_pba_client_connect, - /* BTA_PBA_CLIENT_RESPONSE */ bta_pba_client_response, - /* BTA_PBA_CLIENT_RESPONSE_FINAL */ bta_pba_client_response_final, - /* BTA_PBA_CLIENT_GOEP_CONNECT */ bta_pba_client_goep_connect, - /* BTA_PBA_CLIENT_GOEP_DISCONNECT*/ bta_pba_client_goep_disconnect, - /* BTA_PBA_CLIENT_FORCE_DISCONNECT */ bta_pba_client_force_disconnect, - /* BTA_PBA_CLIENT_FREE_RESPONSE */ bta_pba_client_free_response, + /* BTA_PBA_CLIENT_API_OPEN */ bta_pba_client_api_open, + /* BTA_PBA_CLIENT_API_CLOSE */ bta_pba_client_api_close, + /* BTA_PBA_CLIENT_API_REQ */ bta_pba_client_api_req, + /* BTA_PBA_CLIENT_DO_CONNECT */ bta_pba_client_do_connect, + /* BTA_PBA_CLIENT_AUTHENTICATE */ bta_pba_client_authenticate, + /* BTA_PBA_CLIENT_CONNECT */ bta_pba_client_connect, + /* BTA_PBA_CLIENT_RESPONSE */ bta_pba_client_response, + /* BTA_PBA_CLIENT_RESPONSE_FINAL */ bta_pba_client_response_final, + /* BTA_PBA_CLIENT_GOEP_CONNECT */ bta_pba_client_goep_connect, + /* BTA_PBA_CLIENT_GOEP_DISCONNECT*/ bta_pba_client_goep_disconnect, + /* BTA_PBA_CLIENT_FORCE_DISCONNECT */ bta_pba_client_force_disconnect, + /* BTA_PBA_CLIENT_FREE_RESPONSE */ bta_pba_client_free_response, + /* BTA_PBA_CLIENT_FREE_REQ */ bta_pba_client_free_req, }; /* state table information */ @@ -72,7 +74,7 @@ const uint8_t bta_pba_client_st_init[][BTA_PBA_CLIENT_NUM_COLS] = { /* Event Action Next state */ /* BTA_PBA_CLIENT_API_OPEN_EVT */ {BTA_PBA_CLIENT_API_OPEN, BTA_PBA_CLIENT_OPENING_ST}, /* BTA_PBA_CLIENT_API_CLOSE_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_INIT_ST}, - /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_INIT_ST}, + /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_FREE_REQ, BTA_PBA_CLIENT_INIT_ST}, /* BTA_PBA_CLIENT_DISC_RES_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_INIT_ST}, /* BTA_PBA_CLIENT_AUTHENTICATE_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_INIT_ST}, /* BTA_PBA_CLIENT_CONNECT_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_INIT_ST}, @@ -86,7 +88,7 @@ const uint8_t bta_pba_client_st_opening[][BTA_PBA_CLIENT_NUM_COLS] = { /* Event Action Next state */ /* BTA_PBA_CLIENT_API_OPEN_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_OPENING_ST}, /* BTA_PBA_CLIENT_API_CLOSE_EVT */ {BTA_PBA_CLIENT_FORCE_DISCONNECT, BTA_PBA_CLIENT_INIT_ST}, - /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_OPENING_ST}, + /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_FREE_REQ, BTA_PBA_CLIENT_OPENING_ST}, /* BTA_PBA_CLIENT_DISC_RES_EVT */ {BTA_PBA_CLIENT_DO_CONNECT, BTA_PBA_CLIENT_OPENING_ST}, /* BTA_PBA_CLIENT_AUTHENTICATE_EVT */ {BTA_PBA_CLIENT_AUTHENTICATE, BTA_PBA_CLIENT_OPENING_ST}, /* BTA_PBA_CLIENT_CONNECT_EVT */ {BTA_PBA_CLIENT_CONNECT, BTA_PBA_CLIENT_OPENED_ST}, @@ -114,7 +116,7 @@ const uint8_t bta_pba_client_st_getting[][BTA_PBA_CLIENT_NUM_COLS] = { /* Event Action Next state */ /* BTA_PBA_CLIENT_API_OPEN_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_REQUESTING_ST}, /* BTA_PBA_CLIENT_API_CLOSE_EVT */ {BTA_PBA_CLIENT_FORCE_DISCONNECT, BTA_PBA_CLIENT_INIT_ST}, - /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_REQUESTING_ST}, + /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_FREE_REQ, BTA_PBA_CLIENT_REQUESTING_ST}, /* BTA_PBA_CLIENT_DISC_RES_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_REQUESTING_ST}, /* BTA_PBA_CLIENT_AUTHENTICATE_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_REQUESTING_ST}, /* BTA_PBA_CLIENT_CONNECT_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_REQUESTING_ST}, @@ -128,7 +130,7 @@ const uint8_t bta_pba_client_st_closing[][BTA_PBA_CLIENT_NUM_COLS] = { /* Event Action Next state */ /* BTA_PBA_CLIENT_API_OPEN_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_CLOSING_ST}, /* BTA_PBA_CLIENT_API_CLOSE_EVT */ {BTA_PBA_CLIENT_FORCE_DISCONNECT, BTA_PBA_CLIENT_INIT_ST}, - /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_CLOSING_ST}, + /* BTA_PBA_CLIENT_API_REQ_EVT */ {BTA_PBA_CLIENT_FREE_REQ, BTA_PBA_CLIENT_CLOSING_ST}, /* BTA_PBA_CLIENT_DISC_RES_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_CLOSING_ST}, /* BTA_PBA_CLIENT_AUTHENTICATE_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_CLOSING_ST}, /* BTA_PBA_CLIENT_CONNECT_EVT */ {BTA_PBA_CLIENT_IGNORE, BTA_PBA_CLIENT_CLOSING_ST}, diff --git a/components/bt/host/bluedroid/bta/pba/bta_pba_client_sdp.c b/components/bt/host/bluedroid/bta/pba/bta_pba_client_sdp.c index 9821c893d82..9dd55e1dba6 100644 --- a/components/bt/host/bluedroid/bta/pba/bta_pba_client_sdp.c +++ b/components/bt/host/bluedroid/bta/pba/bta_pba_client_sdp.c @@ -18,6 +18,12 @@ /* Number of elements in service class id list. */ #define BTA_PBA_CLIENT_NUM_SVC_ELEMS 1 +/* Cookie passed as SDP user_data so a stale callback can be detected after CCB reuse */ +typedef struct { + tBTA_PBA_CLIENT_CCB *p_ccb; + UINT8 sdp_seq; +} tBTA_PBA_CLIENT_SDP_CB_DATA; + /******************************************************************************* ** ** Function bta_pba_client_sdp_cback @@ -31,12 +37,24 @@ static void bta_pba_client_sdp_cback(UINT16 status, void *user_data) { tBTA_PBA_CLIENT_DISC_RESULT *p_buf; - tBTA_PBA_CLIENT_CCB *p_ccb = (tBTA_PBA_CLIENT_CCB *)user_data; + tBTA_PBA_CLIENT_SDP_CB_DATA *p_cb_data = (tBTA_PBA_CLIENT_SDP_CB_DATA *)user_data; + tBTA_PBA_CLIENT_CCB *p_ccb; + UINT8 sdp_seq; APPL_TRACE_DEBUG("bta_pba_client_sdp_cback status:0x%x", status); - if (p_ccb == NULL || p_ccb->allocated == 0) { - APPL_TRACE_ERROR("bta_pba_client_sdp_cback EINVAL CCB"); + if (p_cb_data == NULL) { + return; + } + + p_ccb = p_cb_data->p_ccb; + sdp_seq = p_cb_data->sdp_seq; + osi_free(p_cb_data); + + /* Drop callbacks for freed/reused CCBs or cancelled SDP searches */ + if (p_ccb == NULL || p_ccb->allocated == 0 || + p_ccb->sdp_seq != sdp_seq || p_ccb->p_disc_db == NULL) { + APPL_TRACE_WARNING("bta_pba_client_sdp_cback stale or invalid CCB"); return; } @@ -45,6 +63,20 @@ static void bta_pba_client_sdp_cback(UINT16 status, void *user_data) p_buf->hdr.layer_specific = p_ccb->allocated; p_buf->status = status; bta_sys_sendmsg(p_buf); + } else { + /* report connection closed event */ + tBTA_PBA_CLIENT_CONN conn; + conn.handle = p_ccb->allocated; + conn.error = BTA_PBA_CLIENT_NO_RESOURCE; + bdcpy(conn.bd_addr, p_ccb->bd_addr); + + /* free ccb */ + bta_pba_client_free_db(p_ccb); + sdp_seq = p_ccb->sdp_seq; + memset(p_ccb, 0, sizeof(tBTA_PBA_CLIENT_CCB)); + p_ccb->sdp_seq = sdp_seq; + + bta_pba_client_cb.p_cback(BTA_PBA_CLIENT_CONN_CLOSE_EVT, (tBTA_PBA_CLIENT *)&conn); } } @@ -102,6 +134,10 @@ void bta_pba_client_create_record(const char *p_service_name) /* add sdp record if not already registered */ if (bta_pba_client_cb.sdp_handle == 0) { bta_pba_client_cb.sdp_handle = SDP_CreateRecord(); + if (bta_pba_client_cb.sdp_handle == 0) { + APPL_TRACE_ERROR("SDP_CreateRecord failed"); + return; + } bta_pba_client_add_record(p_service_name, bta_pba_client_cb.sdp_handle); bta_sys_add_uuid(UUID_SERVCLASS_PBAP_PCE); } @@ -224,6 +260,7 @@ BOOLEAN bta_pba_client_do_disc(tBTA_PBA_CLIENT_CCB *p_ccb) UINT16 attr_list[6]; UINT8 num_attr = 6; BOOLEAN db_inited = FALSE; + tBTA_PBA_CLIENT_SDP_CB_DATA *p_cb_data = NULL; /* get proto list and features */ attr_list[0] = ATTR_ID_SERVICE_CLASS_ID_LIST; @@ -250,13 +287,23 @@ BOOLEAN bta_pba_client_do_disc(tBTA_PBA_CLIENT_CCB *p_ccb) } if (db_inited) { - /*start service discovery */ - /* todo: avoid p_ccb being free during sdp */ - db_inited = SDP_ServiceSearchAttributeRequest2(p_ccb->bd_addr, p_ccb->p_disc_db, - bta_pba_client_sdp_cback, p_ccb); + p_cb_data = (tBTA_PBA_CLIENT_SDP_CB_DATA *) osi_malloc(sizeof(tBTA_PBA_CLIENT_SDP_CB_DATA)); + if (p_cb_data == NULL) { + db_inited = FALSE; + } else { + /* New generation for this SDP request; must match in callback */ + p_ccb->sdp_seq++; + p_cb_data->p_ccb = p_ccb; + p_cb_data->sdp_seq = p_ccb->sdp_seq; + db_inited = SDP_ServiceSearchAttributeRequest2(p_ccb->bd_addr, p_ccb->p_disc_db, + bta_pba_client_sdp_cback, p_cb_data); + } } if (!db_inited) { + if (p_cb_data != NULL) { + osi_free(p_cb_data); + } /*free discover db */ bta_pba_client_free_db(p_ccb); APPL_TRACE_ERROR("%s start service discovery failed", __FUNCTION__); @@ -278,6 +325,8 @@ BOOLEAN bta_pba_client_do_disc(tBTA_PBA_CLIENT_CCB *p_ccb) void bta_pba_client_free_db(tBTA_PBA_CLIENT_CCB *p_ccb) { if (p_ccb->p_disc_db != NULL) { + /* Bump seq before cancel so a sync/async SDP callback is treated as stale */ + p_ccb->sdp_seq++; SDP_CancelServiceSearch(p_ccb->p_disc_db); osi_free(p_ccb->p_disc_db); p_ccb->p_disc_db = NULL; diff --git a/components/bt/host/bluedroid/bta/pba/include/bta_pba_client_int.h b/components/bt/host/bluedroid/bta/pba/include/bta_pba_client_int.h index 5d48eab9db1..7ef99554626 100644 --- a/components/bt/host/bluedroid/bta/pba/include/bta_pba_client_int.h +++ b/components/bt/host/bluedroid/bta/pba/include/bta_pba_client_int.h @@ -130,6 +130,7 @@ typedef struct { tBTA_PBA_CLIENT_OP operation; /* ongoing or last operations */ UINT8 state; /* main state machine */ UINT8 allocated; /* index + 1 if allocated, otherwise 0 */ + UINT8 sdp_seq; /* SDP request generation; survives CCB reuse */ } tBTA_PBA_CLIENT_CCB; typedef struct { @@ -163,6 +164,7 @@ extern void bta_pba_client_response_final(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_C void bta_pba_client_goep_connect(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data); void bta_pba_client_goep_disconnect(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data); void bta_pba_client_free_response(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data); +void bta_pba_client_free_req(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data); void bta_pba_client_free_sdp_db(tBTA_PBA_CLIENT_CCB *p_ccb, tBTA_PBA_CLIENT_DATA *p_data); extern void bta_pba_client_create_record(const char *p_service_name); diff --git a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c index 1dc09dabcf5..f55aeb0f65f 100644 --- a/components/bt/host/bluedroid/bta/sys/bta_sys_main.c +++ b/components/bt/host/bluedroid/bta/sys/bta_sys_main.c @@ -555,6 +555,7 @@ void bta_sys_register(UINT8 id, const tBTA_SYS_REG *p_reg) void bta_sys_deregister(UINT8 id) { bta_sys_cb.is_reg[id] = FALSE; + bta_sys_cb.reg[id] = NULL; } /******************************************************************************* diff --git a/components/bt/host/bluedroid/btc/profile/std/pba/btc_pba_client.c b/components/bt/host/bluedroid/btc/profile/std/pba/btc_pba_client.c index 2a7bdc068a7..5119a78805b 100644 --- a/components/bt/host/bluedroid/btc/profile/std/pba/btc_pba_client.c +++ b/components/bt/host/bluedroid/btc/profile/std/pba/btc_pba_client.c @@ -67,6 +67,18 @@ static void bte_pba_client_evt(tBTA_PBA_CLIENT_EVT event, tBTA_PBA_CLIENT *p_dat status = btc_transfer_context(&msg, p_data, param_len, NULL, NULL); if (status != BT_STATUS_SUCCESS) { BTC_TRACE_ERROR("context transfer failed"); + switch (event) { + case BTA_PBA_CLIENT_PULL_PHONE_BOOK_RSP_EVT: + case BTA_PBA_CLIENT_SET_PHONE_BOOK_RSP_EVT: + case BTA_PBA_CLIENT_PULL_VCARD_LISTING_RSP_EVT: + case BTA_PBA_CLIENT_PULL_VCARD_ENTRY_RSP_EVT: + if (p_data != NULL && p_data->response.pkt != NULL) { + osi_free(p_data->response.pkt); + } + break; + default: + break; + } } } @@ -93,7 +105,12 @@ static void btc_pba_client_init(void) static void btc_pba_client_deinit(void) { if (s_btc_pba_client_init) { - s_btc_pba_client_init = false; + for (int i = 0; i < BTC_PBA_CLIENT_MAX_CONN_NUM; ++i) { + if (btc_pba_client_cb.ccb[i].path) { + osi_free(btc_pba_client_cb.ccb[i].path); + btc_pba_client_cb.ccb[i].path = NULL; + } + } /* deregister sdp record */ BTA_PbaClientDeregister(); /* disable pba client */ @@ -118,7 +135,9 @@ static bt_status_t connect_int(bt_bdaddr_t *bd_addr, uint16_t uuid) return BT_STATUS_BUSY; } - BTA_PbaClientOpen(bd_addr->address, BTC_PBA_CLIENT_SECURITY, (uint32_t)BTC_PBA_SUPPORTED_FEAT, (uint16_t)BTC_PBA_PREFERRED_MTU); + if (BTA_PbaClientOpen(bd_addr->address, BTC_PBA_CLIENT_SECURITY, (uint32_t)BTC_PBA_SUPPORTED_FEAT, (uint16_t)BTC_PBA_PREFERRED_MTU) != BTA_SUCCESS) { + return BT_STATUS_NOMEM; + } return BT_STATUS_SUCCESS; } @@ -159,6 +178,8 @@ static bool btc_pba_client_pull_phone_book(uint16_t handle, char *name, bool inc bt_status_t err = BT_STATUS_FAIL; uint8_t *app_param_buff = NULL; uint16_t app_param_len = 0; + btc_pba_client_ccb_t *p_ccb = NULL; + bool busy_set = false; do { if (!s_btc_pba_client_init) { @@ -173,7 +194,7 @@ static bool btc_pba_client_pull_phone_book(uint16_t handle, char *name, bool inc break; } - btc_pba_client_ccb_t *p_ccb = &btc_pba_client_cb.ccb[handle - 1]; + p_ccb = &btc_pba_client_cb.ccb[handle - 1]; if (p_ccb->handle != handle) { /* not connect */ err = BT_STATUS_PARM_INVALID; @@ -243,12 +264,19 @@ static bool btc_pba_client_pull_phone_book(uint16_t handle, char *name, bool inc } p_ccb->busy = true; - BTA_PbaClientPullPhoneBook(handle, name, app_param_buff, app_param_len); - err = BT_STATUS_SUCCESS; + busy_set = true; + err = (BTA_PbaClientPullPhoneBook(handle, name, app_param_buff, app_param_len) == BTA_SUCCESS) ? BT_STATUS_SUCCESS : BT_STATUS_NOMEM; } while (0); if (err != BT_STATUS_SUCCESS) { - BTC_TRACE_WARNING("%s failed, handle: %d, reason: %d", __FUNCTION__, handle, err); + BTC_TRACE_WARNING("failed, handle: %d, reason: %d", handle, err); + if (busy_set) { + p_ccb->busy = false; + } + /* BTA did not take ownership on failure */ + if (app_param_buff != NULL) { + osi_free(app_param_buff); + } return false; } @@ -258,6 +286,8 @@ static bool btc_pba_client_pull_phone_book(uint16_t handle, char *name, bool inc static bool btc_pba_client_set_phone_book(uint16_t handle, uint8_t flags, char *name) { bt_status_t err = BT_STATUS_FAIL; + btc_pba_client_ccb_t *p_ccb = NULL; + bool busy_set = false; do { if (!s_btc_pba_client_init) { @@ -272,7 +302,7 @@ static bool btc_pba_client_set_phone_book(uint16_t handle, uint8_t flags, char * break; } - btc_pba_client_ccb_t *p_ccb = &btc_pba_client_cb.ccb[handle - 1]; + p_ccb = &btc_pba_client_cb.ccb[handle - 1]; if (p_ccb->handle != handle) { /* not connect */ err = BT_STATUS_PARM_INVALID; @@ -286,12 +316,15 @@ static bool btc_pba_client_set_phone_book(uint16_t handle, uint8_t flags, char * } p_ccb->busy = true; - BTA_PbaClientSetPhoneBook(handle, flags, (char *)name); - err = BT_STATUS_SUCCESS; + busy_set = true; + err = (BTA_PbaClientSetPhoneBook(handle, flags, (char *)name) == BTA_SUCCESS) ? BT_STATUS_SUCCESS : BT_STATUS_NOMEM; } while (0); if (err != BT_STATUS_SUCCESS) { - BTC_TRACE_WARNING("%s failed, handle: %d, reason: %d", __FUNCTION__, handle, err); + BTC_TRACE_WARNING("failed, handle: %d, reason: %d", handle, err); + if (busy_set) { + p_ccb->busy = false; + } return false; } @@ -301,6 +334,10 @@ static bool btc_pba_client_set_phone_book(uint16_t handle, uint8_t flags, char * static bool btc_pba_client_set_phone_book2(uint16_t handle, char *path) { bt_status_t err = BT_STATUS_FAIL; + btc_pba_client_ccb_t *p_ccb = NULL; + bool busy_set = false; + char *empty_name = NULL; + bool path_empty_root = false; do { if (!s_btc_pba_client_init) { @@ -315,7 +352,7 @@ static bool btc_pba_client_set_phone_book2(uint16_t handle, char *path) break; } - btc_pba_client_ccb_t *p_ccb = &btc_pba_client_cb.ccb[handle - 1]; + p_ccb = &btc_pba_client_cb.ccb[handle - 1]; if (p_ccb->handle != handle) { /* not connect */ err = BT_STATUS_PARM_INVALID; @@ -329,6 +366,7 @@ static bool btc_pba_client_set_phone_book2(uint16_t handle, char *path) } p_ccb->busy = true; + busy_set = true; if (path != NULL) { p_ccb->path_len = strlen(path) + 1; /* ignore the first slash */ @@ -342,26 +380,39 @@ static bool btc_pba_client_set_phone_book2(uint16_t handle, char *path) if (p_ccb->path_len == p_ccb->path_pos + 1) { p_ccb->path_len = 0; p_ccb->path_pos = 0; - osi_free(path); - path = NULL; + path_empty_root = true; } else { p_ccb->path = path; } } /* anyway, go to ROOT first */ - char *empty_name = osi_malloc(1); + empty_name = osi_malloc(1); assert(empty_name != NULL); *empty_name = '\0'; - BTA_PbaClientSetPhoneBook(handle, ESP_PBAC_SET_PHONE_BOOK_FLAGS_ROOT, empty_name); - err = BT_STATUS_SUCCESS; + err = (BTA_PbaClientSetPhoneBook(handle, ESP_PBAC_SET_PHONE_BOOK_FLAGS_ROOT, empty_name) == BTA_SUCCESS) ? BT_STATUS_SUCCESS : BT_STATUS_NOMEM; } while (0); if (err != BT_STATUS_SUCCESS) { - BTC_TRACE_WARNING("%s failed, handle: %d, reason: %d", __FUNCTION__, handle, err); + BTC_TRACE_WARNING("failed, handle: %d, reason: %d", handle, err); + if (busy_set) { + p_ccb->busy = false; + p_ccb->path_len = 0; + p_ccb->path_pos = 0; + /* drop ccb path ownership so deep_free can free the arg buffer */ + p_ccb->path = NULL; + } + /* BTA did not take ownership on failure */ + if (empty_name != NULL) { + osi_free(empty_name); + } return false; } + /* success: empty "/" path is not kept on ccb; free it here (no deep_free on success) */ + if (path_empty_root && path != NULL) { + osi_free(path); + } return true; } @@ -370,6 +421,8 @@ static bool btc_pba_client_pull_vcard_listing(uint16_t handle, char *name, bool bt_status_t err = BT_STATUS_FAIL; uint8_t *app_param_buff = NULL; uint16_t app_param_len = 0; + btc_pba_client_ccb_t *p_ccb = NULL; + bool busy_set = false; do { if (!s_btc_pba_client_init) { @@ -384,7 +437,7 @@ static bool btc_pba_client_pull_vcard_listing(uint16_t handle, char *name, bool break; } - btc_pba_client_ccb_t *p_ccb = &btc_pba_client_cb.ccb[handle - 1]; + p_ccb = &btc_pba_client_cb.ccb[handle - 1]; if (p_ccb->handle != handle) { /* not connect */ err = BT_STATUS_PARM_INVALID; @@ -479,12 +532,19 @@ static bool btc_pba_client_pull_vcard_listing(uint16_t handle, char *name, bool } p_ccb->busy = true; - BTA_PbaClientPullvCardListing(handle, (char *)name, app_param_buff, app_param_len); - err = BT_STATUS_SUCCESS; + busy_set = true; + err = (BTA_PbaClientPullvCardListing(handle, (char *)name, app_param_buff, app_param_len) == BTA_SUCCESS) ? BT_STATUS_SUCCESS : BT_STATUS_NOMEM; } while (0); if (err != BT_STATUS_SUCCESS) { - BTC_TRACE_WARNING("%s failed, handle: %d, reason: %d", __FUNCTION__, handle, err); + BTC_TRACE_WARNING("failed, handle: %d, reason: %d", handle, err); + if (busy_set) { + p_ccb->busy = false; + } + /* BTA did not take ownership on failure */ + if (app_param_buff != NULL) { + osi_free(app_param_buff); + } return false; } @@ -496,6 +556,8 @@ static bool btc_pba_client_pull_vcard_entry(uint16_t handle, char *name, bool in bt_status_t err = BT_STATUS_FAIL; uint8_t *app_param_buff = NULL; uint16_t app_param_len = 0; + btc_pba_client_ccb_t *p_ccb = NULL; + bool busy_set = false; do { if (!s_btc_pba_client_init) { @@ -510,7 +572,7 @@ static bool btc_pba_client_pull_vcard_entry(uint16_t handle, char *name, bool in break; } - btc_pba_client_ccb_t *p_ccb = &btc_pba_client_cb.ccb[handle - 1]; + p_ccb = &btc_pba_client_cb.ccb[handle - 1]; if (p_ccb->handle != handle) { /* not connect */ err = BT_STATUS_PARM_INVALID; @@ -555,12 +617,19 @@ static bool btc_pba_client_pull_vcard_entry(uint16_t handle, char *name, bool in } p_ccb->busy = true; - BTA_PbaClientPullvCardEntry(handle, (char *)name, app_param_buff, app_param_len); - err = BT_STATUS_SUCCESS; + busy_set = true; + err = (BTA_PbaClientPullvCardEntry(handle, (char *)name, app_param_buff, app_param_len) == BTA_SUCCESS) ? BT_STATUS_SUCCESS : BT_STATUS_NOMEM; } while (0); if (err != BT_STATUS_SUCCESS) { - BTC_TRACE_WARNING("%s failed, handle: %d, reason: %d", __FUNCTION__, handle, err); + BTC_TRACE_WARNING("failed, handle: %d, reason: %d", handle, err); + if (busy_set) { + p_ccb->busy = false; + } + /* BTA did not take ownership on failure */ + if (app_param_buff != NULL) { + osi_free(app_param_buff); + } return false; } @@ -1028,7 +1097,17 @@ void btc_pba_client_cb_handler(btc_msg_t *msg) memcpy(dir_name, p_ccb->path + p_ccb->path_pos, dir_name_len); dir_name[dir_name_len] = '\0'; p_ccb->path_pos += dir_name_len + 1; - BTA_PbaClientSetPhoneBook(p_data->response.handle, ESP_PBAC_SET_PHONE_BOOK_FLAGS_DOWN, dir_name); + if (BTA_PbaClientSetPhoneBook(p_data->response.handle, ESP_PBAC_SET_PHONE_BOOK_FLAGS_DOWN, dir_name) != BTA_SUCCESS) { + osi_free(dir_name); + p_ccb->path_len = 0; + p_ccb->path_pos = 0; + osi_free(p_ccb->path); + p_ccb->path = NULL; + p_ccb->busy = false; + param.set_phone_book_rsp.handle = p_data->response.handle; + param.set_phone_book_rsp.result = BTA_FAILURE; + btc_pba_client_callback_to_app(ESP_PBAC_SET_PHONE_BOOK_RESPONSE_EVT, ¶m); + } /* break here, don't report event to upper */ break; } @@ -1086,6 +1165,7 @@ void btc_pba_client_cb_handler(btc_msg_t *msg) break; case BTA_PBA_CLIENT_DISABLE_EVT: /* deinit process: Deregister -> Disable */ + s_btc_pba_client_init = false; btc_pba_client_callback_to_app(ESP_PBAC_DEINIT_EVT, NULL); break; default: