esp_hidh report map length checking function (IDFGH-5889)

[guo-max]

Environment

• Development Kit: [ESP32-Wrover-Kit]
• Kit version (for WroverKit v4]
• Module or chip used: [ESP32-WROOM-32]
• IDF version (run git describe --tags to find it):
  // ESP-IDF v4.4-dev-3042-g220590d599-dirty
• Build System: [idf.py]
• Operating System: [Linux]
• Using an IDE?: [No]
• Power Supply: [USB]

Problem Description

esp_hidh_dev_report_t *esp_hidh_dev_get_input_report_by_proto_and_data(esp_hidh_dev_t *dev, int protocol_mode,
                                                                       size_t len, const uint8_t *data, bool *has_report_id)
{
    esp_hidh_dev_report_t *r = dev->reports;
    *has_report_id = false;
    // first, assume data not include report id
    while (r) {
        if (r->value_len == len && r->report_id == 0 && (r->report_type & 1) &&
            r->protocol_mode == protocol_mode) {
            *has_report_id = false;
            break;
        }
        r = r->next;
    }
    // indicate data include report id
    if (r == NULL) {
        if (*data == 0) {
            ESP_LOGE(TAG, "data not include report id!");
            *has_report_id = false;
            return NULL;
        }
        r = dev->reports;
        while (r) {
            if (r->value_len == len + 1 && r->report_id == *data && (r->report_type & 1) &&

// r->value_len is the report map length and the "len" is the data length. the data should be report id + report data
// which r->value_len == len -1, not len +1

                r->protocol_mode == protocol_mode) {
                *has_report_id = true;
                break;
            }
            r = r->next;
        }
    }
    return r;
}

The fix should be:

diff --git a/components/esp_hid/src/esp_hidh.c b/components/esp_hid/src/esp_hidh.c
index 484092c885..49ff9c9cf3 100644
--- a/components/esp_hid/src/esp_hidh.c
+++ b/components/esp_hid/src/esp_hidh.c
@@ -639,7 +639,7 @@ esp_hidh_dev_report_t *esp_hidh_dev_get_input_report_by_proto_and_data(esp_hidh_
         }
         r = dev->reports;
         while (r) {
-            if (r->value_len == len + 1 && r->report_id == *data && (r->report_type & 1) &&
+            if (r->value_len == len - 1 && r->report_id == *data && (r->report_type & 1) &&
                 r->protocol_mode == protocol_mode) {
                 *has_report_id = true;
                 break;

[ghost]

@guo-max Thanks! We will fix it ASAP.

[finger563]

Any status on this?

The only patch required to fix it is:

--- a/components/esp_hid/src/esp_hidh.c
+++ b/components/esp_hid/src/esp_hidh.c
@@ -639,7 +639,7 @@ esp_hidh_dev_report_t *esp_hidh_dev_get_input_report_by_proto_and_data(esp_hidh_
         }
         r = dev->reports;
         while (r) {
-            if (r->value_len == len + 1 && r->report_id == *data && (r->report_type & 1) &&
+            if (r->value_len == len - 1 && r->report_id == *data && (r->report_type & 1) &&
                 r->protocol_mode == protocol_mode) {
                 *has_report_id = true;
                 break;

If you'd like we can open a PR for it.

[ghost]

@finger563

We already have an MR for this and other tiny bugs of HID internal, but we have to have a throughout test but the test is not all pass.

Please wait for our update.

Thanks