From 291ebdd4e4ae7401cbfcc59c787a121e981ab3d2 Mon Sep 17 00:00:00 2001 From: Joakim Andersson Date: Wed, 29 Jan 2020 18:44:25 +0100 Subject: [PATCH] Bluetooth: Fix infinite recursion in host-based bt_rand Fix infinite recursion in host-based bt_rand function. This would call HCI LE Random Number command, which would in turn call bt_rand, causing an infinite recursion. bt_rand -> prng_reseed -> BT_HCI_OP_LE_RAND -> le_rand -> bt_rand To solve this issue the controller should avoid doing calls into the host, so all calls to bt_rand in the controller should be replaced with a call to a controller function. Fixes #22202 Signed-off-by: Joakim Andersson --- subsys/bluetooth/controller/crypto/crypto.c | 21 ++----------------- subsys/bluetooth/controller/hci/hci.c | 2 +- subsys/bluetooth/controller/ll_sw/ctrl.c | 8 +++---- .../bluetooth/controller/ll_sw/ull_master.c | 10 ++++----- subsys/bluetooth/controller/util/util.c | 19 +++++++++++++++++ subsys/bluetooth/controller/util/util.h | 4 ++++ subsys/bluetooth/host/crypto.c | 1 + 7 files changed, 36 insertions(+), 29 deletions(-) diff --git a/subsys/bluetooth/controller/crypto/crypto.c b/subsys/bluetooth/controller/crypto/crypto.c index 195f51aca1e..d9d9aa133e3 100644 --- a/subsys/bluetooth/controller/crypto/crypto.c +++ b/subsys/bluetooth/controller/crypto/crypto.c @@ -4,33 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -#include - #define BT_DBG_ENABLED IS_ENABLED(CONFIG_BT_DEBUG_HCI_DRIVER) #define LOG_MODULE_NAME bt_ctlr_crypto #include "common/log.h" +#include "../util/util.h" #include "hal/ecb.h" -static struct device *entropy_driver; - int bt_rand(void *buf, size_t len) { - struct device *dev = entropy_driver; - - if (unlikely(!dev)) { - /* Only one entropy device exists, so this is safe even - * if the whole operation isn't atomic. - */ - dev = device_get_binding(CONFIG_ENTROPY_NAME); - __ASSERT((dev != NULL), - "Device driver for %s (CONFIG_ENTROPY_NAME) not found. " - "Check your build configuration!", - CONFIG_ENTROPY_NAME); - entropy_driver = dev; - } - - return entropy_get_entropy(dev, (u8_t *)buf, len); + return util_rand(buf, len); } int bt_encrypt_le(const u8_t key[16], const u8_t plaintext[16], diff --git a/subsys/bluetooth/controller/hci/hci.c b/subsys/bluetooth/controller/hci/hci.c index 9467ae0dac2..4af635f79e0 100644 --- a/subsys/bluetooth/controller/hci/hci.c +++ b/subsys/bluetooth/controller/hci/hci.c @@ -868,7 +868,7 @@ static void le_rand(struct net_buf *buf, struct net_buf **evt) rp = hci_cmd_complete(evt, sizeof(*rp)); rp->status = 0x00; - bt_rand(rp->rand, count); + util_rand(rp->rand, count); } static void le_read_supp_states(struct net_buf *buf, struct net_buf **evt) diff --git a/subsys/bluetooth/controller/ll_sw/ctrl.c b/subsys/bluetooth/controller/ll_sw/ctrl.c index eaf2726aed7..62a0a4fdde1 100644 --- a/subsys/bluetooth/controller/ll_sw/ctrl.c +++ b/subsys/bluetooth/controller/ll_sw/ctrl.c @@ -6489,7 +6489,7 @@ again: LL_ASSERT(retry); retry--; - bt_rand(&access_addr, sizeof(u32_t)); + util_rand(&access_addr, sizeof(u32_t)); bit_idx = 31U; transitions = 0U; @@ -11992,7 +11992,7 @@ u32_t radio_connect_enable(u8_t adv_addr_type, u8_t *adv_addr, u16_t interval, conn->llcp_feature.features = LL_FEAT; access_addr = access_addr_get(); memcpy(&conn->access_addr[0], &access_addr, sizeof(conn->access_addr)); - bt_rand(&conn->crc_init[0], 3); + util_rand(&conn->crc_init[0], 3); memcpy(&conn->data_chan_map[0], &_radio.data_chan_map[0], sizeof(conn->data_chan_map)); conn->data_chan_count = _radio.data_chan_count; @@ -12333,8 +12333,8 @@ u8_t ll_enc_req_send(u16_t handle, u8_t *rand, u8_t *ediv, u8_t *ltk) memcpy(enc_req->rand, rand, sizeof(enc_req->rand)); enc_req->ediv[0] = ediv[0]; enc_req->ediv[1] = ediv[1]; - bt_rand(enc_req->skdm, sizeof(enc_req->skdm)); - bt_rand(enc_req->ivm, sizeof(enc_req->ivm)); + util_rand(enc_req->skdm, sizeof(enc_req->skdm)); + util_rand(enc_req->ivm, sizeof(enc_req->ivm)); } else if (conn->enc_rx && conn->enc_tx) { memcpy(&conn->llcp_enc.rand[0], rand, sizeof(conn->llcp_enc.rand)); diff --git a/subsys/bluetooth/controller/ll_sw/ull_master.c b/subsys/bluetooth/controller/ll_sw/ull_master.c index 17234040c89..c0a8cce4c09 100644 --- a/subsys/bluetooth/controller/ll_sw/ull_master.c +++ b/subsys/bluetooth/controller/ll_sw/ull_master.c @@ -101,7 +101,7 @@ u8_t ll_create_connection(u16_t scan_interval, u16_t scan_window, access_addr_get(access_addr); memcpy(conn_lll->access_addr, &access_addr, sizeof(conn_lll->access_addr)); - bt_rand(&conn_lll->crc_init[0], 3); + util_rand(&conn_lll->crc_init[0], 3); conn_lll->handle = 0xFFFF; conn_lll->interval = interval; @@ -156,7 +156,7 @@ u8_t ll_create_connection(u16_t scan_interval, u16_t scan_window, conn_lll->data_chan_count = ull_conn_chan_map_cpy(conn_lll->data_chan_map); - bt_rand(&hop, sizeof(u8_t)); + util_rand(&hop, sizeof(u8_t)); conn_lll->data_chan_hop = 5 + (hop % 12); conn_lll->data_chan_sel = 0; conn_lll->data_chan_use = 0; @@ -389,8 +389,8 @@ u8_t ll_enc_req_send(u16_t handle, u8_t *rand, u8_t *ediv, u8_t *ltk) memcpy(enc_req->rand, rand, sizeof(enc_req->rand)); enc_req->ediv[0] = ediv[0]; enc_req->ediv[1] = ediv[1]; - bt_rand(enc_req->skdm, sizeof(enc_req->skdm)); - bt_rand(enc_req->ivm, sizeof(enc_req->ivm)); + util_rand(enc_req->skdm, sizeof(enc_req->skdm)); + util_rand(enc_req->ivm, sizeof(enc_req->ivm)); } else if (conn->lll.enc_rx && conn->lll.enc_tx) { memcpy(&conn->llcp_enc.rand[0], rand, sizeof(conn->llcp_enc.rand)); @@ -709,7 +709,7 @@ again: LL_ASSERT(retry); retry--; - bt_rand(access_addr, 4); + util_rand(access_addr, 4); aa = sys_get_le32(access_addr); bit_idx = 31U; diff --git a/subsys/bluetooth/controller/util/util.c b/subsys/bluetooth/controller/util/util.c index f4db43d1b4b..9f9f76fae6b 100644 --- a/subsys/bluetooth/controller/util/util.c +++ b/subsys/bluetooth/controller/util/util.c @@ -6,6 +6,7 @@ */ #include +#include #include "util.h" /** @@ -36,3 +37,21 @@ u8_t util_ones_count_get(u8_t *octets, u8_t octets_len) return one_count; } + +int util_rand(void *buf, size_t len) +{ + static struct device *dev; + + if (unlikely(!dev)) { + /* Only one entropy device exists, so this is safe even + * if the whole operation isn't atomic. + */ + dev = device_get_binding(CONFIG_ENTROPY_NAME); + __ASSERT((dev != NULL), + "Device driver for %s (CONFIG_ENTROPY_NAME) not found. " + "Check your build configuration!", + CONFIG_ENTROPY_NAME); + } + + return entropy_get_entropy(dev, (u8_t *)buf, len); +} diff --git a/subsys/bluetooth/controller/util/util.h b/subsys/bluetooth/controller/util/util.h index 095417b716c..dddd80c2217 100644 --- a/subsys/bluetooth/controller/util/util.h +++ b/subsys/bluetooth/controller/util/util.h @@ -13,4 +13,8 @@ #define TRIPLE_BUFFER_SIZE 3 #endif +#include + u8_t util_ones_count_get(u8_t *octets, u8_t octets_len); + +int util_rand(void *buf, size_t len); diff --git a/subsys/bluetooth/host/crypto.c b/subsys/bluetooth/host/crypto.c index c482d322caf..d1ad31474d0 100644 --- a/subsys/bluetooth/host/crypto.c +++ b/subsys/bluetooth/host/crypto.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include