Repository navigation
Conversation
|
Thanks for this. Before I review it properly I'd like to understand the behaviour change. If an armed verification never times out, what happens for callers that don't cancel, like sudo, polkit or lock screen prompts going through fprintd? Do they just hang? Could the timeout stay as a longer upper bound instead of going away entirely? The driver change itself looks small (sbio.rs and sep.rs), but host-tests.json shows +2,322/-2,304, which looks like reformatting. Could that be a separate commit, or dropped, so the behaviour change is easy to review on its own? |
Avoid restarting a healthy armed verification every minute while retaining a fixed upper bound for callers that never cancel. Keep enrollment and progress limits at sixty seconds. Check expiry before accepting a frame or allowing ARMED to renew its budget, including after a slow status read. Five minutes is a proposed host policy, not a measured firmware lifetime. Document PAM and D-Bus cancellation, claim ownership, tighter expiry boundary handling and remaining hardware qualification limits. Test the production wait and deliberate timeout/cancellation/order regressions with synthetic time. Signed-off-by: Matt <mmdisbro@gmail.com>
bd0c12f to
b5340f8
Compare
|
Thanks. The timeout is now a fixed upper bound of 300 seconds instead of none, so sudo, polkit and lock-screen callers can't hang forever. Cancellation still takes priority, and the existing 60-second arming and progress limits stay. I removed the reformatting in |
scottjones
left a comment
There was a problem hiding this comment.
We'd hold this until after the release candidate unless a default flow needs it.
Thanks for putting a cap back. We reviewed b5340f8. It merges clean with #234 and #250.
There's no hang path as long as SPI calls return. Verification is capped at 300 s from the start of the wait (set at sbio.rs:2871, checked at :2919), cancellation is checked before and after every status read (sbio.rs:2877 and :2890), and the 60 s arming and progress limits still apply. Five minutes is a reasonable cap.
- As you note, pam_fprintd stops at 30 s by default, and Omarchy's stacks don't override it (sudo, polkit-1, omarchy-lock-fingerprint; omarchy-apply-lock writes it without timeout=). So which real flow hit the 60 s teardown the first sentence describes? With default config this changes nothing for sudo, polkit or the lock screen. It only matters for
fprintd-verify, custom clients, or an admin who raised the PAM timeout. - You call out that the deadline check now comes before frame acceptance. For enrollment that turns a frame read at the 60 s boundary into a timeout that ends the enrollment, where before it was used. Nothing in this PR needs that for enrollment, so we'd keep the old order there, or split it out with a hardware run. The trailing status read on timeout is also gone; was that intentional?
What we ran: python3 -m unittest discover -s tools/aurora-sep/tests -p test_capture_wait.py with Rust 1.85.0 passes (31 cases, 4 negative controls). Most cases fail with the integration/aurora sbio.rs and sep.rs restored. Our own mutants were all caught: dropping only the post-read cancel check, renewing the deadline outside the idle state, and treating enrollment as verification. Nothing has run on a sensor.
|
Thanks, Scott. I haven't established a default flow that needs this change. Omarchy's ordinary sudo, polkit and lock PAM configurations do not override pam_fprintd's 30-second default. Direct fprintd-verify, custom clients or an administrator-raised PAM timeout can outlive 60 seconds, but that is not evidence of a stock-flow problem. I'll keep this on hold until after the release candidate. The enrollment ordering change came from sharing the verification wait refactor and was unrelated to the proposed verification extension. Local commit 2323df5c2bc9f1d03851d88601635ab253d20a4a restores the parent's enrollment behavior: accept a ready frame before checking expiry, retain the ignored trailing status read on expiry, and preserve cancellation ordering. Verification still omits that trailing read so expiry does not initiate another SPI transaction. I have not verified on hardware whether that read has a necessary side effect; I shouldn't imply that removing it is proven harmless. The fixed deadline also assumes SPI calls return. The local revision has 34 Rust cases, six negative controls, eight enrollment cases against the actual parent method, and source-matched SEP object compilation. These are local tests, not a new full-kernel or five-minute hardware qualification. I have not pushed this revision. |
The existing 60-second capture timeout tears down a healthy sensor that is still armed and waiting for a finger. Preserve one idle verification for longer, with a fixed five-minute upper bound rather than an indefinite wait.
The proposed five-minute policy leaves margin beyond the earlier combined candidate's 185-second idle window while bounding a live caller that never cancels. It is not a measured sensor/SEP lifetime. This changed branch has not been hardware-qualified at five minutes, and the chosen bound remains subject to maintainer review.
Enrollment retains its 60-second deadline. The check now precedes ready-frame acceptance: a frame arriving at or after that boundary is refused rather than bypassing the timeout. Verification keeps the existing 60-second arming/progress bound, with a non-refreshable 300-second maximum from the start of the wait. Cancellation and suspend invalidate the session before or after a status read; a ready frame arriving at or after the maximum is refused. Transport errors and patch loss keep their existing outcomes. A first ARMED report at or after the initial 60-second deadline cannot renew the expired budget. The bounds assume SPI calls return.
For ordinary sudo/polkit stacks,
pam_fprintdhas its own 30-second default and sendsVerifyStopon expiry; administrator overrides still apply. The Omarchy lock uses PAM, aborts on unlock or its reachability watchdog, and can schedule a new attempt after completion. fprintd cancellation/client disappearance propagates through libfprint to the kernel. A custom connected client that never stops still receives a terminal kernel timeout; this does not automatically release its D-Bus claim.CAPTURE-WAIT.mddocuments these distinct lifetimes and the inspected caller paths.Validation: 31 compiled Rust cases exercise the production wait with synthetic clocks, SPI and IRQ leaves, including a touch after 185 seconds, forgotten-caller expiry, exact-deadline/late-frame rejection and cancellation priority. Four compiled negative controls must fail when the old minute-long idle timeout is restored, cancellation is ignored, the fixed maximum is removed, or progress expiry is moved behind ARMED renewal/frame acceptance. All pass with Rust 1.85.0, including through the registered host-test runner. No sensor is opened. Idle power and real five-minute/suspend behavior remain unqualified.
The final manifest diff contains only the suite and fixture registration; the whole-file formatting change has been removed.
The final SEP Rust object and C shims also compile with Rust 1.85.0 against the archived qualified kernel headers. They emit one inherited unused-constant warning in unchanged
store.rs. This is an object compilation check, not a full kernel build.AI-assisted with OpenAI Codex. Signed-off-by trailers carry the contributor's authorized DCO certification.