Skip to content

fix(usbraw): release claimed interface on open() error paths#369

Merged
BANANASJIM merged 1 commit into
mainfrom
fix/usbraw-claim-leak
Jun 5, 2026
Merged

fix(usbraw): release claimed interface on open() error paths#369
BANANASJIM merged 1 commit into
mainfrom
fix/usbraw-claim-leak

Conversation

@BANANASJIM

@BANANASJIM BANANASJIM commented Jun 5, 2026

Copy link
Copy Markdown
Owner

Summary

Resource-leak fix surfaced by an adversarial review of the #355 work.

UsbrawDevice.open() registered its release_interface + close + exit errdefer only after self.* was populated. A pipe2 (std.posix.pipe2) or allocator (alloc.create) failure between a successful libusb_claim_interface and that point returned an error without releasing the interface or exiting the context — leaking the claim and leaving the USB device ownerless (no kernel driver, not claimed by us).

Fix

  • Move the cleanup errdefer to immediately after the successful claim so it covers the pipe2 and alloc.create failure paths, and re-attach the kernel driver in it (matching close() and the BUSY/ClaimFailed branches).
  • UsbrawSuppress.openSuppress() already released on alloc failure but did not re-attach the kernel driver; add the re-attach for the same consistency.

Error-path only; no change to the success path.

Test plan

  • zig build -Dlibusb=true compiles clean; zig build test -Dlibusb=true (full suite) green in the canonical Docker image.
  • zig build test-tsan -Dlibusb=true green in Docker.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced error handling in USB device operations to ensure proper cleanup of interfaces and kernel driver reattachment during failure scenarios, improving reliability and preventing resource leaks.

UsbrawDevice.open() registered its release/close/exit errdefer only after
self.* was populated, so a pipe2 or allocator failure between a successful
libusb_claim_interface and that point returned without releasing the
interface or exiting the context — leaking the claim and leaving the device
ownerless. Move the cleanup errdefer to immediately after the successful
claim and re-attach the kernel driver, matching close(). Also re-attach the
kernel driver in the UsbrawSuppress alloc-failure path for the same reason.
@qodo-code-review

Copy link
Copy Markdown

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 39563211-1c7b-4fbd-9fc5-9722b8577a42

📥 Commits

Reviewing files that changed from the base of the PR and between 5aad554 and b8828fb.

📒 Files selected for processing (1)
  • src/io/usbraw.zig

📝 Walkthrough

Walkthrough

This PR restructures error-handling in USB device initialization by moving the cleanup guard point earlier in the open flow. After successfully claiming the USB interface in UsbrawDevice.open, an errdefer is now installed immediately to ensure consistent resource teardown (interface release, kernel driver reattach, device close, libusb context exit) if any later setup fails. A redundant cleanup block formerly placed later is removed. A related adjustment in UsbrawSuppress.openSuppress removes the interface release from its allocation-failure cleanup path.

Changes

USB device initialization cleanup

Layer / File(s) Summary
Early errdefer guard and redundant cleanup removal
src/io/usbraw.zig
UsbrawDevice.open installs an errdefer immediately after successful libusb_claim_interface to release the interface, reattach kernel driver, and close libusb resources if later initialization fails, replacing a redundant cleanup block placed later in the flow. UsbrawSuppress.openSuppress adjusts allocation-failure cleanup to remove the interface release while retaining kernel driver reattach.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • BANANASJIM/padctl#10: Prior work on UsbrawDevice.open error-path cleanup logic for interface release and libusb resource teardown.

Poem

🐰 A guard so early, a guard so true,
Catches errors before they break through,
Resources released with care precise,
Cleanup now happens just once, so nice!
No driver left orphaned—all's well, hip-hip!

🚥 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 describes the main fix: ensuring the claimed USB interface is released on error paths during device opening.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
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.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/usbraw-claim-leak

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@BANANASJIM BANANASJIM merged commit 3643941 into main Jun 5, 2026
36 checks passed
@BANANASJIM BANANASJIM deleted the fix/usbraw-claim-leak branch June 5, 2026 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant