hardening/v0.0.7.3: audited libuvc backports for ceralive-v0.0.7.3

CHANGELOG.ceralive.md

@@ -13,6 +13,132 @@ the upstream history, see `changelog.txt`.
 - **License:** BSD-3-Clause, preserved verbatim (`LICENSE.txt`). CeraLive
   additions are also BSD-3-Clause.
 
+## ceralive-v0.0.7.3
+
+Hardening release. Audited, individually-verified backports from upstream
+`libuvc` PRs and the `pupil-labs` / `saki4510t` forks, each landed as a single,
+fork-style-adapted commit on `hardening/v0.0.7.3` (branched from `eae7f49`, tag
+`ceralive-v0.0.7.2`). Every change is either off-by-default or a pure robustness
+guard — no negotiated default changes and byte-identical streaming behavior for
+currently-working devices. Each entry cites its **fork-commit SHA** and upstream
+provenance. Backlog IDs (A1–A14) refer to the equivalence audit in the plugin
+repo's `.omo/evidence/task-1-uvc-camera-compat-stability.md`.
+
+### Added
+
+- **Runtime-configurable USB transfer-buffer count** (A2 — fork `001e8d3`;
+  upstream PR #291 `7620d2f`, `a100ee7`). New public API
+  `uvc_set_transfer_buffers(uvc_device_handle_t *devh, uint8_t count)`
+  (`include/libuvc/libuvc.h`, implemented in `src/stream.c`). The count is stored
+  on the device handle and latched by the next `uvc_start_streaming()` /
+  `uvc_stream_start()`, sizing the transfer allocation/submit loops. `count == 0`
+  (the default, unset state) preserves the prior byte-identical behavior of
+  allocating `LIBUVC_NUM_TRANSFER_BUFS` (100) buffers; a non-zero value is clamped
+  to `[2, 100]`. Setting the count while a stream on the handle is already running
+  is rejected with `UVC_ERROR_BUSY`. The API is pinned device-handle-level rather
+  than the PR's global `uvc_stream_set_default_number_of_transport_buffers()`
+  setter, and the static transfer arrays are retained (only the loop bounds are
+  made configurable). No SONAME change; the existing `uvc_start_streaming()`
+  signature is unchanged.
+
+### Fixed
+
+- **Retry USB alt-setting once on transient failure** (A1 — fork `3195bbc`;
+  upstream PR #293 `212b85d`). `uvc_stream_start()` now retries
+  `libusb_set_interface_alt_setting()` (bounded loop, UVC_DEBUG-logged) before
+  failing, so a stream start that loses a single alt-setting negotiation to a
+  transient USB error recovers instead of aborting. A persistently failing call
+  still propagates the original libusb error via the pre-existing `goto fail`.
+
+- **Free frame-metadata buffer in `uvc_stream_close()`** (A3 — fork `3195bbc`;
+  upstream PR #295 `ca65b0b`). The per-frame metadata buffer
+  (`strmh->frame.metadata` / `meta_outbuf`) was never freed on stream teardown,
+  leaking on every open→start→stop→close cycle. It is now freed alongside
+  `frame.data` in a guarded-free block. (Upstream PR #275 is the identical
+  duplicate; PR #295 was picked per the audit.)
+
+- **Fail loudly on zero submitted transfers** (A2 — fork `001e8d3`; upstream
+  PR #291). `uvc_stream_start()` previously treated `transfer_id >= 0` as success
+  in its submit-failure path, so a device that accepted *no* transfers reported
+  success and then delivered no frames. The test is now `transfer_id > 0`: when at
+  least one transfer is submitted the un-submitted remainder is freed and
+  streaming continues with fewer buffers, but when zero transfers are submitted
+  the allocated transfers are freed and `UVC_ERROR_IO` is returned.
+
+- **Repair degenerate frame descriptors** (A4 — fork `5df5401`; saki4510t
+  `328d14d`). During frame-descriptor parsing in `src/device.c`, a
+  `dwMaxVideoFrameBufferSize == 0` (seen on some DJI action cameras) is now
+  repaired to a sane fallback, and a `dwDefaultFrameInterval` that is zero or
+  outside `[dwMinFrameInterval, dwMaxFrameInterval]` is clamped into range;
+  UVC_DEBUG-logged. The guard is strict on the degenerate cases only —
+  already-sane descriptors are left byte-identical.
+
+- **Bounded wait in `uvc_stream_stop()`; no more indefinite teardown hang**
+  (A5 — fork `ab49e21`; pupil-labs `c534e3d`, upstream PR #59 / issue #152). The
+  indefinite `pthread_cond_wait` on transfer cancellation is replaced with a
+  bounded `pthread_cond_timedwait` (~1 s per iteration, ~5 s cap) that returns
+  `UVC_ERROR_TIMEOUT` if cancellations never complete, while still marking the
+  stream stopped so `uvc_stream_close()` can proceed. Transfers still in flight
+  are not freed (the d3318ae invariant is preserved); the fast-cancel path is
+  unchanged.
+
+- **Zero `GET_MAX` payload fallback** (A7 — fork `69c7da8`; upstream PR #277,
+  issue #276). When a device's `GET_MAX` returns
+  `dwMaxPayloadTransferSize == 0` during stream negotiation, the negotiated
+  control now falls back to the `GET_CUR`/descriptor value instead of propagating
+  a zero payload size. Composes with the existing `f4af02a` smaller-max-payload
+  handling with no double-handling.
+
+- **Corrupt / oversized payload guards** (A9, subsumes A8 — fork `69c7da8`;
+  upstream PR #184 + PR #212 + saki4510t `9e95b8a`). One coherent superset patch
+  in the payload/frame path: (1) bounds-check PTS/SCR reads in
+  `_uvc_process_payload` before dereferencing (`variable_offset + 4 <= header_len`,
+  else zero); (2) grow/realloc guard in `_uvc_populate_frame` to
+  `max(hold_bytes, height*step)` with an explicit `return` on allocation failure
+  (the fork's `UVC_EXIT` is a no-op in non-debug builds), zero-filling any short
+  tail and copying only `hold_bytes` — the DJI/compressed `step == 0` zero-size
+  tolerance from A4 is preserved; (3) a `frame_had_errors` flag on
+  `struct uvc_stream_handle` set on the payload error bit and on non-zero packet
+  status, suppressing delivery of the whole corrupt frame to the callback. The
+  three already-present guards (overflow clamp, per-payload error-bit skip,
+  per-packet status skip) were kept, not duplicated.
+
+- **Preserve VC-header `dwClockFrequency`** (A12 — fork `9874f4c`; pupil-labs
+  `92d2f82`, `74e7a96`). `uvc_parse_vc_header()` now reads `dwClockFrequency`
+  into the control-interface info for the `0x0110` and `0x0150` bcdUVC cases
+  (previously left zeroed), and `uvc_query_stream_ctrl()` stops zeroing it during
+  stream-control parsing. Plumbing only — the clock is not surfaced onto frames
+  and there is no PTS behavior change (see the plugin's `scr-investigation.md`).
+
+### Not backported (audit-confirmed already-equivalent)
+
+The following backlog items were evaluated and deliberately **not** landed —
+each is already covered by the fork's current state or is out of the fork's
+scope. Verdicts are from the todo-1 equivalence audit, independently
+re-verified against the branch HEAD.
+
+- **A6 — Composite-device control interface routing** (pupil-labs `9004351`):
+  the fork already routes unit control requests to the parsed VideoControl
+  interface number (`info->ctrl_if.bInterfaceNumber`, set from the interface
+  scan, not hard-coded to 0). The fork base is already at pupil-labs's
+  post-fix state; re-picking would be a no-op.
+- **A10 — `get_device_descriptor` robustness** (upstream `e001f04`): already
+  present as commit `eae7f49` (its message records "+ backport e001f04"); the
+  `uvc_scan_control` TIS-detection guard is byte-for-byte equivalent.
+- **A11 — Detach only an active kernel driver** (upstream PR #224): covered by
+  fork commit `2f32812`, which sets `libusb_set_auto_detach_kernel_driver(…, 1)`
+  (libusb detaches only when a driver is actually active) and by
+  `uvc_claim_if()` already tolerating the no-active-driver error codes.
+- **A13 — `libusb_device` refcount leak** (saki4510t `2596242`): the
+  libuvc-portion hunk is comment-only for `uvc_ref_device`/`uvc_unref_device`
+  (already correct in the fork) and otherwise touches only the Android-JNI
+  `uvc_get_device_with_fd`, which does not exist in this fork. The real leak fix
+  lives in Android libusb, out of scope. A 1000× enumerate/free loop is
+  ASan/LSan-clean, confirming no pre-existing leak.
+- **A14 — Double-probe workaround** (upstream issue #242): implemented in the
+  plugin (`gstlibuvch264src`) as an opt-in `QUIRK_DOUBLE_PROBE` vid:pid quirk
+  seam that ships with an empty table — not a libuvc fork change.
+
 ## ceralive-v0.0.7.1
 
 ### Added

include/libuvc/libuvc.h

@@ -633,6 +633,8 @@ uvc_error_t uvc_start_iso_streaming(
 
 void uvc_stop_streaming(uvc_device_handle_t *devh);
 
+uvc_error_t uvc_set_transfer_buffers(uvc_device_handle_t *devh, uint8_t count);
+
 uvc_error_t uvc_stream_open_ctrl(uvc_device_handle_t *devh, uvc_stream_handle_t **strmh, uvc_stream_ctrl_t *ctrl);
 uvc_error_t uvc_stream_ctrl(uvc_stream_handle_t *strmh, uvc_stream_ctrl_t *ctrl);
 uvc_error_t uvc_stream_start(uvc_stream_handle_t *strmh,

include/libuvc/libuvc_internal.h

@@ -29,6 +29,14 @@
   (p)[2] = (i) >> 16; \
   (p)[3] = (i) >> 24;
 
+/* Used by the degenerate frame-descriptor repair in device.c (saki4510t 328d14d). */
+#ifndef MIN
+#define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+#ifndef MAX
+#define MAX(a, b) ((a) > (b) ? (a) : (b))
+#endif
+
 /** Selects the nth item in a doubly linked list. n=-1 selects the last item. */
 #define DL_NTH(head, out, n) \
   do { \
@@ -231,6 +239,19 @@ typedef struct uvc_device_info {
 #endif
 #endif
 
+/* Bound uvc_stream_stop()'s wait for transfer cancellation so a wedged event
+ * thread (e.g. a dead/unplugged device that never completes the cancelled
+ * transfers) cannot hang the caller forever. Each iteration waits at most
+ * LIBUVC_STREAM_STOP_TIMEOUT_SECS; after LIBUVC_STREAM_STOP_TIMEOUT_ATTEMPTS
+ * consecutive timeouts (~5s total) uvc_stream_stop returns UVC_ERROR_TIMEOUT.
+ */
+#ifndef LIBUVC_STREAM_STOP_TIMEOUT_SECS
+#define LIBUVC_STREAM_STOP_TIMEOUT_SECS 1
+#endif
+#ifndef LIBUVC_STREAM_STOP_TIMEOUT_ATTEMPTS
+#define LIBUVC_STREAM_STOP_TIMEOUT_ATTEMPTS 5
+#endif
+
 #define LIBUVC_XFER_META_BUF_SIZE ( 4 * 1024 )
 
 struct uvc_stream_handle {
@@ -266,6 +287,10 @@ struct uvc_stream_handle {
   /* raw metadata buffer if available */
   uint8_t *meta_outbuf, *meta_holdbuf;
   size_t meta_got_bytes, meta_hold_bytes;
+
+  /* set when a payload/packet of the in-progress frame reported an error;
+   * a flagged frame is suppressed in _uvc_swap_buffers and never delivered */
+  uint8_t frame_had_errors;
 };
 
 /** Handle on an open UVC device
@@ -291,6 +316,11 @@ struct uvc_device_handle {
   /** Whether the camera is an iSight that sends one header per frame */
   uint8_t is_isight;
   uint32_t claimed;
+  /** Requested number of USB transfer buffers for the NEXT stream start.
+   * 0 = use the library default (LIBUVC_NUM_TRANSFER_BUFS). A non-zero value is
+   * clamped to [2, 100] by uvc_set_transfer_buffers() and latched at
+   * uvc_stream_start() time. */
+  uint8_t transfer_buffer_count;
 };
 
 /** Context within which we communicate with devices */

src/device.c

@@ -1150,8 +1150,10 @@ uvc_error_t uvc_parse_vc_header(uvc_device_t *dev,
     info->ctrl_if.dwClockFrequency = DW_TO_INT(block + 7);
     break;
   case 0x0110:
+    info->ctrl_if.dwClockFrequency = DW_TO_INT(block + 7);
     break;
   case 0x0150:
+    info->ctrl_if.dwClockFrequency = DW_TO_INT(block + 7);
     break;
   default:
     UVC_EXIT(UVC_ERROR_NOT_SUPPORTED);
@@ -1493,7 +1495,57 @@ uvc_error_t uvc_parse_vs_format_mjpeg(uvc_streaming_interface_t *stream_if,
 }
 
 /** @internal
- * @brief Parse a VideoStreaming uncompressed frame block.
+ * @brief Repair degenerate VideoStreaming frame descriptor fields in place.
+ *
+ * Some devices (notably DJI action cameras streaming frame-based H.264/H.265)
+ * ship a bogus frame descriptor: a zero dwMaxVideoFrameBufferSize (the host
+ * uses it to size capture buffers) and/or a dwDefaultFrameInterval that is 0 or
+ * outside the advertised interval range. Only these degenerate values are
+ * touched, so a valid descriptor is left byte-identical.
+ *
+ * Adapted from saki4510t/UVCCamera 328d14d, guarded strictly on the degenerate
+ * cases (upstream rewrote dwMaxVideoFrameBufferSize unconditionally). The buffer
+ * size mirrors the zero-frame-size fixup in uvc_query_stream_ctrl: uncompressed
+ * formats have a real bBitsPerPixel; compressed formats (MJPEG, frame-based)
+ * report 0, so fall back to w*h*2.
+ */
+static void uvc_repair_frame_desc(uvc_format_desc_t *format,
+                                  uvc_frame_desc_t *frame) {
+  uint32_t min_interval, max_interval, repaired;
+
+  if (frame->dwMaxVideoFrameBufferSize == 0 && frame->wWidth && frame->wHeight) {
+    if (format->bBitsPerPixel)
+      frame->dwMaxVideoFrameBufferSize =
+        (uint32_t) format->bBitsPerPixel * frame->wWidth * frame->wHeight / 8;
+    else
+      frame->dwMaxVideoFrameBufferSize =
+        (uint32_t) frame->wWidth * frame->wHeight * 2;
+
+    UVC_DEBUG("repaired dwMaxVideoFrameBufferSize 0 -> %u for %ux%u frame",
+              frame->dwMaxVideoFrameBufferSize, frame->wWidth, frame->wHeight);
+  }
+
+  if (frame->bFrameIntervalType == 0) {
+    min_interval = frame->dwMinFrameInterval;
+    max_interval = frame->dwMaxFrameInterval;
+  } else {
+    min_interval = frame->intervals[0];
+    max_interval = frame->intervals[frame->bFrameIntervalType - 1];
+  }
+
+  if (min_interval && max_interval && min_interval <= max_interval &&
+      (frame->dwDefaultFrameInterval == 0 ||
+       frame->dwDefaultFrameInterval < min_interval ||
+       frame->dwDefaultFrameInterval > max_interval)) {
+    repaired = MIN(max_interval, MAX(min_interval, frame->dwDefaultFrameInterval));
+    UVC_DEBUG("repaired dwDefaultFrameInterval %u -> %u (range [%u, %u])",
+              frame->dwDefaultFrameInterval, repaired, min_interval, max_interval);
+    frame->dwDefaultFrameInterval = repaired;
+  }
+}
+
+/** @internal
+ * @brief Parse a VideoStreaming frame-based frame block.
  * @ingroup device
  */
 uvc_error_t uvc_parse_vs_frame_frame(uvc_streaming_interface_t *stream_if,
@@ -1528,16 +1580,20 @@ uvc_error_t uvc_parse_vs_frame_frame(uvc_streaming_interface_t *stream_if,
     frame->dwMaxFrameInterval = DW_TO_INT(&block[30]);
     frame->dwFrameIntervalStep = DW_TO_INT(&block[34]);
   } else {
+    uint32_t interval;
     frame->intervals = calloc(block[21] + 1, sizeof(frame->intervals[0]));
     p = &block[26];
 
     for (i = 0; i < block[21]; ++i) {
-      frame->intervals[i] = DW_TO_INT(p);
+      interval = DW_TO_INT(p);
+      frame->intervals[i] = interval ? interval : 1;
       p += 4;
     }
     frame->intervals[block[21]] = 0;
   }
 
+  uvc_repair_frame_desc(format, frame);
+
   DL_APPEND(format->frame_descs, frame);
 
   UVC_EXIT(UVC_SUCCESS);
@@ -1580,16 +1636,20 @@ uvc_error_t uvc_parse_vs_frame_uncompressed(uvc_streaming_interface_t *stream_if
     frame->dwMaxFrameInterval = DW_TO_INT(&block[30]);
     frame->dwFrameIntervalStep = DW_TO_INT(&block[34]);
   } else {
+    uint32_t interval;
     frame->intervals = calloc(block[25] + 1, sizeof(frame->intervals[0]));
     p = &block[26];
 
     for (i = 0; i < block[25]; ++i) {
-      frame->intervals[i] = DW_TO_INT(p);
+      interval = DW_TO_INT(p);
+      frame->intervals[i] = interval ? interval : 1;
       p += 4;
     }
     frame->intervals[block[25]] = 0;
   }
 
+  uvc_repair_frame_desc(format, frame);
+
   DL_APPEND(format->frame_descs, frame);
 
   UVC_EXIT(UVC_SUCCESS);