mesh: cfg_cli: Fix possible race condition

If the thread that sends the configuration messages has low priority
and is sending to the local node (a common use case currently) it's
possible that the response arrives before the cli->op_* state
variables are set, resulting in the message never getting properly
processed and the client API call timing out.

Split the initialization into a separete cli_prepare() call and add a
cli_reset() to clean up the variables in case of premature completion
of the client operation (e.g. due to message sending failure).
This commit is contained in:
Michał Narajowski
2018-04-10 12:32:26 +02:00
parent f9e291cc87
commit 0f7a7ccffc
2 changed files with 112 additions and 64 deletions
+73 -41
View File
@@ -474,7 +474,7 @@ const struct bt_mesh_model_op bt_mesh_cfg_cli_op[] = {
BT_MESH_MODEL_OP_END,
};
static int check_cli(void)
static int cli_prepare(void *param, u32_t op)
{
if (!cli) {
BT_ERR("No available Configuration Client context!");
@@ -486,20 +486,25 @@ static int check_cli(void)
return -EBUSY;
}
return 0;
}
static int cli_wait(void *param, u32_t op)
{
int err;
cli->op_param = param;
cli->op_pending = op;
err = k_sem_take(&cli->op_sync, msg_timeout);
return 0;
}
static void cli_reset(void)
{
cli->op_pending = 0;
cli->op_param = NULL;
}
static int cli_wait(void)
{
int err;
err = k_sem_take(&cli->op_sync, msg_timeout);
cli_reset();
return err;
}
@@ -520,7 +525,7 @@ int bt_mesh_cfg_comp_data_get(u16_t net_idx, u16_t addr, u8_t page,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_DEV_COMP_DATA_STATUS);
if (err) {
goto done;
}
@@ -531,10 +536,11 @@ int bt_mesh_cfg_comp_data_get(u16_t net_idx, u16_t addr, u8_t page,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(&param, OP_DEV_COMP_DATA_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -552,7 +558,7 @@ static int get_state_u8(u16_t net_idx, u16_t addr, u32_t op, u32_t rsp,
};
int err;
err = check_cli();
err = cli_prepare(val, rsp);
if (err) {
goto done;
}
@@ -562,10 +568,11 @@ static int get_state_u8(u16_t net_idx, u16_t addr, u32_t op, u32_t rsp,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(val, rsp);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -583,7 +590,7 @@ static int set_state_u8(u16_t net_idx, u16_t addr, u32_t op, u32_t rsp,
};
int err;
err = check_cli();
err = cli_prepare(val, rsp);
if (err) {
goto done;
}
@@ -594,10 +601,11 @@ static int set_state_u8(u16_t net_idx, u16_t addr, u32_t op, u32_t rsp,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(val, rsp);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -668,7 +676,7 @@ int bt_mesh_cfg_relay_get(u16_t net_idx, u16_t addr, u8_t *status,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_RELAY_STATUS);
if (err) {
goto done;
}
@@ -678,10 +686,11 @@ int bt_mesh_cfg_relay_get(u16_t net_idx, u16_t addr, u8_t *status,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(&param, OP_RELAY_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -703,7 +712,7 @@ int bt_mesh_cfg_relay_set(u16_t net_idx, u16_t addr, u8_t new_relay,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_RELAY_STATUS);
if (err) {
goto done;
}
@@ -715,10 +724,11 @@ int bt_mesh_cfg_relay_set(u16_t net_idx, u16_t addr, u8_t new_relay,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(&param, OP_RELAY_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -740,7 +750,7 @@ int bt_mesh_cfg_net_key_add(u16_t net_idx, u16_t addr, u16_t key_net_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_NET_KEY_STATUS);
if (err) {
goto done;
}
@@ -752,14 +762,16 @@ int bt_mesh_cfg_net_key_add(u16_t net_idx, u16_t addr, u16_t key_net_idx,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_NET_KEY_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -783,7 +795,7 @@ int bt_mesh_cfg_app_key_add(u16_t net_idx, u16_t addr, u16_t key_net_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_APP_KEY_STATUS);
if (err) {
goto done;
}
@@ -795,14 +807,16 @@ int bt_mesh_cfg_app_key_add(u16_t net_idx, u16_t addr, u16_t key_net_idx,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_APP_KEY_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -828,7 +842,7 @@ static int mod_app_bind(u16_t net_idx, u16_t addr, u16_t elem_addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_MOD_APP_STATUS);
if (err) {
goto done;
}
@@ -846,14 +860,16 @@ static int mod_app_bind(u16_t net_idx, u16_t addr, u16_t elem_addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_MOD_APP_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -897,7 +913,7 @@ static int mod_sub(u32_t op, u16_t net_idx, u16_t addr, u16_t elem_addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_MOD_SUB_STATUS);
if (err) {
goto done;
}
@@ -915,14 +931,16 @@ static int mod_sub(u32_t op, u16_t net_idx, u16_t addr, u16_t elem_addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_MOD_SUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1005,7 +1023,7 @@ static int mod_sub_va(u32_t op, u16_t net_idx, u16_t addr, u16_t elem_addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_MOD_SUB_STATUS);
if (err) {
goto done;
}
@@ -1027,14 +1045,16 @@ static int mod_sub_va(u32_t op, u16_t net_idx, u16_t addr, u16_t elem_addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_MOD_SUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1122,7 +1142,7 @@ static int mod_pub_get(u16_t net_idx, u16_t addr, u16_t elem_addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_MOD_PUB_STATUS);
if (err) {
goto done;
}
@@ -1140,14 +1160,16 @@ static int mod_pub_get(u16_t net_idx, u16_t addr, u16_t elem_addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_MOD_PUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1192,7 +1214,7 @@ static int mod_pub_set(u16_t net_idx, u16_t addr, u16_t elem_addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_MOD_PUB_STATUS);
if (err) {
goto done;
}
@@ -1215,14 +1237,16 @@ static int mod_pub_set(u16_t net_idx, u16_t addr, u16_t elem_addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_MOD_PUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1263,7 +1287,7 @@ int bt_mesh_cfg_hb_sub_set(u16_t net_idx, u16_t addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEARTBEAT_SUB_STATUS);
if (err) {
goto done;
}
@@ -1276,14 +1300,16 @@ int bt_mesh_cfg_hb_sub_set(u16_t net_idx, u16_t addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEARTBEAT_SUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1305,7 +1331,7 @@ int bt_mesh_cfg_hb_sub_get(u16_t net_idx, u16_t addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEARTBEAT_SUB_STATUS);
if (err) {
goto done;
}
@@ -1315,14 +1341,16 @@ int bt_mesh_cfg_hb_sub_get(u16_t net_idx, u16_t addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEARTBEAT_SUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1343,7 +1371,7 @@ int bt_mesh_cfg_hb_pub_set(u16_t net_idx, u16_t addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEARTBEAT_PUB_STATUS);
if (err) {
goto done;
}
@@ -1359,14 +1387,16 @@ int bt_mesh_cfg_hb_pub_set(u16_t net_idx, u16_t addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEARTBEAT_PUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -1388,7 +1418,7 @@ int bt_mesh_cfg_hb_pub_get(u16_t net_idx, u16_t addr,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEARTBEAT_PUB_STATUS);
if (err) {
goto done;
}
@@ -1398,14 +1428,16 @@ int bt_mesh_cfg_hb_pub_get(u16_t net_idx, u16_t addr,
err = bt_mesh_model_send(cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!status) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEARTBEAT_PUB_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
+39 -23
View File
@@ -167,7 +167,7 @@ const struct bt_mesh_model_op bt_mesh_health_cli_op[] = {
BT_MESH_MODEL_OP_END,
};
static int check_cli(void)
static int cli_prepare(void *param, u32_t op)
{
if (!health_cli) {
BT_ERR("No available Health Client context!");
@@ -179,20 +179,25 @@ static int check_cli(void)
return -EBUSY;
}
return 0;
}
static int cli_wait(void *param, u32_t op)
{
int err;
health_cli->op_param = param;
health_cli->op_pending = op;
err = k_sem_take(&health_cli->op_sync, msg_timeout);
return 0;
}
static void cli_reset(void)
{
health_cli->op_pending = 0;
health_cli->op_param = NULL;
}
static int cli_wait(void)
{
int err;
err = k_sem_take(&health_cli->op_sync, msg_timeout);
cli_reset();
return err;
}
@@ -212,7 +217,7 @@ int bt_mesh_health_attention_get(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_ATTENTION_STATUS);
if (err) {
goto done;
}
@@ -222,10 +227,11 @@ int bt_mesh_health_attention_get(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(&param, OP_ATTENTION_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -246,7 +252,7 @@ int bt_mesh_health_attention_set(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_ATTENTION_STATUS);
if (err) {
goto done;
}
@@ -262,14 +268,16 @@ int bt_mesh_health_attention_set(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!updated_attention) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_ATTENTION_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -290,7 +298,7 @@ int bt_mesh_health_period_get(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEALTH_PERIOD_STATUS);
if (err) {
goto done;
}
@@ -300,10 +308,11 @@ int bt_mesh_health_period_get(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEALTH_PERIOD_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -324,7 +333,7 @@ int bt_mesh_health_period_set(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEALTH_PERIOD_STATUS);
if (err) {
goto done;
}
@@ -340,14 +349,16 @@ int bt_mesh_health_period_set(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!updated_divisor) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEALTH_PERIOD_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -372,7 +383,7 @@ int bt_mesh_health_fault_test(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEALTH_FAULT_STATUS);
if (err) {
goto done;
}
@@ -389,14 +400,16 @@ int bt_mesh_health_fault_test(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!faults) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEALTH_FAULT_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -421,7 +434,7 @@ int bt_mesh_health_fault_clear(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEALTH_FAULT_STATUS);
if (err) {
goto done;
}
@@ -437,14 +450,16 @@ int bt_mesh_health_fault_clear(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
if (!test_id) {
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEALTH_FAULT_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;
@@ -469,7 +484,7 @@ int bt_mesh_health_fault_get(u16_t net_idx, u16_t addr, u16_t app_idx,
};
int err;
err = check_cli();
err = cli_prepare(&param, OP_HEALTH_FAULT_STATUS);
if (err) {
goto done;
}
@@ -480,10 +495,11 @@ int bt_mesh_health_fault_get(u16_t net_idx, u16_t addr, u16_t app_idx,
err = bt_mesh_model_send(health_cli->model, &ctx, msg, NULL, NULL);
if (err) {
BT_ERR("model_send() failed (err %d)", err);
cli_reset();
goto done;
}
err = cli_wait(&param, OP_HEALTH_FAULT_STATUS);
err = cli_wait();
done:
os_mbuf_free_chain(msg);
return err;