Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 33 additions & 4 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,14 @@ jobs:
"libuvc.teardown.high_index_interfaces_released",
"libuvc.teardown.cancel_not_found_still_drains",
"libuvc.teardown.undeliverable_status_xfer_quarantines",
"libuvc.teardown.quarantined_handle_stays_armed",
"libuvc.reattach.rebinds_after_sigkill",
"libuvc.reattach.disarmed_interfaces_are_not_rebound",
"libuvc.reattach.busy_interface_is_retried",
"libuvc.reattach.connect_ioctl_is_what_libusb_would_issue",
"libuvc.reattach.claiming_an_interface_arms_the_guard",
"libuvc.race.close_races_status_callback"
] | sort) and (.tests | length == 27)
] | sort) and (.tests | length == 33)
' "$result_dir/inventory.json" \
2>&1 | tee "$result_dir/inventory-check.log"

Expand Down Expand Up @@ -169,6 +175,29 @@ jobs:
cmake --build build-off --parallel
echo "PASS: OFF variant builds cleanly"

- name: Configure + build + test (reattach guard OFF — rollback path)
run: |
set -o pipefail
cmake -S . -B build-noguard \
-DCMAKE_POLICY_VERSION_MINIMUM=3.5 \
-DCMAKE_BUILD_TYPE=Debug \
-DCMAKE_BUILD_TARGET=Static \
-DBUILD_SHARED_LIBS=OFF \
-DBUILD_EXAMPLE=OFF \
-DBUILD_TEST=OFF \
-DBUILD_TESTING=ON \
-DLIBUVC_REATTACH_GUARD=OFF \
-DCMAKE_C_COMPILER_LAUNCHER=ccache \
-DCMAKE_CXX_COMPILER_LAUNCHER=ccache
cmake --build build-noguard --parallel
# The guard's own cases go with it; everything that predates it must
# still pass, so a rollback is a real rollback and not a smaller gate.
ctest --test-dir build-noguard --show-only=json-v1 \
| jq -e '(.tests | length == 27)
and ([.tests[].name] | map(startswith("libuvc.reattach.")) | any | not)'
ctest --test-dir build-noguard --output-on-failure
echo "PASS: guard OFF variant builds and keeps the pre-guard suite green"

- name: Verify artifacts
run: |
test -f build/libuvc.so
Expand All @@ -191,7 +220,7 @@ jobs:
# stress test; TSan works from vector clocks, so it fails on unsynchronized
# accesses even when they never overlap in wall-clock time.
thread-sanitizer:
name: ThreadSanitizer (teardown + race)
name: ThreadSanitizer (teardown + race + reattach)
runs-on: ubuntu-24.04

steps:
Expand Down Expand Up @@ -222,10 +251,10 @@ jobs:
# negotiation and transfer suites --wrap free(), which a sanitized build
# replaces, so routing those calls back to __real_free would report a
# mismatched allocator rather than anything about this code.
- name: Run teardown and race cases under TSan
- name: Run teardown, race and reattach cases under TSan
run: |
ctest --test-dir build/tsan --output-on-failure \
-R 'libuvc\.(teardown|race)\.'
-R 'libuvc\.(teardown|race|reattach)\.'

- name: Fail on any ThreadSanitizer report
run: |
Expand Down
48 changes: 48 additions & 0 deletions CHANGELOG.ceralive.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,54 @@ the upstream history, see `changelog.txt`.

## Unreleased

### Added

- **A kernel-driver reattach that survives an exit running no user code.**
Every earlier fix in this file hardens a path inside `uvc_close()`. A holder
killed with `SIGKILL` — or `SIGSEGV`, or the systemd watchdog's `SIGABRT` —
executes none of them, and the kernel does not re-probe a UVC interface when
usbfs's claim is dropped at fd-close, so both interfaces stay at
`driver = NONE` indefinitely and only a manual `uvcvideo/bind` or a USB
unbind/bind recovers the camera. Measured on an RK3588 board with a DJI Osmo
Pocket 3: 10 kills across three unrelated holder processes, 10 wedges, none
self-recovering; the same processes stopped with `SIGTERM`, which lets
`uvc_close()` run, recovered 3/3.

Nothing inside the dying process can fix that, because the defining property
of the failure is that the dying process runs nothing. So the reattach now
happens outside it. `uvc_claim_if()` forks a helper on the first claim
(double-forked, so it is reparented to init and the host is never handed a
`SIGCHLD` it did not ask for) and keeps the write end of a pipe. The helper
waits for that pipe's EOF — which the kernel delivers when the last write end
closes, and the kernel closes a dying process's descriptors unconditionally —
then reopens the device by its usbfs path, checks it is still the same device
(bus addresses are reused), and issues the `USBDEVFS_CONNECT` that
`libusb_attach_kernel_driver()` would have issued, retrying while the kernel
answers `EBUSY` because the helper wakes while the dead process's descriptors
are still being torn down.

