fix(hidraw): check ioctl errno in featureReport (HIDIOCSFEATURE failures were silently swallowed)#381
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
|
Warning Review limit reached
More reviews will be available in 3 minutes and 13 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
…res were silently swallowed) - linux.ioctl returns usize, so the `if (rc < 0)` guard was always false; every HIDIOCSFEATURE failure (ENODEV/EPIPE on unplug, EINVAL, EBADF, ...) returned OK and the device-init handshake in init.zig never saw the error. - Interpret the raw syscall return with linux.E.init(rc) and branch on errno != .SUCCESS, mapping ENODEV/EPIPE to Disconnected and the rest to Io. - posix.errno is unsuitable here: under libc linkage it reads the C errno global, which a raw linux.ioctl syscall never sets, so it reported SUCCESS for a -EBADF return. Test plan: - Added Layer-0 regression "hidraw: featureReport surfaces ioctl errno as error": featureReport over fd=-1 (EBADF) must return WriteError.Io. Falsifiable: against the old `if (rc < 0)` code (libc-linked) it fails with "expected error.Io, found void". - `./scripts/padctl-docker test` → EXIT=0 (1635 tests). - `zig fmt --check src/io/hidraw.zig` clean. refs: audit HIGH
BANANASJIM
force-pushed
the
fix/hidraw-featurereport-errno
branch
from
d1158b2 to
5a6ff05
Compare
1 participant
What
HidrawDevice.featureReportguarded theHIDIOCSFEATUREioctl withif (rc < 0).linux.ioctlreturnsusize(unsigned), so the guard was always false — every feature-report failure (ENODEV/EPIPE on unplug, EINVAL, EBADF, ...) returnedOK.The device-init handshake (
src/init.zig) callsfeatureReportand relies on the returned error to abort init; it never saw the failure.Fix
linux.E.init(rc)and branch onerrno != .SUCCESS.ENODEV/EPIPE→WriteError.Disconnected, all others →WriteError.Io.posix.errnois unsuitable here: under libc linkage it reads the Cerrnoglobal, which a rawlinux.ioctlsyscall never sets, so it reportedSUCCESSfor a-EBADFreturn (this is also why the naive port of the guard still passed silently).Test plan
hidraw: featureReport surfaces ioctl errno as error:featureReportoverfd=-1(EBADF) must returnWriteError.Io.if (rc < 0)code (libc-linked, as CI builds), the test fails withexpected error.Io, found void../scripts/padctl-docker test→ EXIT=0 (1635 tests pass).zig fmt --check src/io/hidraw.zigclean.refs: audit HIGH