From 5d847917556c6de8998e91132585b31847c3964a Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Tue, 19 Jun 2018 15:38:59 +0200 Subject: [PATCH 1/7] nimble/ll: Handle negative LTK key request reply separately This make separate handler for each command as well as filling reponse length from handler. Make it consistent with other code. --- nimble/controller/src/ble_ll_conn_hci.c | 73 ++++++++++++++++++++---- nimble/controller/src/ble_ll_conn_priv.h | 5 +- nimble/controller/src/ble_ll_hci.c | 5 +- 3 files changed, 68 insertions(+), 15 deletions(-) diff --git a/nimble/controller/src/ble_ll_conn_hci.c b/nimble/controller/src/ble_ll_conn_hci.c index 570cf1a62..09d9b66d0 100644 --- a/nimble/controller/src/ble_ll_conn_hci.c +++ b/nimble/controller/src/ble_ll_conn_hci.c @@ -1314,7 +1314,7 @@ ble_ll_conn_hci_le_start_encrypt(uint8_t *cmdbuf) * @return int */ int -ble_ll_conn_hci_le_ltk_reply(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t ocf) +ble_ll_conn_hci_le_ltk_reply(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t *rsplen) { int rc; uint16_t handle; @@ -1335,22 +1335,71 @@ ble_ll_conn_hci_le_ltk_reply(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t ocf) } /* The connection should be awaiting a reply. If not, just discard */ - if (connsm->enc_data.enc_state == CONN_ENC_S_LTK_REQ_WAIT) { - if (ocf == BLE_HCI_OCF_LE_LT_KEY_REQ_REPLY) { - swap_buf(connsm->enc_data.enc_block.key, cmdbuf + 2, 16); - ble_ll_calc_session_key(connsm); - ble_ll_ctrl_start_enc_send(connsm, BLE_LL_CTRL_START_ENC_REQ); - } else { - /* We received a negative reply! Send REJECT_IND */ - ble_ll_ctrl_reject_ind_send(connsm, BLE_LL_CTRL_ENC_REQ, - BLE_ERR_PINKEY_MISSING); - connsm->enc_data.enc_state = CONN_ENC_S_LTK_NEG_REPLY; - } + if (connsm->enc_data.enc_state != CONN_ENC_S_LTK_REQ_WAIT) { + rc = BLE_ERR_CMD_DISALLOWED; + goto ltk_key_cmd_complete; } + + swap_buf(connsm->enc_data.enc_block.key, cmdbuf + 2, 16); + ble_ll_calc_session_key(connsm); + ble_ll_ctrl_start_enc_send(connsm, BLE_LL_CTRL_START_ENC_REQ); rc = BLE_ERR_SUCCESS; ltk_key_cmd_complete: put_le16(rspbuf, handle); + *rsplen = sizeof(uint16_t); + return rc; +} + +/** + * Called to process the LE long term key negative reply. + * + * Context: Link Layer Task. + * + * @param cmdbuf + * @param rspbuf + * @param ocf + * + * @return int + */ +int +ble_ll_conn_hci_le_ltk_neg_reply(uint8_t *cmdbuf, uint8_t *rspbuf, + uint8_t *rsplen) +{ + int rc; + uint16_t handle; + struct ble_ll_conn_sm *connsm; + + /* Find connection handle */ + handle = get_le16(cmdbuf); + connsm = ble_ll_conn_find_active_conn(handle); + if (!connsm) { + rc = BLE_ERR_UNK_CONN_ID; + goto ltk_key_cmd_complete; + } + + /* Should never get this if we are a master! */ + if (connsm->conn_role == BLE_LL_CONN_ROLE_MASTER) { + rc = BLE_ERR_UNSPECIFIED; + goto ltk_key_cmd_complete; + } + + /* The connection should be awaiting a reply. If not, just discard */ + if (connsm->enc_data.enc_state != CONN_ENC_S_LTK_REQ_WAIT) { + rc = BLE_ERR_CMD_DISALLOWED; + goto ltk_key_cmd_complete; + } + + /* We received a negative reply! Send REJECT_IND */ + ble_ll_ctrl_reject_ind_send(connsm, BLE_LL_CTRL_ENC_REQ, + BLE_ERR_PINKEY_MISSING); + connsm->enc_data.enc_state = CONN_ENC_S_LTK_NEG_REPLY; + + rc = BLE_ERR_SUCCESS; + +ltk_key_cmd_complete: + put_le16(rspbuf, handle); + *rsplen = sizeof(uint16_t); return rc; } #endif diff --git a/nimble/controller/src/ble_ll_conn_priv.h b/nimble/controller/src/ble_ll_conn_priv.h index 5cc509f0b..db7395f9c 100644 --- a/nimble/controller/src/ble_ll_conn_priv.h +++ b/nimble/controller/src/ble_ll_conn_priv.h @@ -165,7 +165,10 @@ int ble_ll_conn_hci_rd_chan_map(uint8_t *cmdbuf, uint8_t *rspbuf, int ble_ll_conn_hci_set_data_len(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t *rsplen); int ble_ll_conn_hci_le_start_encrypt(uint8_t *cmdbuf); -int ble_ll_conn_hci_le_ltk_reply(uint8_t *cmdbuf, uint8_t *rspbuf, uint8_t ocf); +int ble_ll_conn_hci_le_ltk_reply(uint8_t *cmdbuf, uint8_t *rspbuf, + uint8_t *rsplen); +int ble_ll_conn_hci_le_ltk_neg_reply(uint8_t *cmdbuf, uint8_t *rspbuf, + uint8_t *rsplen); int ble_ll_conn_hci_wr_auth_pyld_tmo(uint8_t *cmdbuf, uint8_t *rsp, uint8_t *rsplen); int ble_ll_conn_hci_rd_auth_pyld_tmo(uint8_t *cmdbuf, uint8_t *rsp, diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index 2e246839f..9bf0a56f8 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -817,9 +817,10 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, rc = ble_ll_conn_hci_le_start_encrypt(cmdbuf); break; case BLE_HCI_OCF_LE_LT_KEY_REQ_REPLY: + rc = ble_ll_conn_hci_le_ltk_reply(cmdbuf, rspbuf, rsplen); + break; case BLE_HCI_OCF_LE_LT_KEY_REQ_NEG_REPLY: - rc = ble_ll_conn_hci_le_ltk_reply(cmdbuf, rspbuf, ocf); - *rsplen = sizeof(uint16_t); + rc = ble_ll_conn_hci_le_ltk_neg_reply(cmdbuf, rspbuf, rsplen); break; #endif case BLE_HCI_OCF_LE_RD_SUPP_STATES : From 2fdca20f885c4c8341497696f08fcc5f92edd9b8 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Wed, 20 Jun 2018 09:55:03 +0200 Subject: [PATCH 2/7] nimble/ll: Fix handling of unknown commands For commands not supported by controller we should not validate length but just reply with Unknown HCI Command error. --- nimble/controller/src/ble_ll_hci.c | 253 +++++++++++++++++++++-------- nimble/host/src/ble_gap.c | 2 +- nimble/host/src/ble_hs_hci_cmd.c | 2 +- nimble/include/nimble/hci_common.h | 6 +- nimble/src/hci_common.c | 109 ------------- 5 files changed, 191 insertions(+), 181 deletions(-) delete mode 100644 nimble/src/hci_common.c diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index 9bf0a56f8..10173ec2c 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -701,7 +701,6 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, ble_ll_hci_post_cmd_complete_cb *cb) { int rc; - uint8_t cmdlen; uint8_t len; uint8_t *rspbuf; @@ -711,12 +710,6 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, /* Get length from command */ len = cmdbuf[sizeof(uint16_t)]; - /* Check the length to make sure it is valid */ - cmdlen = g_ble_hci_le_cmd_len[ocf]; - if (len != cmdlen && cmdlen != BLE_HCI_VARIABLE_LEN) { - goto ll_hci_le_cmd_exit; - } - #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) if (!ble_ll_is_valid_adv_mode(ocf)) { rc = BLE_ERR_CMD_DISALLOWED; @@ -736,211 +729,331 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, switch (ocf) { case BLE_HCI_OCF_LE_SET_EVENT_MASK: - rc = ble_ll_hci_set_le_event_mask(cmdbuf); + if (len == BLE_HCI_SET_LE_EVENT_MASK_LEN) { + rc = ble_ll_hci_set_le_event_mask(cmdbuf); + } break; case BLE_HCI_OCF_LE_RD_BUF_SIZE: - rc = ble_ll_hci_le_read_bufsize(rspbuf, rsplen); + if (len == BLE_HCI_RD_BUF_SIZE_LEN) { + rc = ble_ll_hci_le_read_bufsize(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_RD_LOC_SUPP_FEAT: - rc = ble_ll_hci_le_read_local_features(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_hci_le_read_local_features(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_SET_RAND_ADDR: + if (len == BLE_HCI_SET_RAND_ADDR_LEN) { #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) - rc = ble_ll_set_random_addr(cmdbuf, hci_adv_mode == ADV_MODE_EXT); + rc = ble_ll_set_random_addr(cmdbuf, hci_adv_mode == ADV_MODE_EXT); #else - rc = ble_ll_set_random_addr(cmdbuf, false); + rc = ble_ll_set_random_addr(cmdbuf, false); #endif + } break; case BLE_HCI_OCF_LE_SET_ADV_PARAMS: - rc = ble_ll_adv_set_adv_params(cmdbuf); + if (len == BLE_HCI_SET_ADV_PARAM_LEN) { + rc = ble_ll_adv_set_adv_params(cmdbuf); + } break; case BLE_HCI_OCF_LE_RD_ADV_CHAN_TXPWR: - rc = ble_ll_adv_read_txpwr(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_adv_read_txpwr(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_SET_ADV_DATA: - rc = ble_ll_adv_set_adv_data(cmdbuf, 0, + if (len == BLE_HCI_SET_ADV_DATA_LEN) { + rc = ble_ll_adv_set_adv_data(cmdbuf, 0, BLE_HCI_LE_SET_EXT_ADV_DATA_OPER_COMPLETE); + } break; case BLE_HCI_OCF_LE_SET_SCAN_RSP_DATA: - rc = ble_ll_adv_set_scan_rsp_data(cmdbuf, 0, + if (len == BLE_HCI_SET_SCAN_RSP_DATA_LEN) { + rc = ble_ll_adv_set_scan_rsp_data(cmdbuf, 0, BLE_HCI_LE_SET_EXT_SCAN_RSP_DATA_OPER_COMPLETE); + } break; case BLE_HCI_OCF_LE_SET_ADV_ENABLE: - rc = ble_ll_adv_set_enable(0, cmdbuf[0], -1, 0); + if (len == BLE_HCI_SET_ADV_ENABLE_LEN) { + rc = ble_ll_adv_set_enable(0, cmdbuf[0], -1, 0); + } break; case BLE_HCI_OCF_LE_SET_SCAN_ENABLE: - rc = ble_ll_scan_set_enable(cmdbuf, 0); + if (len == BLE_HCI_SET_SCAN_ENABLE_LEN) { + rc = ble_ll_scan_set_enable(cmdbuf, 0); + } break; case BLE_HCI_OCF_LE_SET_SCAN_PARAMS: - rc = ble_ll_scan_set_scan_params(cmdbuf); + if (len == BLE_HCI_SET_SCAN_PARAM_LEN) { + rc = ble_ll_scan_set_scan_params(cmdbuf); + } break; case BLE_HCI_OCF_LE_CREATE_CONN: - rc = ble_ll_conn_create(cmdbuf); + if (len == BLE_HCI_CREATE_CONN_LEN) { + rc = ble_ll_conn_create(cmdbuf); + } break; case BLE_HCI_OCF_LE_CREATE_CONN_CANCEL: - rc = ble_ll_conn_create_cancel(cb); + if (len == 0) { + rc = ble_ll_conn_create_cancel(cb); + } break; case BLE_HCI_OCF_LE_CLEAR_WHITE_LIST: - rc = ble_ll_whitelist_clear(); + if (len == 0) { + rc = ble_ll_whitelist_clear(); + } break; case BLE_HCI_OCF_LE_RD_WHITE_LIST_SIZE: - rc = ble_ll_whitelist_read_size(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_whitelist_read_size(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_ADD_WHITE_LIST: - rc = ble_ll_whitelist_add(cmdbuf + 1, cmdbuf[0]); + if (len == BLE_HCI_ADD_WHITE_LIST_LEN) { + rc = ble_ll_whitelist_add(cmdbuf + 1, cmdbuf[0]); + } break; case BLE_HCI_OCF_LE_RMV_WHITE_LIST: - rc = ble_ll_whitelist_rmv(cmdbuf + 1, cmdbuf[0]); + if (len == BLE_HCI_RMV_WHITE_LIST_LEN) { + rc = ble_ll_whitelist_rmv(cmdbuf + 1, cmdbuf[0]); + } break; case BLE_HCI_OCF_LE_CONN_UPDATE: - rc = ble_ll_conn_hci_update(cmdbuf); + if (len == BLE_HCI_CONN_UPDATE_LEN) { + rc = ble_ll_conn_hci_update(cmdbuf); + } break; case BLE_HCI_OCF_LE_SET_HOST_CHAN_CLASS: - rc = ble_ll_conn_hci_set_chan_class(cmdbuf); + if (BLE_HCI_SET_HOST_CHAN_CLASS_LEN) { + rc = ble_ll_conn_hci_set_chan_class(cmdbuf); + } break; case BLE_HCI_OCF_LE_RD_CHAN_MAP: - rc = ble_ll_conn_hci_rd_chan_map(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_RD_CHANMAP_LEN) { + rc = ble_ll_conn_hci_rd_chan_map(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_RD_REM_FEAT: - rc = ble_ll_conn_hci_read_rem_features(cmdbuf); + if (len == BLE_HCI_CONN_RD_REM_FEAT_LEN) { + rc = ble_ll_conn_hci_read_rem_features(cmdbuf); + } break; #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION) == 1) case BLE_HCI_OCF_LE_ENCRYPT: - rc = ble_ll_hci_le_encrypt(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_LE_ENCRYPT_LEN) { + rc = ble_ll_hci_le_encrypt(cmdbuf, rspbuf, rsplen); + } break; #endif case BLE_HCI_OCF_LE_RAND: - rc = ble_ll_hci_le_rand(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_hci_le_rand(rspbuf, rsplen); + } break; #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_ENCRYPTION) == 1) case BLE_HCI_OCF_LE_START_ENCRYPT: - rc = ble_ll_conn_hci_le_start_encrypt(cmdbuf); + if (len == BLE_HCI_LE_START_ENCRYPT_LEN) { + rc = ble_ll_conn_hci_le_start_encrypt(cmdbuf); + } break; case BLE_HCI_OCF_LE_LT_KEY_REQ_REPLY: - rc = ble_ll_conn_hci_le_ltk_reply(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_LT_KEY_REQ_REPLY_LEN) { + rc = ble_ll_conn_hci_le_ltk_reply(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_LT_KEY_REQ_NEG_REPLY: - rc = ble_ll_conn_hci_le_ltk_neg_reply(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_LT_KEY_REQ_NEG_REPLY_LEN) { + rc = ble_ll_conn_hci_le_ltk_neg_reply(cmdbuf, rspbuf, rsplen); + } break; #endif case BLE_HCI_OCF_LE_RD_SUPP_STATES : - rc = ble_ll_hci_le_read_supp_states(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_hci_le_read_supp_states(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_REM_CONN_PARAM_NRR: - rc = ble_ll_conn_hci_param_reply(cmdbuf, 0, rspbuf, rsplen); + if (len == BLE_HCI_CONN_PARAM_NEG_REPLY_LEN) { + rc = ble_ll_conn_hci_param_reply(cmdbuf, 0, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_REM_CONN_PARAM_RR: - rc = ble_ll_conn_hci_param_reply(cmdbuf, 1, rspbuf, rsplen); + if (len == BLE_HCI_CONN_PARAM_REPLY_LEN) { + rc = ble_ll_conn_hci_param_reply(cmdbuf, 1, rspbuf, rsplen); + } break; #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_DATA_LEN_EXT) == 1) case BLE_HCI_OCF_LE_SET_DATA_LEN: - rc = ble_ll_conn_hci_set_data_len(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_SET_DATALEN_LEN) { + rc = ble_ll_conn_hci_set_data_len(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_RD_SUGG_DEF_DATA_LEN: - rc = ble_ll_hci_le_rd_sugg_data_len(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_hci_le_rd_sugg_data_len(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_WR_SUGG_DEF_DATA_LEN: - rc = ble_ll_hci_le_wr_sugg_data_len(cmdbuf); + if (len == BLE_HCI_WR_SUGG_DATALEN_LEN) { + rc = ble_ll_hci_le_wr_sugg_data_len(cmdbuf); + } break; #endif #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY) == 1) case BLE_HCI_OCF_LE_ADD_RESOLV_LIST : - rc = ble_ll_resolv_list_add(cmdbuf); + if (len == BLE_HCI_ADD_TO_RESOLV_LIST_LEN) { + rc = ble_ll_resolv_list_add(cmdbuf); + } break; case BLE_HCI_OCF_LE_RMV_RESOLV_LIST: - rc = ble_ll_resolv_list_rmv(cmdbuf); + if (len == BLE_HCI_RMV_FROM_RESOLV_LIST_LEN) { + rc = ble_ll_resolv_list_rmv(cmdbuf); + } break; case BLE_HCI_OCF_LE_CLR_RESOLV_LIST: - rc = ble_ll_resolv_list_clr(); + if (len == 0) { + rc = ble_ll_resolv_list_clr(); + } break; case BLE_HCI_OCF_LE_RD_RESOLV_LIST_SIZE: - rc = ble_ll_resolv_list_read_size(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_resolv_list_read_size(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_RD_PEER_RESOLV_ADDR: - rc = ble_ll_resolv_peer_addr_rd(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_RD_PEER_RESOLV_ADDR_LEN) { + rc = ble_ll_resolv_peer_addr_rd(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_RD_LOCAL_RESOLV_ADDR: - rc = ble_ll_resolv_local_addr_rd(cmdbuf, rspbuf, rsplen); + if(len == BLE_HCI_RD_LOC_RESOLV_ADDR_LEN) { + rc = ble_ll_resolv_local_addr_rd(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_SET_ADDR_RES_EN: - rc = ble_ll_resolv_enable_cmd(cmdbuf); + if (len == BLE_HCI_SET_ADDR_RESOL_ENA_LEN) { + rc = ble_ll_resolv_enable_cmd(cmdbuf); + } break; case BLE_HCI_OCF_LE_SET_RPA_TMO: - rc = ble_ll_resolv_set_rpa_tmo(cmdbuf); + if (len == BLE_HCI_SET_RESOLV_PRIV_ADDR_TO_LEN) { + rc = ble_ll_resolv_set_rpa_tmo(cmdbuf); + } break; case BLE_HCI_OCF_LE_SET_PRIVACY_MODE: - rc = ble_ll_resolve_set_priv_mode(cmdbuf); + if (len == BLE_HCI_LE_SET_PRIVACY_MODE_LEN) { + rc = ble_ll_resolve_set_priv_mode(cmdbuf); + } break; #endif #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) case BLE_HCI_OCF_LE_SET_EXT_SCAN_PARAM: - rc = ble_ll_set_ext_scan_params(cmdbuf); + if (len == BLE_HCI_LE_SET_EXT_SCAN_PARAM_LEN) { + rc = ble_ll_set_ext_scan_params(cmdbuf); + } break; case BLE_HCI_OCF_LE_SET_EXT_SCAN_ENABLE: - rc = ble_ll_scan_set_enable(cmdbuf, 1); + if (len == BLE_HCI_LE_SET_EXT_SCAN_ENABLE_LEN) { + rc = ble_ll_scan_set_enable(cmdbuf, 1); + } break; case BLE_HCI_OCF_LE_EXT_CREATE_CONN: + /* variable length */ rc = ble_ll_ext_conn_create(cmdbuf); break; #endif case BLE_HCI_OCF_LE_RD_MAX_DATA_LEN: - rc = ble_ll_hci_le_rd_max_data_len(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_hci_le_rd_max_data_len(rspbuf, rsplen); + } break; #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) == 1) case BLE_HCI_OCF_LE_SET_ADV_SET_RND_ADDR: - rc = ble_ll_adv_set_random_addr(cmdbuf + 1, cmdbuf[0]); + if (len == BLE_HCI_LE_SET_ADV_SET_RND_ADDR_LEN) { + rc = ble_ll_adv_set_random_addr(cmdbuf + 1, cmdbuf[0]); + } break; case BLE_HCI_OCF_LE_SET_EXT_ADV_PARAM: - rc = ble_ll_adv_ext_set_param(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_LE_SET_EXT_ADV_PARAM_LEN) { + rc = ble_ll_adv_ext_set_param(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_SET_EXT_ADV_DATA: - rc = ble_ll_adv_ext_set_adv_data(cmdbuf, cmdlen); + /* variable length */ + rc = ble_ll_adv_ext_set_adv_data(cmdbuf, len); break; case BLE_HCI_OCF_LE_SET_EXT_SCAN_RSP_DATA: - rc = ble_ll_adv_ext_set_scan_rsp(cmdbuf, cmdlen); + /* variable length */ + rc = ble_ll_adv_ext_set_scan_rsp(cmdbuf, len); break; case BLE_HCI_OCF_LE_SET_EXT_ADV_ENABLE: + /* variable length */ rc = ble_ll_adv_ext_set_enable(cmdbuf, len); break; case BLE_HCI_OCF_LE_RD_MAX_ADV_DATA_LEN: - rc = ble_ll_adv_rd_max_adv_data_len(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_adv_rd_max_adv_data_len(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_RD_NUM_OF_ADV_SETS: - rc = ble_ll_adv_rd_sup_adv_sets(rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_adv_rd_sup_adv_sets(rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_REMOVE_ADV_SET: - rc = ble_ll_ext_adv_set_remove(cmdbuf); + if (len == BLE_HCI_LE_REMOVE_ADV_SET_LEN) { + rc = ble_ll_ext_adv_set_remove(cmdbuf); + } break; case BLE_HCI_OCF_LE_CLEAR_ADV_SETS: - rc = ble_ll_adv_clear_all(); + if (len == 0) { + rc = ble_ll_adv_clear_all(); + } break; #endif #if (BLE_LL_BT5_PHY_SUPPORTED == 1) case BLE_HCI_OCF_LE_RD_PHY: - rc = ble_ll_conn_hci_le_rd_phy(cmdbuf, rspbuf, rsplen); + if (len == BLE_HCI_LE_RD_PHY_LEN) { + rc = ble_ll_conn_hci_le_rd_phy(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_SET_DEFAULT_PHY: - rc = ble_ll_hci_le_set_def_phy(cmdbuf); + if (len == BLE_HCI_LE_SET_DEFAULT_PHY_LEN) { + rc = ble_ll_hci_le_set_def_phy(cmdbuf); + } break; case BLE_HCI_OCF_LE_SET_PHY: - rc = ble_ll_conn_hci_le_set_phy(cmdbuf); + if (len == BLE_HCI_LE_SET_PHY_LEN) { + rc = ble_ll_conn_hci_le_set_phy(cmdbuf); + } break; #endif #if MYNEWT_VAL(BLE_LL_DIRECT_TEST_MODE) == 1 case BLE_HCI_OCF_LE_TX_TEST: - rc = ble_ll_dtm_tx_test(cmdbuf, false); + if (len == BLE_HCI_TX_TEST_LEN) { + rc = ble_ll_dtm_tx_test(cmdbuf, false); + } break; case BLE_HCI_OCF_LE_RX_TEST: - rc = ble_ll_dtm_rx_test(cmdbuf, false); + if (len == BLE_HCI_RX_TEST_LEN) { + rc = ble_ll_dtm_rx_test(cmdbuf, false); + } break; case BLE_HCI_OCF_LE_TEST_END: - rc = ble_ll_dtm_end_test(cmdbuf, rspbuf, rsplen); + if (len == 0) { + rc = ble_ll_dtm_end_test(cmdbuf, rspbuf, rsplen); + } break; case BLE_HCI_OCF_LE_ENH_RX_TEST: - rc = ble_ll_dtm_rx_test(cmdbuf, true); + if (len == BLE_HCI_LE_ENH_RX_TEST_LEN) { + rc = ble_ll_dtm_rx_test(cmdbuf, true); + } break; case BLE_HCI_OCF_LE_ENH_TX_TEST: - rc = ble_ll_dtm_tx_test(cmdbuf, true); + if (len == BLE_HCI_LE_ENH_TX_TEST_LEN) { + rc = ble_ll_dtm_tx_test(cmdbuf, true); + } break; #endif default: @@ -948,12 +1061,14 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, break; } +#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) +ll_hci_le_cmd_exit: +#endif /* * This code is here because we add 256 to the return code to denote * that the reply to this command should be command status (as opposed to * command complete). */ -ll_hci_le_cmd_exit: if (ble_ll_hci_le_cmd_send_cmd_status(ocf)) { rc += (BLE_ERR_MAX + 1); } diff --git a/nimble/host/src/ble_gap.c b/nimble/host/src/ble_gap.c index cb75e878f..bca989251 100644 --- a/nimble/host/src/ble_gap.c +++ b/nimble/host/src/ble_gap.c @@ -1685,7 +1685,7 @@ ble_gap_wl_busy(void) static int ble_gap_wl_tx_add(const ble_addr_t *addr) { - uint8_t buf[BLE_HCI_CHG_WHITE_LIST_LEN]; + uint8_t buf[BLE_HCI_ADD_WHITE_LIST_LEN]; int rc; rc = ble_hs_hci_cmd_build_le_add_to_whitelist(addr->val, addr->type, diff --git a/nimble/host/src/ble_hs_hci_cmd.c b/nimble/host/src/ble_hs_hci_cmd.c index e5acd71ab..7181c9a95 100644 --- a/nimble/host/src/ble_hs_hci_cmd.c +++ b/nimble/host/src/ble_hs_hci_cmd.c @@ -521,7 +521,7 @@ ble_hs_hci_cmd_build_le_add_to_whitelist(const uint8_t *addr, uint8_t addr_type, uint8_t *dst, int dst_len) { - BLE_HS_DBG_ASSERT(dst_len >= BLE_HCI_CHG_WHITE_LIST_LEN); + BLE_HS_DBG_ASSERT(dst_len >= BLE_HCI_ADD_WHITE_LIST_LEN); return ble_hs_hci_cmd_body_le_whitelist_chg(addr, addr_type, dst); } diff --git a/nimble/include/nimble/hci_common.h b/nimble/include/nimble/hci_common.h index d7be6889a..437ed84cf 100644 --- a/nimble/include/nimble/hci_common.h +++ b/nimble/include/nimble/hci_common.h @@ -353,7 +353,8 @@ extern "C" { #define BLE_HCI_SCAN_FILT_MAX (3) /* Whitelist commands */ -#define BLE_HCI_CHG_WHITE_LIST_LEN (7) +#define BLE_HCI_ADD_WHITE_LIST_LEN (7) +#define BLE_HCI_RMV_WHITE_LIST_LEN (7) /* Create Connection */ #define BLE_HCI_CREATE_CONN_LEN (25) @@ -410,6 +411,9 @@ extern "C" { /* --- LE read supported states (OCF 0x001C) --- */ #define BLE_HCI_RD_SUPP_STATES_RSPLEN (8) +/* --- LE receiver test command (OCF 0x001D) --- */ +#define BLE_HCI_RX_TEST_LEN (1) + /* --- LE transitter test command (OCF 0x001E) --- */ #define BLE_HCI_TX_TEST_LEN (3) diff --git a/nimble/src/hci_common.c b/nimble/src/hci_common.c deleted file mode 100644 index 2216e9f49..000000000 --- a/nimble/src/hci_common.c +++ /dev/null @@ -1,109 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ -#include -#include "os/os.h" -#include "nimble/ble.h" -#include "nimble/hci_common.h" - -/* - * Storage for the length of each HCI LE command. This is the length of the - * command parameters in the command; not the response length. - */ -const uint8_t g_ble_hci_le_cmd_len[BLE_HCI_NUM_LE_CMDS] = -{ - 0, /* 0x0000: reserved */ - BLE_HCI_SET_LE_EVENT_MASK_LEN, /* 0x0001: set event mask */ - BLE_HCI_RD_BUF_SIZE_LEN, /* 0x0002: read buffer size */ - 0, /* 0x0003: read local supp features */ - 0, /* 0x0004: not defined */ - BLE_HCI_SET_RAND_ADDR_LEN, /* 0x0005: set random address */ - BLE_HCI_SET_ADV_PARAM_LEN, /* 0x0006: set advertising parameters */ - 0, /* 0x0007: read adv chan tx power */ - BLE_HCI_SET_ADV_DATA_LEN, /* 0x0008: set advertising data */ - BLE_HCI_SET_SCAN_RSP_DATA_LEN, /* 0x0009: set scan rsp data */ - BLE_HCI_SET_ADV_ENABLE_LEN, /* 0x000A: set advertising enable */ - BLE_HCI_SET_SCAN_PARAM_LEN, /* 0x000B: set scan parameters */ - BLE_HCI_SET_SCAN_ENABLE_LEN, /* 0x000C: set scan enable */ - BLE_HCI_CREATE_CONN_LEN, /* 0x000D: create connection */ - 0, /* 0x000E: create connection cancel*/ - 0, /* 0x000F: read whitelist size */ - 0, /* 0x0010: clear white list */ - BLE_HCI_CHG_WHITE_LIST_LEN, /* 0x0011: add to white list */ - BLE_HCI_CHG_WHITE_LIST_LEN, /* 0x0012: remove from white list */ - BLE_HCI_CONN_UPDATE_LEN, /* 0x0013: connection update */ - BLE_HCI_SET_HOST_CHAN_CLASS_LEN, /* 0x0014: set host chan class */ - sizeof(uint16_t), /* 0x0015: read channel map */ - BLE_HCI_CONN_RD_REM_FEAT_LEN, /* 0x0016: read remote features */ - BLE_HCI_LE_ENCRYPT_LEN, /* 0x0017: encrypt */ - 0, /* 0x0018: rand */ - BLE_HCI_LE_START_ENCRYPT_LEN, /* 0x0019: start encryption */ - BLE_HCI_LT_KEY_REQ_REPLY_LEN, /* 0x001A: LTK request reply */ - sizeof(uint16_t), /* 0x001B: LTK request negative reply */ - 0, /* 0x001C: read supported states */ - sizeof(uint8_t), /* 0x001D: receiver test */ - BLE_HCI_TX_TEST_LEN, /* 0x001E: transmitter test */ - 0, /* 0x001F: test end */ - BLE_HCI_CONN_PARAM_REPLY_LEN, /* 0x0020: conn param reply */ - BLE_HCI_CONN_PARAM_NEG_REPLY_LEN, /* 0x0021: conn param neg reply */ - BLE_HCI_SET_DATALEN_LEN, /* 0x0022: set data length */ - 0, /* 0x0023: read sugg data len */ - BLE_HCI_WR_SUGG_DATALEN_LEN, /* 0x0024: write suggested data len */ - 0, /* 0x0025: rd local P256 pub key */ - BLE_HCI_GEN_DHKEY_LEN, /* 0x0026: generate DHKEY */ - BLE_HCI_ADD_TO_RESOLV_LIST_LEN, /* 0x0027: add to resolving list */ - BLE_HCI_RMV_FROM_RESOLV_LIST_LEN, /* 0x0028: rmv from resolving list */ - 0, /* 0x0029: clear resolving list */ - 0, /* 0x002A: read resolving list size */ - BLE_HCI_RD_PEER_RESOLV_ADDR_LEN, /* 0x002B: read peer resolvable addr */ - BLE_HCI_RD_LOC_RESOLV_ADDR_LEN, /* 0x002C: read local resolvable addr */ - sizeof(uint8_t), /* 0x002D: set addr resolution enable */ - sizeof(uint16_t), /* 0x002E: set resolv priv addr tmo */ - 0, /* 0x002F: read max data length */ - BLE_HCI_LE_RD_PHY_LEN, /* 0x0030: read maximum default PHY */ - BLE_HCI_LE_SET_DEFAULT_PHY_LEN, /* 0x0031: set default PHY */ - BLE_HCI_LE_SET_PHY_LEN, /* 0x0032: set PHY */ - BLE_HCI_LE_ENH_RX_TEST_LEN, /* 0x0033: enhanced receiver test */ - BLE_HCI_LE_ENH_TX_TEST_LEN, /* 0x0034: enhanced transmitter test */ - BLE_HCI_LE_SET_ADV_SET_RND_ADDR_LEN,/* 0x0035: set adv. set random address */ - BLE_HCI_LE_SET_EXT_ADV_PARAM_LEN, /* 0x0036: set ext. adv params */ - BLE_HCI_LE_SET_EXT_ADV_DATA_LEN, /* 0x0037: set ext. adv. data */ - BLE_HCI_LE_SET_EXT_SCAN_RSP_DATA_LEN,/* 0x0038: set ext. scan resp. data */ - BLE_HCI_LE_SET_EXT_ADV_ENABLE_LEN, /* 0x0039: set ext. adv. enable */ - 0, /* 0x003A: read max adv. data len */ - 0, /* 0x003B: read number of sup. adv. sets */ - BLE_HCI_LE_REMOVE_ADV_SET_LEN, /* 0x003C: remove adv. set */ - 0, /* 0x003D: clear advertising sets */ - BLE_HCI_LE_SET_PER_ADV_PARAMS_LEN, /* 0x003E: set periodic adv. param. */ - BLE_HCI_LE_SET_PER_ADV_DATA_LEN, /* 0x003F: set periodic adv. data */ - BLE_HCI_LE_SET_PER_ADV_ENABLE_LEN, /* 0x0040: periodic adv. enable */ - BLE_HCI_LE_SET_EXT_SCAN_PARAM_LEN, /* 0x0041: set ext. scan param. */ - BLE_HCI_LE_SET_EXT_SCAN_ENABLE_LEN, /* 0x0042: set ext. scan enable */ - BLE_HCI_LE_EXT_CREATE_CONN_LEN, /* 0x0043: ext. create connection */ - BLE_HCI_LE_PER_ADV_CREATE_SYNC_LEN, /* 0x0044: periodic adv. create sync */ - 0, /* 0x0045: periodic adv. create sync cancel */ - 0, /* 0x0046: periodic adv. terminate sync */ - BLE_HCI_LE_ADD_DEV_TO_PER_ADV_LIST_LEN, /* 0x0047: add dev to per. adv. list */ - BLE_HCI_LE_REM_DEV_FROM_PER_ADV_LIST_LEN,/* 0x0048: remove dev from per. adv. list */ - 0, /* 0x0049: clear periodic adv. list */ - 0, /* 0x004A: read periodic list size */ - 0, /* 0x004B: read transmit power */ - 0, /* 0x004C: read RF path */ - BLE_HCI_LE_WR_RF_PATH_COMPENSATION_LEN, /* 0x004D: write RF path */ - BLE_HCI_LE_SET_PRIVACY_MODE_LEN, /* 0x004E: set privacy mode */ -}; From 8bc3872ca7531ad90e2091fb8b31fa696a346885 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Wed, 20 Jun 2018 09:57:20 +0200 Subject: [PATCH 3/7] nimble/ll: Remove unused parameter from ble_ll_dtm_end_test cmdbuf is not used by this handler. --- nimble/controller/src/ble_ll_dtm.c | 2 +- nimble/controller/src/ble_ll_dtm_priv.h | 2 +- nimble/controller/src/ble_ll_hci.c | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/nimble/controller/src/ble_ll_dtm.c b/nimble/controller/src/ble_ll_dtm.c index 0beef447d..964e7c781 100644 --- a/nimble/controller/src/ble_ll_dtm.c +++ b/nimble/controller/src/ble_ll_dtm.c @@ -466,7 +466,7 @@ int ble_ll_dtm_rx_test(uint8_t *cmdbuf, bool enhanced) return BLE_ERR_SUCCESS; } -int ble_ll_dtm_end_test(uint8_t *cmdbuf, uint8_t *rsp, uint8_t *rsplen) +int ble_ll_dtm_end_test(uint8_t *rsp, uint8_t *rsplen) { put_le16(rsp, g_ble_ll_dtm_ctx. num_of_packets); *rsplen = 2; diff --git a/nimble/controller/src/ble_ll_dtm_priv.h b/nimble/controller/src/ble_ll_dtm_priv.h index 4f21a9ca9..5341cd28d 100644 --- a/nimble/controller/src/ble_ll_dtm_priv.h +++ b/nimble/controller/src/ble_ll_dtm_priv.h @@ -26,7 +26,7 @@ int ble_ll_dtm_tx_test(uint8_t *cmdbuf, bool enhanced); int ble_ll_dtm_rx_test(uint8_t *cmdbuf, bool enhanced); -int ble_ll_dtm_end_test(uint8_t *cmdbuf, uint8_t *rsp, uint8_t *rsplen); +int ble_ll_dtm_end_test(uint8_t *rsp, uint8_t *rsplen); int ble_ll_dtm_rx_isr_start(struct ble_mbuf_hdr *rxhdr, uint32_t aa); int ble_ll_dtm_rx_isr_end(uint8_t *rxbuf, struct ble_mbuf_hdr *rxhdr); diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index 10173ec2c..abb19664d 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -1042,7 +1042,7 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, break; case BLE_HCI_OCF_LE_TEST_END: if (len == 0) { - rc = ble_ll_dtm_end_test(cmdbuf, rspbuf, rsplen); + rc = ble_ll_dtm_end_test(rspbuf, rsplen); } break; case BLE_HCI_OCF_LE_ENH_RX_TEST: From 9103cae9605ee37911dc53b5cb900013944f1a47 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Wed, 20 Jun 2018 12:24:20 +0200 Subject: [PATCH 4/7] nimble/ll: Validate LE Extended Create Connection Cmd buffer Make sure we don't access invalid memory when parsing command buffer. --- nimble/controller/src/ble_ll_conn_hci.c | 22 +++++++++++++++++++++- nimble/controller/src/ble_ll_conn_priv.h | 2 +- nimble/controller/src/ble_ll_hci.c | 2 +- 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/nimble/controller/src/ble_ll_conn_hci.c b/nimble/controller/src/ble_ll_conn_hci.c index 09d9b66d0..5c086fbf3 100644 --- a/nimble/controller/src/ble_ll_conn_hci.c +++ b/nimble/controller/src/ble_ll_conn_hci.c @@ -522,7 +522,7 @@ ble_ll_conn_init_hcc_params(struct hci_ext_create_conn *hcc, } int -ble_ll_ext_conn_create(uint8_t *cmdbuf) +ble_ll_ext_conn_create(uint8_t *cmdbuf, uint8_t cmdlen) { int rc; struct hci_ext_create_conn ccdata; @@ -532,6 +532,11 @@ ble_ll_ext_conn_create(uint8_t *cmdbuf) int valid_param_idx = -1; int iter; + /* validate length */ + if (cmdlen < 10) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + /* If we are already creating a connection we should leave */ if (g_ble_ll_conn_create_sm) { return BLE_ERR_CMD_DISALLOWED; @@ -580,6 +585,11 @@ ble_ll_ext_conn_create(uint8_t *cmdbuf) iter = 10; if (hcc->init_phy_mask & BLE_PHY_MASK_1M) { + /* validate length */ + if (cmdlen < iter + 16) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + hcc_params = &hcc->params[0]; hcc_params->scan_itvl = get_le16(cmdbuf + iter); hcc_params->scan_window = get_le16(cmdbuf + iter + 2); @@ -617,6 +627,11 @@ ble_ll_ext_conn_create(uint8_t *cmdbuf) #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_2M_PHY) if (hcc->init_phy_mask & BLE_PHY_MASK_2M) { + /* validate length */ + if (cmdlen < iter + 16) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + /* Move to connection parameters */ hcc_params = &hcc->params[1]; iter += 4; @@ -647,6 +662,11 @@ ble_ll_ext_conn_create(uint8_t *cmdbuf) #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY) if (hcc->init_phy_mask & BLE_PHY_MASK_CODED) { + /* validate length */ + if (cmdlen < iter + 16) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + hcc_params = &hcc->params[2]; hcc_params->scan_itvl = get_le16(cmdbuf + iter); hcc_params->scan_window = get_le16(cmdbuf + iter + 2); diff --git a/nimble/controller/src/ble_ll_conn_priv.h b/nimble/controller/src/ble_ll_conn_priv.h index db7395f9c..bde7efac0 100644 --- a/nimble/controller/src/ble_ll_conn_priv.h +++ b/nimble/controller/src/ble_ll_conn_priv.h @@ -186,7 +186,7 @@ int ble_ll_conn_hci_le_rd_phy(uint8_t *cmdbuf, uint8_t *rsp, uint8_t *rsplen); int ble_ll_conn_hci_le_set_phy(uint8_t *cmdbuf); int ble_ll_conn_chk_phy_upd_start(struct ble_ll_conn_sm *connsm); #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) -int ble_ll_ext_conn_create(uint8_t *cmdbuf); +int ble_ll_ext_conn_create(uint8_t *cmdbuf, uint8_t cmdlen); #endif #ifdef __cplusplus } diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index abb19664d..01ffeea15 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -960,7 +960,7 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, break; case BLE_HCI_OCF_LE_EXT_CREATE_CONN: /* variable length */ - rc = ble_ll_ext_conn_create(cmdbuf); + rc = ble_ll_ext_conn_create(cmdbuf, len); break; #endif case BLE_HCI_OCF_LE_RD_MAX_DATA_LEN: From 48e48f9f4b683ccdcbc3587f52227162d517605d Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Wed, 20 Jun 2018 15:08:12 +0200 Subject: [PATCH 5/7] nimble/ll: Validate HCI advertising data and scan response Mkae sure no invalid memory is read if malformatted HCI commands are received. --- .../include/controller/ble_ll_adv.h | 7 ++++--- nimble/controller/src/ble_ll_adv.c | 20 +++++++++++++++---- nimble/controller/src/ble_ll_hci.c | 4 ++-- 3 files changed, 22 insertions(+), 9 deletions(-) diff --git a/nimble/controller/include/controller/ble_ll_adv.h b/nimble/controller/include/controller/ble_ll_adv.h index e8fb36c83..64317cbed 100644 --- a/nimble/controller/include/controller/ble_ll_adv.h +++ b/nimble/controller/include/controller/ble_ll_adv.h @@ -120,11 +120,12 @@ int ble_ll_adv_set_enable(uint8_t instance, uint8_t enable, int duration, uint8_t event); /* Set advertising data */ -int ble_ll_adv_set_adv_data(uint8_t *cmd, uint8_t instance, uint8_t operation); +int ble_ll_adv_set_adv_data(uint8_t *cmd, uint8_t cmd_len, uint8_t instance, + uint8_t operation); /* Set scan response data */ -int ble_ll_adv_set_scan_rsp_data(uint8_t *cmd, uint8_t instance, - uint8_t operation); +int ble_ll_adv_set_scan_rsp_data(uint8_t *cmd, uint8_t cmd_len, + uint8_t instance, uint8_t operation); /* Set advertising parameters */ int ble_ll_adv_set_adv_params(uint8_t *cmd); diff --git a/nimble/controller/src/ble_ll_adv.c b/nimble/controller/src/ble_ll_adv.c index ead9ad7e1..675699f0d 100644 --- a/nimble/controller/src/ble_ll_adv.c +++ b/nimble/controller/src/ble_ll_adv.c @@ -1918,7 +1918,8 @@ instance_configured(struct ble_ll_adv_sm *advsm) * @return int */ int -ble_ll_adv_set_scan_rsp_data(uint8_t *cmd, uint8_t instance, uint8_t operation) +ble_ll_adv_set_scan_rsp_data(uint8_t *cmd, uint8_t cmd_len, uint8_t instance, + uint8_t operation) { uint8_t datalen; struct ble_ll_adv_sm *advsm; @@ -1931,6 +1932,10 @@ ble_ll_adv_set_scan_rsp_data(uint8_t *cmd, uint8_t instance, uint8_t operation) advsm = &g_ble_ll_adv_sm[instance]; datalen = cmd[0]; + if (datalen > 251 || datalen > cmd_len - 1) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + if (!instance_configured(advsm)) { return BLE_ERR_UNK_ADV_INDENT; } @@ -2018,7 +2023,8 @@ ble_ll_adv_set_scan_rsp_data(uint8_t *cmd, uint8_t instance, uint8_t operation) * @return int 0: success; BLE_ERR_INV_HCI_CMD_PARMS otherwise. */ int -ble_ll_adv_set_adv_data(uint8_t *cmd, uint8_t instance, uint8_t operation) +ble_ll_adv_set_adv_data(uint8_t *cmd, uint8_t cmd_len, uint8_t instance, + uint8_t operation) { uint8_t datalen; struct ble_ll_adv_sm *advsm; @@ -2031,6 +2037,10 @@ ble_ll_adv_set_adv_data(uint8_t *cmd, uint8_t instance, uint8_t operation) advsm = &g_ble_ll_adv_sm[instance]; datalen = cmd[0]; + if (datalen > 251 || datalen > cmd_len - 1) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + if (!instance_configured(advsm)) { return BLE_ERR_UNK_ADV_INDENT; } @@ -2354,7 +2364,8 @@ ble_ll_adv_ext_set_adv_data(uint8_t *cmdbuf, uint8_t cmdlen) /* TODO fragment preference ignored for now */ - return ble_ll_adv_set_adv_data(cmdbuf + 3, cmdbuf[0], cmdbuf[1]); + return ble_ll_adv_set_adv_data(cmdbuf + 3, cmdlen - 3, cmdbuf[0], + cmdbuf[1]); } int @@ -2367,7 +2378,8 @@ ble_ll_adv_ext_set_scan_rsp(uint8_t *cmdbuf, uint8_t cmdlen) /* TODO fragment preference ignored for now */ - return ble_ll_adv_set_scan_rsp_data(cmdbuf + 3, cmdbuf[0], cmdbuf[1]); + return ble_ll_adv_set_scan_rsp_data(cmdbuf + 3, cmdlen - 3, cmdbuf[0], + cmdbuf[1]); } struct ext_adv_set { diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index 01ffeea15..e30b0753a 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -764,13 +764,13 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, break; case BLE_HCI_OCF_LE_SET_ADV_DATA: if (len == BLE_HCI_SET_ADV_DATA_LEN) { - rc = ble_ll_adv_set_adv_data(cmdbuf, 0, + rc = ble_ll_adv_set_adv_data(cmdbuf, len, 0, BLE_HCI_LE_SET_EXT_ADV_DATA_OPER_COMPLETE); } break; case BLE_HCI_OCF_LE_SET_SCAN_RSP_DATA: if (len == BLE_HCI_SET_SCAN_RSP_DATA_LEN) { - rc = ble_ll_adv_set_scan_rsp_data(cmdbuf, 0, + rc = ble_ll_adv_set_scan_rsp_data(cmdbuf, len, 0, BLE_HCI_LE_SET_EXT_SCAN_RSP_DATA_OPER_COMPLETE); } break; From a6ff03329a195d040393d1bed7309d2406654218 Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Wed, 20 Jun 2018 15:33:01 +0200 Subject: [PATCH 6/7] nimble/ll: Sort HCI commands handlers in increasing order This make is simpler to check which commands are supported. --- nimble/controller/src/ble_ll_hci.c | 144 +++++++++++++++-------------- 1 file changed, 74 insertions(+), 70 deletions(-) diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index e30b0753a..fe448befa 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -779,16 +779,16 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, rc = ble_ll_adv_set_enable(0, cmdbuf[0], -1, 0); } break; - case BLE_HCI_OCF_LE_SET_SCAN_ENABLE: - if (len == BLE_HCI_SET_SCAN_ENABLE_LEN) { - rc = ble_ll_scan_set_enable(cmdbuf, 0); - } - break; case BLE_HCI_OCF_LE_SET_SCAN_PARAMS: if (len == BLE_HCI_SET_SCAN_PARAM_LEN) { rc = ble_ll_scan_set_scan_params(cmdbuf); } break; + case BLE_HCI_OCF_LE_SET_SCAN_ENABLE: + if (len == BLE_HCI_SET_SCAN_ENABLE_LEN) { + rc = ble_ll_scan_set_enable(cmdbuf, 0); + } + break; case BLE_HCI_OCF_LE_CREATE_CONN: if (len == BLE_HCI_CREATE_CONN_LEN) { rc = ble_ll_conn_create(cmdbuf); @@ -799,16 +799,16 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, rc = ble_ll_conn_create_cancel(cb); } break; - case BLE_HCI_OCF_LE_CLEAR_WHITE_LIST: - if (len == 0) { - rc = ble_ll_whitelist_clear(); - } - break; case BLE_HCI_OCF_LE_RD_WHITE_LIST_SIZE: if (len == 0) { rc = ble_ll_whitelist_read_size(rspbuf, rsplen); } break; + case BLE_HCI_OCF_LE_CLEAR_WHITE_LIST: + if (len == 0) { + rc = ble_ll_whitelist_clear(); + } + break; case BLE_HCI_OCF_LE_ADD_WHITE_LIST: if (len == BLE_HCI_ADD_WHITE_LIST_LEN) { rc = ble_ll_whitelist_add(cmdbuf + 1, cmdbuf[0]); @@ -873,16 +873,33 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, rc = ble_ll_hci_le_read_supp_states(rspbuf, rsplen); } break; - case BLE_HCI_OCF_LE_REM_CONN_PARAM_NRR: - if (len == BLE_HCI_CONN_PARAM_NEG_REPLY_LEN) { - rc = ble_ll_conn_hci_param_reply(cmdbuf, 0, rspbuf, rsplen); +#if MYNEWT_VAL(BLE_LL_DIRECT_TEST_MODE) == 1 + case BLE_HCI_OCF_LE_TX_TEST: + if (len == BLE_HCI_TX_TEST_LEN) { + rc = ble_ll_dtm_tx_test(cmdbuf, false); } break; + case BLE_HCI_OCF_LE_RX_TEST: + if (len == BLE_HCI_RX_TEST_LEN) { + rc = ble_ll_dtm_rx_test(cmdbuf, false); + } + break; + case BLE_HCI_OCF_LE_TEST_END: + if (len == 0) { + rc = ble_ll_dtm_end_test(rspbuf, rsplen); + } + break; +#endif case BLE_HCI_OCF_LE_REM_CONN_PARAM_RR: if (len == BLE_HCI_CONN_PARAM_REPLY_LEN) { rc = ble_ll_conn_hci_param_reply(cmdbuf, 1, rspbuf, rsplen); } break; + case BLE_HCI_OCF_LE_REM_CONN_PARAM_NRR: + if (len == BLE_HCI_CONN_PARAM_NEG_REPLY_LEN) { + rc = ble_ll_conn_hci_param_reply(cmdbuf, 0, rspbuf, rsplen); + } + break; #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_DATA_LEN_EXT) == 1) case BLE_HCI_OCF_LE_SET_DATA_LEN: if (len == BLE_HCI_SET_DATALEN_LEN) { @@ -941,33 +958,41 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, rc = ble_ll_resolv_set_rpa_tmo(cmdbuf); } break; - case BLE_HCI_OCF_LE_SET_PRIVACY_MODE: - if (len == BLE_HCI_LE_SET_PRIVACY_MODE_LEN) { - rc = ble_ll_resolve_set_priv_mode(cmdbuf); - } - break; -#endif -#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) - case BLE_HCI_OCF_LE_SET_EXT_SCAN_PARAM: - if (len == BLE_HCI_LE_SET_EXT_SCAN_PARAM_LEN) { - rc = ble_ll_set_ext_scan_params(cmdbuf); - } - break; - case BLE_HCI_OCF_LE_SET_EXT_SCAN_ENABLE: - if (len == BLE_HCI_LE_SET_EXT_SCAN_ENABLE_LEN) { - rc = ble_ll_scan_set_enable(cmdbuf, 1); - } - break; - case BLE_HCI_OCF_LE_EXT_CREATE_CONN: - /* variable length */ - rc = ble_ll_ext_conn_create(cmdbuf, len); - break; -#endif case BLE_HCI_OCF_LE_RD_MAX_DATA_LEN: if (len == 0) { rc = ble_ll_hci_le_rd_max_data_len(rspbuf, rsplen); } break; +#endif +#if (BLE_LL_BT5_PHY_SUPPORTED == 1) + case BLE_HCI_OCF_LE_RD_PHY: + if (len == BLE_HCI_LE_RD_PHY_LEN) { + rc = ble_ll_conn_hci_le_rd_phy(cmdbuf, rspbuf, rsplen); + } + break; + case BLE_HCI_OCF_LE_SET_DEFAULT_PHY: + if (len == BLE_HCI_LE_SET_DEFAULT_PHY_LEN) { + rc = ble_ll_hci_le_set_def_phy(cmdbuf); + } + break; + case BLE_HCI_OCF_LE_SET_PHY: + if (len == BLE_HCI_LE_SET_PHY_LEN) { + rc = ble_ll_conn_hci_le_set_phy(cmdbuf); + } + break; +#endif +#if MYNEWT_VAL(BLE_LL_DIRECT_TEST_MODE) == 1 + case BLE_HCI_OCF_LE_ENH_RX_TEST: + if (len == BLE_HCI_LE_ENH_RX_TEST_LEN) { + rc = ble_ll_dtm_rx_test(cmdbuf, true); + } + break; + case BLE_HCI_OCF_LE_ENH_TX_TEST: + if (len == BLE_HCI_LE_ENH_TX_TEST_LEN) { + rc = ble_ll_dtm_tx_test(cmdbuf, true); + } + break; +#endif #if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) == 1) case BLE_HCI_OCF_LE_SET_ADV_SET_RND_ADDR: if (len == BLE_HCI_LE_SET_ADV_SET_RND_ADDR_LEN) { @@ -1012,47 +1037,26 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, } break; #endif -#if (BLE_LL_BT5_PHY_SUPPORTED == 1) - case BLE_HCI_OCF_LE_RD_PHY: - if (len == BLE_HCI_LE_RD_PHY_LEN) { - rc = ble_ll_conn_hci_le_rd_phy(cmdbuf, rspbuf, rsplen); +#if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) + case BLE_HCI_OCF_LE_SET_EXT_SCAN_PARAM: + if (len == BLE_HCI_LE_SET_EXT_SCAN_PARAM_LEN) { + rc = ble_ll_set_ext_scan_params(cmdbuf); } break; - case BLE_HCI_OCF_LE_SET_DEFAULT_PHY: - if (len == BLE_HCI_LE_SET_DEFAULT_PHY_LEN) { - rc = ble_ll_hci_le_set_def_phy(cmdbuf); + case BLE_HCI_OCF_LE_SET_EXT_SCAN_ENABLE: + if (len == BLE_HCI_LE_SET_EXT_SCAN_ENABLE_LEN) { + rc = ble_ll_scan_set_enable(cmdbuf, 1); } break; - case BLE_HCI_OCF_LE_SET_PHY: - if (len == BLE_HCI_LE_SET_PHY_LEN) { - rc = ble_ll_conn_hci_le_set_phy(cmdbuf); - } + case BLE_HCI_OCF_LE_EXT_CREATE_CONN: + /* variable length */ + rc = ble_ll_ext_conn_create(cmdbuf, len); break; #endif -#if MYNEWT_VAL(BLE_LL_DIRECT_TEST_MODE) == 1 - case BLE_HCI_OCF_LE_TX_TEST: - if (len == BLE_HCI_TX_TEST_LEN) { - rc = ble_ll_dtm_tx_test(cmdbuf, false); - } - break; - case BLE_HCI_OCF_LE_RX_TEST: - if (len == BLE_HCI_RX_TEST_LEN) { - rc = ble_ll_dtm_rx_test(cmdbuf, false); - } - break; - case BLE_HCI_OCF_LE_TEST_END: - if (len == 0) { - rc = ble_ll_dtm_end_test(rspbuf, rsplen); - } - break; - case BLE_HCI_OCF_LE_ENH_RX_TEST: - if (len == BLE_HCI_LE_ENH_RX_TEST_LEN) { - rc = ble_ll_dtm_rx_test(cmdbuf, true); - } - break; - case BLE_HCI_OCF_LE_ENH_TX_TEST: - if (len == BLE_HCI_LE_ENH_TX_TEST_LEN) { - rc = ble_ll_dtm_tx_test(cmdbuf, true); +#if (MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_PRIVACY) == 1) + case BLE_HCI_OCF_LE_SET_PRIVACY_MODE: + if (len == BLE_HCI_LE_SET_PRIVACY_MODE_LEN) { + rc = ble_ll_resolve_set_priv_mode(cmdbuf); } break; #endif From b01e42bfc94dafbd2a7d674bbe63508356041d0f Mon Sep 17 00:00:00 2001 From: Szymon Janc Date: Wed, 20 Jun 2018 15:36:02 +0200 Subject: [PATCH 7/7] nimble/ll: Improve LE Set Extended Scan Parameters command parsing This commands is variable length and should verify if all required bytes were provided by host. --- nimble/controller/include/controller/ble_ll_scan.h | 2 +- nimble/controller/src/ble_ll_hci.c | 5 ++--- nimble/controller/src/ble_ll_scan.c | 14 +++++++++++++- 3 files changed, 16 insertions(+), 5 deletions(-) diff --git a/nimble/controller/include/controller/ble_ll_scan.h b/nimble/controller/include/controller/ble_ll_scan.h index efb856812..b4415b790 100644 --- a/nimble/controller/include/controller/ble_ll_scan.h +++ b/nimble/controller/include/controller/ble_ll_scan.h @@ -161,7 +161,7 @@ int ble_ll_scan_set_scan_params(uint8_t *cmd); int ble_ll_scan_set_enable(uint8_t *cmd, uint8_t ext); #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) -int ble_ll_set_ext_scan_params(uint8_t *cmd); +int ble_ll_set_ext_scan_params(uint8_t *cmd, uint8_t cmdlen); #endif /*--- Controller Internal API ---*/ diff --git a/nimble/controller/src/ble_ll_hci.c b/nimble/controller/src/ble_ll_hci.c index fe448befa..b3c4bcf8a 100644 --- a/nimble/controller/src/ble_ll_hci.c +++ b/nimble/controller/src/ble_ll_hci.c @@ -1039,9 +1039,8 @@ ble_ll_hci_le_cmd_proc(uint8_t *cmdbuf, uint16_t ocf, uint8_t *rsplen, #endif #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LL_EXT_ADV) case BLE_HCI_OCF_LE_SET_EXT_SCAN_PARAM: - if (len == BLE_HCI_LE_SET_EXT_SCAN_PARAM_LEN) { - rc = ble_ll_set_ext_scan_params(cmdbuf); - } + /* variable length */ + rc = ble_ll_set_ext_scan_params(cmdbuf, len); break; case BLE_HCI_OCF_LE_SET_EXT_SCAN_ENABLE: if (len == BLE_HCI_LE_SET_EXT_SCAN_ENABLE_LEN) { diff --git a/nimble/controller/src/ble_ll_scan.c b/nimble/controller/src/ble_ll_scan.c index d39305d98..82d80de8e 100644 --- a/nimble/controller/src/ble_ll_scan.c +++ b/nimble/controller/src/ble_ll_scan.c @@ -2632,7 +2632,7 @@ ble_ll_check_scan_params(uint8_t type, uint16_t itvl, uint16_t window) } int -ble_ll_set_ext_scan_params(uint8_t *cmd) +ble_ll_set_ext_scan_params(uint8_t *cmd, uint8_t cmdlen) { struct ble_ll_scan_params new_params[BLE_LL_SCAN_PHY_NUMBER] = { }; struct ble_ll_scan_params *uncoded = &new_params[PHY_UNCODED]; @@ -2640,6 +2640,10 @@ ble_ll_set_ext_scan_params(uint8_t *cmd) uint8_t idx; int rc; + if (cmdlen < 3) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + /* If already enabled, we return an error */ if (g_ble_ll_scan_sm.scan_enabled) { return BLE_ERR_CMD_DISALLOWED; @@ -2671,6 +2675,10 @@ ble_ll_set_ext_scan_params(uint8_t *cmd) idx = 3; if (cmd[2] & BLE_HCI_LE_PHY_1M_PREF_MASK) { + if (cmdlen < idx + 5) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + uncoded->scan_type = cmd[idx]; idx++; uncoded->scan_itvl = get_le16(cmd + idx); @@ -2691,6 +2699,10 @@ ble_ll_set_ext_scan_params(uint8_t *cmd) #if MYNEWT_VAL(BLE_LL_CFG_FEAT_LE_CODED_PHY) if (cmd[2] & BLE_HCI_LE_PHY_CODED_PREF_MASK) { + if (cmdlen < idx + 5) { + return BLE_ERR_INV_HCI_CMD_PARMS; + } + coded->scan_type = cmd[idx]; idx++; coded->scan_itvl = get_le16(cmd + idx);