From 7ee511e2f08c53d79d57d7ba8de415f7894d769a Mon Sep 17 00:00:00 2001 From: Mayank Tyagi Date: Fri, 15 May 2026 17:51:53 +0530 Subject: [PATCH] fix(nimble): Forward HID report/control/protocol writes as HIDD events --- components/bt/host/nimble/nimble | 2 +- .../host/nimble/port/include/esp_nimble_cfg.h | 2 + components/esp_hid/src/ble_hidh.c | 15 + components/esp_hid/src/nimble_hidd.c | 279 +++++++++++++++++- 4 files changed, 286 insertions(+), 12 deletions(-) diff --git a/components/bt/host/nimble/nimble b/components/bt/host/nimble/nimble index 7be10cd22ac..bc528ad5491 160000 --- a/components/bt/host/nimble/nimble +++ b/components/bt/host/nimble/nimble @@ -1 +1 @@ -Subproject commit 7be10cd22ac795b717e8801a8f3c80cea5f27bbb +Subproject commit bc528ad54918bbacdf864f55f3257ee45711735e diff --git a/components/bt/host/nimble/port/include/esp_nimble_cfg.h b/components/bt/host/nimble/port/include/esp_nimble_cfg.h index 673a0c58040..7ca0f368a23 100644 --- a/components/bt/host/nimble/port/include/esp_nimble_cfg.h +++ b/components/bt/host/nimble/port/include/esp_nimble_cfg.h @@ -1029,6 +1029,8 @@ #ifndef MYNEWT_VAL_BLE_SM_SC_LVL #ifdef CONFIG_BT_NIMBLE_SM_SC_LVL #define MYNEWT_VAL_BLE_SM_SC_LVL CONFIG_BT_NIMBLE_SM_SC_LVL +#elif defined(CONFIG_BT_NIMBLE_SM_LVL) +#define MYNEWT_VAL_BLE_SM_SC_LVL CONFIG_BT_NIMBLE_SM_LVL #else #define MYNEWT_VAL_BLE_SM_SC_LVL (0) #endif diff --git a/components/esp_hid/src/ble_hidh.c b/components/esp_hid/src/ble_hidh.c index 2b31cfbd7ef..4b3fdaf7c0c 100644 --- a/components/esp_hid/src/ble_hidh.c +++ b/components/esp_hid/src/ble_hidh.c @@ -302,6 +302,21 @@ static void attach_report_listeners(esp_gatt_if_t gattc_if, esp_hidh_dev_t *dev) //subscribe to battery notifications if (dev->ble.battery_handle) { + uint8_t *rdata = NULL; + uint16_t rlen = 0; + + if (event_loop_handle && + read_char(gattc_if, dev->ble.conn_id, dev->ble.battery_handle, + ESP_GATT_AUTH_REQ_NO_MITM, &rdata, &rlen) == ESP_GATT_OK && + rlen >= 1 && rdata != NULL) { + esp_hidh_event_data_t p = {0}; + p.battery.dev = dev; + p.battery.level = rdata[0]; + esp_event_post_to(event_loop_handle, ESP_HIDH_EVENTS, ESP_HIDH_BATTERY_EVENT, + &p, sizeof(esp_hidh_event_data_t), portMAX_DELAY); + } + free(rdata); + register_for_notify(gattc_if, dev->bda, dev->ble.battery_handle); if (dev->ble.battery_ccc_handle) { //Write CCC descr to enable notifications diff --git a/components/esp_hid/src/nimble_hidd.c b/components/esp_hid/src/nimble_hidd.c index 2eae6ee46da..18d9dc51678 100644 --- a/components/esp_hid/src/nimble_hidd.c +++ b/components/esp_hid/src/nimble_hidd.c @@ -26,6 +26,9 @@ #include "services/hid/ble_svc_hid.h" #include "services/dis/ble_svc_dis.h" #include "services/sps/ble_svc_sps.h" +#include +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" #if CONFIG_BT_NIMBLE_HID_SERVICE @@ -41,6 +44,22 @@ static esp_ble_hidd_dev_t *s_dev = NULL; Assuming the first instance of the hid service is registered first. Increment service index as the hid services get registered */ static int service_index = -1; +static SemaphoreHandle_t s_hidd_cb_mutex = NULL; +static struct ble_gap_event_listener nimble_gap_event_listener; + +static inline void hidd_cb_lock(void) +{ + if (s_hidd_cb_mutex) { + xSemaphoreTake(s_hidd_cb_mutex, portMAX_DELAY); + } +} + +static inline void hidd_cb_unlock(void) +{ + if (s_hidd_cb_mutex) { + xSemaphoreGive(s_hidd_cb_mutex); + } +} typedef hidd_report_item_t hidd_le_report_item_t; @@ -75,8 +94,13 @@ struct esp_ble_hidd_dev_s { uint8_t pnp[7]; /* something related to device info service */ hidd_dev_map_t *devices; uint8_t devices_len; + atomic_uint_fast32_t event_loop_ref; /* refcount for in-flight posts */ }; +static void nimble_report_write_cb(uint16_t attr_handle, uint8_t report_type, uint8_t report_id, + const uint8_t *data, uint16_t len); +static void nimble_char_write_cb(uint16_t attr_handle, uint16_t char_uuid16, uint8_t value); + // HID Information characteristic value static const uint8_t hidInfo[4] = { 0x11, 0x01, // bcdHID (USB HID version) @@ -287,8 +311,22 @@ static int ble_hid_free_config(esp_ble_hidd_dev_t *dev) return ESP_OK; } +static void nimble_hidd_stop_event_ack_handler(void *arg, esp_event_base_t event_base, + int32_t event_id, void *event_data) +{ + (void)event_base; + (void)event_id; + (void)event_data; + SemaphoreHandle_t sem = (SemaphoreHandle_t)arg; + if (sem != NULL) { + xSemaphoreGive(sem); + } +} + static int nimble_hidd_dev_deinit(void *devp) { esp_ble_hidd_dev_t *dev = (esp_ble_hidd_dev_t *)devp; + SemaphoreHandle_t stop_event_sem = NULL; + esp_err_t reg_rc = ESP_FAIL; if (!s_dev) { ESP_LOGE(TAG, "HID device profile already uninitialized"); return ESP_OK; @@ -298,11 +336,57 @@ static int nimble_hidd_dev_deinit(void *devp) { ESP_LOGE(TAG, "Wrong HID device provided"); return ESP_FAIL; } + hidd_cb_lock(); + nimble_hid_stop_gatts(dev); + ble_svc_hid_register_report_write_cb(NULL); + ble_svc_hid_register_char_write_cb(NULL); + /* unregister GAP event listener to prevent callbacks accessing s_dev + after we clear it; clear NimBLE host callbacks too for extra safety */ + ble_gap_event_listener_unregister(&nimble_gap_event_listener); + ble_hs_cfg.sync_cb = NULL; + ble_hs_cfg.gatts_register_cb = NULL; s_dev = NULL; service_index = -1; // resetting the value + hidd_cb_unlock(); + + stop_event_sem = xSemaphoreCreateBinary(); + if (stop_event_sem != NULL) { + reg_rc = esp_event_handler_register_with(dev->event_loop_handle, ESP_HIDD_EVENTS, + ESP_HIDD_STOP_EVENT, + nimble_hidd_stop_event_ack_handler, + stop_event_sem); + } - nimble_hid_stop_gatts(dev); esp_event_post_to(dev->event_loop_handle, ESP_HIDD_EVENTS, ESP_HIDD_STOP_EVENT, NULL, 0, portMAX_DELAY); + + if (reg_rc == ESP_OK) { + xSemaphoreTake(stop_event_sem, pdMS_TO_TICKS(100)); + esp_event_handler_unregister_with(dev->event_loop_handle, ESP_HIDD_EVENTS, + ESP_HIDD_STOP_EVENT, nimble_hidd_stop_event_ack_handler); + } + + if (stop_event_sem != NULL) { + vSemaphoreDelete(stop_event_sem); + } + + /* Wait for any in-flight event posts to complete before deleting the event loop. + Callbacks increment event_loop_ref while holding hidd_cb_lock, so once s_dev + is set to NULL no new increments will occur. Wait up to 1000ms for refcount + to drain to zero to avoid use-after-free; proceed after timeout with a warn. */ + { + int waited_ms = 0; + const int max_wait_ms = 1000; + while (atomic_load(&dev->event_loop_ref) != 0 && waited_ms < max_wait_ms) { + vTaskDelay(pdMS_TO_TICKS(10)); + waited_ms += 10; + } + if (atomic_load(&dev->event_loop_ref) != 0) { + ESP_LOGW(TAG, "Event loop posts still in flight (%u); leaving resources allocated to avoid UAF", + (unsigned)atomic_load(&dev->event_loop_ref)); + return ESP_FAIL; + } + } + ble_hid_free_config(dev); free(dev); return ESP_OK; @@ -362,12 +446,163 @@ static hidd_le_report_item_t* find_report_by_usage_and_type(uint8_t usage, uint8 return NULL; } +static hidd_le_report_item_t *find_report_by_handle(uint16_t attr_handle, uint8_t *map_index) +{ + for (uint8_t d = 0; d < s_dev->devices_len; d++) { + for (uint8_t i = 0; i < s_dev->devices[d].reports_len; i++) { + hidd_le_report_item_t *rpt = &s_dev->devices[d].reports[i]; + if (rpt->handle == attr_handle) { + if (map_index) { + *map_index = d; + } + return rpt; + } + } + } + return NULL; +} + +static void nimble_report_write_cb(uint16_t attr_handle, uint8_t report_type, uint8_t report_id, + const uint8_t *data, uint16_t len) +{ + esp_hidd_event_data_t *p_cb_param = NULL; + size_t event_data_size; + uint8_t map_index = 0; + hidd_le_report_item_t *match = NULL; + esp_event_loop_handle_t event_loop = NULL; + esp_ble_hidd_dev_t *dev_handle = NULL; + esp_hidd_dev_t *dev = NULL; + uint8_t usage = 0; + + hidd_cb_lock(); + if (s_dev == NULL || s_dev->event_loop_handle == NULL || data == NULL || len == 0) { + hidd_cb_unlock(); + return; + } + + if (report_type != ESP_HID_REPORT_TYPE_OUTPUT && report_type != ESP_HID_REPORT_TYPE_FEATURE) { + hidd_cb_unlock(); + return; + } + + match = find_report_by_handle(attr_handle, &map_index); + if (match == NULL) { + hidd_cb_unlock(); + return; + } + + event_data_size = sizeof(esp_hidd_event_data_t) + len; + p_cb_param = (esp_hidd_event_data_t *)calloc(1, event_data_size); + if (p_cb_param == NULL) { + ESP_LOGE(TAG, "%s malloc event data failed!", __func__); + hidd_cb_unlock(); + return; + } + + memcpy(((uint8_t *)p_cb_param) + sizeof(esp_hidd_event_data_t), data, len); + /* copy out data we need while holding lock and bump refcount */ + dev_handle = s_dev; + atomic_fetch_add(&dev_handle->event_loop_ref, 1); + event_loop = dev_handle->event_loop_handle; + dev = dev_handle->dev; + usage = match->usage; + hidd_cb_unlock(); + + if (report_type == ESP_HID_REPORT_TYPE_OUTPUT) { + p_cb_param->output.dev = dev; + p_cb_param->output.usage = usage; + p_cb_param->output.report_id = report_id; + p_cb_param->output.length = len; + p_cb_param->output.data = (uint8_t *)data; /* fixed by esp_hidd_process_event_data_handler */ + p_cb_param->output.map_index = map_index; + esp_event_post_to(event_loop, ESP_HIDD_EVENTS, ESP_HIDD_OUTPUT_EVENT, + p_cb_param, event_data_size, portMAX_DELAY); + atomic_fetch_sub(&dev_handle->event_loop_ref, 1); + } else { + p_cb_param->feature.dev = dev; + p_cb_param->feature.usage = usage; + p_cb_param->feature.report_id = report_id; + p_cb_param->feature.length = len; + p_cb_param->feature.data = (uint8_t *)data; /* fixed by esp_hidd_process_event_data_handler */ + p_cb_param->feature.map_index = map_index; + esp_event_post_to(event_loop, ESP_HIDD_EVENTS, ESP_HIDD_FEATURE_EVENT, + p_cb_param, event_data_size, portMAX_DELAY); + atomic_fetch_sub(&dev_handle->event_loop_ref, 1); + } + free(p_cb_param); +} + +static void nimble_char_write_cb(uint16_t attr_handle, uint16_t char_uuid16, uint8_t value) +{ + uint8_t map_index = 0; + bool found = false; + esp_hidd_event_data_t cb_param = {0}; + esp_event_loop_handle_t event_loop = NULL; + esp_ble_hidd_dev_t *dev_handle = NULL; + + hidd_cb_lock(); + if (s_dev == NULL || s_dev->event_loop_handle == NULL) { + hidd_cb_unlock(); + return; + } + + for (uint8_t d = 0; d < s_dev->devices_len; d++) { + if (char_uuid16 == BLE_SVC_HID_CHR_UUID16_PROTOCOL_MODE && + s_dev->devices[d].hid_protocol_handle == attr_handle) { + found = true; + map_index = d; + break; + } + if (char_uuid16 == BLE_SVC_HID_CHR_UUID16_HID_CTRL_PT && + s_dev->devices[d].hid_control_handle == attr_handle) { + found = true; + map_index = d; + break; + } + } + + if (!found) { + hidd_cb_unlock(); + return; + } + + if (char_uuid16 == BLE_SVC_HID_CHR_UUID16_PROTOCOL_MODE) { + s_dev->protocol = value; + cb_param.protocol_mode.dev = s_dev->dev; + cb_param.protocol_mode.protocol_mode = value; + cb_param.protocol_mode.map_index = map_index; + /* bump refcount while holding lock */ + dev_handle = s_dev; + atomic_fetch_add(&dev_handle->event_loop_ref, 1); + event_loop = dev_handle->event_loop_handle; + hidd_cb_unlock(); + esp_event_post_to(event_loop, ESP_HIDD_EVENTS, ESP_HIDD_PROTOCOL_MODE_EVENT, + &cb_param, sizeof(esp_hidd_event_data_t), portMAX_DELAY); + atomic_fetch_sub(&dev_handle->event_loop_ref, 1); + } else if (char_uuid16 == BLE_SVC_HID_CHR_UUID16_HID_CTRL_PT) { + s_dev->control = value; + cb_param.control.dev = s_dev->dev; + cb_param.control.control = value; + cb_param.control.map_index = map_index; + /* bump refcount while holding lock */ + dev_handle = s_dev; + atomic_fetch_add(&dev_handle->event_loop_ref, 1); + event_loop = dev_handle->event_loop_handle; + hidd_cb_unlock(); + esp_event_post_to(event_loop, ESP_HIDD_EVENTS, ESP_HIDD_CONTROL_EVENT, + &cb_param, sizeof(esp_hidd_event_data_t), portMAX_DELAY); + atomic_fetch_sub(&dev_handle->event_loop_ref, 1); + } else { + hidd_cb_unlock(); + } +} + static int nimble_hidd_dev_input_set(void *devp, size_t index, size_t id, uint8_t *data, size_t length) { hidd_le_report_item_t *p_rpt; esp_ble_hidd_dev_t *dev = (esp_ble_hidd_dev_t *)devp; int rc; - struct os_mbuf *om; + struct os_mbuf *om = NULL; if (!dev || s_dev != dev) { return ESP_FAIL; } @@ -386,6 +621,7 @@ static int nimble_hidd_dev_input_set(void *devp, size_t index, size_t id, uint8_ } rc = ble_hs_mbuf_to_flat(om, &dev->protocol, sizeof(dev->protocol), NULL); if(rc != 0) { + os_mbuf_free_chain(om); return ESP_FAIL; } /* free the mbuf */ @@ -413,7 +649,7 @@ static int nimble_hidd_dev_feature_set(void *devp, size_t index, size_t id, uint hidd_le_report_item_t *p_rpt; esp_ble_hidd_dev_t *dev = (esp_ble_hidd_dev_t *)devp; int rc; - struct os_mbuf *om; + struct os_mbuf *om = NULL; if (!dev || s_dev != dev) { return ESP_FAIL; } @@ -432,6 +668,7 @@ static int nimble_hidd_dev_feature_set(void *devp, size_t index, size_t id, uint } rc = ble_hs_mbuf_to_flat(om, &dev->protocol, sizeof(dev->protocol), NULL); if(rc != 0) { + os_mbuf_free_chain(om); return ESP_FAIL; } /* free the mbuf */ @@ -481,7 +718,7 @@ static void ble_hidd_dev_free(void) static int nimble_hid_gap_event(struct ble_gap_event *event, void *arg) { struct ble_gap_conn_desc desc; - struct os_mbuf *om; + struct os_mbuf *om = NULL; uint8_t data; int rc; @@ -504,12 +741,14 @@ static int nimble_hid_gap_event(struct ble_gap_event *event, void *arg) /* 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 */ + /* allocate a fresh mbuf for each device to avoid use-after-free */ for(int i = 0; i < s_dev->devices_len; i++) { + om = ble_hs_mbuf_from_flat(&data, 1); + if(om == NULL) { + ESP_LOGD(TAG, "No memory to allocate mbuf"); + continue; + } + /* NOTE : om is freed by stack in ble_att_svr_write_local */ rc = ble_att_svr_write_local(s_dev->devices[i].hid_protocol_handle, om); if (rc != 0) { ESP_LOGE(TAG, "Write on Protocol Mode Failed: %d", rc); @@ -540,7 +779,7 @@ static void nimble_gatt_svr_register_cb(struct ble_gatt_register_ctxt *ctxt, voi { char buf[BLE_UUID_STR_LEN]; hidd_le_report_item_t *rpt = NULL; - struct os_mbuf *om; + struct os_mbuf *om = NULL; uint16_t uuid16; uint16_t report_info; uint8_t report_type, report_id; @@ -642,7 +881,6 @@ void nimble_host_reset(int 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; @@ -663,6 +901,13 @@ esp_err_t esp_ble_hidd_dev_init(esp_hidd_dev_t *dev_p, const esp_hid_device_conf s_dev->protocol = ESP_HID_PROTOCOL_MODE_REPORT; s_dev->event_loop_handle = NULL; s_dev->dev = dev_p; + if (s_hidd_cb_mutex == NULL) { + s_hidd_cb_mutex = xSemaphoreCreateMutex(); + if (s_hidd_cb_mutex == NULL) { + ble_hidd_dev_free(); + return ESP_ERR_NO_MEM; + } + } esp_event_loop_args_t event_task_args = { .queue_size = 5, @@ -710,8 +955,20 @@ esp_err_t esp_ble_hidd_dev_init(esp_hidd_dev_t *dev_p, const esp_hid_device_conf 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; + ble_svc_hid_register_report_write_cb(nimble_report_write_cb); + ble_svc_hid_register_char_write_cb(nimble_char_write_cb); rc = nimble_hid_start_gatts(); if(rc != ESP_OK) { + ble_svc_hid_register_report_write_cb(NULL); + ble_svc_hid_register_char_write_cb(NULL); + /* Stop any GATT services that were initialized by nimble_hid_start_gatts */ + nimble_hid_stop_gatts(s_dev); + /* Prevent NimBLE host callbacks from running against freed state */ + ble_hs_cfg.sync_cb = NULL; + ble_hs_cfg.gatts_register_cb = NULL; + /* reset service index to initial state to avoid stale index on re-init */ + service_index = -1; + ble_hidd_dev_free(); return rc; } ble_gap_event_listener_register(&nimble_gap_event_listener,