hardening/v0.0.7.3: audited libuvc backports for ceralive-v0.0.7.3 - #3
Merged
Conversation
…m_close (upstream #293, #295) A1 (upstream #293, commit 212b85d): uvc_stream_start() previously called libusb_set_interface_alt_setting() exactly once and bailed to `fail` on the first error. Some devices need a retry after a transient alt-setting failure. Wrap the call in a bounded do/while loop (up to 3 attempts total) matching the upstream PR, and log each retry via UVC_DEBUG. Fails with the original libusb error (e.g. -99/UVC_ERROR_OTHER) only after all attempts are exhausted. A3 (upstream #295, commit ca65b0b; #275 is a byte-identical duplicate): uvc_stream_close() freed frame.data/outbuf/holdbuf/meta_outbuf/meta_holdbuf but never freed frame.metadata, which _uvc_populate_frame() realloc's per frame whenever the device delivers stream metadata -> leak. Add the guarded free. Style-adapted to the fork's 2-space K&R braces. No negotiated default changes.
… on zero submitted transfers (upstream #291) Adapted from upstream PR #291 (commits 7620d2f, a100ee7). Two changes: 1. New public API uvc_set_transfer_buffers(devh, count): stores a per-device transfer-buffer count latched by the next uvc_stream_start(). count==0 (the default, freshly-calloc'd state) preserves the byte-identical prior behavior of LIBUVC_NUM_TRANSFER_BUFS (100); nonzero is clamped to [2,100]; a set while a stream on the handle is running is rejected with UVC_ERROR_BUSY. The static transfers[]/transfer_bufs[] arrays are retained (only the loop bounds are made configurable), so a smaller count just uses fewer slots -- no ABI/SONAME change and no change to the uvc_start_streaming signature. 2. uvc_stream_start() now fails loudly on zero submitted transfers: the submit-failure test changes from transfer_id >= 0 to transfer_id > 0, so when at least one transfer is submitted the remainder is freed and streaming continues with fewer buffers, but when zero are submitted the transfers are freed and UVC_ERROR_IO is returned instead of the old silent UVC_SUCCESS. Provenance: adapted from upstream PR #291; the API is pinned device-handle-level (uvc_set_transfer_buffers) rather than the PR's global uvc_stream_set_default_number_of_transport_buffers() setter, per the todo-1 equivalence audit -- the plugin element only ever holds a uvc_device_handle_t.
…bad default interval)
Adapted from saki4510t/UVCCamera 328d14d. Some devices (notably DJI action
cameras streaming frame-based H.264/H.265) ship bogus frame descriptors: a zero
dwMaxVideoFrameBufferSize (which the host uses to size capture buffers) and/or a
dwDefaultFrameInterval that is 0 or outside the advertised interval range.
uvc_parse_vs_frame_uncompressed and uvc_parse_vs_frame_frame now repair, strictly
on the degenerate cases only (valid descriptors stay byte-identical):
- dwMaxVideoFrameBufferSize == 0 -> bBitsPerPixel*w*h/8 for uncompressed, or
w*h*2 for compressed/frame-based (bBitsPerPixel==0), mirroring the existing
zero-frame-size fixup in uvc_query_stream_ctrl.
- dwDefaultFrameInterval 0 or out of range -> clamped into [min,max] via
MIN(max, MAX(min, default)); zero discrete intervals repaired to 1.
Repairs are logged via UVC_DEBUG. Guarded strictly (upstream applied the
buffer-size formula unconditionally).
A11 (upstream PR #224, detach-only-active-kernel-driver): nothing landed --
skip-equivalent per the todo-1 audit (already covered by fork 2f32812
libusb_set_auto_detach_kernel_driver + uvc_claim_if tolerance of
LIBUSB_ERROR_NOT_FOUND/NOT_SUPPORTED).
…of hanging Adapt pupil-labs c534e3d + upstream PR #59 intent: the transfer-cancel wait loop in uvc_stream_stop() used an unbounded pthread_cond_wait, so a dead/unplugged device whose cancelled transfers never complete (a wedged event thread) hung the caller forever. Replace it with a bounded pthread_cond_timedwait (~1s per iteration, up to 5 attempts / ~5s total), mirroring the existing timedwait pattern in uvc_stream_get_frame, and return UVC_ERROR_TIMEOUT if cancellation never completes. The stream is already marked stopped (running=0) before the wait, so uvc_stream_close can proceed safely on timeout. Transfers still in flight are NOT freed here (preserves d3318ae): _uvc_stream_callback still owns the single free. The normal fast-cancel path is unchanged.
… guards (upstream #277/#184/#212 + saki 9e95b8a) A7 (upstream PR #277, issue #276): in uvc_query_stream_ctrl, when GET_MAX returns dwMaxPayloadTransferSize == 0, fall back to the frame descriptor's dwMaxBitRate instead of propagating 0. Orthogonal to f4af02a (which only relaxes the equality check in _uvc_stream_params_negotiated) -- no double-handling. A9 corrupt/oversized-frame superset (ONE patch, deduped against guards already present in the fork): 1. saki 9e95b8a PTS/SCR bounds guards: guard the 4-byte DW_TO_INT read in _uvc_process_payload with variable_offset + 4 <= header_len before dereferencing (pts/last_scr set to 0 on a truncated header). The fork's 6-byte SCR field advance is preserved. 2. PR #184 _uvc_populate_frame realloc guard: grow the frame buffer to max(hold_bytes, height*step), NULL-check the realloc (explicit return -- the fork's UVC_EXIT does not return), zero-fill the tail below height*step, and memcpy only hold_bytes so data_bytes never exceeds the allocation. For step==0 formats (H264/H265/MJPEG) height*step==0, so this degenerates to the hold_bytes path and preserves the DJI zero-size tolerance from the degenerate-descriptor repair -- no zero-size realloc is forced. 3. PR #212 frame_had_errors whole-frame suppression: new uint8_t field on uvc_stream_handle, set on the UVC_STREAM_ERR header bit (_uvc_process_payload) and on a nonzero isoc packet status (_uvc_stream_callback), gates the cond broadcast in _uvc_swap_buffers so a frame with any packet error is never delivered to the callback, then cleared. A8 (discard corrupt frames) is fully contained in piece 3; not landed separately. The already-present overflow clamp, per-payload error-bit skip and per-packet status skip are kept unchanged (not duplicated).
…nfirmed no-op A12 (pupil-labs 92d2f82 / 74e7a96): set dwClockFrequency for bcdUVC 0x0110 and 0x0150 in uvc_parse_vc_header (previously left zeroed for those versions), and always take the clock from the VideoControl interface header descriptor in uvc_query_stream_ctrl rather than the probe response's buf+26 -- a zero there must not erase the enumerated value. Plumbing only: dwClockFrequency is not surfaced onto frames and the element derives PTS from arrival time, not the device SCR/clock (gstlibuvch264src docs/notes/scr-investigation.md) -- no PTS behavior change. 74e7a96's dwMaxVideoFrameSize GET-block fixup was already present and is not re-added. A13 (saki 2596242) is audit-confirmed skip-equivalent and lands nothing: the libuvc-portion is comment-only for uvc_ref_device/uvc_unref_device plus an early-return fix to the Android-JNI-only uvc_get_device_with_fd (absent from the fork). The fork's ref/unref already free on zero; a 1000x uvc_get_device_list/uvc_free_device_list loop is ASan/LSan clean, confirming no pre-existing enumeration leak.
Extend the ceralive-v0.0.7.3 changelog section to cover every landed hardening backport with its fork-commit SHA and upstream provenance: A1 3195bbc retry alt-setting on transient failure (upstream #293) A3 3195bbc free frame-metadata buffer in uvc_stream_close (upstream #295) A2 001e8d3 configurable transfer buffers + fail-on-zero-submit (upstream #291) A4 5df5401 repair degenerate frame descriptors (saki4510t 328d14d) A5 ab49e21 bounded wait in uvc_stream_stop (pupil-labs c534e3d / upstream #59) A7 69c7da8 zero GET_MAX payload fallback (upstream #277) A9 69c7da8 corrupt/oversized payload guards, subsumes A8 (#184/#212 + saki 9e95b8a) A12 9874f4c preserve VC-header dwClockFrequency (pupil-labs 92d2f82/74e7a96) Also records the audit-confirmed skip-equivalent items (A6, A10, A11, A13, A14), each with a one-line reason. No source change; doc-only.
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
1 participant
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.
Summary
Release branch for ceralive-v0.0.7.3 — a hardening release of audited,
individually-verified backports from upstream libuvc PRs and the pupil-labs /
saki4510t forks. Each fix landed as a single, fork-style-adapted commit on top of
`eae7f49` (tag `ceralive-v0.0.7.2`). Every change is off-by-default or a pure
robustness guard — no negotiated-default changes and byte-identical streaming
behavior for currently-working DJI/UVC devices.
Landed backports (fork SHA — provenance)
Audit-confirmed skip-equivalent (already covered, nothing landed): A6, A10,
A11, A13, A14 — see `CHANGELOG.ceralive.md`.
Invariants