From 8c7ffac4a6964e8ffd67fdd92093b8f3cd27b0e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Anders=20Storr=C3=B8?= Date: Wed, 5 Jul 2023 14:52:55 +0200 Subject: [PATCH] Bluetooth: Mesh: Fix err in mod_app_list packing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes erroneous packing/unpacking of model app list messages in the configuration client and server. According to the mesh 1.1 protcol spec (4.3.1.1) two app indexes shall be packed in a 3 octet interleaved format. The current implementation packs them in 4 octets. This commit also provide a helper function for unpacking key indexes as public API to facilitate future config model callback API. Signed-off-by: Anders Storrø --- include/zephyr/bluetooth/mesh/cfg_cli.h | 13 +++ subsys/bluetooth/mesh/cfg_cli.c | 133 +++++++++++++++--------- subsys/bluetooth/mesh/cfg_srv.c | 56 +++++----- subsys/bluetooth/mesh/foundation.h | 6 +- 4 files changed, 129 insertions(+), 79 deletions(-) diff --git a/include/zephyr/bluetooth/mesh/cfg_cli.h b/include/zephyr/bluetooth/mesh/cfg_cli.h index 6a827be3805..ea48c3b30d5 100644 --- a/include/zephyr/bluetooth/mesh/cfg_cli.h +++ b/include/zephyr/bluetooth/mesh/cfg_cli.h @@ -1644,6 +1644,19 @@ struct bt_mesh_comp_p1_model_item *bt_mesh_comp_p1_item_pull( struct bt_mesh_comp_p1_ext_item *bt_mesh_comp_p1_pull_ext_item( struct bt_mesh_comp_p1_model_item *item, struct bt_mesh_comp_p1_ext_item *ext_item); +/** @brief Unpack a list of key index entries from a buffer. + * + * On success, @c dst_cnt is set to the amount of unpacked key index entries. + * + * @param buf Message buffer containing encoded AppKey or NetKey Indexes. + * @param dst_arr Destination array for the unpacked list. + * @param dst_cnt Size of the destination array. + * + * @return 0 on success. + * @return -EMSGSIZE if dst_arr size is to small to parse full message. + */ +int bt_mesh_key_idx_unpack_list(struct net_buf_simple *buf, uint16_t *dst_arr, size_t *dst_cnt); + /** @cond INTERNAL_HIDDEN */ extern const struct bt_mesh_model_op bt_mesh_cfg_cli_op[]; extern const struct bt_mesh_model_cb bt_mesh_cfg_cli_cb; diff --git a/subsys/bluetooth/mesh/cfg_cli.c b/subsys/bluetooth/mesh/cfg_cli.c index 200d6d93538..167d1c64264 100644 --- a/subsys/bluetooth/mesh/cfg_cli.c +++ b/subsys/bluetooth/mesh/cfg_cli.c @@ -308,38 +308,48 @@ struct net_key_list_param { size_t *key_cnt; }; +int bt_mesh_key_idx_unpack_list(struct net_buf_simple *buf, uint16_t *dst_arr, + size_t *dst_cnt) +{ + size_t i; + + if (!dst_cnt) { + return 0; + } + + for (i = 0; (i + 1) < *dst_cnt && buf->len >= 3; i += 2) { + key_idx_unpack_pair(buf, &dst_arr[i], &dst_arr[i + 1]); + } + + if (i < *dst_cnt && buf->len >= 2) { + dst_arr[i++] = net_buf_simple_pull_le16(buf) & 0xfff; + } + + *dst_cnt = i; + + return buf->len > 0 ? -EMSGSIZE : 0; +} + static int net_key_list(struct bt_mesh_model *model, struct bt_mesh_msg_ctx *ctx, struct net_buf_simple *buf) { struct net_key_list_param *param; - int i; LOG_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s", ctx->net_idx, ctx->app_idx, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); - if (bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_NET_KEY_LIST, ctx->addr, - (void **)¶m)) { + if (bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_NET_KEY_LIST, ctx->addr, (void **)¶m)) { if (param->keys && param->key_cnt) { - for (i = 0; i < *param->key_cnt && buf->len >= 3; i += 2) { - key_idx_unpack(buf, ¶m->keys[i], - ¶m->keys[i + 1]); - } + int err = bt_mesh_key_idx_unpack_list(buf, param->keys, param->key_cnt); - if (i < *param->key_cnt && buf->len >= 2) { - param->keys[i++] = - net_buf_simple_pull_le16(buf) & 0xfff; - } - - if (buf->len > 0) { + if (err) { LOG_ERR("The message size for the application opcode is " "incorrect."); - return -EMSGSIZE; + return err; } - - *param->key_cnt = i; } bt_mesh_msg_ack_ctx_rx(&cli->ack_ctx); @@ -392,7 +402,7 @@ static int app_key_status(struct bt_mesh_model *model, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); status = net_buf_simple_pull_u8(buf); - key_idx_unpack(buf, &net_idx, &app_idx); + key_idx_unpack_pair(buf, &net_idx, &app_idx); if (bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, OP_APP_KEY_STATUS, ctx->addr, (void **)¶m)) { @@ -435,7 +445,6 @@ static int app_key_list(struct bt_mesh_model *model, struct app_key_list_param *param; uint16_t net_idx; uint8_t status; - int i; LOG_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s", ctx->net_idx, ctx->app_idx, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); @@ -453,22 +462,13 @@ static int app_key_list(struct bt_mesh_model *model, if (param->keys && param->key_cnt) { - for (i = 0; i < *param->key_cnt && buf->len >= 3; i += 2) { - key_idx_unpack(buf, ¶m->keys[i], - ¶m->keys[i + 1]); - } + int err = bt_mesh_key_idx_unpack_list(buf, param->keys, param->key_cnt); - if (i < *param->key_cnt && buf->len == 2) { - param->keys[i++] = net_buf_simple_pull_le16(buf) & 0xfff; - } - - if (buf->len > 0U) { + if (err) { LOG_ERR("The message size for the application opcode is " "incorrect."); - return -EMSGSIZE; + return err; } - - *param->key_cnt = i; } if (param->status) { @@ -555,20 +555,14 @@ struct mod_member_list_param { size_t *member_cnt; }; -static int mod_member_list_handle(struct bt_mesh_msg_ctx *ctx, - struct net_buf_simple *buf, uint16_t op, - bool vnd) +static int mod_sub_list_handle(struct bt_mesh_msg_ctx *ctx, struct net_buf_simple *buf, uint16_t op, + bool vnd) { struct mod_member_list_param *param; uint16_t elem_addr, mod_id, cid; uint8_t status; int i; - if ((vnd && buf->len < 7U) || (buf->len < 5U)) { - LOG_ERR("The message size for the application opcode is incorrect."); - return -EMSGSIZE; - } - status = net_buf_simple_pull_u8(buf); elem_addr = net_buf_simple_pull_le16(buf); if (vnd) { @@ -577,11 +571,10 @@ static int mod_member_list_handle(struct bt_mesh_msg_ctx *ctx, mod_id = net_buf_simple_pull_le16(buf); - if (bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, op, ctx->addr, - (void **)¶m)) { + if (bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, op, ctx->addr, (void **)¶m)) { if (param->elem_addr != elem_addr || param->mod_id != mod_id || - (vnd && param->cid != cid)) { + (vnd && param->cid != cid)) { LOG_WRN("Model Member List parameters did not match"); return -ENOENT; } @@ -609,13 +602,57 @@ static int mod_member_list_handle(struct bt_mesh_msg_ctx *ctx, return 0; } +static int mod_app_list_handle(struct bt_mesh_msg_ctx *ctx, struct net_buf_simple *buf, uint16_t op, + bool vnd) +{ + struct mod_member_list_param *param; + uint16_t elem_addr, mod_id, cid; + uint8_t status; + + status = net_buf_simple_pull_u8(buf); + elem_addr = net_buf_simple_pull_le16(buf); + if (vnd) { + cid = net_buf_simple_pull_le16(buf); + } + + mod_id = net_buf_simple_pull_le16(buf); + + if (bt_mesh_msg_ack_ctx_match(&cli->ack_ctx, op, ctx->addr, (void **)¶m)) { + + if (param->elem_addr != elem_addr || param->mod_id != mod_id || + (vnd && param->cid != cid)) { + LOG_WRN("Model Member List parameters did not match"); + return -ENOENT; + } + + if (param->member_cnt && param->members) { + + int err = + bt_mesh_key_idx_unpack_list(buf, param->members, param->member_cnt); + + if (err) { + LOG_ERR("The message size for the application opcode is " + "incorrect."); + return err; + } + } + + if (param->status) { + *param->status = status; + } + + bt_mesh_msg_ack_ctx_rx(&cli->ack_ctx); + } + return 0; +} + static int mod_app_list(struct bt_mesh_model *model, struct bt_mesh_msg_ctx *ctx, struct net_buf_simple *buf) { LOG_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s", ctx->net_idx, ctx->app_idx, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); - return mod_member_list_handle(ctx, buf, OP_SIG_MOD_APP_LIST, false); + return mod_app_list_handle(ctx, buf, OP_SIG_MOD_APP_LIST, false); } static int mod_app_list_vnd(struct bt_mesh_model *model, @@ -625,7 +662,7 @@ static int mod_app_list_vnd(struct bt_mesh_model *model, LOG_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s", ctx->net_idx, ctx->app_idx, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); - return mod_member_list_handle(ctx, buf, OP_VND_MOD_APP_LIST, true); + return mod_app_list_handle(ctx, buf, OP_VND_MOD_APP_LIST, true); } struct mod_pub_param { @@ -777,7 +814,7 @@ static int mod_sub_list(struct bt_mesh_model *model, LOG_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s", ctx->net_idx, ctx->app_idx, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); - return mod_member_list_handle(ctx, buf, OP_MOD_SUB_LIST, false); + return mod_sub_list_handle(ctx, buf, OP_MOD_SUB_LIST, false); } static int mod_sub_list_vnd(struct bt_mesh_model *model, @@ -787,7 +824,7 @@ static int mod_sub_list_vnd(struct bt_mesh_model *model, LOG_DBG("net_idx 0x%04x app_idx 0x%04x src 0x%04x len %u: %s", ctx->net_idx, ctx->app_idx, ctx->addr, buf->len, bt_hex(buf->data, buf->len)); - return mod_member_list_handle(ctx, buf, OP_MOD_SUB_LIST_VND, true); + return mod_sub_list_handle(ctx, buf, OP_MOD_SUB_LIST_VND, true); } struct hb_sub_param { @@ -1315,7 +1352,7 @@ int bt_mesh_cfg_cli_app_key_add(uint16_t net_idx, uint16_t addr, uint16_t key_ne }; bt_mesh_model_msg_init(&msg, OP_APP_KEY_ADD); - key_idx_pack(&msg, key_net_idx, key_app_idx); + key_idx_pack_pair(&msg, key_net_idx, key_app_idx); net_buf_simple_add_mem(&msg, app_key, 16); return bt_mesh_msg_ackd_send(cli->model, &ctx, &msg, !status ? NULL : &rsp); @@ -1339,7 +1376,7 @@ int bt_mesh_cfg_cli_app_key_update(uint16_t net_idx, uint16_t addr, uint16_t key }; bt_mesh_model_msg_init(&msg, OP_APP_KEY_UPDATE); - key_idx_pack(&msg, key_net_idx, key_app_idx); + key_idx_pack_pair(&msg, key_net_idx, key_app_idx); net_buf_simple_add_mem(&msg, app_key, 16); return bt_mesh_msg_ackd_send(cli->model, &ctx, &msg, !status ? NULL : &rsp); @@ -1409,7 +1446,7 @@ int bt_mesh_cfg_cli_app_key_del(uint16_t net_idx, uint16_t addr, uint16_t key_ne }; bt_mesh_model_msg_init(&msg, OP_APP_KEY_DEL); - key_idx_pack(&msg, key_net_idx, key_app_idx); + key_idx_pack_pair(&msg, key_net_idx, key_app_idx); return bt_mesh_msg_ackd_send(cli->model, &ctx, &msg, !status ? NULL : &rsp); } diff --git a/subsys/bluetooth/mesh/cfg_srv.c b/subsys/bluetooth/mesh/cfg_srv.c index 512f2eb57f9..09eee4af3a0 100644 --- a/subsys/bluetooth/mesh/cfg_srv.c +++ b/subsys/bluetooth/mesh/cfg_srv.c @@ -280,6 +280,28 @@ static uint8_t mod_unbind(struct bt_mesh_model *model, uint16_t key_idx, bool st return STATUS_SUCCESS; } +static void key_idx_pack_list(struct net_buf_simple *buf, uint16_t *arr, size_t cnt) +{ + uint16_t *idx = NULL; + + for (int i = 0; i < cnt; i++) { + if (arr[i] != BT_MESH_KEY_UNUSED) { + if (!idx) { + idx = &arr[i]; + continue; + } + + key_idx_pack_pair(buf, *idx, arr[i]); + idx = NULL; + } + } + + if (idx) { + net_buf_simple_add_le16(buf, *idx); + } + +} + static int send_app_key_status(struct bt_mesh_model *model, struct bt_mesh_msg_ctx *ctx, uint8_t status, @@ -289,7 +311,7 @@ static int send_app_key_status(struct bt_mesh_model *model, bt_mesh_model_msg_init(&msg, OP_APP_KEY_STATUS); net_buf_simple_add_u8(&msg, status); - key_idx_pack(&msg, net_idx, app_idx); + key_idx_pack_pair(&msg, net_idx, app_idx); if (bt_mesh_model_send(model, ctx, &msg, NULL, NULL)) { LOG_ERR("Unable to send App Key Status response"); @@ -305,7 +327,7 @@ static int app_key_add(struct bt_mesh_model *model, uint16_t key_net_idx, key_app_idx; uint8_t status; - key_idx_unpack(buf, &key_net_idx, &key_app_idx); + key_idx_unpack_pair(buf, &key_net_idx, &key_app_idx); LOG_DBG("AppIdx 0x%04x NetIdx 0x%04x", key_app_idx, key_net_idx); @@ -321,7 +343,7 @@ static int app_key_update(struct bt_mesh_model *model, uint16_t key_net_idx, key_app_idx; uint8_t status; - key_idx_unpack(buf, &key_net_idx, &key_app_idx); + key_idx_unpack_pair(buf, &key_net_idx, &key_app_idx); LOG_DBG("AppIdx 0x%04x NetIdx 0x%04x", key_app_idx, key_net_idx); @@ -357,7 +379,7 @@ static int app_key_del(struct bt_mesh_model *model, uint16_t key_net_idx, key_app_idx; uint8_t status; - key_idx_unpack(buf, &key_net_idx, &key_app_idx); + key_idx_unpack_pair(buf, &key_net_idx, &key_app_idx); LOG_DBG("AppIdx 0x%04x NetIdx 0x%04x", key_app_idx, key_net_idx); @@ -379,7 +401,6 @@ static int app_key_get(struct bt_mesh_model *model, uint16_t get_idx; uint8_t status; ssize_t count; - int i; get_idx = net_buf_simple_pull_le16(buf); if (get_idx > 0xfff) { @@ -409,13 +430,7 @@ static int app_key_get(struct bt_mesh_model *model, count = ARRAY_SIZE(app_idx); } - for (i = 0; i < count - 1; i += 2) { - key_idx_pack(&msg, app_idx[i], app_idx[i + 1]); - } - - if (i < count) { - net_buf_simple_add_le16(&msg, app_idx[i]); - } + key_idx_pack_list(&msg, app_idx, count); send_status: if (bt_mesh_model_send(model, ctx, &msg, NULL, NULL)) { @@ -1731,7 +1746,6 @@ static int net_key_get(struct bt_mesh_model *model, IDX_LEN(CONFIG_BT_MESH_SUBNET_COUNT)); uint16_t net_idx[CONFIG_BT_MESH_SUBNET_COUNT]; ssize_t count; - int i; bt_mesh_model_msg_init(&msg, OP_NET_KEY_LIST); @@ -1740,13 +1754,7 @@ static int net_key_get(struct bt_mesh_model *model, count = ARRAY_SIZE(net_idx); } - for (i = 0; i < count - 1; i += 2) { - key_idx_pack(&msg, net_idx[i], net_idx[i + 1]); - } - - if (i < count) { - net_buf_simple_add_le16(&msg, net_idx[i]); - } + key_idx_pack_list(&msg, net_idx, count); if (bt_mesh_model_send(model, ctx, &msg, NULL, NULL)) { LOG_ERR("Unable to send NetKey List"); @@ -2037,13 +2045,7 @@ send_list: } if (mod) { - int i; - - for (i = 0; i < mod->keys_cnt; i++) { - if (mod->keys[i] != BT_MESH_KEY_UNUSED) { - net_buf_simple_add_le16(&msg, mod->keys[i]); - } - } + key_idx_pack_list(&msg, mod->keys, mod->keys_cnt); } if (bt_mesh_model_send(model, ctx, &msg, NULL, NULL)) { diff --git a/subsys/bluetooth/mesh/foundation.h b/subsys/bluetooth/mesh/foundation.h index 164ff0cb5aa..01cbf93c39e 100644 --- a/subsys/bluetooth/mesh/foundation.h +++ b/subsys/bluetooth/mesh/foundation.h @@ -154,15 +154,13 @@ void bt_mesh_attention(struct bt_mesh_model *model, uint8_t time); #include -static inline void key_idx_pack(struct net_buf_simple *buf, - uint16_t idx1, uint16_t idx2) +static inline void key_idx_pack_pair(struct net_buf_simple *buf, uint16_t idx1, uint16_t idx2) { net_buf_simple_add_le16(buf, idx1 | ((idx2 & 0x00f) << 12)); net_buf_simple_add_u8(buf, idx2 >> 4); } -static inline void key_idx_unpack(struct net_buf_simple *buf, - uint16_t *idx1, uint16_t *idx2) +static inline void key_idx_unpack_pair(struct net_buf_simple *buf, uint16_t *idx1, uint16_t *idx2) { *idx1 = sys_get_le16(&buf->data[0]) & 0xfff; *idx2 = sys_get_le16(&buf->data[1]) >> 4;