From 39208448425d5f0875e5ee52fbcccfd799b83cb8 Mon Sep 17 00:00:00 2001 From: Joakim Andersson Date: Mon, 30 Sep 2019 13:21:11 +0200 Subject: [PATCH] Bluetooth: Host: Fix usage of re-using Bluetooth log buffers Fix calling bt_hex and bt_addr_le_str multiple times in the same logging call could result in string overwritten since log_strdup is not guaranteed to duplicate the string buffer in all logging configurations. Signed-off-by: Joakim Andersson --- subsys/bluetooth/common/log.h | 8 +++++ subsys/bluetooth/common/rpa.c | 3 +- subsys/bluetooth/controller/crypto/crypto.c | 6 ++-- subsys/bluetooth/host/crypto.c | 6 ++-- subsys/bluetooth/host/smp.c | 34 +++++++++++++-------- subsys/bluetooth/mesh/net.c | 4 +-- 6 files changed, 42 insertions(+), 19 deletions(-) diff --git a/subsys/bluetooth/common/log.h b/subsys/bluetooth/common/log.h index cff20ec37df..e090ba88ee3 100644 --- a/subsys/bluetooth/common/log.h +++ b/subsys/bluetooth/common/log.h @@ -65,10 +65,18 @@ LOG_MODULE_REGISTER(LOG_MODULE_NAME); #define BT_HEXDUMP_DBG(_data, _length, _str) \ LOG_HEXDUMP_DBG((const u8_t *)_data, _length, _str) +/* NOTE: These helper functions always encodes into the same buffer storage. + * It is the responsibility of the user of this function to copy the information + * in this string if needed. + */ const char *bt_hex_real(const void *buf, size_t len); const char *bt_addr_str_real(const bt_addr_t *addr); const char *bt_addr_le_str_real(const bt_addr_le_t *addr); +/* NOTE: log_strdup does not guarantee a duplication of the string. + * It is therefore still the responsibility of the user to handle the + * restrictions in the underlying function call. + */ #define bt_hex(buf, len) log_strdup(bt_hex_real(buf, len)) #define bt_addr_str(addr) log_strdup(bt_addr_str_real(addr)) #define bt_addr_le_str(addr) log_strdup(bt_addr_le_str_real(addr)) diff --git a/subsys/bluetooth/common/rpa.c b/subsys/bluetooth/common/rpa.c index 56f63dee926..9554818a234 100644 --- a/subsys/bluetooth/common/rpa.c +++ b/subsys/bluetooth/common/rpa.c @@ -33,7 +33,8 @@ static int ah(const u8_t irk[16], const u8_t r[3], u8_t out[3]) u8_t res[16]; int err; - BT_DBG("irk %s, r %s", bt_hex(irk, 16), bt_hex(r, 3)); + BT_DBG("irk %s", bt_hex(irk, 16)); + BT_DBG("r %s", bt_hex(r, 3)); /* r' = padding || r */ memcpy(res, r, 3); diff --git a/subsys/bluetooth/controller/crypto/crypto.c b/subsys/bluetooth/controller/crypto/crypto.c index 74abbde1bde..164647021e4 100644 --- a/subsys/bluetooth/controller/crypto/crypto.c +++ b/subsys/bluetooth/controller/crypto/crypto.c @@ -36,7 +36,8 @@ int bt_rand(void *buf, size_t len) int bt_encrypt_le(const u8_t key[16], const u8_t plaintext[16], u8_t enc_data[16]) { - BT_DBG("key %s plaintext %s", bt_hex(key, 16), bt_hex(plaintext, 16)); + BT_DBG("key %s", bt_hex(key, 16)); + BT_DBG("plaintext %s", bt_hex(plaintext, 16)); ecb_encrypt(key, plaintext, enc_data, NULL); @@ -48,7 +49,8 @@ int bt_encrypt_le(const u8_t key[16], const u8_t plaintext[16], int bt_encrypt_be(const u8_t key[16], const u8_t plaintext[16], u8_t enc_data[16]) { - BT_DBG("key %s plaintext %s", bt_hex(key, 16), bt_hex(plaintext, 16)); + BT_DBG("key %s", bt_hex(key, 16)); + BT_DBG("plaintext %s", bt_hex(plaintext, 16)); ecb_encrypt_be(key, plaintext, enc_data); diff --git a/subsys/bluetooth/host/crypto.c b/subsys/bluetooth/host/crypto.c index e614841c290..c482d322caf 100644 --- a/subsys/bluetooth/host/crypto.c +++ b/subsys/bluetooth/host/crypto.c @@ -119,7 +119,8 @@ int bt_encrypt_le(const u8_t key[16], const u8_t plaintext[16], struct tc_aes_key_sched_struct s; u8_t tmp[16]; - BT_DBG("key %s plaintext %s", bt_hex(key, 16), bt_hex(plaintext, 16)); + BT_DBG("key %s", bt_hex(key, 16)); + BT_DBG("plaintext %s", bt_hex(plaintext, 16)); sys_memcpy_swap(tmp, key, 16); @@ -145,7 +146,8 @@ int bt_encrypt_be(const u8_t key[16], const u8_t plaintext[16], { struct tc_aes_key_sched_struct s; - BT_DBG("key %s plaintext %s", bt_hex(key, 16), bt_hex(plaintext, 16)); + BT_DBG("key %s", bt_hex(key, 16)); + BT_DBG("plaintext %s", bt_hex(plaintext, 16)); if (tc_aes128_set_encrypt_key(&s, key) == TC_CRYPTO_FAIL) { return -EINVAL; diff --git a/subsys/bluetooth/host/smp.c b/subsys/bluetooth/host/smp.c index 157fe838585..d95a44e8b17 100644 --- a/subsys/bluetooth/host/smp.c +++ b/subsys/bluetooth/host/smp.c @@ -485,7 +485,8 @@ static int smp_f5(const u8_t *w, const u8_t *n1, const u8_t *n2, int err; BT_DBG("w %s", bt_hex(w, 32)); - BT_DBG("n1 %s n2 %s", bt_hex(n1, 16), bt_hex(n2, 16)); + BT_DBG("n1 %s", bt_hex(n1, 16)); + BT_DBG("n2 %s", bt_hex(n2, 16)); sys_memcpy_swap(ws, w, 32); @@ -536,9 +537,12 @@ static int smp_f6(const u8_t *w, const u8_t *n1, const u8_t *n2, int err; BT_DBG("w %s", bt_hex(w, 16)); - BT_DBG("n1 %s n2 %s", bt_hex(n1, 16), bt_hex(n2, 16)); - BT_DBG("r %s io_cap %s", bt_hex(r, 16), bt_hex(iocap, 3)); - BT_DBG("a1 %s a2 %s", bt_hex(a1, 7), bt_hex(a2, 7)); + BT_DBG("n1 %s", bt_hex(n1, 16)); + BT_DBG("n2 %s", bt_hex(n2, 16)); + BT_DBG("r %s", bt_hex(r, 16)); + BT_DBG("io_cap %s", bt_hex(iocap, 3)); + BT_DBG("a1 %s", bt_hex(a1, 7)); + BT_DBG("a2 %s", bt_hex(a2, 7)); sys_memcpy_swap(m, n1, 16); sys_memcpy_swap(m + 16, n2, 16); @@ -575,7 +579,8 @@ static int smp_g2(const u8_t u[32], const u8_t v[32], BT_DBG("u %s", bt_hex(u, 32)); BT_DBG("v %s", bt_hex(v, 32)); - BT_DBG("x %s y %s", bt_hex(x, 16), bt_hex(y, 16)); + BT_DBG("x %s", bt_hex(x, 16)); + BT_DBG("y %s", bt_hex(y, 16)); sys_memcpy_swap(m, u, 32); sys_memcpy_swap(m + 32, v, 32); @@ -1794,9 +1799,12 @@ static int smp_c1(const u8_t k[16], const u8_t r[16], u8_t p1[16], p2[16]; int err; - BT_DBG("k %s r %s", bt_hex(k, 16), bt_hex(r, 16)); - BT_DBG("ia %s ra %s", bt_addr_le_str(ia), bt_addr_le_str(ra)); - BT_DBG("preq %s pres %s", bt_hex(preq, 7), bt_hex(pres, 7)); + BT_DBG("k %s", bt_hex(k, 16)); + BT_DBG("r %s", bt_hex(r, 16)); + BT_DBG("ia %s", bt_addr_le_str(ia)); + BT_DBG("ra %s", bt_addr_le_str(ra)); + BT_DBG("preq %s", bt_hex(preq, 7)); + BT_DBG("pres %s", bt_hex(pres, 7)); /* pres, preq, rat and iat are concatenated to generate p1 */ p1[0] = ia->type; @@ -2215,7 +2223,8 @@ static u8_t legacy_pairing_random(struct bt_smp *smp) return BT_SMP_ERR_UNSPECIFIED; } - BT_DBG("pcnf %s cfm %s", bt_hex(smp->pcnf, 16), bt_hex(tmp, 16)); + BT_DBG("pcnf %s", bt_hex(smp->pcnf, 16)); + BT_DBG("cfm %s", bt_hex(tmp, 16)); if (memcmp(smp->pcnf, tmp, sizeof(smp->pcnf))) { return BT_SMP_ERR_CONFIRM_FAILED; @@ -3135,7 +3144,8 @@ static u8_t sc_smp_check_confirm(struct bt_smp *smp) return BT_SMP_ERR_UNSPECIFIED; } - BT_DBG("pcnf %s cfm %s", bt_hex(smp->pcnf, 16), bt_hex(cfm, 16)); + BT_DBG("pcnf %s", bt_hex(smp->pcnf, 16)); + BT_DBG("cfm %s", bt_hex(cfm, 16)); if (memcmp(smp->pcnf, cfm, 16)) { return BT_SMP_ERR_CONFIRM_FAILED; @@ -3393,8 +3403,8 @@ static u8_t smp_ident_addr_info(struct bt_smp *smp, struct net_buf *buf) BT_DBG("identity %s", bt_addr_le_str(&req->addr)); if (!bt_addr_le_is_identity(&req->addr)) { - BT_ERR("Invalid identity %s for %s", - bt_addr_le_str(&req->addr), bt_addr_le_str(&conn->le.dst)); + BT_ERR("Invalid identity %s", bt_addr_le_str(&req->addr)); + BT_ERR(" for %s", bt_addr_le_str(&conn->le.dst)); return BT_SMP_ERR_INVALID_PARAMS; } diff --git a/subsys/bluetooth/mesh/net.c b/subsys/bluetooth/mesh/net.c index 07ca58f9876..f2638a462e9 100644 --- a/subsys/bluetooth/mesh/net.c +++ b/subsys/bluetooth/mesh/net.c @@ -944,8 +944,8 @@ static bool auth_match(struct bt_mesh_subnet_keys *keys, net_auth); if (memcmp(auth, net_auth, 8)) { - BT_WARN("Authentication Value %s != %s", - bt_hex(auth, 8), bt_hex(net_auth, 8)); + BT_WARN("Authentication Value %s", bt_hex(auth, 8)); + BT_WARN(" != %s", bt_hex(net_auth, 8)); return false; }