From 3b9fe36494cf35160b1ea4269f6979d61b8b608c Mon Sep 17 00:00:00 2001 From: lly Date: Fri, 20 Dec 2019 14:08:33 +0800 Subject: [PATCH] ble_mesh: Fix memory leak when node is reset When node is being reset, the init functions of each sig-defined models will be invoked again, this will cause memory leak because some model internal data will be allocated again. Hence before trying to allocate memory for them, we add some check to make sure no memory has been allocated previously. And for client model, when the init functions are invoked again, we will clear the list items. --- .../bt/esp_ble_mesh/mesh_core/cfg_cli.c | 27 ++++++----- .../bt/esp_ble_mesh/mesh_core/health_cli.c | 27 ++++++----- .../mesh_models/client/client_common.c | 47 ++++++++++++++----- .../mesh_models/client/generic_client.c | 27 ++++++----- .../client/include/client_common.h | 2 + .../mesh_models/client/lighting_client.c | 27 ++++++----- .../mesh_models/client/sensor_client.c | 27 ++++++----- .../mesh_models/client/time_scene_client.c | 27 ++++++----- .../mesh_models/server/server_common.c | 6 ++- 9 files changed, 132 insertions(+), 85 deletions(-) diff --git a/components/bt/esp_ble_mesh/mesh_core/cfg_cli.c b/components/bt/esp_ble_mesh/mesh_core/cfg_cli.c index c3e73c23f..d1c04f604 100644 --- a/components/bt/esp_ble_mesh/mesh_core/cfg_cli.c +++ b/components/bt/esp_ble_mesh/mesh_core/cfg_cli.c @@ -1649,20 +1649,23 @@ int bt_mesh_cfg_cli_init(struct bt_mesh_model *model, bool primary) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked*/ - internal = osi_calloc(sizeof(config_internal_data_t)); - if (!internal) { - BT_ERR("Allocate memory for Configuration Client internal data fail"); - return -ENOMEM; + if (!client->internal_data) { + internal = osi_calloc(sizeof(config_internal_data_t)); + if (!internal) { + BT_ERR("Allocate memory for Configuration Client internal data fail"); + return -ENOMEM; + } + + sys_slist_init(&internal->queue); + + client->model = model; + client->op_pair_size = ARRAY_SIZE(cfg_op_pair); + client->op_pair = cfg_op_pair; + client->internal_data = internal; + } else { + bt_mesh_client_clear_list(client->internal_data); } - sys_slist_init(&internal->queue); - - client->model = model; - client->op_pair_size = ARRAY_SIZE(cfg_op_pair); - client->op_pair = cfg_op_pair; - client->internal_data = internal; - cli = client; /* Configuration Model security is device-key based */ diff --git a/components/bt/esp_ble_mesh/mesh_core/health_cli.c b/components/bt/esp_ble_mesh/mesh_core/health_cli.c index 0d5d5c813..ee32e9ddd 100644 --- a/components/bt/esp_ble_mesh/mesh_core/health_cli.c +++ b/components/bt/esp_ble_mesh/mesh_core/health_cli.c @@ -454,20 +454,23 @@ int bt_mesh_health_cli_init(struct bt_mesh_model *model, bool primary) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked*/ - internal = osi_calloc(sizeof(health_internal_data_t)); - if (!internal) { - BT_ERR("%s, Failed to allocate memory", __func__); - return -ENOMEM; + if (!client->internal_data) { + internal = osi_calloc(sizeof(health_internal_data_t)); + if (!internal) { + BT_ERR("%s, Failed to allocate memory", __func__); + return -ENOMEM; + } + + sys_slist_init(&internal->queue); + + client->model = model; + client->op_pair_size = ARRAY_SIZE(health_op_pair); + client->op_pair = health_op_pair; + client->internal_data = internal; + } else { + bt_mesh_client_clear_list(client->internal_data); } - sys_slist_init(&internal->queue); - - client->model = model; - client->op_pair_size = ARRAY_SIZE(health_op_pair); - client->op_pair = health_op_pair; - client->internal_data = internal; - bt_mesh_health_client_mutex_new(); /* Set the default health client pointer */ diff --git a/components/bt/esp_ble_mesh/mesh_models/client/client_common.c b/components/bt/esp_ble_mesh/mesh_models/client/client_common.c index 5f9374b34..c309519ae 100644 --- a/components/bt/esp_ble_mesh/mesh_models/client/client_common.c +++ b/components/bt/esp_ble_mesh/mesh_models/client/client_common.c @@ -266,19 +266,22 @@ int bt_mesh_client_init(struct bt_mesh_model *model) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked */ - data = osi_calloc(sizeof(bt_mesh_client_internal_data_t)); - if (!data) { - BT_ERR("%s, Failed to allocate memory", __func__); - return -ENOMEM; + if (!cli->internal_data) { + data = osi_calloc(sizeof(bt_mesh_client_internal_data_t)); + if (!data) { + BT_ERR("%s, Failed to allocate memory", __func__); + return -ENOMEM; + } + + /* Init the client data queue */ + sys_slist_init(&data->queue); + + cli->model = model; + cli->internal_data = data; + } else { + bt_mesh_client_clear_list(cli->internal_data); } - /* Init the client data queue */ - sys_slist_init(&data->queue); - - cli->model = model; - cli->internal_data = data; - bt_mesh_client_model_mutex_new(); return 0; @@ -316,6 +319,28 @@ int bt_mesh_client_free_node(bt_mesh_client_node_t *node) return 0; } +int bt_mesh_client_clear_list(void *data) +{ + bt_mesh_client_internal_data_t *internal = NULL; + bt_mesh_client_node_t *node = NULL; + + if (!data) { + BT_ERR("%s, Invalid parameter", __func__); + return -EINVAL; + } + + internal = (bt_mesh_client_internal_data_t *)data; + + bt_mesh_list_lock(); + while (!sys_slist_is_empty(&internal->queue)) { + node = (void *)sys_slist_get_not_empty(&internal->queue); + osi_free(node); + } + bt_mesh_list_unlock(); + + return 0; +} + int bt_mesh_set_client_model_role(bt_mesh_role_param_t *common) { bt_mesh_client_user_data_t *client = NULL; diff --git a/components/bt/esp_ble_mesh/mesh_models/client/generic_client.c b/components/bt/esp_ble_mesh/mesh_models/client/generic_client.c index 48b540b41..8352d1bf4 100644 --- a/components/bt/esp_ble_mesh/mesh_models/client/generic_client.c +++ b/components/bt/esp_ble_mesh/mesh_models/client/generic_client.c @@ -1172,20 +1172,23 @@ static int generic_client_init(struct bt_mesh_model *model, bool primary) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked*/ - internal = osi_calloc(sizeof(generic_internal_data_t)); - if (!internal) { - BT_ERR("%s, Failed to allocate memory", __func__); - return -ENOMEM; + if (!client->internal_data) { + internal = osi_calloc(sizeof(generic_internal_data_t)); + if (!internal) { + BT_ERR("%s, Failed to allocate memory", __func__); + return -ENOMEM; + } + + sys_slist_init(&internal->queue); + + client->model = model; + client->op_pair_size = ARRAY_SIZE(gen_op_pair); + client->op_pair = gen_op_pair; + client->internal_data = internal; + } else { + bt_mesh_client_clear_list(client->internal_data); } - sys_slist_init(&internal->queue); - - client->model = model; - client->op_pair_size = ARRAY_SIZE(gen_op_pair); - client->op_pair = gen_op_pair; - client->internal_data = internal; - bt_mesh_generic_client_mutex_new(); return 0; diff --git a/components/bt/esp_ble_mesh/mesh_models/client/include/client_common.h b/components/bt/esp_ble_mesh/mesh_models/client/include/client_common.h index 74eb366ca..ff4e52609 100644 --- a/components/bt/esp_ble_mesh/mesh_models/client/include/client_common.h +++ b/components/bt/esp_ble_mesh/mesh_models/client/include/client_common.h @@ -108,6 +108,8 @@ int bt_mesh_client_send_msg(struct bt_mesh_model *model, int bt_mesh_client_free_node(bt_mesh_client_node_t *node); +int bt_mesh_client_clear_list(void *data); + enum { NODE = 0, PROVISIONER, diff --git a/components/bt/esp_ble_mesh/mesh_models/client/lighting_client.c b/components/bt/esp_ble_mesh/mesh_models/client/lighting_client.c index 78b80d7b0..87d2922c1 100644 --- a/components/bt/esp_ble_mesh/mesh_models/client/lighting_client.c +++ b/components/bt/esp_ble_mesh/mesh_models/client/lighting_client.c @@ -1362,20 +1362,23 @@ static int light_client_init(struct bt_mesh_model *model, bool primary) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked*/ - internal = osi_calloc(sizeof(light_internal_data_t)); - if (!internal) { - BT_ERR("%s, Failed to allocate memory", __func__); - return -ENOMEM; + if (!client->internal_data) { + internal = osi_calloc(sizeof(light_internal_data_t)); + if (!internal) { + BT_ERR("%s, Failed to allocate memory", __func__); + return -ENOMEM; + } + + sys_slist_init(&internal->queue); + + client->model = model; + client->op_pair_size = ARRAY_SIZE(light_op_pair); + client->op_pair = light_op_pair; + client->internal_data = internal; + } else { + bt_mesh_client_clear_list(client->internal_data); } - sys_slist_init(&internal->queue); - - client->model = model; - client->op_pair_size = ARRAY_SIZE(light_op_pair); - client->op_pair = light_op_pair; - client->internal_data = internal; - bt_mesh_light_client_mutex_new(); return 0; diff --git a/components/bt/esp_ble_mesh/mesh_models/client/sensor_client.c b/components/bt/esp_ble_mesh/mesh_models/client/sensor_client.c index 7b93aecfb..d8eb7bd2d 100644 --- a/components/bt/esp_ble_mesh/mesh_models/client/sensor_client.c +++ b/components/bt/esp_ble_mesh/mesh_models/client/sensor_client.c @@ -604,20 +604,23 @@ int bt_mesh_sensor_cli_init(struct bt_mesh_model *model, bool primary) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked*/ - internal = osi_calloc(sizeof(sensor_internal_data_t)); - if (!internal) { - BT_ERR("%s, Failed to allocate memory", __func__); - return -ENOMEM; + if (!client->internal_data) { + internal = osi_calloc(sizeof(sensor_internal_data_t)); + if (!internal) { + BT_ERR("%s, Failed to allocate memory", __func__); + return -ENOMEM; + } + + sys_slist_init(&internal->queue); + + client->model = model; + client->op_pair_size = ARRAY_SIZE(sensor_op_pair); + client->op_pair = sensor_op_pair; + client->internal_data = internal; + } else { + bt_mesh_client_clear_list(client->internal_data); } - sys_slist_init(&internal->queue); - - client->model = model; - client->op_pair_size = ARRAY_SIZE(sensor_op_pair); - client->op_pair = sensor_op_pair; - client->internal_data = internal; - bt_mesh_sensor_client_mutex_new(); return 0; diff --git a/components/bt/esp_ble_mesh/mesh_models/client/time_scene_client.c b/components/bt/esp_ble_mesh/mesh_models/client/time_scene_client.c index 878ef3da3..b7456d2a4 100644 --- a/components/bt/esp_ble_mesh/mesh_models/client/time_scene_client.c +++ b/components/bt/esp_ble_mesh/mesh_models/client/time_scene_client.c @@ -667,20 +667,23 @@ static int time_scene_client_init(struct bt_mesh_model *model, bool primary) return -EINVAL; } - /* TODO: call osi_free() when deinit function is invoked*/ - internal = osi_calloc(sizeof(time_scene_internal_data_t)); - if (!internal) { - BT_ERR("%s, Failed to allocate memory", __func__); - return -ENOMEM; + if (!client->internal_data) { + internal = osi_calloc(sizeof(time_scene_internal_data_t)); + if (!internal) { + BT_ERR("%s, Failed to allocate memory", __func__); + return -ENOMEM; + } + + sys_slist_init(&internal->queue); + + client->model = model; + client->op_pair_size = ARRAY_SIZE(time_scene_op_pair); + client->op_pair = time_scene_op_pair; + client->internal_data = internal; + } else { + bt_mesh_client_clear_list(client->internal_data); } - sys_slist_init(&internal->queue); - - client->model = model; - client->op_pair_size = ARRAY_SIZE(time_scene_op_pair); - client->op_pair = time_scene_op_pair; - client->internal_data = internal; - bt_mesh_time_scene_client_mutex_new(); return 0; diff --git a/components/bt/esp_ble_mesh/mesh_models/server/server_common.c b/components/bt/esp_ble_mesh/mesh_models/server/server_common.c index 5723c640e..041cc3b00 100644 --- a/components/bt/esp_ble_mesh/mesh_models/server/server_common.c +++ b/components/bt/esp_ble_mesh/mesh_models/server/server_common.c @@ -193,8 +193,10 @@ void bt_mesh_server_alloc_ctx(struct k_work *work) * Here we use the allocated heap memory to store the "struct bt_mesh_msg_ctx". */ __ASSERT(work, "%s, Invalid parameter", __func__); - work->_reserved = osi_calloc(sizeof(struct bt_mesh_msg_ctx)); - __ASSERT(work->_reserved, "%s, Failed to allocate memory", __func__); + if (!work->_reserved) { + work->_reserved = osi_calloc(sizeof(struct bt_mesh_msg_ctx)); + __ASSERT(work->_reserved, "%s, Failed to allocate memory", __func__); + } } bool bt_mesh_is_server_recv_last_msg(struct bt_mesh_last_msg_info *last,