michaelh/zephyr
1
0
Fork 0
Code Releases Activity

Bluetooth: Mesh: cfg_cli: Fix possible race condition

Browse source 
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).

Signed-off-by: Johan Hedberg <johan.hedberg@intel.com>
This commit is contained in:
Johan Hedberg 2018-03-19 16:45:09 +02:00 • committed by Johan Hedberg
commit 0cee4ed747
2 changed files with 112 additions and 64 deletions
Download patch file Download diff file Expand all files Collapse all files

114
subsys/bluetooth/host/mesh/cfg_cli.c
View file

@ -479,7 +479,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!");
@ -491,20 +491,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;
}
@ -525,7 +530,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) {
return err;
}
@ -536,10 +541,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();
return err;
}
return cli_wait(&param, OP_DEV_COMP_DATA_STATUS);
return cli_wait();
}
static int get_state_u8(u16_t net_idx, u16_t addr, u32_t op, u32_t rsp,
@ -554,7 +560,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) {
return err;
}
@ -564,10 +570,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();
return err;
}
return cli_wait(val, rsp);
return cli_wait();
}
static int set_state_u8(u16_t net_idx, u16_t addr, u32_t op, u32_t rsp,
@ -582,7 +589,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) {
return err;
}
@ -593,10 +600,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();
return err;
}
return cli_wait(val, rsp);
return cli_wait();
}
int bt_mesh_cfg_beacon_get(u16_t net_idx, u16_t addr, u8_t *status)
@ -664,7 +672,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) {
return err;
}
@ -674,10 +682,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();
return err;
}
return cli_wait(&param, OP_RELAY_STATUS);
return cli_wait();
}
int bt_mesh_cfg_relay_set(u16_t net_idx, u16_t addr, u8_t new_relay,
@ -696,7 +705,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) {
return err;
}
@ -708,10 +717,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();
return err;
}
return cli_wait(&param, OP_RELAY_STATUS);
return cli_wait();
}
int bt_mesh_cfg_net_key_add(u16_t net_idx, u16_t addr, u16_t key_net_idx,
@ -730,7 +740,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) {
return err;
}
@ -742,14 +752,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_NET_KEY_STATUS);
return cli_wait();
}
int bt_mesh_cfg_app_key_add(u16_t net_idx, u16_t addr, u16_t key_net_idx,
@ -770,7 +782,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) {
return err;
}
@ -782,14 +794,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_APP_KEY_STATUS);
return cli_wait();
}
static int mod_app_bind(u16_t net_idx, u16_t addr, u16_t elem_addr,
@ -812,7 +826,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) {
return err;
}
@ -830,14 +844,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_MOD_APP_STATUS);
return cli_wait();
}
int bt_mesh_cfg_mod_app_bind(u16_t net_idx, u16_t addr, u16_t elem_addr,
@ -878,7 +894,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) {
return err;
}
@ -896,14 +912,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_MOD_SUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_mod_sub_add(u16_t net_idx, u16_t addr, u16_t elem_addr,
@ -983,7 +1001,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) {
return err;
}
@ -1005,14 +1023,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_MOD_SUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_mod_sub_va_add(u16_t net_idx, u16_t addr, u16_t elem_addr,
@ -1097,7 +1117,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) {
return err;
}
@ -1115,14 +1135,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_MOD_PUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_mod_pub_get(u16_t net_idx, u16_t addr, u16_t elem_addr,
@ -1164,7 +1186,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) {
return err;
}
@ -1187,14 +1209,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_MOD_PUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_mod_pub_set(u16_t net_idx, u16_t addr, u16_t elem_addr,
@ -1232,7 +1256,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) {
return err;
}
@ -1245,14 +1269,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEARTBEAT_SUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_hb_sub_get(u16_t net_idx, u16_t addr,
@ -1271,7 +1297,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) {
return err;
}
@ -1281,14 +1307,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEARTBEAT_SUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_hb_pub_set(u16_t net_idx, u16_t addr,
@ -1306,7 +1334,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) {
return err;
}
@ -1322,14 +1350,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEARTBEAT_PUB_STATUS);
return cli_wait();
}
int bt_mesh_cfg_hb_pub_get(u16_t net_idx, u16_t addr,
@ -1348,7 +1378,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) {
return err;
}
@ -1358,14 +1388,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();
return err;
}
if (!status) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEARTBEAT_PUB_STATUS);
return cli_wait();
}
s32_t bt_mesh_cfg_cli_timeout_get(void)

62
subsys/bluetooth/host/mesh/health_cli.c
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) {
return err;
}
@ -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();
return err;
}
return cli_wait(&param, OP_ATTENTION_STATUS);
return cli_wait();
}
int bt_mesh_health_attention_set(u16_t net_idx, u16_t addr, u16_t app_idx,
@ -243,7 +249,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) {
return err;
}
@ -259,14 +265,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();
return err;
}
if (!updated_attention) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_ATTENTION_STATUS);
return cli_wait();
}
int bt_mesh_health_period_get(u16_t net_idx, u16_t addr, u16_t app_idx,
@ -284,7 +292,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) {
return err;
}
@ -294,10 +302,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();
return err;
}
return cli_wait(&param, OP_HEALTH_PERIOD_STATUS);
return cli_wait();
}
int bt_mesh_health_period_set(u16_t net_idx, u16_t addr, u16_t app_idx,
@ -315,7 +324,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) {
return err;
}
@ -331,14 +340,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();
return err;
}
if (!updated_divisor) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEALTH_PERIOD_STATUS);
return cli_wait();
}
int bt_mesh_health_fault_test(u16_t net_idx, u16_t addr, u16_t app_idx,
@ -360,7 +371,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) {
return err;
}
@ -377,14 +388,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();
return err;
}
if (!faults) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEALTH_FAULT_STATUS);
return cli_wait();
}
int bt_mesh_health_fault_clear(u16_t net_idx, u16_t addr, u16_t app_idx,
@ -406,7 +419,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) {
return err;
}
@ -422,14 +435,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();
return err;
}
if (!test_id) {
cli_reset();
return 0;
}
return cli_wait(&param, OP_HEALTH_FAULT_STATUS);
return cli_wait();
}
int bt_mesh_health_fault_get(u16_t net_idx, u16_t addr, u16_t app_idx,
@ -451,7 +466,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) {
return err;
}
@ -462,10 +477,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();
return err;
}
return cli_wait(&param, OP_HEALTH_FAULT_STATUS);
return cli_wait();
}
s32_t bt_mesh_health_cli_timeout_get(void)