fix(hid): Store client instance per-device instead of per-class

[kajteklau]

USBX HID Shared Client Instance Pointer Bug

Target Repo

eclipse-threadx/usbx (MIT license, Category A Simple Contribution)

---

Symptom

This bug surfaces when a user plugs in two or more HID devices of the same class to the hub tree. Removing any one HID device of a given type (keyboard or mouse) causes all remaining devices of that type to stop reporting input. Callbacks write into freed memory.

---

Background: The Global Client Table

At startup, the application registers HID client types (keyboard, mouse) by calling ux_host_class_hid_client_register(). This function allocates a single flat array of UX_HOST_CLASS_HID_CLIENT structs, stored at class->ux_host_class_client on the UX_HOST_CLASS container for HID. In this application there are two entries:

class->ux_host_class_client -> [ [0]: keyboard handler | local_instance ]
                               [ [1]: mouse handler    | local_instance ]

Each entry holds a handler function pointer (e.g. ux_host_class_hid_keyboard_entry) and a VOID *ux_host_class_hid_client_local_instance field intended to point to the per-device instance (e.g. UX_HOST_CLASS_HID_KEYBOARD).

---

Original Code: Device Activation Sequence

When a USB HID device enumerates, the middleware creates a per-interface UX_HOST_CLASS_HID struct (hid) and calls _ux_host_class_hid_client_search(hid). This function:

1. Iterates the global client table to find a matching client (by HID usage page/usage).
2. Stores a direct pointer to the matching table entry on the HID instance:

   hid->ux_host_class_hid_client = hid_client;  // points into the shared table

3. Calls the client's activate handler (e.g. _ux_host_class_hid_keyboard_activate).

The activate handler then:

1. Reads the client pointer back: hid_client = hid->ux_host_class_hid_client
2. Allocates a new UX_HOST_CLASS_HID_KEYBOARD struct (keyboard_instance)
3. Stores it on the shared table entry:

   hid_client->ux_host_class_hid_client_local_instance = (VOID *)keyboard_instance;

The problem: hid_client points to the shared table entry. Every keyboard shares the same entry. The local_instance field is a single pointer — the last keyboard to activate overwrites it.

---

Original Code: Device Deactivation Sequence

When a device is unplugged, _ux_host_class_hid_keyboard_deactivate() runs:

1. Reads the client pointer: hid_client = hid->ux_host_class_hid_client (shared entry)
2. Reads the keyboard instance: keyboard_instance = hid_client->local_instance
3. Frees the keyboard instance's thread, semaphore, buffers, and the instance itself.

The deactivation has no other way to find the keyboard instance. The address was only
ever stored in one place: hid_client->local_instance on the shared table entry.

---

Failure Sequence (Two Keyboards, A and B)

1. Keyboard A activates
   -> shared_entry.local_instance = keyboard_instance_A

2. Keyboard B activates
   -> shared_entry.local_instance = keyboard_instance_B  (overwrites A)

   State: hid_A->hid_client -> shared_entry -> keyboard_instance_B
          hid_B->hid_client -> shared_entry -> keyboard_instance_B
          keyboard_instance_A exists in memory but nothing points to it

3. Keyboard A is unplugged
   -> deactivate reads shared_entry.local_instance -> gets keyboard_instance_B
   -> frees keyboard_instance_B (WRONG — this belongs to the still-connected keyboard)

   State: hid_B is still active, interrupt endpoint still delivering data
          hid_B's callback reads shared_entry.local_instance -> dangling pointer
          keyboard_instance_A is leaked (never freed, nothing references it)

Keyboard B's interrupt reports continue to arrive, but the callback writes into freed memory. The result is silent data loss and memory leak.

---

Fix

Instead of storing a pointer to the shared table entry, allocate a per-instance copy of the client struct so each HID device gets its own local_instance pointer.

In _ux_host_class_hid_client_search() (lines 119–155):

UX_HOST_CLASS_HID_CLIENT *hid_client_instance;
hid_client_instance = _ux_utility_memory_allocate(..., sizeof(UX_HOST_CLASS_HID_CLIENT));
_ux_utility_memory_copy(hid_client_instance, hid_client, sizeof(UX_HOST_CLASS_HID_CLIENT));
hid_client_instance->ux_host_class_hid_client_local_instance = UX_NULL;
hid->ux_host_class_hid_client = hid_client_instance;

The copy inherits the handler function pointer and name from the shared entry but has its own local_instance field. local_instance is explicitly set to NULL so the copy doesn't carry a stale pointer from a previous activation of the same device type. The activate handler then writes the freshly allocated keyboard instance to this private
copy's local_instance.

In _ux_host_class_hid_deactivate(), the per-instance copy is freed after the client deactivate handler runs:

_ux_utility_memory_free(hid->ux_host_class_hid_client);
hid->ux_host_class_hid_client = UX_NULL;

---

Fixed Sequence (Two Keyboards, A and B)

1. Keyboard A activates
   -> private_copy_A.local_instance = keyboard_instance_A
   -> hid_A->hid_client = private_copy_A

2. Keyboard B activates
   -> private_copy_B.local_instance = keyboard_instance_B
   -> hid_B->hid_client = private_copy_B

3. Keyboard A is unplugged
   -> deactivate reads private_copy_A.local_instance -> keyboard_instance_A (CORRECT)
   -> frees keyboard_instance_A, then frees private_copy_A

   Keyboard B is completely untouched.

---

Files Changed

• common/usbx_host_classes/src/ux_host_class_hid_client_search.c (lines 119–155)
• common/usbx_host_classes/src/ux_host_class_hid_deactivate.c

---

Memory Cost

One additional sizeof(UX_HOST_CLASS_HID_CLIENT) allocation (~40 bytes) per connected
HID device. Freed on device removal.

[kajteklau]

Hi, sorry I made a mistake when signing the ECA. It should be fixed now. Please let me know what can I do to contribute.

-- Kajtek