daemon: bounds-check pack_* functions against CAPTURE plane size #21

Merged
marfrit merged 1 commits from noether/daemon-pack-bounds-check into main 2026-05-23 15:11:57 +00:00
Owner

Fixes the daemon SEGV that fires when libavcodec decodes a frame larger than the V4L2 client's currently-mapped CAPTURE dmabuf.

Crash scenario

YouTube DASH stepping resolution mid-stream (e.g. 480p → 720p when bandwidth improves). libva is supposed to handle V4L2_EVENT_SOURCE_CHANGE with STREAMOFF / S_FMT / REQBUFS, but in practice a stale CAPTURE request with the old buffer size sometimes slips through carrying the new (larger) frame. The chroma-interleave inner loop walks past the mapping boundary and the daemon takes SIGSEGV mid-frame.

Follow-on damage: the daemon crash leaves V4L2 clients hanging in vb2_core_dqbuf (D-state — separate follow-up task #146).

Fix

All three pack functions get a bounds-check on entry:

  • pack_nv12_single_to_plane (V4L2_PIX_FMT_NV12, contiguous Y+UV)
  • pack_nv12_to_planes (V4L2_PIX_FMT_NV12M, separate Y and UV planes)
  • pack_p010_to_plane (V4L2_PIX_FMT_P010, 10-bit HDR)

Each computes required = y_size + uv_size from the frame dimensions and compares to planes->size[N] before any write. On mismatch:

  • log_warn with both sizes and frame dimensions
  • return -EOVERFLOW

Error contract

The caller (process_decode_request loop) already handles negative pack returns:

if (prc < 0)
    log_warn("decoder: pack failed (...) — kernel will see metadata only");

The kernel still gets the response with metadata only; the V4L2 client sees a frame whose pixels are stale but whose buffer-done event fires normally. The next SOURCE_CHANGE the client processes resyncs the buffer size — graceful degradation instead of a hard crash that locks every consumer of /dev/video0.

The canonical comment lives on pack_nv12_single_to_plane; the other two reference it.

Verification

  • decoder.c compiles clean against trixie aarch64 + daedalus-fourier installed prefix (hertz local build).
  • No behavioural change on the happy path — bounds check is a single size compare; on a correctly-sized CAPTURE buffer it's effectively a 1-cycle pass before the existing memcpy loops.

Closes task #145 (daemon SEGV in pack_nv12_single on resolution change). Follow-up #146 (D-state on /dev/video0 release) likely still needs a separate kernel-side fix, but with this change the SEGV that triggers it should be gone.

Fixes the daemon SEGV that fires when libavcodec decodes a frame larger than the V4L2 client's currently-mapped CAPTURE dmabuf. ## Crash scenario YouTube DASH stepping resolution mid-stream (e.g. 480p → 720p when bandwidth improves). libva is supposed to handle `V4L2_EVENT_SOURCE_CHANGE` with `STREAMOFF` / `S_FMT` / `REQBUFS`, but in practice a stale CAPTURE request with the old buffer size sometimes slips through carrying the new (larger) frame. The chroma-interleave inner loop walks past the mapping boundary and the daemon takes SIGSEGV mid-frame. Follow-on damage: the daemon crash leaves V4L2 clients hanging in `vb2_core_dqbuf` (D-state — separate follow-up task #146). ## Fix All three pack functions get a bounds-check on entry: - `pack_nv12_single_to_plane` (V4L2_PIX_FMT_NV12, contiguous Y+UV) - `pack_nv12_to_planes` (V4L2_PIX_FMT_NV12M, separate Y and UV planes) - `pack_p010_to_plane` (V4L2_PIX_FMT_P010, 10-bit HDR) Each computes `required = y_size + uv_size` from the frame dimensions and compares to `planes->size[N]` before any write. On mismatch: - `log_warn` with both sizes and frame dimensions - return `-EOVERFLOW` ## Error contract The caller (`process_decode_request` loop) already handles negative pack returns: ```c if (prc < 0) log_warn("decoder: pack failed (...) — kernel will see metadata only"); ``` The kernel still gets the response with metadata only; the V4L2 client sees a frame whose pixels are stale but whose buffer-done event fires normally. The next `SOURCE_CHANGE` the client processes resyncs the buffer size — graceful degradation instead of a hard crash that locks every consumer of `/dev/video0`. The canonical comment lives on `pack_nv12_single_to_plane`; the other two reference it. ## Verification - `decoder.c` compiles clean against trixie aarch64 + daedalus-fourier installed prefix (hertz local build). - No behavioural change on the happy path — bounds check is a single size compare; on a correctly-sized CAPTURE buffer it's effectively a 1-cycle pass before the existing memcpy loops. Closes task #145 (daemon SEGV in pack_nv12_single on resolution change). Follow-up #146 (D-state on /dev/video0 release) likely still needs a separate kernel-side fix, but with this change the SEGV that triggers it should be gone.
marfrit added 1 commit 2026-05-23 13:33:10 +00:00
The three NV12/P010 pack functions (pack_nv12_single_to_plane,
pack_nv12_to_planes, pack_p010_to_plane) wrote into the V4L2
client's CAPTURE dmabuf without checking that the mapped size
covers the frame libavcodec just decoded.

Crash scenario: YouTube DASH stepping resolution mid-stream
(e.g. 480p -> 720p when bandwidth improves) — libva is supposed
to handle the V4L2_EVENT_SOURCE_CHANGE with STREAMOFF / S_FMT /
REQBUFS, but in practice a stale CAPTURE request with the old
buffer size sometimes slips through carrying the new (larger)
frame.  The chroma-interleave inner loop walks past the mapping
boundary and the daemon takes SIGSEGV mid-frame, which in turn
leaves V4L2 clients hanging in vb2_core_dqbuf — see the followup
ticket on the D-state symptom.

Fix: compute required = y_size + uv_size against planes->size[N]
BEFORE any write.  On mismatch, log_warn with both sizes and the
frame dimensions, and return -EOVERFLOW.

The caller (process_decode_request loop) already handles a
negative pack return with a log_warn and proceeds without
aborting the decode — the kernel still gets the response with
metadata-only and the V4L2 client sees a frame whose pixels are
stale but whose buffer-done event fires normally.  The next
SOURCE_CHANGE the client processes resyncs the buffer size.

All three pack paths get the same bounds-check; the comment on
pack_nv12_single is the canonical explanation, the other two
reference it.

Verified: builds clean against trixie aarch64; no behavioural
change on the happy path (the bounds check is a single size
compare; on a correctly-sized CAPTURE buffer it's a 1-cycle pass).

Closes daedalus-v4l2 task #145 (daemon SEGV in pack_nv12_single
on resolution change).
marfrit merged commit 4cfe0b470f into main 2026-05-23 15:11:57 +00:00
marfrit deleted branch noether/daemon-pack-bounds-check 2026-05-23 15:11:57 +00:00
Sign in to join this conversation.
No Reviewers
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: reauktion/daedalus-v4l2#21