From a43296c1ed992271aef7256c1cabeb42639e163e Mon Sep 17 00:00:00 2001 From: claude-noether Date: Sat, 23 May 2026 15:31:50 +0200 Subject: [PATCH] daemon: bounds-check pack_* functions against CAPTURE plane size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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). --- daemon/src/decoder.c | 55 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 53 insertions(+), 2 deletions(-) diff --git a/daemon/src/decoder.c b/daemon/src/decoder.c index 10f70f5..83e0b24 100644 --- a/daemon/src/decoder.c +++ b/daemon/src/decoder.c @@ -273,6 +273,20 @@ static int pack_p010_to_plane(struct AVFrame *fr, if (!base) return -EINVAL; + /* Bounds-check (see pack_nv12_single comment). P010 stores 16 + * bits per sample on both Y and CbCr planes; stride is in bytes + * and already accounts for the 2× expansion. */ + { + size_t y_size_chk = (size_t) stride * (size_t) h; + size_t required = y_size_chk + (size_t) stride * (size_t) ch; + if (planes->size[0] < required) { + log_warn("pack_p010: frame %dx%d (stride=%u required=%zu) " + "exceeds CAPTURE plane[0] size %zu — skipping pack", + w, h, stride, required, planes->size[0]); + return -EOVERFLOW; + } + } + dst_y = base; y_size = (size_t) stride * (size_t) h; dst_uv = base + y_size; @@ -320,7 +334,7 @@ static int pack_nv12_single_to_plane(struct AVFrame *fr, uint8_t *base; uint32_t stride; uint8_t *dst_y, *dst_uv; - size_t y_size; + size_t y_size, required; if (!desc || !planes || planes->nr < 1) return -EINVAL; @@ -339,8 +353,27 @@ static int pack_nv12_single_to_plane(struct AVFrame *fr, if (!base) return -EINVAL; + /* + * Bounds-check before any write — the V4L2 client's CAPTURE + * dmabuf may have been sized for a smaller frame than what + * libavcodec just decoded (e.g. YouTube DASH stepping + * resolution mid-stream — libva is supposed to handle the + * SOURCE_CHANGE event with STREAMOFF + S_FMT + REQBUFS but + * sometimes a stale request slips through carrying the old + * buffer size). Writing the chroma interleave loop into an + * undersized mapping faults the daemon with SIGSEGV mid-frame. + * Bail loudly with a warn instead. + */ + y_size = (size_t) stride * (size_t) h; + required = y_size + (size_t) stride * (size_t) ch; + if (planes->size[0] < required) { + log_warn("pack_nv12_single: frame %dx%d (stride=%u required=%zu) " + "exceeds CAPTURE plane[0] size %zu — skipping pack", + w, h, stride, required, planes->size[0]); + return -EOVERFLOW; + } + dst_y = base; - y_size = (size_t) stride * (size_t) h; dst_uv = base + y_size; for (y = 0; y < h; y++) @@ -395,6 +428,24 @@ static int pack_nv12_to_planes(struct AVFrame *fr, if (!dst_y || !dst_uv) return -EINVAL; + /* + * Bounds-check both planes against the mapped dmabuf size. See + * pack_nv12_single_to_plane comment for the resolution-change- + * mid-stream crash story this protects against. + */ + { + size_t y_required = (size_t) dst_y_stride * (size_t) h; + size_t uv_required = (size_t) dst_uv_stride * (size_t) ch; + if (planes->size[0] < y_required || + planes->size[1] < uv_required) { + log_warn("pack_nv12_2plane: frame %dx%d " + "(y=%zu/%zu uv=%zu/%zu) exceeds CAPTURE — skipping pack", + w, h, y_required, planes->size[0], + uv_required, planes->size[1]); + return -EOVERFLOW; + } + } + /* Y plane copy — strip source stride padding. */ for (y = 0; y < h; y++) memcpy(dst_y + (size_t) y * dst_y_stride,