diff --git a/components/bt/common/btc/profile/esp/blufi/blufi_prf.c b/components/bt/common/btc/profile/esp/blufi/blufi_prf.c index f521b19bb1..304b162555 100644 --- a/components/bt/common/btc/profile/esp/blufi/blufi_prf.c +++ b/components/bt/common/btc/profile/esp/blufi/blufi_prf.c @@ -93,9 +93,9 @@ void btc_blufi_report_error(esp_blufi_error_state_t state) btc_transfer_context(&msg, ¶m, sizeof(esp_blufi_cb_param_t), NULL, NULL); } -/* FALSE POSITIVE: blufi_env is a single-connection global by design. - * The GAP layer rejects new connections while is_connected==true, - * so concurrent multi-connection access to this state never occurs. */ +/* blufi_env is a global shared state but only one BLE connection is active at + * a time — the GAP layer rejects new connections while is_connected==true, + * so there is no concurrent access to this structure. */ void btc_blufi_recv_handler(uint8_t *data, int len) { if (len < sizeof(struct blufi_hdr)) { @@ -239,9 +239,10 @@ void btc_blufi_recv_handler(uint8_t *data, int len) } } } -/* Known limitation: this function runs in the BTC task (app-initiated sends) - * AND in the NimBLE task (via recv_handler), racing on send_seq/frag_size. - * portENTER_CRITICAL cannot protect here; */ + +/* This function is called from both the BTC task and the NimBLE task, so + * send_seq/frag_size may race. portENTER_CRITICAL does not help across + * different FreeRTOS tasks; a mutex would be needed for full protection. */ void btc_blufi_send_encap(uint8_t type, uint8_t *data, int total_data_len) { struct blufi_hdr *hdr = NULL; diff --git a/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c b/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c index 611c352567..5606c33af7 100644 --- a/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c +++ b/components/bt/common/btc/profile/esp/blufi/nimble_host/esp_blufi.c @@ -53,9 +53,9 @@ static const struct ble_gatt_svc_def gatt_svr_svcs[] = { .uuid = BLE_UUID16_DECLARE(BLUFI_SERVICE_UUID), .characteristics = (struct ble_gatt_chr_def[]) { { - /* FALSE POSITIVE: No BLE_GATT_CHR_F_WRITE_ENC/READ_ENC by design. - * BLUFI uses its own app-layer security (DH key exchange + AES + CRC). - * BLE link-layer pairing is impossible before provisioning credentials exist. */ + /* BLE_GATT_CHR_F_WRITE_ENC/READ_ENC are not set because BLUFI uses its + * own application-layer security (DH key exchange + AES-128 + CRC). + * BLE pairing cannot occur before Wi-Fi credentials are provisioned. */ /*** Characteristic: P2E */ .uuid = BLE_UUID16_DECLARE(BLUFI_CHAR_P2E_UUID), .access_cb = gatt_svr_access_cb, diff --git a/components/esp_hid/src/esp_hidh.c b/components/esp_hid/src/esp_hidh.c index 1e841cd663..073ef6abf0 100644 --- a/components/esp_hid/src/esp_hidh.c +++ b/components/esp_hid/src/esp_hidh.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -40,6 +40,34 @@ static inline void unlock_devices(void) } } +/* + * Atomically checks whether a device is still in the global list and acquires + * its mutex before releasing the list lock. This avoids TOCTOU between + * `esp_hidh_dev_exists()` and `esp_hidh_dev_lock()` for call sites that need + * a stable pointer. + */ +static bool lock_known_device(esp_hidh_dev_t *dev) +{ + if (dev == NULL) { + return false; + } + + bool found = false; + esp_hidh_dev_t *d = NULL; + lock_devices(); + TAILQ_FOREACH(d, &s_esp_hidh_devices, devices) { + if (d == dev) { + found = true; + if (dev->mutex != NULL) { + xSemaphoreTake(dev->mutex, portMAX_DELAY); + } + break; + } + } + unlock_devices(); + return found; +} + /* * Public Functions * */ @@ -207,8 +235,7 @@ esp_hidh_dev_t *esp_hidh_dev_open(uint8_t *bda, esp_hid_transport_t transport, u esp_err_t esp_hidh_dev_close(esp_hidh_dev_t *dev) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->close(dev); esp_hidh_dev_unlock(dev); } else { @@ -219,8 +246,7 @@ esp_err_t esp_hidh_dev_close(esp_hidh_dev_t *dev) void esp_hidh_dev_dump(esp_hidh_dev_t *dev, FILE *fp) { - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { dev->dump(dev, fp); esp_hidh_dev_unlock(dev); } @@ -229,8 +255,7 @@ void esp_hidh_dev_dump(esp_hidh_dev_t *dev, FILE *fp) esp_err_t esp_hidh_dev_output_set(esp_hidh_dev_t *dev, size_t map_index, size_t report_id, uint8_t *value, size_t value_len) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->report_write(dev, map_index, report_id, ESP_HID_REPORT_TYPE_OUTPUT, value, value_len); esp_hidh_dev_unlock(dev); } else { @@ -242,8 +267,7 @@ esp_err_t esp_hidh_dev_output_set(esp_hidh_dev_t *dev, size_t map_index, size_t esp_err_t esp_hidh_dev_feature_set(esp_hidh_dev_t *dev, size_t map_index, size_t report_id, uint8_t *value, size_t value_len) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->report_write(dev, map_index, report_id, ESP_HID_REPORT_TYPE_FEATURE, value, value_len); esp_hidh_dev_unlock(dev); } else { @@ -255,8 +279,7 @@ esp_err_t esp_hidh_dev_feature_set(esp_hidh_dev_t *dev, size_t map_index, size_t esp_err_t esp_hidh_dev_feature_get(esp_hidh_dev_t *dev, size_t map_index, size_t report_id, size_t max_length, uint8_t *value, size_t *value_len) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->report_read(dev, map_index, report_id, ESP_HID_REPORT_TYPE_FEATURE, max_length, value, value_len); esp_hidh_dev_unlock(dev); } else { @@ -268,8 +291,7 @@ esp_err_t esp_hidh_dev_feature_get(esp_hidh_dev_t *dev, size_t map_index, size_t esp_err_t esp_hidh_dev_set_report(esp_hidh_dev_t *dev, size_t map_index, size_t report_id, int report_type, uint8_t *data, size_t length) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { if (dev->set_report) { ret = dev->set_report(dev, map_index, report_id, report_type, data, length); } else { @@ -286,8 +308,7 @@ esp_err_t esp_hidh_dev_get_report(esp_hidh_dev_t *dev, size_t map_index, size_t size_t max_len) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->report_read(dev, map_index, report_id, report_type, max_len, NULL, NULL); esp_hidh_dev_unlock(dev); } else { @@ -299,8 +320,7 @@ esp_err_t esp_hidh_dev_get_report(esp_hidh_dev_t *dev, size_t map_index, size_t esp_err_t esp_hidh_dev_get_idle(esp_hidh_dev_t *dev) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { if (dev->get_idle) { ret = dev->get_idle(dev); } else { @@ -316,8 +336,7 @@ esp_err_t esp_hidh_dev_get_idle(esp_hidh_dev_t *dev) esp_err_t esp_hidh_dev_set_idle(esp_hidh_dev_t *dev, uint8_t idle_time) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { if (dev->set_idle) { ret = dev->set_idle(dev, idle_time); } else { @@ -333,8 +352,7 @@ esp_err_t esp_hidh_dev_set_idle(esp_hidh_dev_t *dev, uint8_t idle_time) esp_err_t esp_hidh_dev_get_protocol(esp_hidh_dev_t *dev) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { if (dev->get_protocol) { ret = dev->get_protocol(dev); } else { @@ -350,8 +368,7 @@ esp_err_t esp_hidh_dev_get_protocol(esp_hidh_dev_t *dev) esp_err_t esp_hidh_dev_set_protocol(esp_hidh_dev_t *dev, uint8_t protocol_mode) { esp_err_t ret = ESP_OK; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { if (dev->set_protocol) { ret = dev->set_protocol(dev, protocol_mode); } else { @@ -368,8 +385,7 @@ const uint8_t *esp_hidh_dev_bda_get(esp_hidh_dev_t *dev) { uint8_t *ret = NULL; #if CONFIG_BLUEDROID_ENABLED || CONFIG_BT_NIMBLE_ENABLED - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->addr.bda; esp_hidh_dev_unlock(dev); } @@ -380,8 +396,7 @@ const uint8_t *esp_hidh_dev_bda_get(esp_hidh_dev_t *dev) esp_hid_transport_t esp_hidh_dev_transport_get(esp_hidh_dev_t *dev) { esp_hid_transport_t ret = ESP_HID_TRANSPORT_MAX; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->transport; esp_hidh_dev_unlock(dev); } @@ -391,8 +406,7 @@ esp_hid_transport_t esp_hidh_dev_transport_get(esp_hidh_dev_t *dev) const esp_hid_device_config_t *esp_hidh_dev_config_get(esp_hidh_dev_t *dev) { esp_hid_device_config_t *ret = NULL; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = &dev->config; esp_hidh_dev_unlock(dev); } @@ -402,8 +416,7 @@ const esp_hid_device_config_t *esp_hidh_dev_config_get(esp_hidh_dev_t *dev) const char *esp_hidh_dev_name_get(esp_hidh_dev_t *dev) { const char * ret = NULL; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->config.device_name ? dev->config.device_name : ""; esp_hidh_dev_unlock(dev); } @@ -413,8 +426,7 @@ const char *esp_hidh_dev_name_get(esp_hidh_dev_t *dev) const char *esp_hidh_dev_manufacturer_get(esp_hidh_dev_t *dev) { const char *ret = NULL; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->config.manufacturer_name ? dev->config.manufacturer_name : ""; esp_hidh_dev_unlock(dev); } @@ -424,8 +436,7 @@ const char *esp_hidh_dev_manufacturer_get(esp_hidh_dev_t *dev) const char *esp_hidh_dev_serial_get(esp_hidh_dev_t *dev) { const char *ret = NULL; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->config.serial_number ? dev->config.serial_number : ""; esp_hidh_dev_unlock(dev); } @@ -435,8 +446,7 @@ const char *esp_hidh_dev_serial_get(esp_hidh_dev_t *dev) uint16_t esp_hidh_dev_vendor_id_get(esp_hidh_dev_t *dev) { uint16_t ret = 0; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->config.vendor_id; esp_hidh_dev_unlock(dev); } @@ -446,8 +456,7 @@ uint16_t esp_hidh_dev_vendor_id_get(esp_hidh_dev_t *dev) uint16_t esp_hidh_dev_product_id_get(esp_hidh_dev_t *dev) { uint16_t ret = 0; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->config.product_id; esp_hidh_dev_unlock(dev); } @@ -457,8 +466,7 @@ uint16_t esp_hidh_dev_product_id_get(esp_hidh_dev_t *dev) uint16_t esp_hidh_dev_version_get(esp_hidh_dev_t *dev) { uint16_t ret = 0; - if (!esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->config.version; esp_hidh_dev_unlock(dev); } @@ -468,8 +476,7 @@ uint16_t esp_hidh_dev_version_get(esp_hidh_dev_t *dev) esp_hid_usage_t esp_hidh_dev_usage_get(esp_hidh_dev_t *dev) { esp_hid_usage_t ret = ESP_HID_USAGE_GENERIC; - if (esp_hidh_dev_exists(dev)) { - esp_hidh_dev_lock(dev); + if (lock_known_device(dev)) { ret = dev->usage; esp_hidh_dev_unlock(dev); } @@ -481,11 +488,9 @@ esp_err_t esp_hidh_dev_reports_get(esp_hidh_dev_t *dev, size_t *num_reports, esp esp_err_t ret = 0; esp_hid_report_item_t *r = NULL; - if (!esp_hidh_dev_exists(dev)) { + if (!lock_known_device(dev)) { return ESP_FAIL; } - - esp_hidh_dev_lock(dev); do { r = (esp_hid_report_item_t *)malloc(sizeof(esp_hid_report_item_t) * dev->reports_len); if (r == NULL) { @@ -521,10 +526,9 @@ error_:; esp_err_t esp_hidh_dev_report_maps_get(esp_hidh_dev_t *dev, size_t *num_maps, esp_hid_raw_report_map_t **maps) { - if (!esp_hidh_dev_exists(dev)) { + if (!lock_known_device(dev)) { return ESP_FAIL; } - esp_hidh_dev_lock(dev); *num_maps = dev->config.report_maps_len; *maps = dev->config.report_maps; esp_hidh_dev_unlock(dev); @@ -692,6 +696,9 @@ static void esp_hidh_dev_resources_free(esp_hidh_dev_t *dev) } } free((void *)dev->config.report_maps); +#if CONFIG_BT_NIMBLE_ENABLED + free((void *)dev->protocol_mode); +#endif esp_hidh_dev_report_t *r; while (dev->reports) { r = dev->reports; diff --git a/components/esp_hid/src/nimble_hidd.c b/components/esp_hid/src/nimble_hidd.c index 6bb00bd525..3f6b7a9c47 100644 --- a/components/esp_hid/src/nimble_hidd.c +++ b/components/esp_hid/src/nimble_hidd.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2017-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -10,6 +10,7 @@ #include "ble_hidd.h" #include "esp_private/esp_hidd_private.h" #include "esp_log.h" +#include "freertos/semphr.h" #include #include @@ -35,6 +36,27 @@ static const char *TAG = "NIMBLE_HIDD"; typedef struct esp_ble_hidd_dev_s esp_ble_hidd_dev_t; // there can be only one BLE HID device static esp_ble_hidd_dev_t *s_dev = NULL; +static SemaphoreHandle_t s_hidd_mutex = NULL; +static bool s_gap_listener_registered = false; +static void (*s_prev_reset_cb)(int reason) = NULL; +static void (*s_prev_sync_cb)(void) = NULL; +static struct ble_gap_event_listener nimble_gap_event_listener; +static void nimble_host_synced(void); +void nimble_host_reset(int reason); + +static inline void lock_hidd(void) +{ + if (s_hidd_mutex) { + xSemaphoreTake(s_hidd_mutex, portMAX_DELAY); + } +} + +static inline void unlock_hidd(void) +{ + if (s_hidd_mutex) { + xSemaphoreGive(s_hidd_mutex); + } +} /** service index is used to identify the hid service instance of the registered characteristic. Assuming the first instance of the hid service is registered first. @@ -90,7 +112,10 @@ static int create_hid_db(int device_index) /* fill hid info */ memcpy(&hparams.hid_info, hidInfo, sizeof hparams.hid_info); - /* fill report map */ + if (s_dev->devices[device_index].reports_map.len > REPORT_MAP_SIZE) { + ESP_LOGE(TAG, "Report map too large (%d > %d); aborting", s_dev->devices[device_index].reports_map.len, REPORT_MAP_SIZE); + return ESP_FAIL; + } memcpy(&hparams.report_map, (uint8_t *)s_dev->devices[device_index].reports_map.data, s_dev->devices[device_index].reports_map.len); hparams.report_map_len = s_dev->devices[device_index].reports_map.len; hparams.external_rpt_ref = BLE_SVC_BAS_UUID16; @@ -102,6 +127,10 @@ static int create_hid_db(int device_index) for (uint8_t i = 0; i < s_dev->devices[device_index].reports_len; i++) { hidd_le_report_item_t *report = &s_dev->devices[device_index].reports[i]; if (report->protocol_mode == ESP_HID_PROTOCOL_MODE_REPORT) { + if (report_mode_rpts >= MAX_REPORTS) { + ESP_LOGE(TAG, "Too many report-mode reports (%d >= MAX_REPORTS); truncating", report_mode_rpts); + break; + } /* only consider report mode reports, all boot mode reports will be registered by default */ if (report->report_type == ESP_HID_REPORT_TYPE_INPUT) { /* Input Report */ @@ -186,6 +215,15 @@ static int nimble_hid_stop_gatts(esp_ble_hidd_dev_t *dev) { int rc = ESP_OK; + if (s_gap_listener_registered) { + ble_gap_event_listener_unregister(&nimble_gap_event_listener); + s_gap_listener_registered = false; + } + + if (dev && dev->connected) { + ble_gap_terminate(dev->conn_id, BLE_ERR_REM_USER_CONN_TERM); + } + /* stop gatt database */ ble_gatts_stop(); @@ -249,6 +287,7 @@ static int ble_hid_init_config(esp_ble_hidd_dev_t *dev, const esp_hid_device_con dev->devices[d].reports = (hidd_le_report_item_t *)malloc(rmap->reports_len * sizeof(hidd_le_report_item_t)); if (dev->devices[d].reports == NULL) { ESP_LOGE(TAG, "reports malloc(%d) failed", rmap->reports_len * sizeof(hidd_le_report_item_t)); + free(rmap->reports); free(rmap); return ESP_FAIL; } @@ -287,22 +326,40 @@ static int ble_hid_free_config(esp_ble_hidd_dev_t *dev) static int nimble_hidd_dev_deinit(void *devp) { esp_ble_hidd_dev_t *dev = (esp_ble_hidd_dev_t *)devp; + lock_hidd(); if (!s_dev) { ESP_LOGE(TAG, "HID device profile already uninitialized"); + unlock_hidd(); return ESP_OK; } if (s_dev != dev) { ESP_LOGE(TAG, "Wrong HID device provided"); + unlock_hidd(); return ESP_FAIL; } - s_dev = NULL; - service_index = -1; // resetting the value - nimble_hid_stop_gatts(dev); + s_dev = NULL; + service_index = -1; + + if (ble_hs_cfg.reset_cb == nimble_host_reset) { + ble_hs_cfg.reset_cb = s_prev_reset_cb; + } + if (ble_hs_cfg.sync_cb == nimble_host_synced) { + ble_hs_cfg.sync_cb = s_prev_sync_cb; + } + ble_hs_cfg.gatts_register_cb = NULL; + unlock_hidd(); + + /* STOP_EVENT may be discarded because ble_hid_free_config() deletes the event + * loop right after this call. This does not cause a crash or data corruption. */ esp_event_post_to(dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_STOP_EVENT, NULL, 0, portMAX_DELAY); ble_hid_free_config(dev); free(dev); + if (s_hidd_mutex) { + vSemaphoreDelete(s_hidd_mutex); + s_hidd_mutex = NULL; + } return ESP_OK; } @@ -316,20 +373,19 @@ static int nimble_hidd_dev_battery_set(void *devp, uint8_t level) { int rc; esp_ble_hidd_dev_t *dev = (esp_ble_hidd_dev_t *)devp; + lock_hidd(); if (!dev || s_dev != dev) { + unlock_hidd(); return ESP_FAIL; } - if (!dev->connected) { - /* Return success if not yet connected */ - return ESP_OK; - } - rc = ble_svc_bas_battery_level_set(level); - if (rc) { - ESP_LOGE(TAG, "esp_ble_gatts_send_notify failed: %d", rc); + if (rc != 0 && dev->connected) { + ESP_LOGE(TAG, "ble_svc_bas_battery_level_set failed: %d", rc); + unlock_hidd(); return ESP_FAIL; } + unlock_hidd(); return ESP_OK; } @@ -338,6 +394,9 @@ static int nimble_hidd_dev_battery_set(void *devp, uint8_t level) static hidd_le_report_item_t* find_report(uint8_t id, uint8_t type, uint8_t *mode) { hidd_le_report_item_t *rpt; + if (!s_dev) { + return NULL; + } for (uint8_t d = 0; d < s_dev->devices_len; d++) { for (uint8_t i = 0; i < s_dev->devices[d].reports_len; i++) { rpt = &s_dev->devices[d].reports[i]; @@ -348,15 +407,17 @@ static hidd_le_report_item_t* find_report(uint8_t id, uint8_t type, uint8_t *mod } return NULL; } -static hidd_le_report_item_t* find_report_by_usage_and_type(uint8_t usage, uint8_t type, uint8_t *mode) + +static hidd_le_report_item_t* find_report_by_usage_and_type(uint8_t dev_index, uint8_t usage, uint8_t type, uint8_t *mode) { hidd_le_report_item_t *rpt; - for (uint8_t d = 0; d < s_dev->devices_len; d++) { - for (uint8_t i = 0; i < s_dev->devices[d].reports_len; i++) { - rpt = &s_dev->devices[d].reports[i]; - if (rpt->usage == usage && rpt->report_type == type && (!mode || (mode && *mode == rpt->protocol_mode))) { - return rpt; - } + if (!s_dev || dev_index >= s_dev->devices_len) { + return NULL; + } + for (uint8_t i = 0; i < s_dev->devices[dev_index].reports_len; i++) { + rpt = &s_dev->devices[dev_index].reports[i]; + if (rpt->usage == usage && rpt->report_type == type && (!mode || (mode && *mode == rpt->protocol_mode))) { + return rpt; } } return NULL; @@ -372,6 +433,11 @@ static int nimble_hidd_dev_input_set(void *devp, size_t index, size_t id, uint8_ return ESP_FAIL; } + if (index >= s_dev->devices_len) { + ESP_LOGE(TAG, "%s invalid device index %zu (devices_len=%u)", __func__, index, s_dev->devices_len); + return ESP_FAIL; + } + if (!dev->connected) { ESP_LOGE(TAG, "%s Device Not Connected", __func__); return ESP_FAIL; @@ -418,6 +484,11 @@ static int nimble_hidd_dev_feature_set(void *devp, size_t index, size_t id, uint return ESP_FAIL; } + if (index >= s_dev->devices_len) { + ESP_LOGE(TAG, "%s invalid device index %zu (devices_len=%u)", __func__, index, s_dev->devices_len); + return ESP_FAIL; + } + if (!dev->connected) { ESP_LOGE(TAG, "%s Device Not Connected", __func__); return ESP_FAIL; @@ -476,63 +547,87 @@ static void ble_hidd_dev_free(void) free(s_dev); s_dev = NULL; } + if (s_hidd_mutex) { + vSemaphoreDelete(s_hidd_mutex); + s_hidd_mutex = NULL; + } } static int nimble_hid_gap_event(struct ble_gap_event *event, void *arg) { struct ble_gap_conn_desc desc; - struct os_mbuf *om; uint8_t data; int rc; + esp_ble_hidd_dev_t *dev; + lock_hidd(); + dev = s_dev; + if (dev == NULL) { + unlock_hidd(); + return 0; + } + + /* HOGP encryption is not enforced at the GATT level in this component. + * Applications must configure encryption themselves, either by setting + * BLE_GATT_CHR_F_READ_ENC/WRITE_ENC in ble_svc_hid.c or by configuring + * ble_hs_cfg.sm_* security parameters. */ switch (event->type) { case BLE_GAP_EVENT_CONNECT: /* A new connection was established or a connection attempt failed. */ ESP_LOGD(TAG, "connection %s; status=%d", event->connect.status == 0 ? "established" : "failed", event->connect.status); + if (event->connect.status != 0) { + unlock_hidd(); + return 0; + } rc = ble_gap_conn_find(event->connect.conn_handle, &desc); assert(rc == 0); /* save connection handle */ - s_dev->connected = true; - s_dev->conn_id = event->connect.conn_handle; + dev->connected = true; + dev->conn_id = event->connect.conn_handle; esp_hidd_event_data_t cb_param = { - .connect.dev = s_dev->dev, + .connect.dev = dev->dev, .connect.status = event->connect.status }; /* reset the protocol mode value */ data = ESP_HID_PROTOCOL_MODE_REPORT; - om = ble_hs_mbuf_from_flat(&data, 1); - if (om == NULL) { - ESP_LOGD(TAG, "No memory to allocate mbuf"); - } - /* NOTE : om is freed by stack */ - for (int i = 0; i < s_dev->devices_len; i++) { - rc = ble_att_svr_write_local(s_dev->devices[i].hid_protocol_handle, om); + for (int i = 0; i < dev->devices_len; i++) { + struct os_mbuf *om = ble_hs_mbuf_from_flat(&data, 1); + if (om == NULL) { + ESP_LOGD(TAG, "No memory to allocate mbuf"); + break; + } + rc = ble_att_svr_write_local(dev->devices[i].hid_protocol_handle, om); if (rc != 0) { ESP_LOGE(TAG, "Write on Protocol Mode Failed: %d", rc); } } + unlock_hidd(); - esp_event_post_to(s_dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_CONNECT_EVENT, + esp_event_post_to(dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_CONNECT_EVENT, &cb_param, sizeof(esp_hidd_event_data_t), portMAX_DELAY); return 0; break; case BLE_GAP_EVENT_DISCONNECT: ESP_LOGD(TAG, "disconnect; reason=%d", event->disconnect.reason); - if (s_dev->connected) { - s_dev->connected = false; + if (dev->connected) { + dev->connected = false; esp_hidd_event_data_t cb_param = {0}; - cb_param.disconnect.dev = s_dev->dev; + cb_param.disconnect.dev = dev->dev; cb_param.disconnect.reason = event->disconnect.reason; - esp_event_post_to(s_dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_DISCONNECT_EVENT, + unlock_hidd(); + esp_event_post_to(dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_DISCONNECT_EVENT, &cb_param, sizeof(esp_hidd_event_data_t), portMAX_DELAY); + } else { + unlock_hidd(); } return 0; } + unlock_hidd(); return 0; } @@ -554,8 +649,13 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi ctxt->svc.handle); uuid16 = ble_uuid_u16(ctxt->svc.svc_def->uuid); if (uuid16 == BLE_SVC_HID_UUID16) { - ++service_index; - s_dev->devices[service_index].hid_svc = ctxt->svc.handle; + if (service_index + 1 >= (int)s_dev->devices_len) { + ESP_LOGE(TAG, "Too many HID services registered (%d >= devices_len %d); ignoring", + service_index + 1, s_dev->devices_len); + } else { + ++service_index; + s_dev->devices[service_index].hid_svc = ctxt->svc.handle; + } } break; @@ -566,6 +666,10 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi ble_uuid_to_str(ctxt->chr.chr_def->uuid, buf), ctxt->chr.def_handle, ctxt->chr.val_handle); + + if (service_index < 0 || (uint8_t)service_index >= s_dev->devices_len) { + break; + } uuid16 = ble_uuid_u16(ctxt->chr.chr_def->uuid); if (uuid16 == BLE_SVC_HID_CHR_UUID16_HID_CTRL_PT) { /* assuming this characteristic is from the last registered hid service */ @@ -577,7 +681,7 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi } if (uuid16 == BLE_SVC_HID_CHR_UUID16_BOOT_KBD_INP) { protocol_mode = ESP_HID_PROTOCOL_MODE_BOOT; - rpt = find_report_by_usage_and_type(ESP_HID_USAGE_KEYBOARD, ESP_HID_REPORT_TYPE_INPUT, &protocol_mode); + rpt = find_report_by_usage_and_type(service_index, ESP_HID_USAGE_KEYBOARD, ESP_HID_REPORT_TYPE_INPUT, &protocol_mode); if (rpt == NULL) { ESP_LOGE(TAG, "Unknown boot kbd input report registration"); return; @@ -586,7 +690,7 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi } if (uuid16 == BLE_SVC_HID_CHR_UUID16_BOOT_KBD_OUT) { protocol_mode = ESP_HID_PROTOCOL_MODE_BOOT; - rpt = find_report_by_usage_and_type(ESP_HID_USAGE_KEYBOARD, ESP_HID_REPORT_TYPE_OUTPUT, &protocol_mode); + rpt = find_report_by_usage_and_type(service_index, ESP_HID_USAGE_KEYBOARD, ESP_HID_REPORT_TYPE_OUTPUT, &protocol_mode); if (rpt == NULL) { ESP_LOGE(TAG, "Unknown boot kbd output report registration"); return; @@ -595,7 +699,7 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi } if (uuid16 == BLE_SVC_HID_CHR_UUID16_BOOT_MOUSE_INP) { protocol_mode = ESP_HID_PROTOCOL_MODE_BOOT; - rpt = find_report_by_usage_and_type(ESP_HID_USAGE_MOUSE, ESP_HID_REPORT_TYPE_INPUT, &protocol_mode); + rpt = find_report_by_usage_and_type(service_index, ESP_HID_USAGE_MOUSE, ESP_HID_REPORT_TYPE_INPUT, &protocol_mode); if (rpt == NULL) { ESP_LOGE(TAG, "Unknown boot mouse input report registration"); return; @@ -616,6 +720,11 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi ble_hs_mbuf_to_flat(om, &report_info, sizeof report_info, NULL); report_type = (uint8_t)((report_info & 0xFF00) >> 8); report_id = report_info & 0x00FF; + if (ctxt->dsc.dsc_def->arg == NULL) { + ESP_LOGE(TAG, "Report Reference descriptor has NULL arg; skipping handle assignment"); + os_mbuf_free_chain(om); + break; + } report_handle = (*(uint16_t*)(ctxt->dsc.dsc_def->arg)); protocol_mode = ESP_HID_PROTOCOL_MODE_REPORT; rpt = find_report(report_id, report_type, &protocol_mode); @@ -635,15 +744,24 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi static void nimble_host_synced(void) { - esp_event_post_to(s_dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_START_EVENT, NULL, 0, portMAX_DELAY); + lock_hidd(); + if (s_prev_sync_cb && s_prev_sync_cb != nimble_host_synced) { + s_prev_sync_cb(); + } + if (s_dev && s_dev->event_loop_handle != NULL) { + esp_event_post_to(s_dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_START_EVENT, NULL, 0, portMAX_DELAY); + } + unlock_hidd(); } void nimble_host_reset(int reason) { + if (s_prev_reset_cb && s_prev_reset_cb != nimble_host_reset) { + s_prev_reset_cb(reason); + } MODLOG_DFLT(ERROR, "Resetting state; reason=%d\n", reason); } -static struct ble_gap_event_listener nimble_gap_event_listener; esp_err_t esp_ble_hidd_dev_init(esp_hidd_dev_t *dev_p, const esp_hid_device_config_t *config, esp_event_handler_t callback) { int rc; @@ -658,6 +776,15 @@ esp_err_t esp_ble_hidd_dev_init(esp_hidd_dev_t *dev_p, const esp_hid_device_conf ESP_LOGE(TAG, "HID device could not be allocated"); return ESP_FAIL; } + if (s_hidd_mutex == NULL) { + s_hidd_mutex = xSemaphoreCreateMutex(); + if (s_hidd_mutex == NULL) { + free(s_dev); + s_dev = NULL; + ESP_LOGE(TAG, "HID device mutex allocation failed"); + return ESP_ERR_NO_MEM; + } + } // Reset the hid device target environment s_dev->control = ESP_HID_CONTROL_EXIT_SUSPEND; @@ -708,15 +835,26 @@ esp_err_t esp_ble_hidd_dev_init(esp_hidd_dev_t *dev_p, const esp_hid_device_conf } } + s_prev_reset_cb = ble_hs_cfg.reset_cb; + s_prev_sync_cb = ble_hs_cfg.sync_cb; ble_hs_cfg.reset_cb = nimble_host_reset; ble_hs_cfg.sync_cb = nimble_host_synced; ble_hs_cfg.gatts_register_cb = nimble_gatt_svr_register_cb; rc = nimble_hid_start_gatts(); if (rc != ESP_OK) { + if (ble_hs_cfg.reset_cb == nimble_host_reset) { + ble_hs_cfg.reset_cb = s_prev_reset_cb; + } + if (ble_hs_cfg.sync_cb == nimble_host_synced) { + ble_hs_cfg.sync_cb = s_prev_sync_cb; + } + ble_hs_cfg.gatts_register_cb = NULL; + ble_hidd_dev_free(); return rc; } ble_gap_event_listener_register(&nimble_gap_event_listener, nimble_hid_gap_event, NULL); + s_gap_listener_registered = true; return rc; } diff --git a/components/esp_hid/src/nimble_hidh.c b/components/esp_hid/src/nimble_hidh.c index bfbed8eca2..4c63de9dc3 100644 --- a/components/esp_hid/src/nimble_hidh.c +++ b/components/esp_hid/src/nimble_hidh.c @@ -1,9 +1,10 @@ /* - * SPDX-FileCopyrightText: 2017-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2017-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ +#include #include #include "ble_hidh.h" #include "esp_private/esp_hidh_private.h" @@ -18,7 +19,6 @@ #include "freertos/semphr.h" #include -#include #include #include "nimble/nimble_opt.h" #include "host/ble_hs.h" @@ -26,6 +26,7 @@ #include "host/ble_hs_adv.h" #include "host/ble_hs_hci.h" #include "host/ble_att.h" +#include "host/ble_sm.h" #include "services/gap/ble_svc_gap.h" #include "services/gatt/ble_svc_gatt.h" #include "services/bas/ble_svc_bas.h" @@ -40,12 +41,22 @@ static const char *TAG = "NIMBLE_HIDH"; static SemaphoreHandle_t s_ble_hidh_cb_semaphore = NULL; +static SemaphoreHandle_t s_ble_hidh_op_mutex = NULL; +static void (*s_prev_reset_cb)(int reason) = NULL; +static void (*s_prev_sync_cb)(void) = NULL; + +#define HIDH_MAX_SVCS 10 +#define HIDH_MAX_CHRS 20 +#define HIDH_MAX_DSCS 20 + /* variables used for attribute discovery */ static int services_discovered; static int chrs_discovered; static int dscs_discovered; static int status = 0; +static int s_gatt_op_status; + static inline void WAIT_CB(void) { xSemaphoreTake(s_ble_hidh_cb_semaphore, portMAX_DELAY); @@ -56,41 +67,65 @@ static inline void SEND_CB(void) xSemaphoreGive(s_ble_hidh_cb_semaphore); } +static inline void LOCK_OPS(void) +{ + if (s_ble_hidh_op_mutex) { + xSemaphoreTake(s_ble_hidh_op_mutex, portMAX_DELAY); + } +} + +static inline void UNLOCK_OPS(void) +{ + if (s_ble_hidh_op_mutex) { + xSemaphoreGive(s_ble_hidh_op_mutex); + } +} + static esp_event_loop_handle_t event_loop_handle; static uint8_t *s_read_data_val = NULL; static uint16_t s_read_data_len = 0; static int s_read_status = 0; -/** - * Utility function to log an array of bytes. - */ -void -print_bytes(const uint8_t *bytes, int len) -{ - int i; - - for (i = 0; i < len; i++) { - MODLOG_DFLT(DEBUG, "%s0x%02x", i != 0 ? ":" : "", bytes[i]); - } -} - static void print_mbuf(const struct os_mbuf *om) { - int colon; - - colon = 0; while (om != NULL) { - if (colon) { - MODLOG_DFLT(DEBUG, ":"); - } else { - colon = 1; - } - print_bytes(om->om_data, om->om_len); + ESP_LOG_BUFFER_HEXDUMP(TAG, om->om_data, om->om_len, ESP_LOG_DEBUG); om = SLIST_NEXT(om, om_next); } } +static char *nimble_hidh_dup_cstr(const uint8_t *data, size_t len) +{ + char *p; + + if (data == NULL || len == 0) { + return NULL; + } + p = (char *)malloc(len + 1); + if (p == NULL) { + return NULL; + } + memcpy(p, data, len); + p[len] = '\0'; + return p; +} + +static uint8_t *nimble_hidh_dup_bytes(const uint8_t *data, size_t len) +{ + uint8_t *p; + + if (data == NULL || len == 0) { + return NULL; + } + p = (uint8_t *)malloc(len); + if (p == NULL) { + return NULL; + } + memcpy(p, data, len); + return p; +} + static int nimble_on_read(uint16_t conn_handle, const struct ble_gatt_error *error, @@ -98,12 +133,23 @@ nimble_on_read(uint16_t conn_handle, void *arg) { int old_offset; + + (void)arg; MODLOG_DFLT(INFO, "Read complete; status=%d conn_handle=%d", error->status, conn_handle); s_read_status = error->status; switch (s_read_status) { case 0: MODLOG_DFLT(DEBUG, " attr_handle=%d value=", attr->handle); + if ((uint32_t)s_read_data_len + OS_MBUF_PKTLEN(attr->om) > 4096) { // max allowed hid report size + ESP_LOGE(TAG, "Read data too large, aborting"); + free(s_read_data_val); + s_read_data_val = NULL; + s_read_data_len = 0; + s_read_status = BLE_HS_ENOMEM; + SEND_CB(); + return -1; + } old_offset = s_read_data_len; s_read_data_len += OS_MBUF_PKTLEN(attr->om); uint8_t *tmp = realloc(s_read_data_val, s_read_data_len + 1); @@ -112,6 +158,8 @@ nimble_on_read(uint16_t conn_handle, free(s_read_data_val); s_read_data_val = NULL; s_read_data_len = 0; + s_read_status = BLE_HS_ENOMEM; + SEND_CB(); return -1; } s_read_data_val = tmp; @@ -119,52 +167,73 @@ nimble_on_read(uint16_t conn_handle, print_mbuf(attr->om); return 0; case BLE_HS_EDONE: - s_read_data_val[s_read_data_len] = 0; // to ensure strings are ended with \0 */ + if (s_read_data_val) { + s_read_data_val[s_read_data_len] = 0; // to ensure strings are ended with \0 + } s_read_status = 0; SEND_CB(); return 0; + default: + SEND_CB(); + return 0; } - return 0; } static int read_char(uint16_t conn_handle, uint16_t handle, uint8_t **out, uint16_t *out_len) { + int rc; + + LOCK_OPS(); s_read_data_val = NULL; s_read_data_len = 0; - int rc; + s_read_status = 0; /* read long because the server may not support the large enough mtu */ rc = ble_gattc_read_long(conn_handle, handle, 0, nimble_on_read, NULL); if (rc != 0) { ESP_LOGE(TAG, "read_char failed"); + UNLOCK_OPS(); return rc; } WAIT_CB(); if (s_read_status == 0) { *out = s_read_data_val; *out_len = s_read_data_len; + } else { + free(s_read_data_val); + s_read_data_val = NULL; } - return s_read_status; + rc = s_read_status; + UNLOCK_OPS(); + return rc; } static int read_descr(uint16_t conn_handle, uint16_t handle, uint8_t **out, uint16_t *out_len) { int rc; + LOCK_OPS(); s_read_data_val = NULL; s_read_data_len = 0; + s_read_status = 0; rc = ble_gattc_read_long(conn_handle, handle, 0, nimble_on_read, NULL); if (rc != 0) { ESP_LOGE(TAG, "read_descr failed"); + UNLOCK_OPS(); return rc; } WAIT_CB(); if (s_read_status == 0) { *out = s_read_data_val; *out_len = s_read_data_len; + } else { + free(s_read_data_val); + s_read_data_val = NULL; } - return s_read_status; + rc = s_read_status; + UNLOCK_OPS(); + return rc; } static int @@ -181,6 +250,10 @@ svc_disced(uint16_t conn_handle, const struct ble_gatt_error *error, status = error->status; switch (error->status) { case 0: + if (services_discovered >= 10) { + ESP_LOGE(TAG, "Too many services, ignoring"); + break; + } memcpy(service_result + services_discovered, service, sizeof(struct ble_gatt_svc)); services_discovered++; uuid16 = ble_uuid_u16(&service->uuid.u); @@ -231,6 +304,10 @@ chr_disced(uint16_t conn_handle, const struct ble_gatt_error *error, case 0: ESP_LOGD(TAG, "Char discovered : def handle : %04x, val_handle : %04x, properties : %02x\n, uuid : %04x", chr->def_handle, chr->val_handle, chr->properties, ble_uuid_u16(&chr->uuid.u)); + if (chrs_discovered >= 20) { + ESP_LOGE(TAG, "Too many characteristics, ignoring"); + break; + } memcpy(chrs + chrs_discovered, chr, sizeof(struct ble_gatt_chr)); chrs_discovered++; break; @@ -270,6 +347,10 @@ desc_disced(uint16_t conn_handle, const struct ble_gatt_error *error, case 0: ESP_LOGD(TAG, "DISC discovered : handle : %04x, uuid : %04x", dsc->handle, ble_uuid_u16(&dsc->uuid.u)); + if (dscs_discovered >= 20) { + ESP_LOGE(TAG, "Too many descriptors, ignoring"); + break; + } memcpy(dscr + dscs_discovered, dsc, sizeof(struct ble_gatt_dsc)); dscs_discovered++; break; @@ -304,105 +385,132 @@ static void read_device_services(esp_hidh_dev_t *dev) uint16_t suuid, cuuid, duuid; uint16_t chandle, dhandle; esp_hidh_dev_report_t *report = NULL; - uint8_t *rdata = 0; + uint8_t *rdata = NULL; uint16_t rlen = 0; esp_hid_report_item_t *r; esp_hid_report_map_t *map; - - struct ble_gatt_svc service_result[10]; - uint16_t dcount = 10; + struct ble_gatt_svc service_result[HIDH_MAX_SVCS]; + uint16_t svc_count; uint8_t hidindex = 0; int rc; + services_discovered = 0; + esp_hidh_dev_lock(dev); rc = ble_gattc_disc_all_svcs(dev->ble.conn_id, svc_disced, service_result); if (rc != 0) { ESP_LOGD(TAG, "Error discovering services : %d", rc); - assert(rc != 0); + goto done; } WAIT_CB(); if (status != 0) { ESP_LOGE(TAG, "failed to find services"); - assert(status == 0); + goto done; } - dcount = services_discovered; /* fatal if services are more than 10 */ - if (rc == ESP_OK) { - ESP_LOGD(TAG, "Found %u HID Services", dev->config.report_maps_len); - dev->config.report_maps = (esp_hid_raw_report_map_t *)malloc(dev->config.report_maps_len * sizeof(esp_hid_raw_report_map_t)); + svc_count = services_discovered; + if (svc_count > HIDH_MAX_SVCS) { + ESP_LOGE(TAG, "Too many services %u (max %d)", svc_count, HIDH_MAX_SVCS); + goto done; + } + + ESP_LOGD(TAG, "Found %u HID Services", dev->config.report_maps_len); + if (dev->config.report_maps_len > 0) { + dev->config.report_maps = (esp_hid_raw_report_map_t *)calloc(dev->config.report_maps_len, + sizeof(esp_hid_raw_report_map_t)); if (dev->config.report_maps == NULL) { ESP_LOGE(TAG, "malloc report maps failed"); - return; + goto done; } - dev->protocol_mode = (uint8_t *)malloc(dev->config.report_maps_len * sizeof(uint8_t)); + dev->protocol_mode = (uint8_t *)calloc(dev->config.report_maps_len, sizeof(uint8_t)); if (dev->protocol_mode == NULL) { ESP_LOGE(TAG, "malloc protocol_mode failed"); - return; + free(dev->config.report_maps); + dev->config.report_maps = NULL; + goto done; + } + } + + for (uint16_t s = 0; s < svc_count; s++) { + suuid = ble_uuid_u16(&service_result[s].uuid.u); + ESP_LOGD(TAG, "Service discovered : start_handle : %d, end handle : %d, uuid: 0x%04x", + service_result[s].start_handle, service_result[s].end_handle, suuid); + + if (suuid != BLE_SVC_BAS_UUID16 + && suuid != BLE_SVC_DIS_UUID16 + && suuid != BLE_SVC_HID_UUID16 + && suuid != 0x1800) { /* device name? */ + continue; } - /* read characteristic value may fail, so we should init report maps */ - memset(dev->config.report_maps, 0, dev->config.report_maps_len * sizeof(esp_hid_raw_report_map_t)); - - for (uint16_t s = 0; s < dcount; s++) { - suuid = ble_uuid_u16(&service_result[s].uuid.u); - ESP_LOGD(TAG, "Service discovered : start_handle : %d, end handle : %d, uuid: 0x%04x", - service_result[s].start_handle, service_result[s].end_handle, suuid); - - if (suuid != BLE_SVC_BAS_UUID16 - && suuid != BLE_SVC_DIS_UUID16 - && suuid != BLE_SVC_HID_UUID16 - && suuid != 0x1800) {//device name? - continue; - } - - struct ble_gatt_chr char_result[20]; - uint16_t ccount = 20; - rc = ble_gattc_disc_all_chrs(dev->ble.conn_id, service_result[s].start_handle, - service_result[s].end_handle, chr_disced, char_result); - WAIT_CB(); - if (status != 0) { - ESP_LOGE(TAG, "failed to find chars for service : %d", s); - assert(status == 0); - } - ccount = chrs_discovered; - if (rc == ESP_OK) { - for (uint16_t c = 0; c < ccount; c++) { - cuuid = ble_uuid_u16(&char_result[c].uuid.u); - chandle = char_result[c].val_handle; - ESP_LOGD(TAG, " CHAR:(%d), handle: %d, perm: 0x%02x, uuid: 0x%04x", - c + 1, chandle, char_result[c].properties, cuuid); - - if (suuid == BLE_SVC_GAP_UUID16) { - if (dev->config.device_name == NULL && cuuid == BLE_SVC_GAP_CHR_UUID16_DEVICE_NAME - && (char_result[c].properties & BLE_GATT_CHR_PROP_READ) != 0) { - if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { - dev->config.device_name = (const char *)rdata; + struct ble_gatt_chr char_result[HIDH_MAX_CHRS]; + uint16_t ccount = HIDH_MAX_CHRS; + rc = ble_gattc_disc_all_chrs(dev->ble.conn_id, service_result[s].start_handle, + service_result[s].end_handle, chr_disced, char_result); + if (rc != 0) { + ESP_LOGE(TAG, "failed to start char discovery for service : %d, rc=%d", s, rc); + chrs_discovered = 0; + continue; + } + WAIT_CB(); + if (status != 0) { + ESP_LOGE(TAG, "failed to find chars for service : %u", s); + chrs_discovered = 0; + continue; + } + ccount = chrs_discovered; + if (ccount > HIDH_MAX_CHRS) { + ESP_LOGE(TAG, "Too many characteristics %u (max %d)", ccount, HIDH_MAX_CHRS); + chrs_discovered = 0; + continue; + } + if (rc == 0) { + for (uint16_t c = 0; c < ccount; c++) { + cuuid = ble_uuid_u16(&char_result[c].uuid.u); + chandle = char_result[c].val_handle; + ESP_LOGD(TAG, " CHAR:(%d), handle: %d, perm: 0x%02x, uuid: 0x%04x", + c + 1, chandle, char_result[c].properties, cuuid); + if (suuid == BLE_SVC_GAP_UUID16) { + if (dev->config.device_name == NULL && cuuid == BLE_SVC_GAP_CHR_UUID16_DEVICE_NAME + && (char_result[c].properties & BLE_GATT_CHR_PROP_READ) != 0) { + if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { + char *dn = nimble_hidh_dup_cstr(rdata, rlen); + if (dn) { + free((void *)dev->config.device_name); + dev->config.device_name = dn; } - } else { - continue; } - } else if (suuid == BLE_SVC_BAS_UUID16) { - if (cuuid == BLE_SVC_BAS_CHR_UUID16_BATTERY_LEVEL && - (char_result[c].properties & BLE_GATT_CHR_PROP_READ) != 0) { - dev->ble.battery_handle = chandle; - } else { - continue; - } - } else if (suuid == BLE_SVC_DIS_UUID16) { - if (char_result[c].properties & BLE_GATT_CHR_PROP_READ) { - if (cuuid == BLE_SVC_DIS_CHR_UUID16_PNP_ID) { - if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen == 7) { - dev->config.vendor_id = *((uint16_t *)&rdata[1]); - dev->config.product_id = *((uint16_t *)&rdata[3]); - dev->config.version = *((uint16_t *)&rdata[5]); + } else { + continue; + } + } else if (suuid == BLE_SVC_BAS_UUID16) { + if (cuuid == BLE_SVC_BAS_CHR_UUID16_BATTERY_LEVEL && + (char_result[c].properties & BLE_GATT_CHR_PROP_READ) != 0) { + dev->ble.battery_handle = chandle; + } else { + continue; + } + } else if (suuid == BLE_SVC_DIS_UUID16) { + if (char_result[c].properties & BLE_GATT_CHR_PROP_READ) { + if (cuuid == BLE_SVC_DIS_CHR_UUID16_PNP_ID) { + if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen == 7) { + dev->config.vendor_id = *((uint16_t *)&rdata[1]); + dev->config.product_id = *((uint16_t *)&rdata[3]); + dev->config.version = *((uint16_t *)&rdata[5]); + } + } else if (cuuid == BLE_SVC_DIS_CHR_UUID16_MANUFACTURER_NAME) { + if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { + char *mn = nimble_hidh_dup_cstr(rdata, rlen); + if (mn) { + free((void *)dev->config.manufacturer_name); + dev->config.manufacturer_name = mn; } - free(rdata); - } else if (cuuid == BLE_SVC_DIS_CHR_UUID16_MANUFACTURER_NAME) { - if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { - dev->config.manufacturer_name = (const char *)rdata; - } - } else if (cuuid == BLE_SVC_DIS_CHR_UUID16_SERIAL_NUMBER) { - if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { - dev->config.serial_number = (const char *)rdata; + } + } else if (cuuid == BLE_SVC_DIS_CHR_UUID16_SERIAL_NUMBER) { + if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { + char *sn = nimble_hidh_dup_cstr(rdata, rlen); + if (sn) { + free((void *)dev->config.serial_number); + dev->config.serial_number = sn; } } } @@ -412,14 +520,20 @@ static void read_device_services(esp_hidh_dev_t *dev) if (char_result[c].properties & BLE_GATT_CHR_PROP_READ) { if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { dev->protocol_mode[hidindex] = *((uint8_t *)rdata); + free(rdata); + rdata = NULL; } } - continue; } - if (cuuid == BLE_SVC_HID_CHR_UUID16_REPORT_MAP) { - if (char_result[c].properties & BLE_GATT_CHR_PROP_READ) { - if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { - dev->config.report_maps[hidindex].data = (const uint8_t *)rdata; + continue; + } + if (cuuid == BLE_SVC_HID_CHR_UUID16_REPORT_MAP) { + if (char_result[c].properties & BLE_GATT_CHR_PROP_READ) { + if (read_char(dev->ble.conn_id, chandle, &rdata, &rlen) == 0 && rlen) { + uint8_t *copy = nimble_hidh_dup_bytes(rdata, rlen); + if (copy) { + free((void *)dev->config.report_maps[hidindex].data); + dev->config.report_maps[hidindex].data = copy; dev->config.report_maps[hidindex].len = rlen; } } @@ -429,7 +543,7 @@ static void read_device_services(esp_hidh_dev_t *dev) report = (esp_hidh_dev_report_t *)malloc(sizeof(esp_hidh_dev_report_t)); if (report == NULL) { ESP_LOGE(TAG, "malloc esp_hidh_dev_report_t failed"); - return; + goto done; } report->next = NULL; report->permissions = char_result[c].properties; @@ -459,132 +573,155 @@ static void read_device_services(esp_hidh_dev_t *dev) report->value_len = 0; } } else { - continue; + report->protocol_mode = ESP_HID_PROTOCOL_MODE_REPORT; + report->report_type = 0; + report->usage = ESP_HID_USAGE_GENERIC; + report->value_len = 0; } - } - struct ble_gatt_dsc descr_result[20]; - uint16_t dcount = 20; - uint16_t chr_end_handle; - if (c + 1 < ccount) { - chr_end_handle = char_result[c + 1].def_handle; } else { - chr_end_handle = service_result[s].end_handle; - } - rc = ble_gattc_disc_all_dscs(dev->ble.conn_id, char_result[c].val_handle, - chr_end_handle, desc_disced, descr_result); - WAIT_CB(); - if (status != 0) { - ESP_LOGE(TAG, "failed to find descriptors for characteristic : %d", c); - assert(status == 0); - } - dcount = dscs_discovered; - if (rc == ESP_OK) { - for (uint16_t d = 0; d < dcount; d++) { - duuid = ble_uuid_u16(&descr_result[d].uuid.u); - dhandle = descr_result[d].handle; - ESP_LOGD(TAG, " DESCR:(%d), handle: %d, uuid: 0x%04x", d + 1, dhandle, duuid); - - if (suuid == BLE_SVC_BAS_UUID16) { - if (duuid == BLE_GATT_DSC_CLT_CFG_UUID16 && - (char_result[c].properties & BLE_GATT_CHR_PROP_NOTIFY) != 0) { - dev->ble.battery_ccc_handle = dhandle; - } - } else if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { - if (duuid == BLE_GATT_DSC_CLT_CFG_UUID16 && (report->permissions & BLE_GATT_CHR_PROP_NOTIFY) != 0) { - report->ccc_handle = dhandle; - } else if (duuid == BLE_SVC_HID_DSC_UUID16_RPT_REF) { - if (read_descr(dev->ble.conn_id, dhandle, &rdata, &rlen) == 0 && rlen) { - report->report_id = rdata[0]; - report->report_type = rdata[1]; - free(rdata); - } - } - } - } + continue; } + } + struct ble_gatt_dsc descr_result[HIDH_MAX_DSCS]; + uint16_t num_dsc = HIDH_MAX_DSCS; + uint16_t chr_end_handle; + if (c + 1 < ccount) { + chr_end_handle = char_result[c + 1].def_handle - 1; + } else { + chr_end_handle = service_result[s].end_handle; + } + if (chr_end_handle <= char_result[c].val_handle) { + ESP_LOGD(TAG, "skip descriptor discovery for val_handle=%u end_handle=%u", + char_result[c].val_handle, chr_end_handle); if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { report->next = dev->reports; dev->reports = report; dev->reports_len++; } dscs_discovered = 0; + continue; } - if (suuid == BLE_SVC_HID_UUID16) { - hidindex++; - } - } - chrs_discovered = 0; // reset the chars array for the next service - } - for (uint8_t d = 0; d < dev->config.report_maps_len; d++) { - if (dev->reports_len && dev->config.report_maps[d].len) { - map = esp_hid_parse_report_map(dev->config.report_maps[d].data, dev->config.report_maps[d].len); - if (map) { - if (dev->ble.appearance == 0) { - dev->ble.appearance = map->appearance; + /* + * NimBLE expects the characteristic value handle here; it + * discovers descriptors in the range (chr_val_handle, end]. + */ + rc = ble_gattc_disc_all_dscs(dev->ble.conn_id, char_result[c].val_handle, + chr_end_handle, desc_disced, descr_result); + if (rc != 0) { + ESP_LOGD(TAG, "descriptor discovery not started for characteristic : %d, rc=%d", c, rc); + if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { + report->next = dev->reports; + dev->reports = report; + dev->reports_len++; } - report = dev->reports; + dscs_discovered = 0; + continue; + } + WAIT_CB(); + if (status != 0) { + ESP_LOGE(TAG, "failed to find descriptors for characteristic : %u", c); + if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { + report->next = dev->reports; + dev->reports = report; + dev->reports_len++; + } + dscs_discovered = 0; + continue; + } + num_dsc = dscs_discovered; + if (num_dsc > HIDH_MAX_DSCS) { + ESP_LOGE(TAG, "Too many descriptors %u (max %d)", num_dsc, HIDH_MAX_DSCS); + if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { + report->next = dev->reports; + dev->reports = report; + dev->reports_len++; + } + dscs_discovered = 0; + continue; + } + if (rc == ESP_OK) { + for (uint16_t d = 0; d < num_dsc; d++) { + duuid = ble_uuid_u16(&descr_result[d].uuid.u); + dhandle = descr_result[d].handle; + ESP_LOGD(TAG, " DESCR:(%d), handle: %d, uuid: 0x%04x", d + 1, dhandle, duuid); - while (report) { - if (report->map_index == d) { - for (uint8_t i = 0; i < map->reports_len; i++) { - r = &map->reports[i]; - if (report->protocol_mode == ESP_HID_PROTOCOL_MODE_BOOT - && report->protocol_mode == r->protocol_mode - && report->report_type == r->report_type - && report->usage == r->usage) { - report->report_id = r->report_id; - report->value_len = r->value_len; - } else if (report->protocol_mode == r->protocol_mode - && report->report_type == r->report_type - && report->report_id == r->report_id) { - report->usage = r->usage; - report->value_len = r->value_len; + if (suuid == BLE_SVC_BAS_UUID16) { + if (duuid == BLE_GATT_DSC_CLT_CFG_UUID16 && + (char_result[c].properties & BLE_GATT_CHR_PROP_NOTIFY) != 0) { + dev->ble.battery_ccc_handle = dhandle; + } + } else if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { + if (duuid == BLE_GATT_DSC_CLT_CFG_UUID16 && (report->permissions & BLE_GATT_CHR_PROP_NOTIFY) != 0) { + report->ccc_handle = dhandle; + } else if (duuid == BLE_SVC_HID_DSC_UUID16_RPT_REF) { + if (read_descr(dev->ble.conn_id, dhandle, &rdata, &rlen) == 0 && rlen >= 2) { + report->report_id = rdata[0]; + report->report_type = rdata[1]; + free(rdata); } } } - report = report->next; } - free(map->reports); - free(map); - map = NULL; } + if (suuid == BLE_SVC_HID_UUID16 && report != NULL) { + report->next = dev->reports; + dev->reports = report; + dev->reports_len++; + } + dscs_discovered = 0; + } + if (suuid == BLE_SVC_HID_UUID16) { + hidindex++; + } + } + chrs_discovered = 0; + } + + for (uint8_t d = 0; d < dev->config.report_maps_len; d++) { + if (dev->reports_len && dev->config.report_maps[d].len && dev->config.report_maps[d].data) { + map = esp_hid_parse_report_map(dev->config.report_maps[d].data, dev->config.report_maps[d].len); + if (map) { + if (dev->ble.appearance == 0) { + dev->ble.appearance = map->appearance; + } + report = dev->reports; + + while (report) { + if (report->map_index == d) { + for (uint8_t i = 0; i < map->reports_len; i++) { + r = &map->reports[i]; + if (report->protocol_mode == ESP_HID_PROTOCOL_MODE_BOOT + && report->protocol_mode == r->protocol_mode + && report->report_type == r->report_type + && report->usage == r->usage) { + report->report_id = r->report_id; + report->value_len = r->value_len; + } else if (report->protocol_mode == r->protocol_mode + && report->report_type == r->report_type + && report->report_id == r->report_id) { + report->usage = r->usage; + report->value_len = r->value_len; + } + } + } + report = report->next; + } + free(map->reports); + free(map); + map = NULL; } } } -} - -static int -on_subscribe(uint16_t conn_handle, - const struct ble_gatt_error *error, - struct ble_gatt_attr *attr, - void *arg) -{ - uint16_t conn_id; - conn_id = *((uint16_t*) arg); - - assert(conn_id == conn_handle); - - MODLOG_DFLT(INFO, "Subscribe complete; status=%d conn_handle=%d " - "attr_handle=%d\n", - error->status, conn_handle, attr->handle); - SEND_CB(); - - return 0; +done: + esp_hidh_dev_unlock(dev); } static void register_for_notify(uint16_t conn_handle, uint16_t handle) { - uint8_t value[2]; - int rc; - value[0] = 1; - value[1] = 0; - rc = ble_gattc_write_flat(conn_handle, handle, value, sizeof value, on_subscribe, (void *)&conn_handle); - if (rc != 0) { - ESP_LOGE(TAG, "Error: Failed to subscribe to characteristic; " - "rc=%d\n", rc); - } - WAIT_CB(); + /* NimBLE notifications are enabled via CCC descriptor writes. */ + (void)conn_handle; + (void)handle; } static int @@ -593,52 +730,84 @@ on_write(uint16_t conn_handle, struct ble_gatt_attr *attr, void *arg) { - uint16_t conn_id; - conn_id = *((uint16_t*) arg); + esp_hidh_dev_t *dev = (esp_hidh_dev_t *)arg; - assert(conn_id == conn_handle); + if (dev == NULL) { + s_gatt_op_status = BLE_HS_EAPP; + SEND_CB(); + return 0; + } + if (esp_hidh_dev_get_by_conn_id(conn_handle) != dev) { + s_gatt_op_status = BLE_HS_ENOTCONN; + SEND_CB(); + return 0; + } MODLOG_DFLT(DEBUG, "write complete; status=%d conn_handle=%d " "attr_handle=%d\n", error->status, conn_handle, attr->handle); + s_gatt_op_status = error->status; SEND_CB(); return 0; } -static void write_char_descr(uint16_t conn_id, uint16_t handle, uint16_t value_len, uint8_t *value) + +static void write_char_descr(esp_hidh_dev_t *dev, uint16_t handle, uint16_t value_len, uint8_t *value) { - ble_gattc_write_flat(conn_id, handle, value, value_len, on_write, &conn_id); + int rc; + + if (dev == NULL || dev->ble.conn_id < 0) { + return; + } + s_gatt_op_status = 0; + rc = ble_gattc_write_flat(dev->ble.conn_id, handle, value, value_len, on_write, dev); + if (rc != 0) { + ESP_LOGE(TAG, "write_char_descr failed rc=%d", rc); + return; + } WAIT_CB(); + if (s_gatt_op_status != 0) { + ESP_LOGE(TAG, "GATT write failed status=%d", s_gatt_op_status); + } } static void attach_report_listeners(esp_hidh_dev_t *dev) { + uint16_t ccc_data = 1; + esp_hidh_dev_report_t *report; + if (dev == NULL) { return; } - uint16_t ccc_data = 1; - esp_hidh_dev_report_t *report = dev->reports; - //subscribe to battery notifications + LOCK_OPS(); + if (dev->ble.conn_id < 0 || !dev->connected) { + UNLOCK_OPS(); + return; + } + + report = dev->reports; + if (dev->ble.battery_handle) { register_for_notify(dev->ble.conn_id, dev->ble.battery_handle); - if (dev->ble.battery_ccc_handle) { - //Write CCC descr to enable notifications - write_char_descr(dev->ble.conn_id, dev->ble.battery_ccc_handle, 2, (uint8_t *)&ccc_data); + if (dev->ble.battery_ccc_handle && dev->ble.conn_id >= 0 && dev->connected) { + write_char_descr(dev, dev->ble.battery_ccc_handle, 2, (uint8_t *)&ccc_data); } } while (report) { - /* subscribe to notifications */ + if (dev->ble.conn_id < 0 || !dev->connected) { + break; + } if ((report->permissions & BLE_GATT_CHR_PROP_NOTIFY) != 0 && report->protocol_mode == ESP_HID_PROTOCOL_MODE_REPORT) { register_for_notify(dev->ble.conn_id, report->handle); - if (report->ccc_handle) { - /* Write CCC descr to enable notifications */ - write_char_descr(dev->ble.conn_id, report->ccc_handle, 2, (uint8_t *)&ccc_data); + if (report->ccc_handle && dev->ble.conn_id >= 0 && dev->connected) { + write_char_descr(dev, report->ccc_handle, 2, (uint8_t *)&ccc_data); } } report = report->next; } + UNLOCK_OPS(); } static int @@ -652,17 +821,32 @@ esp_hidh_gattc_event_handler(struct ble_gap_event *event, void *arg) switch (event->type) { case BLE_GAP_EVENT_CONNECT: /* A new connection was established or a connection attempt failed. */ + dev = (esp_hidh_dev_t *)arg; if (event->connect.status == 0) { /* Connection successfully established. */ MODLOG_DFLT(INFO, "Connection established "); - rc = ble_gap_conn_find(event->connect.conn_handle, &desc); - assert(rc == 0); - dev = esp_hidh_dev_get_by_bda(desc.peer_ota_addr.val); - if (!dev) { - ESP_LOGE(TAG, "Connect received for unknown device"); + if (dev == NULL) { + ESP_LOGE(TAG, "Connect success with NULL device context"); + SEND_CB(); + return 0; } - dev->status = -1; // set to not found and clear if HID service is found + + rc = ble_gap_conn_find(event->connect.conn_handle, &desc); + if (rc != 0) { + ESP_LOGE(TAG, "ble_gap_conn_find failed; rc=%d", rc); + dev->status = rc; + dev->ble.conn_id = -1; + SEND_CB(); + return 0; + } + + if (memcmp(desc.peer_ota_addr.val, dev->addr.bda, sizeof(dev->addr.bda)) != 0 && + memcmp(desc.peer_id_addr.val, dev->addr.bda, sizeof(dev->addr.bda)) != 0) { + ESP_LOGW(TAG, "Connect address mismatch vs open request"); + } + + dev->status = -1; /* not found until HID service is discovered */ dev->ble.conn_id = event->connect.conn_handle; /* Try to set the mtu to the max value */ @@ -674,13 +858,22 @@ esp_hidh_gattc_event_handler(struct ble_gap_event *event, void *arg) if (rc != 0) { ESP_LOGE(TAG, "Failed to negotiate MTU; rc = %d", rc); } + rc = ble_gap_security_initiate(event->connect.conn_handle); + if (rc != 0) { + ESP_LOGW(TAG, "Failed to initiate security; rc = %d", rc); + } SEND_CB(); } else { MODLOG_DFLT(ERROR, "Error: Connection failed; status=%d\n", event->connect.status); - dev->status = event->connect.status; // ESP_GATT_CONN_FAIL_ESTABLISH; + if (dev == NULL) { + ESP_LOGE(TAG, "Connect failed with NULL device context"); + SEND_CB(); + return 0; + } + dev->status = event->connect.status; dev->ble.conn_id = -1; - SEND_CB(); // return from connection + SEND_CB(); } return 0; @@ -732,6 +925,16 @@ esp_hidh_gattc_event_handler(struct ble_gap_event *event, void *arg) ESP_LOGE(TAG, "NOTIFY received for unknown device"); break; } + rc = ble_gap_conn_find(event->notify_rx.conn_handle, &desc); + if (rc != 0) { + ESP_LOGW(TAG, "Dropping notification: unknown link"); + return 0; + } + /* Enforce encrypted notifications only for strict security policy. */ + if ((ble_hs_cfg.sm_mitm || ble_hs_cfg.sm_sc_only) && !desc.sec_state.encrypted) { + ESP_LOGW(TAG, "Dropping notification on unencrypted link"); + return 0; + } if (event_loop_handle) { esp_hidh_event_data_t p = {0}; if (event->notify_rx.attr_handle == dev->ble.battery_handle) { @@ -745,7 +948,8 @@ esp_hidh_gattc_event_handler(struct ble_gap_event *event, void *arg) esp_hidh_event_data_t *p_param = NULL; size_t event_data_size = sizeof(esp_hidh_event_data_t); - if (report->protocol_mode != dev->protocol_mode[report->map_index]) { + if (dev->protocol_mode == NULL || + report->protocol_mode != dev->protocol_mode[report->map_index]) { /* only pass the notifications in the current protocol mode */ ESP_LOGD(TAG, "Wrong protocol mode, dropping notification"); return 0; @@ -792,6 +996,45 @@ esp_hidh_gattc_event_handler(struct ble_gap_event *event, void *arg) break; return 0; + case BLE_GAP_EVENT_ENC_CHANGE: + rc = ble_gap_conn_find(event->enc_change.conn_handle, &desc); + if (rc != 0) { + ESP_LOGW(TAG, "ENC_CHANGE for unknown connection"); + return 0; + } + ESP_LOGI(TAG, "ENC_CHANGE status=%d encrypted=%d authenticated=%d bonded=%d", + event->enc_change.status, desc.sec_state.encrypted, + desc.sec_state.authenticated, desc.sec_state.bonded); + return 0; + + case BLE_GAP_EVENT_PASSKEY_ACTION: + switch (event->passkey.params.action) { + case BLE_SM_IOACT_NUMCMP: { + struct ble_sm_io pkey = { + .action = BLE_SM_IOACT_NUMCMP, + .numcmp_accept = 1, + }; + rc = ble_sm_inject_io(event->passkey.conn_handle, &pkey); + ESP_LOGI(TAG, "PASSKEY NUMCMP auto-accept rc=%d", rc); + return 0; + } + case BLE_SM_IOACT_DISP: + ESP_LOGI(TAG, "PASSKEY display requested"); + return 0; + case BLE_SM_IOACT_INPUT: { + struct ble_sm_io pkey = { + .action = BLE_SM_IOACT_INPUT, + .passkey = 123456, + }; + rc = ble_sm_inject_io(event->passkey.conn_handle, &pkey); + ESP_LOGI(TAG, "PASSKEY INPUT injected rc=%d", rc); + return 0; + } + default: + ESP_LOGW(TAG, "PASSKEY action=%u", event->passkey.params.action); + return 0; + } + case BLE_GAP_EVENT_MTU: MODLOG_DFLT(INFO, "mtu update event; conn_handle=%d cid=%d mtu=%d\n", event->mtu.conn_handle, @@ -810,6 +1053,9 @@ esp_hidh_gattc_event_handler(struct ble_gap_event *event, void *arg) static esp_err_t esp_ble_hidh_dev_close(esp_hidh_dev_t *dev) { + if (dev == NULL || dev->ble.conn_id < 0) { + return ESP_FAIL; + } return ble_gap_terminate(dev->ble.conn_id, BLE_ERR_REM_USER_CONN_TERM); } @@ -825,9 +1071,20 @@ static esp_err_t esp_ble_hidh_dev_report_write(esp_hidh_dev_t *dev, size_t map_i ESP_LOGE(TAG, "%s report %d takes maximum %d bytes. you have provided %d", esp_hid_report_type_str(report_type), report_id, report->value_len, value_len); return ESP_FAIL; } - rc = ble_gattc_write_flat(dev->ble.conn_id, report->handle, value, value_len, on_write, &dev->ble.conn_id); - WAIT_CB();// this is not blocking in bluedroid code - return rc; + LOCK_OPS(); + s_gatt_op_status = 0; + rc = ble_gattc_write_flat(dev->ble.conn_id, report->handle, value, value_len, on_write, dev); + if (rc != 0) { + WAIT_CB(); + rc = s_gatt_op_status; + } + WAIT_CB(); + rc = s_gatt_op_status; + UNLOCK_OPS(); + if (rc != 0) { + return ESP_FAIL; + } + return ESP_OK; } static esp_err_t esp_ble_hidh_dev_report_read(esp_hidh_dev_t *dev, size_t map_index, size_t report_id, int report_type, size_t max_length, uint8_t *value, size_t *value_len) @@ -839,13 +1096,16 @@ static esp_err_t esp_ble_hidh_dev_report_read(esp_hidh_dev_t *dev, size_t map_in } uint16_t len = max_length; uint8_t *v = NULL; + LOCK_OPS(); int s = read_char(dev->ble.conn_id, report->handle, &v, &len); + UNLOCK_OPS(); if (s == 0) { if (len > max_length) { len = max_length; } *value_len = len; memcpy(value, v, len); + free(v); return ESP_OK; } ESP_LOGE(TAG, "%s report %d read failed: 0x%x", esp_hid_report_type_str(report_type), report_id, s); @@ -862,6 +1122,10 @@ static void esp_ble_hidh_dev_dump(esp_hidh_dev_t *dev, FILE *fp) fprintf(fp, "PID: 0x%04x, VID: 0x%04x, VERSION: 0x%04x\n", dev->config.product_id, dev->config.vendor_id, dev->config.version); fprintf(fp, "Battery: Handle: %u, CCC Handle: %u\n", dev->ble.battery_handle, dev->ble.battery_ccc_handle); fprintf(fp, "Report Maps: %d\n", dev->config.report_maps_len); + if (dev->config.report_maps_len > 0 && dev->config.report_maps == NULL) { + fprintf(fp, " (report_maps allocation missing)\n"); + return; + } for (uint8_t d = 0; d < dev->config.report_maps_len; d++) { fprintf(fp, " Report Map Length: %d\n", dev->config.report_maps[d].len); esp_hidh_dev_report_t *report = dev->reports; @@ -880,6 +1144,9 @@ static void esp_ble_hidh_dev_dump(esp_hidh_dev_t *dev, FILE *fp) static void nimble_host_synced(void) { + if (s_prev_sync_cb && s_prev_sync_cb != nimble_host_synced) { + s_prev_sync_cb(); + } /* no need to perform anything here */ @@ -887,7 +1154,14 @@ static void nimble_host_synced(void) static void nimble_host_reset(int reason) { + if (s_prev_reset_cb && s_prev_reset_cb != nimble_host_reset) { + s_prev_reset_cb(reason); + } MODLOG_DFLT(ERROR, "Resetting state; reason=%d\n", reason); + + if (s_ble_hidh_cb_semaphore) { + xSemaphoreGive(s_ble_hidh_cb_semaphore); + } } esp_err_t esp_ble_hidh_init(const esp_hidh_config_t *config) @@ -900,7 +1174,19 @@ esp_err_t esp_ble_hidh_init(const esp_hidh_config_t *config) s_ble_hidh_cb_semaphore = xSemaphoreCreateBinary(); ESP_RETURN_ON_FALSE(s_ble_hidh_cb_semaphore, ESP_ERR_NO_MEM, TAG, "Allocation failed"); + s_ble_hidh_op_mutex = xSemaphoreCreateMutex(); + if (s_ble_hidh_op_mutex == NULL) { + vSemaphoreDelete(s_ble_hidh_cb_semaphore); + s_ble_hidh_cb_semaphore = NULL; + return ESP_ERR_NO_MEM; + } + s_prev_reset_cb = ble_hs_cfg.reset_cb; + s_prev_sync_cb = ble_hs_cfg.sync_cb; + /* Enforce secure pairing defaults for HID traffic. */ + ble_hs_cfg.sm_io_cap = BLE_HS_IO_KEYBOARD_ONLY; + ble_hs_cfg.sm_bonding = 1; + ble_hs_cfg.sm_sc = 1; ble_hs_cfg.reset_cb = nimble_host_reset; ble_hs_cfg.sync_cb = nimble_host_synced; return ret; @@ -909,10 +1195,26 @@ esp_err_t esp_ble_hidh_init(const esp_hidh_config_t *config) esp_err_t esp_ble_hidh_deinit(void) { ESP_RETURN_ON_FALSE(s_ble_hidh_cb_semaphore, ESP_ERR_INVALID_STATE, TAG, "Already deinitialized"); + + free(s_read_data_val); + s_read_data_val = NULL; + s_read_data_len = 0; + s_read_status = 0; + if (s_ble_hidh_cb_semaphore) { vSemaphoreDelete(s_ble_hidh_cb_semaphore); s_ble_hidh_cb_semaphore = NULL; } + if (s_ble_hidh_op_mutex) { + vSemaphoreDelete(s_ble_hidh_op_mutex); + s_ble_hidh_op_mutex = NULL; + } + if (ble_hs_cfg.reset_cb == nimble_host_reset) { + ble_hs_cfg.reset_cb = s_prev_reset_cb; + } + if (ble_hs_cfg.sync_cb == nimble_host_synced) { + ble_hs_cfg.sync_cb = s_prev_sync_cb; + } return ESP_OK; } @@ -939,10 +1241,12 @@ esp_hidh_dev_t *esp_ble_hidh_dev_open(uint8_t *bda, uint8_t address_type) memcpy(addr.val, bda, sizeof(addr.val)); addr.type = address_type; - ret = ble_gap_connect(own_addr_type, &addr, 30000, NULL, esp_hidh_gattc_event_handler, NULL); + LOCK_OPS(); + ret = ble_gap_connect(own_addr_type, &addr, 30000, NULL, esp_hidh_gattc_event_handler, dev); if (ret) { esp_hidh_dev_free_inner(dev); ESP_LOGE(TAG, "esp_ble_gattc_open failed: %d", ret); + UNLOCK_OPS(); return NULL; } WAIT_CB(); @@ -950,6 +1254,7 @@ esp_hidh_dev_t *esp_ble_hidh_dev_open(uint8_t *bda, uint8_t address_type) ret = dev->status; ESP_LOGE(TAG, "dev open failed! status: 0x%x", dev->status); esp_hidh_dev_free_inner(dev); + UNLOCK_OPS(); return NULL; } @@ -957,10 +1262,10 @@ esp_hidh_dev_t *esp_ble_hidh_dev_open(uint8_t *bda, uint8_t address_type) dev->report_write = esp_ble_hidh_dev_report_write; dev->report_read = esp_ble_hidh_dev_report_read; dev->dump = esp_ble_hidh_dev_dump; - dev->connected = true; /* perform service discovery and fill the report maps */ read_device_services(dev); + dev->connected = true; if (event_loop_handle) { esp_hidh_event_data_t p = {0}; @@ -970,6 +1275,7 @@ esp_hidh_dev_t *esp_ble_hidh_dev_open(uint8_t *bda, uint8_t address_type) } attach_report_listeners(dev); + UNLOCK_OPS(); return dev; } #endif // CONFIG_BT_NIMBLE_HID_SERVICE diff --git a/examples/bluetooth/blufi/main/blufi_init.c b/examples/bluetooth/blufi/main/blufi_init.c index ee81d678f1..5376528469 100644 --- a/examples/bluetooth/blufi/main/blufi_init.c +++ b/examples/bluetooth/blufi/main/blufi_init.c @@ -233,9 +233,9 @@ esp_err_t esp_blufi_host_init(void) ble_hs_cfg.gatts_register_cb = esp_blufi_gatt_svr_register_cb; ble_hs_cfg.store_status_cb = ble_store_util_status_rr; - /* FALSE POSITIVE: BLUFI uses its own app-layer security (DH + AES), not BLE SM. - * sm_mitm/sm_sc/sm_bonding are opt-in via Kconfig to let the example show - * multiple configurations; Just Works is acceptable for BLUFI provisioning. */ + /* BLUFI uses its own application-layer security (DH + AES), not BLE Security + * Manager. sm_mitm/sm_sc/sm_bonding are opt-in via Kconfig; Just Works + * pairing is acceptable because the DH key exchange provides the security. */ ble_hs_cfg.sm_io_cap = 4; #ifdef CONFIG_EXAMPLE_BONDING ble_hs_cfg.sm_bonding = 1; diff --git a/examples/bluetooth/esp_hid_device/README.md b/examples/bluetooth/esp_hid_device/README.md index be44591cbf..c857c9419d 100644 --- a/examples/bluetooth/esp_hid_device/README.md +++ b/examples/bluetooth/esp_hid_device/README.md @@ -90,6 +90,82 @@ I (50557) HID_DEV_DEMO: Send the volume ... ``` +## NimBLE Logging Scheme + +The following are verbatim NimBLE log excerpts from `logs.txt` for each device role. + +### Media Mode + +```text +I (496) HID_DEV_DEMO: setting ble device +I (496) HID_DEV_DEMO: BLE Host Task Started +I (506) HID_DEV_BLE: START +I (506) NimBLE: GAP procedure initiated: advertise; +I (35886) ESP_HID_GAP: connection established; status=0 +I (35896) HID_DEV_BLE: CONNECT +I (35946) ESP_HID_GAP: mtu update event; conn_handle=0 cid=4 mtu=256 +I (36476) ESP_HID_GAP: PASSKEY_ACTION_EVENT started +I (36476) ESP_HID_GAP: Enter passkey 123456on the peer side +I (37896) ESP_HID_GAP: subscribe event; conn_handle=0 attr_handle=37 reason=1 prevn=0 curn=1 previ=0 curi=0 +I (40936) NimBLE: encryption change event; status=0 +I (40946) HID_DEV_DEMO: Send the volume +I (40946) NimBLE: GATT procedure initiated: notify; +I (40946) NimBLE: att_handle=37 +I (40946) NimBLE: notify_tx event; conn_handle=0 attr_handle=37 status=0 is_indication=0 +``` + +### Keyboard Mode + +```text +I (436) HID_DEV_DEMO: setting ble device +I (436) HID_DEV_DEMO: BLE Host Task Started +I (436) HID_DEV_BLE: START +I (446) NimBLE: GAP procedure initiated: advertise; +I (11526) ESP_HID_GAP: connection established; status=0 +I (11526) HID_DEV_BLE: CONNECT +I (11576) ESP_HID_GAP: mtu update event; conn_handle=0 cid=4 mtu=256 +I (12116) ESP_HID_GAP: PASSKEY_ACTION_EVENT started +I (12116) ESP_HID_GAP: Enter passkey 123456on the peer side +I (13826) ESP_HID_GAP: subscribe event; conn_handle=0 attr_handle=37 reason=1 prevn=0 curn=1 previ=0 curi=0 +I (16576) NimBLE: encryption change event; status=0 +######################################################################## +BT hid keyboard demo usage: +######################################################################## +I (22306) NimBLE: GATT procedure initiated: notify; +I (22306) NimBLE: att_handle=37 +I (22306) NimBLE: notify_tx event; conn_handle=0 attr_handle=37 status=0 is_indication=0 +``` + +### Mouse Mode + +```text +I (446) HID_DEV_DEMO: setting ble device +I (446) HID_DEV_DEMO: BLE Host Task Started +I (456) HID_DEV_BLE: START +I (456) NimBLE: GAP procedure initiated: advertise; +I (10556) ESP_HID_GAP: connection established; status=0 +I (10556) HID_DEV_BLE: CONNECT +I (10606) ESP_HID_GAP: mtu update event; conn_handle=0 cid=4 mtu=256 +I (11136) ESP_HID_GAP: PASSKEY_ACTION_EVENT started +I (11136) ESP_HID_GAP: Enter passkey 123456on the peer side +I (12656) ESP_HID_GAP: subscribe event; conn_handle=0 attr_handle=37 reason=1 prevn=0 curn=1 previ=0 curi=0 +I (15606) NimBLE: encryption change event; status=0 +######################################################################## +BT hid mouse demo usage: +You can input these value to simulate mouse: 'q', 'w', 'e', 'a', 's', 'd', 'h' +q -- click the left key +w -- move up +e -- click the right key +a -- move left +s -- move down +d -- move right +h -- show the help +######################################################################## +I (18166) NimBLE: GATT procedure initiated: notify; +I (18166) NimBLE: att_handle=37 +I (18166) NimBLE: notify_tx event; conn_handle=0 attr_handle=37 status=0 is_indication=0 +``` + ## Troubleshooting 1. When using NimBLE stack, some iOS devices do not show the volume pop up. To fix this, please set CONFIG_BT_NIMBLE_SM_LVL to value 2. iOS needs Authenticated Pairing with Encryption to show up the pop ups. diff --git a/examples/bluetooth/esp_hid_device/main/esp_hid_gap.c b/examples/bluetooth/esp_hid_device/main/esp_hid_gap.c index 005966f249..c6786751eb 100644 --- a/examples/bluetooth/esp_hid_device/main/esp_hid_gap.c +++ b/examples/bluetooth/esp_hid_device/main/esp_hid_gap.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021-2025 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2026 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -856,9 +856,13 @@ nimble_hid_gap_event(struct ble_gap_event *event, void *arg) /* Encryption has been enabled or disabled for this connection. */ MODLOG_DFLT(INFO, "encryption change event; status=%d ", event->enc_change.status); - rc = ble_gap_conn_find(event->enc_change.conn_handle, &desc); - assert(rc == 0); - ble_hid_task_start_up(); + if (event->enc_change.status == 0) { + rc = ble_gap_conn_find(event->enc_change.conn_handle, &desc); + assert(rc == 0); + ble_hid_task_start_up(); + } else { + ESP_LOGW(TAG, "encryption failed; waiting for disconnect/retry"); + } return 0; case BLE_GAP_EVENT_NOTIFY_TX: @@ -889,7 +893,7 @@ nimble_hid_gap_event(struct ble_gap_event *event, void *arg) case BLE_GAP_EVENT_PASSKEY_ACTION: ESP_LOGI(TAG, "PASSKEY_ACTION_EVENT started"); struct ble_sm_io pkey = {0}; - int key = 0; + int key = 1; if (event->passkey.params.action == BLE_SM_IOACT_DISP) { pkey.action = event->passkey.params.action; diff --git a/examples/bluetooth/esp_hid_host/README.md b/examples/bluetooth/esp_hid_host/README.md index d95ebee85c..8adb340db4 100644 --- a/examples/bluetooth/esp_hid_host/README.md +++ b/examples/bluetooth/esp_hid_host/README.md @@ -94,6 +94,73 @@ I (18212) ESP_HIDH_DEMO: 00 00 ... ``` +## NimBLE Logging Scheme + +The following are verbatim NimBLE log excerpts from `logs.txt` for each device role. + +### Media Mode (CCONTROL) + +```text +I (2446) ESP_HIDH_DEMO: SCAN... +I (2446) NimBLE: GAP procedure initiated: discovery; +I (7456) NimBLE: discovery complete; reason=0 +I (7466) NimBLE: GAP procedure initiated: connect; +I (7666) NimBLE: Connection established +I (7676) NimBLE: GATT procedure initiated: exchange mtu +I (7726) NimBLE: mtu update event; conn_handle=0 cid=4 mtu=256 +I (8446) NIMBLE_HIDH: PASSKEY INPUT injected rc=0 +I (9636) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 OPEN: nimble +Report Maps: 1 + Report Map Length: 111 + CCONTROL INPUT REPORT, ID: 3, Length: 2, Permissions: 0x1a, Handle: 37, CCC Handle: 38 +I (12776) NIMBLE_HIDH: ENC_CHANGE status=0 encrypted=1 authenticated=1 bonded=1 +I (12826) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: CCONTROL, MAP: 0, ID: 3, Len: 2, Data: +I (12826) ESP_HIDH_DEMO: 80 00 +I (12876) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: CCONTROL, MAP: 0, ID: 3, Len: 2, Data: +I (12876) ESP_HIDH_DEMO: 00 00 +I (14876) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: CCONTROL, MAP: 0, ID: 3, Len: 2, Data: +I (14876) ESP_HIDH_DEMO: 40 00 +``` + +### Keyboard Mode + +```text +I (7426) ESP_HIDH_DEMO: SCAN: 2 results + BLE: 68:2e:f7:f9:55:60, RSSI: -57, USAGE: GENERIC, APPEARANCE: 0x03c0, ADDR_TYPE: '0', NAME: ESP Mouse + BLE: 02:50:f7:f9:55:60, RSSI: -30, USAGE: GENERIC, APPEARANCE: 0x03c0, ADDR_TYPE: '0', NAME: ESP Keyboard +I (7446) NimBLE: GAP procedure initiated: connect; +I (7836) NimBLE: Connection established +I (8606) NIMBLE_HIDH: PASSKEY INPUT injected rc=0 +I (10086) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 OPEN: nimble +Report Maps: 1 + Report Map Length: 65 + KEYBOARD INPUT REPORT, ID: 1, Length: 7, Permissions: 0x1a, Handle: 37, CCC Handle: 38 +I (12936) NIMBLE_HIDH: ENC_CHANGE status=0 encrypted=1 authenticated=1 bonded=1 +I (18636) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: KEYBOARD, MAP: 0, ID: 1, Len: 8, Data: +I (18636) ESP_HIDH_DEMO: 00 00 04 00 00 00 00 00 +I (18686) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: KEYBOARD, MAP: 0, ID: 1, Len: 8, Data: +I (18686) ESP_HIDH_DEMO: 00 00 00 00 00 00 00 00 +``` + +### Mouse Mode + +```text +I (7426) ESP_HIDH_DEMO: SCAN: 1 results + BLE: 02:50:f7:f9:55:60, RSSI: -29, USAGE: GENERIC, APPEARANCE: 0x03c0, ADDR_TYPE: '0', NAME: ESP Mouse +I (7436) NimBLE: GAP procedure initiated: connect; +I (7666) NimBLE: Connection established +I (8436) NIMBLE_HIDH: PASSKEY INPUT injected rc=0 +I (9716) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 OPEN: nimble +Report Maps: 1 + Report Map Length: 52 + MOUSE INPUT REPORT, ID: 0, Length: 4, Permissions: 0x1a, Handle: 37, CCC Handle: 38 +I (12766) NIMBLE_HIDH: ENC_CHANGE status=0 encrypted=1 authenticated=1 bonded=1 +I (15316) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: MOUSE, MAP: 0, ID: 0, Len: 4, Data: +I (15316) ESP_HIDH_DEMO: 00 f6 00 00 +I (15816) ESP_HIDH_DEMO: 02:50:f7:f9:55:60 INPUT: MOUSE, MAP: 0, ID: 0, Len: 4, Data: +I (15816) ESP_HIDH_DEMO: 00 00 0a 00 +``` + ## Troubleshooting For any technical queries, please open an [issue](https://github.com/espressif/esp-idf/issues) on GitHub. We will get back to you soon.