A normal teardown disarms each interface as it hands the driver back, so the
helper wakes with nothing to do. The guard is a backstop, not a second
teardown path.

It is also what makes the quarantine paths whole. A quarantining `uvc_close()`
cannot release anything — libusb still owns a URB on the interface, and
releasing it either re-creates the usbfs-eviction defect below or makes
`uvcvideo` probe with the streaming interfaces still held and register no
video node. It therefore leaves the guard armed and undestroyed, which turns
the quarantine from a permanently wedged camera into a leak bounded by the
process lifetime.

Fail-safe by construction: the helper holds no reference to the device while
it waits, so a helper that dies leaves behaviour exactly as it was before this
change, never worse. Linux-only, and configurable:

cmake .. -DLIBUVC_REATTACH_GUARD=OFF

Six new regression cases cover it, five of which really do `SIGKILL` a forked
victim, since a test that calls a cleanup function proves nothing here. CI
builds and tests the `OFF` variant too.

### Fixed

- **`uvc_close()` left the camera's kernel driver detached, so `/dev/videoN`
Expand Down
53 changes: 47 additions & 6 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ option(ENABLE_UVC_DEBUGGING "Enable UVC debugging" OFF)
option(LIBUVC_AUTO_DETACH_KERNEL_DRIVER
"Auto-detach the kernel driver (e.g. uvcvideo) when claiming UVC interfaces (CeraLive)"
ON)
option(LIBUVC_REATTACH_GUARD
"Fork a helper that reattaches the kernel driver if this process dies without closing (Linux only, CeraLive)"
ON)
set(LIBUVC_SANITIZE "" CACHE STRING
"Sanitizer to build library and tests with: thread, address, or empty (CeraLive)")
set_property(CACHE LIBUVC_SANITIZE PROPERTY STRINGS "" thread address)
Expand Down Expand Up @@ -59,6 +62,7 @@ set(SOURCES
src/diag.c
src/frame.c
src/init.c
src/reattach_guard.c
src/stream.c
src/misc.c
)
Expand Down Expand Up @@ -183,12 +187,21 @@ if(BUILD_TESTING)
add_executable(uvc_teardown_assertions tests/teardown_assertions.c)
add_executable(uvc_status_race_assertions tests/status_race_assertions.c)

foreach(test_target
uvc_descriptor_assertions
uvc_negotiation_assertions
uvc_transfer_assertions
uvc_teardown_assertions
uvc_status_race_assertions)
set(UVC_TEST_TARGETS
uvc_descriptor_assertions
uvc_negotiation_assertions
uvc_transfer_assertions
uvc_teardown_assertions
uvc_status_race_assertions)

# The backstop is Linux-only and can be configured out, so its cases only
# exist when it does.
if(LIBUVC_REATTACH_GUARD)
add_executable(uvc_reattach_guard_assertions tests/reattach_guard_assertions.c)
list(APPEND UVC_TEST_TARGETS uvc_reattach_guard_assertions)
endif()

foreach(test_target ${UVC_TEST_TARGETS})
target_link_libraries(${test_target}
PRIVATE uvc_static LibUSB::LibUSB ${threads})
if(JPEG_FOUND)
Expand Down Expand Up @@ -252,6 +265,34 @@ if(BUILD_TESTING)
libuvc.teardown.undeliverable_status_xfer_quarantines
PROPERTIES TIMEOUT 5)

if(LIBUVC_REATTACH_GUARD)
# Asserts what a quarantining close leaves behind FOR the backstop, so it
# only means anything when the backstop is compiled in.
add_test(NAME libuvc.teardown.quarantined_handle_stays_armed
COMMAND uvc_teardown_assertions --case quarantined_handle_stays_armed)
set_tests_properties(libuvc.teardown.quarantined_handle_stays_armed
PROPERTIES TIMEOUT 5)

