Apply hard timeout to compare.py subprocess in pam.py#1
Open
ds17f wants to merge 1 commit into
Open
Conversation
Replace subprocess.call with subprocess.run(timeout=...) so a wedged compare.py (e.g. a camera that opens but never returns a frame, or a v4l2 ioctl that blocks forever) cannot hang the PAM stack indefinitely. The timeout bound is video.timeout + 5s. video.timeout is already the per-attempt limit compare.py applies internally; the +5s grace lets compare.py exit cleanly under normal conditions and only triggers the SIGKILL path if compare.py itself is unresponsive. On TimeoutExpired we emit the same conversation message and syslog entry as the existing status==11 path, and return PAM_AUTH_ERR. The log message also records the configured bound for diagnostics. Observed before this change: a stuck IR camera causing 'sudo' to hang forever with no way to fall back to password auth. Observed after: sudo waits ~timeout+5 seconds, prints 'Face detection timeout reached', and falls through to the next auth method.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Replace
subprocess.callwithsubprocess.run(timeout=...)so a wedgedcompare.pycannot hang the PAM stack indefinitely.The bug
pam.pyinvokescompare.pywithsubprocess.call(...), which has no timeout. Ifcompare.pyhangs — for example, the IR camera opens but never returns a frame, or a v4l2 ioctl blocks forever — the entire PAM auth path is stuck waiting on the subprocess. Real-world symptom:sudohangs forever with no way to fall back to password auth.compare.pydoes enforce its own per-attempt timeout (video.timeout) internally, but only when its read loop is running. If the camera never returns the first frame, that timer never starts.The fix
Bound is
video.timeout + 5s. The +5s is grace forcompare.pyto exit cleanly under normal conditions; the SIGKILL path only triggers ifcompare.pyitself is unresponsive.On
TimeoutExpiredwe emit the same conversation message and return code as the existingstatus == 11path, plus a more diagnostic syslog line that records the configured bound.Observed before/after
sudohangs forever, can't fall back to password.sudowaits ~timeout + 5seconds →Face detection timeout reached→ falls through to next auth method.Scope
compare.pylegitimately taking longer than its own configured timeout to exit cleanly. In practice that means something is already wrong, and the bound (timeout + 5) gives a generous grace window.Note: filed against
principis/howdybecausepam.pywas removed fromboltgolt/howdymaster in the C++ PAM rewrite (3.0.0). The bug is identical in scope toboltgolt/howdyv2.6.1, which principis tracks.See also boltgolt#1107 (ffmpeg_reader bug fixes) and boltgolt#1108 (dark_threshold doc clarification) — adjacent fixes against the upstream Python codebase that principis pulls from.