Patch NUT 2.8.5 usbhid-ups out-of-bounds read causing driver segfaults

[frenck]

Proposed Changes

(Describe the changes and rationale behind them)

This applies an upstream NUT fix to the source we compile, to address the driver restart loop reported in #504.

The bug

In drivers/libusb1.c, nut_libusb_open() iterates the two candidate HID report descriptor lengths:

int32_t rdlen1, rdlen2, rdlens[2];
size_t j;
...
for (j = 0; j < sizeof(rdlens); j++) {
	rdlen = rdlens[j];

sizeof(rdlens) is 8 (bytes), not 2 (elements), so the loop reads rdlens[2..7] — 24 bytes past the end of the array — and uses that stack garbage as rdlen. The "ran out of candidates" check afterwards has the same defect, so it never fires as intended either. Garbage that happens to survive the sanity checks is handed to libusb_control_transfer() and on to the HID parser callback, which is where it crashes. drivers/libusb0.c carries the identical defect in the same two places.

The out-of-range indices are only reached when neither real candidate satisfies the caller, which is exactly the state the reports in #504 describe: string descriptor 0 request failed, get Manufacturer string failed, get Product string failed, get Serial Number string failed, and upstream's device->Product is NULL.

Why it lines up with the 0.18.0 boundary

This is a regression from aba6f43 (#3136), first released in NUT v2.8.5. v0.17.0 of this app shipped Debian's NUT 2.8.1 packages; v0.18.0 switched to building 2.8.5 from source. That is why downgrading to v0.17.0 makes the restart loop go away for everyone in that thread, and why no configuration change affects it.

Why a patch rather than a version bump

Fixed upstream in edc06fb, merged 2026-07-30. But v2.8.5 (April 2026) is still the latest NUT release, so there is nothing to bump to yet. The patch can be dropped again when 2.8.6 lands.

The patch file is vendored under nut/patches/ rather than fetched at build time, so the build stays hermetic and the change is reviewable in the diff, consistent with how the release tarball is already checksum verified. The build step applies every *.patch in that directory in sorted order, with --forward, so a patch that has become redundant fails the build loudly instead of being silently skipped. patch is pinned explicitly in the builder rather than relying on it arriving transitively via build-essential → dpkg-dev.

Verification

• Confirmed the defect is present in the exact nut-2.8.5.tar.gz we compile (drivers/libusb1.c:726 and :816, drivers/libusb0.c:661 and :726).
• Confirmed the patch applies during the build and that the compiled tree has SIZEOF_ARRAY(rdlens) in all four places.
• Full image builds; no NUT binary has an unresolved shared library; the driver set is unchanged at 69 drivers.

Important

I have no affected UPS hardware, so I could not reproduce the crash or confirm first hand that this stops it. The source-level defect, its 2.8.5-only provenance, and the log signatures in #504 all line up, but it would be good to have one of the reporters confirm against a build of this branch before we call #504 fixed.

Not fixed here

invalid libusb bus number 0, which appears throughout #504, is a red herring. Despite the wording it reports libusb_get_port_number() returning 0, not the bus number, and it is upsdebugx(1)-only and harmless (drivers/libusb1.c:398-410). #504 has also accumulated several unrelated problems (the integration's localhost host setting, an SNMP case, "no UPS definitions in ups.conf", a pwned-password startup exit) that want splitting into their own issues.

Related Issues

(Github link to related issues or pull requests)

Addresses the USB restart loop in #504.
Follows #527.
Upstream: networkupstools/nut#3422, networkupstools/nut#3550.

Summary by CodeRabbit

• Bug Fixes

  • Improved USB device detection reliability by preventing invalid memory access during device discovery.
  • Corrected device candidate handling to ensure all supported entries are evaluated properly.

• Build Improvements

  • Added support for applying source patches automatically during container builds.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bafe452b-29a3-4da7-bc6e-9e4588cf51d7

📥 Commits

Reviewing files that changed from the base of the PR and between 8c58471 and 4d14d9c.

📒 Files selected for processing (2)

• nut/Dockerfile
• nut/patches/0001-libusb-bound-rdlens-loop-by-element-count.patch

Included review availability: Your plan provides up to 2 included reviews per hour; 0 remain after this review.

---

Walkthrough

The Docker builder now copies and applies local patches before compiling NUT. The included patch bounds report-length loops in the libusb 0 and libusb 1 drivers by using the array element count.

Changes

NUT patch build

Layer / File(s)	Summary
Bound libusb report-length loops nut/patches/0001-libusb-bound-rdlens-loop-by-element-count.patch	The libusb 0 and libusb 1 driver loops and exhaustion checks now use SIZEOF_ARRAY(rdlens).
Apply patches during the build nut/Dockerfile	The builder copies the patches directory, installs patch, and applies each patch before configuration and compilation.

Estimated code review effort: 2 (Simple) | ~10 minutes

Merge Risk: ⚪ Minimal · up to 4d14d

The PR applies a localized fix for the out-of-bounds HID report-length iteration and integrates it into the build; no actionable merge-blocking risk remains after normal checks and review.

Poem

A rabbit hops through build-time snow,
Applying patches in a row.
Two libusb loops now count with care,
No extra steps beyond the pair.
NUT compiles, neat and bright.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely describes the NUT 2.8.5 out-of-bounds read fix and its effect on usbhid-ups driver stability.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0 files. (2 skipped: 2 unsupported.)
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix-usbhid-segfault

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}