set_property(TARGET uvc_reattach_guard_assertions APPEND_STRING
PROPERTY LINK_FLAGS " -Wl,--wrap=ioctl -Wl,--wrap=libusb_detach_kernel_driver -Wl,--wrap=libusb_claim_interface -Wl,--wrap=libusb_release_interface -Wl,--wrap=libusb_set_interface_alt_setting -Wl,--wrap=libusb_attach_kernel_driver -Wl,--wrap=libusb_get_device_descriptor -Wl,--wrap=libusb_get_bus_number -Wl,--wrap=libusb_get_device_address")
foreach(case_name
rebinds_after_sigkill
disarmed_interfaces_are_not_rebound
busy_interface_is_retried
connect_ioctl_is_what_libusb_would_issue
claiming_an_interface_arms_the_guard)
add_test(NAME libuvc.reattach.${case_name}
COMMAND uvc_reattach_guard_assertions --case ${case_name})
endforeach()
# These wait on a forked helper rather than on anything they control, so the
# bound is the helper's settle budget plus slack, not the work itself.
set_tests_properties(
libuvc.reattach.rebinds_after_sigkill
libuvc.reattach.disarmed_interfaces_are_not_rebound
libuvc.reattach.busy_interface_is_retried
PROPERTIES TIMEOUT 30)
endif()

add_test(NAME libuvc.race.close_races_status_callback
COMMAND uvc_status_race_assertions --case close_races_status_callback)
# Generous because the case is deliberately iterated, and a sanitized build
Expand Down
62 changes: 55 additions & 7 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,21 @@ CeraLive changes on top of the base:

cmake .. -DLIBUVC_AUTO_DETACH_KERNEL_DRIVER=OFF

4. **Kernel-driver reattach that survives an uncatchable exit** — detaching a
driver is undone by a userspace call in the release path, so a process killed
with `SIGKILL`/`SIGSEGV`/`SIGABRT` never undoes it and leaves both UVC
interfaces with `driver = NONE` for good. libuvc now forks a small helper
when it first claims an interface; the helper's wakeup is the pipe EOF the
kernel delivers on the arming process's death, whatever killed it, and it
re-probes the interfaces that are still armed. Gated by the CMake option
**`LIBUVC_REATTACH_GUARD` (default `ON`, Linux only)**:

cmake .. -DLIBUVC_REATTACH_GUARD=OFF

See `include/libuvc/reattach_guard.h` for the mechanism and its fail-safe
properties, and "Device teardown contract" below for how it relates to
`uvc_close()`.

The library remains **BSD-3-Clause**; see `LICENSE.txt`. CeraLive additions are
also BSD-3-Clause. No license change.

Expand Down Expand Up @@ -64,21 +79,31 @@ Linux CTest suite. Configure, build, inspect, and run its static build with:
-DBUILD_TESTING=ON
cmake --build build/regression --parallel
ctest --test-dir build/regression --show-only=json-v1 \
| jq -e '.tests | length == 27'
| jq -e '.tests | length == 33'
ctest --test-dir build/regression --output-on-failure

The 27 cases are grouped as descriptor (11: `h264`, `h265`,
The 33 cases are grouped as descriptor (11: `h264`, `h265`,
`truncated_format`, `truncated_frame`, `degenerate_h26x`,
`scanner_vc_header_short`, `scanner_vc_oversized`, `scanner_vc_zero`,
`scanner_vs_header_short`, `scanner_vs_oversized`, `scanner_vs_zero`),
negotiation (5: `h264`, `h265`, `near_match`, `probe_set_error`,
`probe_get_error`), transfer (3: `terminal_statuses`, `retry_success`,
`retry_failure`), teardown (7: `status_xfer_stops_before_control_release`,
`retry_failure`), teardown (8: `status_xfer_stops_before_control_release`,
`every_claimed_interface_released_control_last`,
`no_status_endpoint_unchanged`, `sparse_interfaces_control_released_last`,
`high_index_interfaces_released`, `cancel_not_found_still_drains`,
`undeliverable_status_xfer_quarantines`), and race
`undeliverable_status_xfer_quarantines`, `quarantined_handle_stays_armed`),
reattach (5: `rebinds_after_sigkill`, `disarmed_interfaces_are_not_rebound`,
`busy_interface_is_retried`, `connect_ioctl_is_what_libusb_would_issue`,
`claiming_an_interface_arms_the_guard`), and race
(1: `close_races_status_callback`).

The six guard cases exist only when `LIBUVC_REATTACH_GUARD` is `ON`; with
`-DLIBUVC_REATTACH_GUARD=OFF` the suite is the 27 cases that predate it, and CI
runs that configuration too so a rollback stays a real rollback. The reattach
cases really do `SIGKILL` a forked victim process — that is the point, since the
defect is defined by cleanup code never running.

