daemon: bounds-check pack_* functions against CAPTURE plane size
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).
This commit is contained in:
+53
-2
@@ -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,
|
||||
|
||||
Reference in New Issue
Block a user