From 04fc15a4f36d5c90b92bfc012862ddceb23b3bcb 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 c3e73c23fe..d1c04f6043 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 0d5d5c8138..ee32e9ddd7 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 5f9374b340..c309519ae1 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 48b540b415..8352d1bf42 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 74eb366ca8..ff4e526095 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 78b80d7b05..87d2922c13 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 7b93aecfb1..d8eb7bd2d7 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 878ef3da3a..b7456d2a44 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 5723c640ec..041cc3b002 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,