Merge "Fix use-after-free while setting advertisement data."
diff --git a/btif/include/btif_gatt_multi_adv_util.h b/btif/include/btif_gatt_multi_adv_util.h
index 4b44a51..fd1426f 100644
--- a/btif/include/btif_gatt_multi_adv_util.h
+++ b/btif/include/btif_gatt_multi_adv_util.h
@@ -83,13 +83,14 @@
BOOLEAN stop_timer);
// Free a buffer and reset *buf to NULL.
extern void btif_gattc_cleanup(void** buf);
-extern BOOLEAN btif_gattc_copy_datacb(int arrindex, btif_adv_data_t *p_adv_data,
+extern BOOLEAN btif_gattc_copy_datacb(int arrindex, const btif_adv_data_t *p_adv_data,
BOOLEAN bInstData);
extern void btif_gattc_adv_data_packager(int client_if, bool set_scan_rsp,
bool include_name, bool include_txpower, int min_interval, int max_interval,
int appearance, int manufacturer_len, char* manufacturer_data,
int service_data_len, char* service_data, int service_uuid_len,
char* service_uuid, btif_adv_data_t *p_multi_adv_inst);
+extern void btif_gattc_adv_data_cleanup(const btif_adv_data_t* adv);
void btif_multi_adv_timer_ctrl(int client_if, TIMER_CBACK cb);
#endif
diff --git a/btif/src/btif_gatt_client.c b/btif/src/btif_gatt_client.c
index df766c1..171725c 100644
--- a/btif/src/btif_gatt_client.c
+++ b/btif/src/btif_gatt_client.c
@@ -1058,6 +1058,23 @@
(char*) &btif_cb, sizeof(btgatt_adv_filter_cb_t), NULL);
}
+static void btgattc_free_event_data(UINT16 event, char *event_data)
+{
+ switch (event)
+ {
+ case BTIF_GATTC_ADV_INSTANCE_SET_DATA:
+ case BTIF_GATTC_SET_ADV_DATA:
+ {
+ const btif_adv_data_t *adv_data = (btif_adv_data_t*) event_data;
+ btif_gattc_adv_data_cleanup(adv_data);
+ break;
+ }
+
+ default:
+ break;
+ }
+}
+
static void btgattc_handle_event(uint16_t event, char* p_param)
{
tBTA_GATT_STATUS status;
@@ -1502,14 +1519,11 @@
case BTIF_GATTC_SET_ADV_DATA:
{
- btif_adv_data_t *p_adv_data = (btif_adv_data_t*) p_param;
- int cbindex = CLNT_IF_IDX;
- if (cbindex >= 0 && NULL != p_adv_data)
+ const btif_adv_data_t *p_adv_data = (btif_adv_data_t*) p_param;
+ const int cbindex = CLNT_IF_IDX;
+ if (cbindex >= 0 && btif_gattc_copy_datacb(cbindex, p_adv_data, false))
{
btgatt_multi_adv_common_data *p_multi_adv_data_cb = btif_obtain_multi_adv_data_cb();
- if (!btif_gattc_copy_datacb(cbindex, p_adv_data, false))
- return;
-
if (!p_adv_data->set_scan_rsp)
{
BTA_DmBleSetAdvConfig(p_multi_adv_data_cb->inst_cb[cbindex].mask,
@@ -1520,8 +1534,13 @@
BTA_DmBleSetScanRsp(p_multi_adv_data_cb->inst_cb[cbindex].mask,
&p_multi_adv_data_cb->inst_cb[cbindex].data, bta_gattc_set_adv_data_cback);
}
- break;
}
+ else
+ {
+ BTIF_TRACE_ERROR("%s:%s: failed to get instance data cbindex: %d",
+ __func__, "BTIF_GATTC_SET_ADV_DATA", cbindex);
+ }
+ break;
}
case BTIF_GATTC_ADV_INSTANCE_ENABLE:
@@ -1578,19 +1597,23 @@
btif_adv_data_t *p_adv_data = (btif_adv_data_t*) p_param;
int cbindex = btif_gattc_obtain_idx_for_datacb(p_adv_data->client_if, CLNT_IF_IDX);
int inst_id = btif_multi_adv_instid_for_clientif(p_adv_data->client_if);
- if (inst_id < 0 || cbindex < 0)
+ if (inst_id >= 0 && cbindex >= 0 && btif_gattc_copy_datacb(cbindex, p_adv_data, true))
{
- BTIF_TRACE_ERROR("%s invalid index in BTIF_GATTC_SETADV_INST_DATA", __FUNCTION__);
- return;
+ btgatt_multi_adv_common_data *p_multi_adv_data_cb =
+ btif_obtain_multi_adv_data_cb();
+ BTA_BleCfgAdvInstData(
+ (UINT8)inst_id,
+ p_multi_adv_data_cb->inst_cb[cbindex].is_scan_rsp,
+ p_multi_adv_data_cb->inst_cb[cbindex].mask,
+ &p_multi_adv_data_cb->inst_cb[cbindex].data);
}
-
- if (!btif_gattc_copy_datacb(cbindex, p_adv_data, true))
- return;
-
- btgatt_multi_adv_common_data *p_multi_adv_data_cb = btif_obtain_multi_adv_data_cb();
- BTA_BleCfgAdvInstData((UINT8)inst_id, p_multi_adv_data_cb->inst_cb[cbindex].is_scan_rsp,
- p_multi_adv_data_cb->inst_cb[cbindex].mask,
- &p_multi_adv_data_cb->inst_cb[cbindex].data);
+ else
+ {
+ BTIF_TRACE_ERROR(
+ "%s:%s: failed to get invalid instance data: inst_id:%d "
+ "cbindex:%d",
+ __func__, "BTIF_GATTC_ADV_INSTANCE_SET_DATA", inst_id, cbindex);
+ }
break;
}
@@ -1666,6 +1689,8 @@
LOG_ERROR("%s: Unknown event (%d)!", __FUNCTION__, event);
break;
}
+
+ btgattc_free_event_data(event, p_param);
}
/*******************************************************************************
@@ -1732,6 +1757,44 @@
(char*) &btif_cb, sizeof(btif_gattc_cb_t), NULL);
}
+static void btif_gattc_deep_copy(UINT16 event, char *p_dest, char *p_src)
+{
+ switch (event)
+ {
+ case BTIF_GATTC_ADV_INSTANCE_SET_DATA:
+ case BTIF_GATTC_SET_ADV_DATA:
+ {
+ const btif_adv_data_t *src = (btif_adv_data_t*) p_src;
+ btif_adv_data_t *dst = (btif_adv_data_t*) p_dest;
+ memcpy(dst, src, sizeof(*src));
+
+ if (src->p_manufacturer_data)
+ {
+ dst->p_manufacturer_data = GKI_getbuf(src->manufacturer_len);
+ memcpy(dst->p_manufacturer_data, src->p_manufacturer_data,
+ src->manufacturer_len);
+ }
+
+ if (src->p_service_data)
+ {
+ dst->p_service_data = GKI_getbuf(src->service_data_len);
+ memcpy(dst->p_service_data, src->p_service_data, src->service_data_len);
+ }
+
+ if (src->p_service_uuid)
+ {
+ dst->p_service_uuid = GKI_getbuf(src->service_uuid_len);
+ memcpy(dst->p_service_uuid, src->p_service_uuid, src->service_uuid_len);
+ }
+ break;
+ }
+
+ default:
+ ASSERTC(false, "Unhandled deep copy", event);
+ break;
+ }
+}
+
static bt_status_t btif_gattc_set_adv_data(int client_if, bool set_scan_rsp, bool include_name,
bool include_txpower, int min_interval, int max_interval, int appearance,
uint16_t manufacturer_len, char* manufacturer_data,
@@ -1739,8 +1802,6 @@
uint16_t service_uuid_len, char* service_uuid)
{
CHECK_BTGATT_INIT();
- bt_status_t status =0;
-
btif_adv_data_t adv_data;
btif_gattc_adv_data_packager(client_if, set_scan_rsp, include_name,
@@ -1748,18 +1809,9 @@
manufacturer_data, service_data_len, service_data, service_uuid_len, service_uuid,
&adv_data);
- status = btif_transfer_context(btgattc_handle_event, BTIF_GATTC_SET_ADV_DATA,
- (char*) &adv_data, sizeof(btif_adv_data_t), NULL);
-
- if (NULL != adv_data.p_service_data)
- GKI_freebuf(adv_data.p_service_data);
-
- if (NULL != adv_data.p_service_uuid)
- GKI_freebuf(adv_data.p_service_uuid);
-
- if (NULL != adv_data.p_manufacturer_data)
- GKI_freebuf(adv_data.p_manufacturer_data);
-
+ bt_status_t status = btif_transfer_context(btgattc_handle_event, BTIF_GATTC_SET_ADV_DATA,
+ (char*) &adv_data, sizeof(adv_data), btif_gattc_deep_copy);
+ btif_gattc_adv_data_cleanup(&adv_data);
return status;
}
@@ -2141,28 +2193,21 @@
{
CHECK_BTGATT_INIT();
- int min_interval = 0, max_interval = 0;
- bt_status_t status =0;
-
btif_adv_data_t multi_adv_data_inst;
- memset(&multi_adv_data_inst, 0, sizeof(btif_adv_data_t));
+ memset(&multi_adv_data_inst, 0, sizeof(multi_adv_data_inst));
+
+ const int min_interval = 0;
+ const int max_interval = 0;
btif_gattc_adv_data_packager(client_if, set_scan_rsp, include_name, incl_txpower,
min_interval, max_interval, appearance, manufacturer_len, manufacturer_data,
service_data_len, service_data, service_uuid_len, service_uuid, &multi_adv_data_inst);
- status = btif_transfer_context(btgattc_handle_event, BTIF_GATTC_ADV_INSTANCE_SET_DATA,
- (char*) &multi_adv_data_inst, sizeof(btif_adv_data_t), NULL);
-
- if (NULL != multi_adv_data_inst.p_service_data)
- GKI_freebuf(multi_adv_data_inst.p_service_data);
-
- if (NULL != multi_adv_data_inst.p_service_uuid)
- GKI_freebuf(multi_adv_data_inst.p_service_uuid);
-
- if (NULL != multi_adv_data_inst.p_manufacturer_data)
- GKI_freebuf(multi_adv_data_inst.p_manufacturer_data);
-
+ bt_status_t status = btif_transfer_context(
+ btgattc_handle_event, BTIF_GATTC_ADV_INSTANCE_SET_DATA,
+ (char *)&multi_adv_data_inst, sizeof(multi_adv_data_inst),
+ btif_gattc_deep_copy);
+ btif_gattc_adv_data_cleanup(&multi_adv_data_inst);
return status;
}
diff --git a/btif/src/btif_gatt_multi_adv_util.c b/btif/src/btif_gatt_multi_adv_util.c
index 4837474..63bca08 100644
--- a/btif/src/btif_gatt_multi_adv_util.c
+++ b/btif/src/btif_gatt_multi_adv_util.c
@@ -252,8 +252,21 @@
}
}
-BOOLEAN btif_gattc_copy_datacb(int cbindex, btif_adv_data_t *p_adv_data, BOOLEAN bInstData)
+void btif_gattc_adv_data_cleanup(const btif_adv_data_t* adv)
{
+ if (adv->p_service_data)
+ GKI_freebuf(adv->p_service_data);
+
+ if (adv->p_service_uuid)
+ GKI_freebuf(adv->p_service_uuid);
+
+ if (adv->p_manufacturer_data)
+ GKI_freebuf(adv->p_manufacturer_data);
+}
+
+
+BOOLEAN btif_gattc_copy_datacb(int cbindex, const btif_adv_data_t *p_adv_data,
+ BOOLEAN bInstData) {
btgatt_multi_adv_common_data *p_multi_adv_data_cb = btif_obtain_multi_adv_data_cb();
if (NULL == p_multi_adv_data_cb || cbindex < 0)
return false;
@@ -377,18 +390,18 @@
}
}
- if (p_adv_data->service_uuid_len > 0 && NULL != p_adv_data->p_service_uuid)
+ if (p_adv_data->service_uuid_len && p_adv_data->p_service_uuid)
{
UINT16 *p_uuid_out16 = NULL;
UINT32 *p_uuid_out32 = NULL;
- while (p_adv_data->service_uuid_len >= LEN_UUID_128)
+ for (int position = 0; position < p_adv_data->service_uuid_len; position += LEN_UUID_128)
{
bt_uuid_t uuid;
- memset(&uuid, 0, sizeof(bt_uuid_t));
- memcpy(&uuid.uu, p_adv_data->p_service_uuid, LEN_UUID_128);
+ memset(&uuid, 0, sizeof(uuid));
+ memcpy(&uuid.uu, p_adv_data->p_service_uuid + position, LEN_UUID_128);
tBT_UUID bt_uuid;
- memset(&bt_uuid, 0, sizeof(tBT_UUID));
+ memset(&bt_uuid, 0, sizeof(bt_uuid));
btif_to_bta_uuid(&bt_uuid, &uuid);
switch(bt_uuid.len)
@@ -467,9 +480,6 @@
default:
break;
}
-
- p_adv_data->p_service_uuid += LEN_UUID_128;
- p_adv_data->service_uuid_len -= LEN_UUID_128;
}
}
