From bfcb28603100506b06d6cdbbca8bdbab940bde3b Mon Sep 17 00:00:00 2001 From: claude-noether Date: Thu, 21 May 2026 12:14:48 +0200 Subject: [PATCH] picture: bounds-check codec_store_buffer slice writes against source_size MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit surface_object->source_data points at an OUTPUT-pool mmap of fixed size source_size, negotiated by v4l2_query_buffer at request_pool_init time (kernel sizeimage at S_FMT). codec_store_buffer's VASliceDataBufferType branch appended to it at three sites (H.264 Annex-B start code, VP8 uncompressed-header pad, slice payload) without consulting that capacity — a stream-level resolution upshift would walk past the mmap and SIGSEGV inside the memcpy (mpv --hwdec=vaapi-copy on the daedalus path, issue #13) or corrupt adjacent heap (Firefox RDD). Add a check at each append site that fails the RenderPicture call with VA_STATUS_ERROR_ALLOCATION_FAILED when slices_size+payload exceeds source_size, and logs the over-budget request for postmortem. libavcodec recreates the surface at the new dimensions on the next BeginPicture, so a refused upshift slice is recoverable. Doesn't address the root cause (surfaces should be re-created on resolution change, or source_data should be grown on demand) but removes the memory-safety hazard while the larger refactor waits. Closes marfrit/libva-v4l2-request-fourier#13. --- src/picture.c | 50 +++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/src/picture.c b/src/picture.c index 97e3563..9e703a8 100644 --- a/src/picture.c +++ b/src/picture.c @@ -62,16 +62,37 @@ static VAStatus codec_store_buffer(struct request_data *driver_data, struct object_buffer *buffer_object) { switch (buffer_object->type) { - case VASliceDataBufferType: + case VASliceDataBufferType: { /* * Since there is no guarantee that the allocation * order is the same as the submission order (via * RenderPicture), we can't use a V4L2 buffer directly * and have to copy from a regular buffer. + * + * Bounds check (issue #13): surface_object->source_data points + * at an OUTPUT-pool mmap of fixed size source_size, negotiated + * at S_FMT time. A stream-level resolution upshift can produce + * a slice larger than this allocation; without the guard, the + * memcpy walks past the mmap and SIGSEGVs (mpv --hwdec=vaapi- + * copy) or corrupts adjacent heap (Firefox RDD). Each append + * site below checks the running total against source_size and + * fails the RenderPicture call instead of corrupting memory; + * libavcodec re-creates the surface at the new resolution on + * the next BeginPicture. */ + size_t cap = surface_object->source_size; + size_t need; + if (context->h264_start_code) { static const char start_code[3] = { 0x00, 0x00, 0x01 }; + need = (size_t)surface_object->slices_size + + sizeof(start_code); + if (need > cap) { + request_log("codec_store_buffer: H.264 start code would overflow OUTPUT buffer (%zu > %zu) — resolution upshift mid-stream?\n", + need, cap); + return VA_STATUS_ERROR_ALLOCATION_FAILED; + } memcpy(surface_object->source_data + surface_object->slices_size, start_code, sizeof(start_code)); @@ -105,19 +126,34 @@ static VAStatus codec_store_buffer(struct request_data *driver_data, unsigned int header_size = surface_object->params.vp8.picture.pic_fields.bits.key_frame == 0 ? 10 : 3; + need = (size_t)surface_object->slices_size + header_size; + if (need > cap) { + request_log("codec_store_buffer: VP8 header pad would overflow OUTPUT buffer (%zu > %zu)\n", + need, cap); + return VA_STATUS_ERROR_ALLOCATION_FAILED; + } memset(surface_object->source_data + surface_object->slices_size, 0, header_size); surface_object->slices_size += header_size; } - memcpy(surface_object->source_data + - surface_object->slices_size, - buffer_object->data, - buffer_object->size * buffer_object->count); - surface_object->slices_size += - buffer_object->size * buffer_object->count; + { + size_t payload = (size_t)buffer_object->size * + buffer_object->count; + need = (size_t)surface_object->slices_size + payload; + if (need > cap) { + request_log("codec_store_buffer: slice payload would overflow OUTPUT buffer (%zu > %zu) — resolution upshift mid-stream?\n", + need, cap); + return VA_STATUS_ERROR_ALLOCATION_FAILED; + } + memcpy(surface_object->source_data + + surface_object->slices_size, + buffer_object->data, payload); + surface_object->slices_size += payload; + } surface_object->slices_count++; break; + } case VAPictureParameterBufferType: switch (profile) {