CI runs this suite without camera hardware on Ubuntu 22.04
and Ubuntu 24.04. See
`docs/evidence/uvc-camera-compat-stability.md` for its exact scope.
Expand All @@ -99,7 +124,8 @@ the teardown races live in `src/device.c` and run on the libusb event thread:
-DBUILD_TESTING=ON \
-DLIBUVC_SANITIZE=thread
cmake --build build/tsan --parallel
ctest --test-dir build/tsan --output-on-failure -R 'libuvc\.(teardown|race)\.'
ctest --test-dir build/tsan --output-on-failure \
-R 'libuvc\.(teardown|race|reattach)\.'

`libuvc.race.close_races_status_callback` is the case this exists for: it drives
`uvc_close()` against a real libusb event thread, and only an instrumented run
Expand All @@ -110,9 +136,9 @@ anything about this code. A dedicated CI job runs exactly this.

### Device teardown contract

`uvc_close()` owns the whole USB teardown and must keep four invariants that are
`uvc_close()` owns the whole USB teardown and must keep five invariants that are
not visible from the call site — all regression-locked by the
`libuvc.teardown.*` and `libuvc.race.*` cases:
`libuvc.teardown.*`, `libuvc.reattach.*` and `libuvc.race.*` cases:

1. **The VideoControl status interrupt transfer is stopped before any interface
is released.** It re-arms itself from `_uvc_status_callback()`, so a
Expand Down Expand Up @@ -141,6 +167,28 @@ not visible from the call site — all regression-locked by the
freeing the transfer there is undefined behaviour by libusb's own contract.
`_uvc_status_callback()` is therefore the only thing that ever clears
`status_xfer_submitted`, and the bounded drain is the only way out of the stop.
5. **Every interface libuvc claims is handed back, on every exit — including the
ones where `uvc_close()` never runs.** Invariants 1–4 all assume the process
lives long enough to execute them; `SIGKILL`, `SIGSEGV` and a watchdog's
`SIGABRT` execute nothing at all, and the kernel does not re-probe an
interface when usbfs's claim is dropped at fd-close. `uvc_claim_if()`
therefore arms the interface with the reattach guard and `uvc_release_if()`
disarms it only once the driver is genuinely back; the guard's forked helper
does the rest from outside the process. `uvc_reattach_guard_destroy()` runs
from `uvc_free_devh()` — the one place the handle is truly released, and
reached from neither quarantine path.

**The quarantine paths deliberately reattach nothing, and must stay that
way.** With a quarantined status transfer libusb still owns a URB on the
VideoControl status endpoint, so releasing that interface re-creates
invariant 1's defect exactly; with a quarantined stream the in-flight URB
rides a VideoStreaming interface, so releasing VideoControl alone would make
`uvcvideo` probe while usbfs still holds the streaming interfaces and
register no video node at all (invariant 2, in reverse). Both leave the
handle armed and let the helper repair the binding once this process — and
the transfer it was racing — is gone. A quarantine is a leak bounded by the
process lifetime, not a wedged camera.
`libuvc.teardown.quarantined_handle_stays_armed` locks that down.

## Developing with libuvc

Expand Down
2 changes: 2 additions & 0 deletions include/libuvc/libuvc_config.h.in
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@

#cmakedefine01 LIBUVC_AUTO_DETACH_KERNEL_DRIVER

#cmakedefine01 LIBUVC_REATTACH_GUARD

#endif // !def(LIBUVC_CONFIG_H)
10 changes: 10 additions & 0 deletions include/libuvc/libuvc_internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <time.h>
#include <libusb.h>
#include "utlist.h"
#include "libuvc/reattach_guard.h"

/** Converts an unaligned four-byte little-endian integer into an int32 */
#define DW_TO_INT(p) ((p)[0] | ((p)[1] << 8) | ((p)[2] << 16) | ((p)[3] << 24))
Expand Down Expand Up @@ -399,6 +400,15 @@ struct uvc_device_handle {
* still owned by libusb. Quarantines the handle for the same reason
* has_quarantined_stream does: a late callback dereferences devh. */
uint8_t has_quarantined_status_xfer;
/** Out-of-process backstop that hands the kernel driver back for every
* interface this handle still holds, however the process ends -- including
* the exits that run no user code at all (SIGKILL, SIGSEGV, the watchdog's
* SIGABRT) and the quarantine paths above, which deliberately never release
* anything. Created lazily by uvc_claim_if() and released by uvc_free_devh(),
* i.e. only where the handle is genuinely freed. NULL when the guard is
* compiled out or could not be started; every entry point tolerates that.
* See reattach_guard.h. */
uvc_reattach_guard_t *reattach_guard;
};

/** Context within which we communicate with devices */
Expand Down
Loading