From 14703e8ee829bb328c2dac4c2b139a8da22a8c1e Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sat, 12 Sep 2026 15:55:09 -0500 Subject: [PATCH 1/2] fix(usb_host): clamp copied input-report length; document the failed-teardown state Two follow-ups from the Copilot review of #792 that concern code now on main: - on_interface_event(): never trust the driver-reported input-report length beyond the buffer we handed it -- clamp `len` to the copy capacity (logging the truncation) so Event::data() can never form an out-of-bounds span. - deinitialize(): when hid_host_uninstall() keeps failing the root port is deliberately left powered off (powering it back up would only make the driver re-track the device a retry needs gone). Document that state in the header and the error log: event delivery is stopped, devices are retired, and the only valid next steps are retrying deinitialize() or destroying the object. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/usb_host/include/usb_host.hpp | 10 ++++++++-- components/usb_host/src/usb_host.cpp | 17 +++++++++++++++-- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/components/usb_host/include/usb_host.hpp b/components/usb_host/include/usb_host.hpp index 931eddc7e..10681f939 100644 --- a/components/usb_host/include/usb_host.hpp +++ b/components/usb_host/include/usb_host.hpp @@ -264,8 +264,14 @@ class UsbHost : public BaseComponent { /// @param ec Set on failure. If any step of the teardown fails (a device the /// driver cannot release, or the library refusing to uninstall) the host /// stays initialized (is_initialized() remains true) and false is - /// returned, rather than tearing down under a live driver. Destroying - /// a UsbHost in that state aborts (see the destructor). + /// returned, rather than tearing down under a live driver. In that + /// state event delivery has already stopped, every device has been + /// retired, and the root port is left powered off (powering it back up + /// would only make the driver re-track the device that a retry needs + /// gone), so the host is not usable: the only valid next steps are to + /// call deinitialize() again (which waits for the driver again) or to + /// destroy the object, which aborts if teardown still fails (see the + /// destructor). /// @return true on success. bool deinitialize(std::error_code &ec); diff --git a/components/usb_host/src/usb_host.cpp b/components/usb_host/src/usb_host.cpp index c063add3a..29d560ee3 100644 --- a/components/usb_host/src/usb_host.cpp +++ b/components/usb_host/src/usb_host.cpp @@ -413,8 +413,14 @@ bool UsbHost::deinitialize(std::error_code &ec) { } if (err != ESP_OK) { // Tearing down under a driver that still references us would be a - // use-after-free waiting to happen; stay initialized and report it. - logger_.error("hid_host_uninstall failed: {} (a device could not be released)", + // use-after-free waiting to happen; stay initialized and report it. The + // root port deliberately stays powered OFF: powering it back up would make + // the driver re-enumerate (and track) the attached device again, which is + // exactly what a retry of deinitialize() needs to have gone away. Event + // delivery is already stopped, so the only valid next steps are retrying + // deinitialize() or destroying the object (see the header). + logger_.error("hid_host_uninstall failed: {} (a device could not be released); " + "root port left powered off, retry deinitialize()", esp_err_to_name(err)); ec = make_ec(err); return false; @@ -569,6 +575,13 @@ void UsbHost::on_interface_event(hid_host_device_handle_t handle, if (err != ESP_OK) { return; } + if (len > cap) { + // The driver copies at most `cap` bytes, so this only happens if it ever + // reports the report's full length rather than the copied length; never + // let it turn into an out-of-bounds span. + logger_.warn("input report of {} bytes truncated to {} (max_input_report_size)", len, cap); + len = cap; + } ev.len = len; enqueue(std::move(ev)); break; From b27a5be2ccacaedfa1d398a7de56e209c5c11383 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Sat, 12 Sep 2026 16:13:32 -0500 Subject: [PATCH 2/2] fix(usb_host): don't hard-code a cause in the hid_host_uninstall error log Only ESP_ERR_INVALID_STATE means the driver still tracks a device; report any other error as-is instead of guessing at a cause. Keeps the "root port left powered off, retry deinitialize()" guidance. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01DFjjnq3XCTRAXENSxsCxJU --- components/usb_host/src/usb_host.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/components/usb_host/src/usb_host.cpp b/components/usb_host/src/usb_host.cpp index 29d560ee3..64b995fcc 100644 --- a/components/usb_host/src/usb_host.cpp +++ b/components/usb_host/src/usb_host.cpp @@ -419,9 +419,12 @@ bool UsbHost::deinitialize(std::error_code &ec) { // exactly what a retry of deinitialize() needs to have gone away. Event // delivery is already stopped, so the only valid next steps are retrying // deinitialize() or destroying the object (see the header). - logger_.error("hid_host_uninstall failed: {} (a device could not be released); " - "root port left powered off, retry deinitialize()", - esp_err_to_name(err)); + // ESP_ERR_INVALID_STATE is what the driver returns while it still tracks a + // device; anything else is reported as-is rather than guessed at. + logger_.error("hid_host_uninstall failed: {}{}; root port left powered off, retry " + "deinitialize()", + esp_err_to_name(err), + err == ESP_ERR_INVALID_STATE ? " (the driver still tracks a device)" : ""); ec = make_ec(err); return false; }