From f1b22951740fee3451819773d851088b738ddda6 Mon Sep 17 00:00:00 2001 From: lusoris Date: Sat, 27 Jun 2026 16:52:22 +0200 Subject: [PATCH] fix: 9 confirmed defects from an adversarial bug hunt (ADR-0138) A verified sweep (5 finders -> 15 candidates -> 9 confirmed after independent source verification) across the filter family, GPU dispatch lifecycle, interop ABI, and patch stack. Two fire on DEFAULT configs. must-fix: - denoise: stat_buffer (binding 5) statically referenced by the shader but bound only inside if(want_meta); on the DEFAULT meta=0 path it was left unbound -> invalid Vulkan usage (validation error / UB / device-lost on strict drivers; tolerated on the dev-box NVIDIA driver, masking it). Now always allocated + bound every dispatch (mirroring mv/conf); zero-fill/readback still gated on meta. - denoise: uninitialized FFVkExecContext *exec read on an early-RET fail path. should-fix: - mc: s_part[128] OOB on subgroupSize<8 -> size to BLOCK_DIM*BLOCK_DIM, lockstep. - interop pel_blob_pack: uint32 total_size overflow -> heap overflow -> uint64 + reject > UINT32_MAX with PEL_ERR_RANGE. - denoise MC consumer: cells = grid_cols*grid_rows signed overflow -> uint64 + cap. - interop pel_blob_find_section: missing R5 8-byte alignment check -> PEL_ERR_ABI. - mc: av_realloc on a libc-calloc'd blob (cross-allocator UB) -> libc realloc. - analyze coalesce_roi: dropped ROI by raster position, not score -> sort by banding strength, keep top max. Interop conformance fixture extended. No wire-ABI change. meson fast 11/11; full n8.1.1 patch-stack build clean; GPU smokes pass incl. denoise meta=0. Regenerates patches 0002/0003/0007. ADR: docs/adr/0138-bug-hunt-remediation.md Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 1 + changelog.d/fixed/0138-bug-hunt.md | 1 + docs/adr/0138-bug-hunt-remediation.md | 59 +++++++++++++ docs/adr/README.md | 1 + .../0002-add-vf_pelorus_analyze_vulkan.patch | 87 ++++++++++++++++--- .../0003-add-vf_pelorus_denoise_vulkan.patch | 73 ++++++++++------ .../0007-add-vf_pelorus_mc_vulkan.patch | 25 ++++-- .../files/vf_pelorus_analyze_vulkan.c | 77 ++++++++++++++-- .../files/vf_pelorus_denoise_vulkan.c | 63 +++++++++----- ffmpeg-patches/files/vf_pelorus_mc_vulkan.c | 15 +++- libpelorus/shaders/pelorus_mc.comp | 6 +- libpelorus/src/interop.c | 22 ++++- libpelorus/test/interop_test.c | 64 ++++++++++++++ 13 files changed, 415 insertions(+), 79 deletions(-) create mode 100644 changelog.d/fixed/0138-bug-hunt.md create mode 100644 docs/adr/0138-bug-hunt-remediation.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 4e633d3..a881821 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -74,6 +74,7 @@ All notable changes to Pelorus are documented here. The format is - **`vf_pelorus_grain_estimate_vulkan` SIGSEGV on the first GPU frame**: the inline GLSL `% SLICES` modulo was passed unescaped through an `av_bprintf` format string, so `%S` was interpreted as a wide-string (`%ls`) conversion and dereferenced garbage when the shader was assembled. Escaped to `%%` so the literal `%` reaches the GLSL. ADR-0115. - **Chroma-passthrough macro across five Vulkan filters (`dehalo`, `aa`, `deblock`, `deband`, `borderfix`)**: the per-plane passthrough branch was emitted as a two-line split `GLSLF(1, imageStore(output_images[%i], pos, ,i);` / `GLSLF(2, imageLoad(input_images[%i], pos)); ,i);` whose `imageStore(` parenthesis never closed inside the macro call. The C preprocessor swallowed the next line and emitted the raw `do { av_bprintf(...); } while (0)` macro body **into the shader source**, so shaderc aborted the SPIR-V compile at first frame with `syntax error, unexpected COMMA` and the filter failed with `-22 (Invalid argument)`. Active on the luma-only filters (`dehalo`, `aa`, `deblock` — the chroma planes always took the broken branch); latent on the all-planes filters (`deband`, `borderfix` — triggered by any non-default `planes` mask). Invisible to every static gate: the `.o` compiles (valid C, garbage only at `av_bprintf` runtime), `glslangValidator` only sees the `.comp` (the chroma loop is C-emitted), and the patch-stack CI never instantiates a Vulkan device. Surfaced by on-device execution on the RTX 4090. Fixed by collapsing to a single balanced `GLSLF(1, imageStore(output_images[%i], pos, imageLoad(input_images[%i], pos)); ,i, i);` — the idiom `denoise` already used; chroma is now passed through bit-identically (infinite PSNR on U/V vs a no-op round-trip). ADR-0129. - **`vf_pelorus_mc_vulkan` out-of-bounds descriptor write**: the `cur_image` and `ref_image` bindings were declared `.elems = 1` (the shader reads only plane 0), but the host fill `ff_vk_shader_update_img_array()` writes one descriptor per frame plane (`dstArrayElement` 0..nb_planes−1). On any multi-plane input (yuv420p = 3 planes) the plane-1/2 writes landed past the one-element array — an out-of-bounds `VkWriteDescriptorSet` (VUID-…-dstArrayElement-00321) that a strict driver or the validation layers reject, and that only "worked" on the dev box's NVIDIA driver with validation absent. The per-plane image views already exist, so the binding size was the only defect. Fixed by sizing both bindings to `.elems = av_pix_fmt_count_planes(vkctx->input_format)`, matching the `deband`/`denoise` precedent. ADR-0129. +- **Bug-hunt remediation** ([ADR-0138](docs/adr/0138-bug-hunt-remediation.md)) — a verified sweep fixed 9 defects: **(must-fix)** `vf_pelorus_denoise_vulkan` left descriptor binding 5 (`stat_buffer`, statically used by the shader) **unbound on the default `meta=0` path** (invalid Vulkan / device-lost on strict drivers) — now always bound; and an uninitialized `exec` read on an error path. **(should-fix)** `vf_pelorus_mc_vulkan` `s_part[]` OOB on GPUs with `subgroupSize < 8`; `libpelorus` `pel_blob_pack` `uint32` size overflow → heap overflow (now rejected with `PEL_ERR_RANGE`) and `pel_blob_find_section` missing the R5 8-byte-alignment check (now `PEL_ERR_ABI`); `denoise` MC-consumer `grid_cols*grid_rows` signed-int overflow from untrusted side-data; `mc` growing a libc-allocated blob with `av_realloc` (cross-allocator UB); and `analyze` `coalesce_roi` truncating the ROI by raster position instead of by banding strength. Interop conformance fixture extended with malformed-blob rejection tests. No wire-ABI change. diff --git a/changelog.d/fixed/0138-bug-hunt.md b/changelog.d/fixed/0138-bug-hunt.md new file mode 100644 index 0000000..270b1d5 --- /dev/null +++ b/changelog.d/fixed/0138-bug-hunt.md @@ -0,0 +1 @@ +- **Bug-hunt remediation** ([ADR-0138](docs/adr/0138-bug-hunt-remediation.md)) — a verified sweep fixed 9 defects: **(must-fix)** `vf_pelorus_denoise_vulkan` left descriptor binding 5 (`stat_buffer`, statically used by the shader) **unbound on the default `meta=0` path** (invalid Vulkan / device-lost on strict drivers) — now always bound; and an uninitialized `exec` read on an error path. **(should-fix)** `vf_pelorus_mc_vulkan` `s_part[]` OOB on GPUs with `subgroupSize < 8`; `libpelorus` `pel_blob_pack` `uint32` size overflow → heap overflow (now rejected with `PEL_ERR_RANGE`) and `pel_blob_find_section` missing the R5 8-byte-alignment check (now `PEL_ERR_ABI`); `denoise` MC-consumer `grid_cols*grid_rows` signed-int overflow from untrusted side-data; `mc` growing a libc-allocated blob with `av_realloc` (cross-allocator UB); and `analyze` `coalesce_roi` truncating the ROI by raster position instead of by banding strength. Interop conformance fixture extended with malformed-blob rejection tests. No wire-ABI change. diff --git a/docs/adr/0138-bug-hunt-remediation.md b/docs/adr/0138-bug-hunt-remediation.md new file mode 100644 index 0000000..9d5de96 --- /dev/null +++ b/docs/adr/0138-bug-hunt-remediation.md @@ -0,0 +1,59 @@ + +# ADR-0138: Adversarial bug-hunt remediation — 9 confirmed defects fixed + +- **Status**: Accepted +- **Date**: 2026-06-20 +- **Deciders**: Lusoris + +## Context + +A systematic, adversarially-verified bug hunt swept the whole filter family, the +GPU dispatch lifecycle, the libpelorus interop ABI, and the patch stack: five +parallel finders (one per defect class) proposed 15 candidates; each was then +verified against the source by an independent agent that defaulted to +"not a bug". Nine were confirmed (two must-fix, six should-fix, one a duplicate). +Two are correctness defects that fire on **default** configurations. + +## Decision + +Fix all nine, with a conformance-fixture regression for the two parser hardenings. +Adopt the adversarially-verified hunt as a periodic practice (it complements the +on-device validation release gate of ADR-0129, which catches runtime GPU defects +the C-only patch CI cannot). + +### The defects + fixes + +| # | Sev | Defect | Fix | +|---|---|---|---| +| 1 | must-fix | `denoise` declares `stat_buffer` at binding 5; the SPIR-V statically references it but it is bound only inside `if(want_meta)` — so on the **default `meta=0`** path a used storage descriptor is left **unbound** (validation error / UB / device-lost on intolerant drivers). No `PARTIALLY_BOUND` escape exists in the FFmpeg helper. | Always allocate the (small) stat buffer and bind binding 5 every dispatch (mirroring the mv/conf 6/7 treatment) + add it as an exec dep on the async path; zero-fill + readback stay gated on `want_meta`. | +| 2 | must-fix | `denoise_dispatch` reads an uninitialized `FFVkExecContext *exec` on an early-`RET` fail path (the `if(exec)` cleanup guard sees garbage). | `FFVkExecContext *exec = NULL;`. | +| 3 | should-fix | `mc` block-SAD reduction OOB-writes `s_part[128]` on GPUs with `subgroupSize < 8` (8×8 subgroups would need up to 1024 slots). | Size `s_part` to the worst case `BLOCK_DIM*BLOCK_DIM` (=1024) in both the inline GLSL and the `.comp` (lockstep). | +| 4 | should-fix | `pel_blob_pack` computes `total_size` (a `uint32` wire field) with 32-bit arithmetic → a crafted oversized section wraps to an undersized allocation → heap overflow. | Accumulate in `uint64_t`, 8-align each section in 64-bit, `return PEL_ERR_RANGE` if `> UINT32_MAX` before allocating. No wire-format change. | +| 5 | should-fix | `denoise` MC consumer computes `cells = grid_cols * grid_rows` (signed `int`) from untrusted side-data → signed overflow → UB / bad size validation. | Compute in `uint64_t`, bound by a new `PEL_DENOISE_MAX_CELLS` cap, validate field sizes in 64-bit before any use. | +| 6 | should-fix | `pel_blob_find_section` returns a castable section pointer without checking the R5 8-byte-alignment invariant on the read side (the packer guarantees it on write). | Reject `(off & 7) != 0` with `PEL_ERR_ABI` before the bounds check. | +| 7 | should-fix | `mc` grows a libc-`calloc`'d Pelorus blob with `av_realloc` → cross-allocator UB (the blob is later `free`'d). | Use libc `realloc`; the error path already preserves the original for `pel_blob_free`. | +| 8 | should-fix | `analyze` `coalesce_roi` documents that it drops the lowest-**score** remainder when the `PEL_MAX_ROI` cap is hit, but actually truncates by **raster position** — so on a busy frame it can drop the strongest banding regions. | Collect all coalesced runs into a scratch buffer, sort by descending banding strength (most-negative qoffset), keep the top `max`. | + +## Validation + +`meson test --suite=fast` 11/11 (incl. the interop fixture, now with +`test_misaligned_offset` → `PEL_ERR_ABI` and `test_pack_size_overflow` → +`PEL_ERR_RANGE`, and the `pelorus_mc` SPIR-V compile validating the `s_part` +resize). Full n8.1.1 patch-stack apply + `make ffmpeg` clean; GPU smokes on +`vk:0` all pass — notably **denoise at the default `meta=0`** (the must-fix +binding-5 path), plus mc, analyze-roi, denoise meta=1 and denoise mc=1 (the +rewritten 64-bit cells path). + +## Consequences + +- The interop fixes (#4, #6) are defensive hardenings of the shared, vmafx-vendored + parser — append-only safe (no struct/offset/wire change); vmafx picks them up on + its next interop.c sync. +- No behaviour change on valid input; the must-fix descriptor binding makes the + default denoise path valid Vulkan on strict drivers (it happened to be tolerated + on the dev-box NVIDIA driver, masking the defect). + +## References + +- ADR-0129 (on-device validation release gate — the complementary runtime-defect + gate). The hunt + verification transcripts are this PR's provenance. diff --git a/docs/adr/README.md b/docs/adr/README.md index f87624f..c0151e3 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -50,3 +50,4 @@ sibling keep vmafx's number (e.g. 0100, 0108) for an easy cross-walk. | [0135](0135-analyze-perceptual-aq-map.md) | Source-side perceptual bit-allocation map (`aq` perceptual-AQ on the clean source) for analyze, aiming to beat the encoder's in-loop variance-AQ. Implemented (Chou-Li luma + NAMM masking + edge/banding floors, signed mean-centered qoffsets) + measured: strongly cuts banding (CAMBI −14–25%) but at a net fidelity cost (SSIMULACRA2 BD-rate +13–20%, PSNR +7–14%), and a pure loss on real non-banding content. Fails the kill-criterion; `roi=1` already does banding without the texture-starving harm. Code not merged | Rejected | | [0136](0136-analyze-frame-metadata.md) | analyze emits its per-frame scalars (complexity/texture/motion/variance/edge/banding/scene_cut) as `lavfi.pelorus.*` frame metadata (the vf_scdet idiom), readable via ffprobe / `metadata=print`. The interop blob isn't muxed to file, so this is the host-side extraction path for per-shot CRF / autotune orchestration + shell debugging. Additive, host-side; no interop-ABI or shader change | Accepted | | [0137](0137-denoise-forward-lookahead-cadence.md) | Forward-lookahead (bidirectional) temporal denoise — cadence-aware. The causal walk only denoises the trailing frame of each held animation drawing (2s/3s cadence); the leading frame sees a different drawing (tcut-breaks) → under-denoised. Premise validated: causal recovers +1.5 dB on a synthetic 2s-cadence clip vs ~+3 dB potential. Design: a 1-frame-delay buffered model (activate) + a next-frame shader tap, tcut-gated. First positive direction after the encoder-RC negatives. Build pending | Proposed | +| [0138](0138-bug-hunt-remediation.md) | Adversarial bug-hunt remediation: a verified sweep (15 candidates → 9 confirmed) fixed two must-fix denoise defects (the default `meta=0` unbound stat-buffer descriptor + an uninitialized `exec`) and six should-fix hardenings — mc `s_part` OOB on small subgroups, interop `pel_blob_pack` uint32 overflow → heap overflow + `find_section` R5 alignment, denoise `cells` int overflow, mc cross-allocator `av_realloc`, analyze `coalesce_roi` score-truncation. Conformance fixture extended. Complements the ADR-0129 on-device gate | Accepted | diff --git a/ffmpeg-patches/0002-add-vf_pelorus_analyze_vulkan.patch b/ffmpeg-patches/0002-add-vf_pelorus_analyze_vulkan.patch index 0909ebb..035e069 100644 --- a/ffmpeg-patches/0002-add-vf_pelorus_analyze_vulkan.patch +++ b/ffmpeg-patches/0002-add-vf_pelorus_analyze_vulkan.patch @@ -1,6 +1,6 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Lusoris -Date: Sat, 20 Jun 2026 22:05:42 +0200 +Date: Sat, 27 Jun 2026 16:51:25 +0200 Subject: [PATCH 02/18] feat(analyze): add vf_pelorus_analyze_vulkan (measured banding/variance maps) MIME-Version: 1.0 @@ -42,8 +42,8 @@ docs/adr/0133-analyze-coarse-banding-cambi.md configure | 2 + libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + - libavfilter/vf_pelorus_analyze_vulkan.c | 845 ++++++++++++++++++++++++ - 4 files changed, 849 insertions(+) + libavfilter/vf_pelorus_analyze_vulkan.c | 912 ++++++++++++++++++++++++ + 4 files changed, 916 insertions(+) create mode 100644 libavfilter/vf_pelorus_analyze_vulkan.c diff --git a/configure b/configure @@ -92,10 +92,10 @@ index c02a049dbc..41761c6146 100644 extern const FFFilter ff_vf_perspective; diff --git a/libavfilter/vf_pelorus_analyze_vulkan.c b/libavfilter/vf_pelorus_analyze_vulkan.c new file mode 100644 -index 0000000000..1d4b3ac145 +index 0000000000..1cd20e3b9c --- /dev/null +++ b/libavfilter/vf_pelorus_analyze_vulkan.c -@@ -0,0 +1,845 @@ +@@ -0,0 +1,912 @@ +/* + * Copyright 2026 Lusoris + * @@ -149,6 +149,7 @@ index 0000000000..1d4b3ac145 +#include "filters.h" + +#include ++#include +#include + +#include @@ -437,9 +438,24 @@ index 0000000000..1d4b3ac145 + return av_make_q(milli, 1000); +} + ++/* qsort comparator: order coalesced runs by descending banding strength. A ++ * stronger banding tile gets a more-negative qoffset.num (down to -1000), so the ++ * strongest run is the most negative one — sort ascending by qoffset.num. */ ++static int roi_strength_cmp(const void *pa, const void *pb) ++{ ++ const AVRegionOfInterest *a = pa; ++ const AVRegionOfInterest *b = pb; ++ if (a->qoffset.num < b->qoffset.num) ++ return -1; /* a is stronger banding -> earlier */ ++ if (a->qoffset.num > b->qoffset.num) ++ return 1; ++ return 0; ++} ++ +/* Greedy row-run coalescing: scan tiles row-major, merge horizontally adjacent -+ * banding tiles of the same qoffset bucket into one rectangle, cap at -+ * PEL_MAX_ROI rectangles (drop the lowest-score remainder once full). Per-tile ++ * banding tiles of the same qoffset bucket into one rectangle. ALL runs are ++ * collected first, then ranked by banding strength (|qoffset|) and the top ++ * `max` are kept (drop the lowest-score remainder once full). Per-tile + * rectangles fall out naturally when neighbours differ. Writes into out[] and + * returns the rectangle count. */ +static int coalesce_roi(const PelorusAnalyzeVulkanContext *s, @@ -448,10 +464,53 @@ index 0000000000..1d4b3ac145 + int cols = s->grid_cols, rows = s->grid_rows; + int n = 0; + int ty, tx; ++ AVRegionOfInterest *runs; ++ /* Worst case: every tile is a singleton run (no horizontal merge). */ ++ size_t cap = (size_t)cols * (size_t)rows; ++ ++ if (max <= 0) ++ return 0; ++ ++ runs = av_calloc(cap > 0 ? cap : 1, sizeof(*runs)); ++ if (!runs) { ++ /* Fallback: positional emission (the prior behaviour) so a transient ++ * alloc failure still yields a usable, bounded ROI set rather than none. */ ++ for (ty = 0; ty < rows && n < max; ty++) { ++ tx = 0; ++ while (tx < cols && n < max) { ++ float sc = score[ty * cols + tx]; ++ int q0, run, x; ++ if (sc <= 0.0f) { ++ tx++; ++ continue; ++ } ++ q0 = score_to_qoffset(s, sc).num; ++ run = 1; ++ for (x = tx + 1; x < cols; x++) { ++ float scn = score[ty * cols + x]; ++ if (scn <= 0.0f || score_to_qoffset(s, scn).num != q0) ++ break; ++ run++; ++ } ++ out[n] = (AVRegionOfInterest) { ++ .self_size = sizeof(AVRegionOfInterest), ++ .top = ty * PEL_TILE, ++ .bottom = FFMIN((ty + 1) * PEL_TILE, s->vkctx.output_height), ++ .left = tx * PEL_TILE, ++ .right = FFMIN((tx + run) * PEL_TILE, s->vkctx.output_width), ++ .qoffset = av_make_q(q0, 1000), ++ }; ++ n++; ++ tx += run; ++ } ++ } ++ return n; ++ } + -+ for (ty = 0; ty < rows && n < max; ty++) { ++ /* Collect every coalesced run (uncapped). */ ++ for (ty = 0; ty < rows; ty++) { + tx = 0; -+ while (tx < cols && n < max) { ++ while (tx < cols) { + float sc = score[ty * cols + tx]; + int q0, run, x; + if (sc <= 0.0f) { @@ -467,7 +526,7 @@ index 0000000000..1d4b3ac145 + break; + run++; + } -+ out[n] = (AVRegionOfInterest) { ++ runs[n] = (AVRegionOfInterest) { + .self_size = sizeof(AVRegionOfInterest), + .top = ty * PEL_TILE, + .bottom = FFMIN((ty + 1) * PEL_TILE, s->vkctx.output_height), @@ -479,6 +538,14 @@ index 0000000000..1d4b3ac145 + tx += run; + } + } ++ ++ /* Rank by banding strength and keep the strongest `max`. */ ++ if (n > 1) ++ qsort(runs, (size_t)n, sizeof(*runs), roi_strength_cmp); ++ if (n > max) ++ n = max; ++ memcpy(out, runs, (size_t)n * sizeof(*out)); ++ av_free(runs); + return n; +} + diff --git a/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch b/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch index 11fbef5..611ce41 100644 --- a/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch +++ b/ffmpeg-patches/0003-add-vf_pelorus_denoise_vulkan.patch @@ -1,6 +1,6 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Lusoris -Date: Sat, 27 Jun 2026 16:03:53 +0200 +Date: Sat, 27 Jun 2026 16:51:25 +0200 Subject: [PATCH 03/18] feat(denoise): add vf_pelorus_denoise_vulkan (temporal denoise) MIME-Version: 1.0 @@ -40,8 +40,8 @@ ADR: docs/adr/0112-temporal-denoise.md configure | 2 + libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + - libavfilter/vf_pelorus_denoise_vulkan.c | 1237 +++++++++++++++++++++++ - 4 files changed, 1241 insertions(+) + libavfilter/vf_pelorus_denoise_vulkan.c | 1258 +++++++++++++++++++++++ + 4 files changed, 1262 insertions(+) create mode 100644 libavfilter/vf_pelorus_denoise_vulkan.c diff --git a/configure b/configure @@ -90,10 +90,10 @@ index 41761c6146..8f49f54808 100644 extern const FFFilter ff_vf_phase; diff --git a/libavfilter/vf_pelorus_denoise_vulkan.c b/libavfilter/vf_pelorus_denoise_vulkan.c new file mode 100644 -index 0000000000..4a0bf8e5f5 +index 0000000000..0294cc6275 --- /dev/null +++ b/libavfilter/vf_pelorus_denoise_vulkan.c -@@ -0,0 +1,1237 @@ +@@ -0,0 +1,1258 @@ +/* + * Copyright 2026 Lusoris + * @@ -166,6 +166,13 @@ index 0000000000..4a0bf8e5f5 + +#define PEL_SLICES 16 + ++/* Upper bound on the MV/conf grid cell count accepted from untrusted side data. ++ * grid_cols/grid_rows are each uint16 on the wire (up to 65535), so the raw ++ * product can reach ~4.29e9 — past INT_MAX and a multi-GB allocation. Cap it to a ++ * value that still covers any realistic frame grid (8K at a small block edge) ++ * while rejecting absurd dimensions before the cell count is used for sizing. */ ++#define PEL_DENOISE_MAX_CELLS (8192u * 8192u) ++ +/* Host-readback accumulator for the meta=1 residual free-ride. Sliced to spread + * atomic contention, exactly as vf_pelorus_analyze does; summed host-side. */ +typedef struct PelorusDenoiseBuf { @@ -802,7 +809,7 @@ index 0000000000..4a0bf8e5f5 + int i; + FFVulkanContext *vkctx = &s->vkctx; + FFVulkanFunctions *vk = &vkctx->vkfn; -+ FFVkExecContext *exec; ++ FFVkExecContext *exec = NULL; + AVFrame *bound_prev[PEL_DENOISE_MAX_PREV]; + AVFrame *bound_next; + VkImageView cur_views[AV_NUM_DATA_POINTERS]; @@ -833,16 +840,21 @@ index 0000000000..4a0bf8e5f5 + s->opts.actual_next = (next != NULL) ? 1 : 0; + s->opts.want_meta = want_meta; + -+ if (want_meta) { -+ RET(ff_vk_get_pooled_buffer(vkctx, &s->stat_buf_pool, &buf, -+ VK_BUFFER_USAGE_TRANSFER_DST_BIT | -+ VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, -+ NULL, sizeof(PelorusDenoiseBuf), -+ VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | -+ VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | -+ VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)); -+ buf_vk = (FFVkBuffer *)buf->data; -+ } ++ /* The shader statically references the binding-5 stat SSBO regardless of ++ * want_meta (the meta accumulators are gated by a runtime push-const branch, ++ * not removed from the SPIR-V), so a valid buffer MUST be bound at 5 on every ++ * dispatch — Vulkan forbids a used-but-unbound descriptor. Allocate it ++ * unconditionally; the zero-fill + host readback below stay gated on ++ * want_meta (when meta=0 the SSBO is never written, so its contents are ++ * don't-care — it only has to be a live, bound descriptor). */ ++ RET(ff_vk_get_pooled_buffer(vkctx, &s->stat_buf_pool, &buf, ++ VK_BUFFER_USAGE_TRANSFER_DST_BIT | ++ VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, ++ NULL, sizeof(PelorusDenoiseBuf), ++ VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | ++ VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | ++ VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)); ++ buf_vk = (FFVkBuffer *)buf->data; + + /* --- motion-compensated taps: parse the MV + confidence grids from cur's + * Pelorus side data and stage them into host-visible SSBOs. Something must @@ -856,7 +868,9 @@ index 0000000000..4a0bf8e5f5 + const void *mp = NULL, *cp = NULL; + size_t msz = 0, csz = 0; + const uint8_t *mv_field = NULL, *conf_field = NULL; -+ int gc = 0, gr = 0, cells = 0, ncells, j; ++ int gc = 0, gr = 0; ++ uint64_t cells = 0, ncells; ++ uint64_t j; + + if (sd && + pel_blob_find_section(sd->data, sd->size, PEL_SEC_MOTION, @@ -869,11 +883,13 @@ index 0000000000..4a0bf8e5f5 + (const PelorusSideData *)(sd->data + PELORUS_SIDEDATA_UUID_LEN); + gc = bh->grid_cols; + gr = bh->grid_rows; -+ cells = gc * gr; ++ /* grid dims are untrusted uint16; the product can exceed INT_MAX, so ++ * compute in 64-bit and bound it before any sizing/loop use. */ ++ cells = (uint64_t)gc * (uint64_t)gr; + /* Validate against the untrusted side-data length before deref. */ -+ if (gc > 0 && gr > 0 && -+ mo->mv_field_size == (uint32_t)cells * 4 && -+ mcs->conf_field_size == (uint32_t)cells && ++ if (gc > 0 && gr > 0 && cells <= PEL_DENOISE_MAX_CELLS && ++ mo->mv_field_size == cells * 4 && ++ mcs->conf_field_size == cells && + (size_t)PELORUS_SIDEDATA_UUID_LEN + mo->mv_field_offset + + mo->mv_field_size <= sd->size && + (size_t)PELORUS_SIDEDATA_UUID_LEN + mcs->conf_field_offset + @@ -958,6 +974,11 @@ index 0000000000..4a0bf8e5f5 + ff_vk_shader_update_img_array(vkctx, exec, &s->shd, bound_prev[i], + prev_views[i], 0, 1 + i, + VK_IMAGE_LAYOUT_GENERAL, VK_NULL_HANDLE); ++ /* Bind the stat SSBO at 5 on EVERY dispatch (the shader statically uses it); ++ * the zero-fill + readback stay gated on want_meta below. */ ++ RET(ff_vk_shader_update_desc_buffer(vkctx, exec, &s->shd, 0, 5, 0, ++ buf_vk, 0, buf_vk->size, ++ VK_FORMAT_UNDEFINED)); + RET(ff_vk_shader_update_desc_buffer(vkctx, exec, &s->shd, 0, 6, 0, + mvbuf_vk, 0, mvbuf_vk->size, + VK_FORMAT_UNDEFINED)); @@ -1037,10 +1058,6 @@ index 0000000000..4a0bf8e5f5 + }, + .bufferMemoryBarrierCount = 1, + }); -+ -+ RET(ff_vk_shader_update_desc_buffer(vkctx, exec, &s->shd, 0, 5, 0, -+ buf_vk, 0, buf_vk->size, -+ VK_FORMAT_UNDEFINED)); + } else { + vk->CmdPipelineBarrier2(exec->buf, &(VkDependencyInfo) { + .sType = VK_STRUCTURE_TYPE_DEPENDENCY_INFO, @@ -1086,7 +1103,11 @@ index 0000000000..4a0bf8e5f5 + return 0; + } + -+ /* meta=0: no readback, no sync point — just submit and return. */ ++ /* meta=0: no readback, no sync point — just submit and return. The stat ++ * buffer bound at 5 is never read back, but it must outlive the async submit ++ * (same treatment as the mv/conf buffers above). */ ++ RET(ff_vk_exec_add_dep_buf(vkctx, exec, &buf, 1, 0)); ++ buf = NULL; + RET(ff_vk_exec_submit(vkctx, exec)); + return 0; + diff --git a/ffmpeg-patches/0007-add-vf_pelorus_mc_vulkan.patch b/ffmpeg-patches/0007-add-vf_pelorus_mc_vulkan.patch index eaca8ee..cb54896 100644 --- a/ffmpeg-patches/0007-add-vf_pelorus_mc_vulkan.patch +++ b/ffmpeg-patches/0007-add-vf_pelorus_mc_vulkan.patch @@ -1,6 +1,6 @@ From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Lusoris -Date: Sat, 20 Jun 2026 18:39:47 +0200 +Date: Sat, 27 Jun 2026 16:51:25 +0200 Subject: [PATCH 07/18] feat(mc): add vf_pelorus_mc_vulkan (block-matching motion estimator) MIME-Version: 1.0 @@ -47,8 +47,8 @@ ADR: docs/adr/0116-pelorus-mc.md configure | 2 + libavfilter/Makefile | 1 + libavfilter/allfilters.c | 1 + - libavfilter/vf_pelorus_mc_vulkan.c | 976 +++++++++++++++++++++++++++++ - 4 files changed, 980 insertions(+) + libavfilter/vf_pelorus_mc_vulkan.c | 985 +++++++++++++++++++++++++++++ + 4 files changed, 989 insertions(+) create mode 100644 libavfilter/vf_pelorus_mc_vulkan.c diff --git a/configure b/configure @@ -97,10 +97,10 @@ index 5a5f777074..45949d7b8e 100644 extern const FFFilter ff_vf_phase; diff --git a/libavfilter/vf_pelorus_mc_vulkan.c b/libavfilter/vf_pelorus_mc_vulkan.c new file mode 100644 -index 0000000000..640f6ec979 +index 0000000000..9de6748eb8 --- /dev/null +++ b/libavfilter/vf_pelorus_mc_vulkan.c -@@ -0,0 +1,976 @@ +@@ -0,0 +1,985 @@ +/* + * Copyright 2026 Lusoris + * @@ -163,6 +163,7 @@ index 0000000000..640f6ec979 +#include "filters.h" + +#include ++#include +#include + +#include @@ -190,8 +191,11 @@ index 0000000000..640f6ec979 + * ref_image[0]. */ +static const char mc_glsl[] = + "const float SAD_SCALE = 256.0;\n" -+ "shared float s_part[128];\n" /* per-subgroup SAD partials (wg / min-subgroup)*/ -+ "shared float s_sad0;\n" /* block-SAD reduction result */ ++ /* One slot per invocation (worst case subgroupSize==1 => BLOCK_DIM*BLOCK_DIM ++ * subgroups). gl_SubgroupID indexes this, so a smaller size OOB-writes. Sized ++ * from the lane-count macro so a BLOCK_DIM change propagates. */ ++ "shared float s_part[" PEL_MC_SAD_LANES_STR "];\n" ++ "shared float s_sad0;\n" /* block-SAD reduction result */ + "shared int s_best_x;\n" + "shared int s_best_y;\n" + "shared float s_best_cost;\n" @@ -648,7 +652,12 @@ index 0000000000..640f6ec979 + size_t mv_bytes = (size_t)nblocks * 2 * sizeof(int16_t); + size_t conf_bytes = (size_t)nblocks; + size_t uuid = PELORUS_SIDEDATA_UUID_LEN; -+ uint8_t *grown = av_realloc(blob, len + mv_bytes + conf_bytes); ++ /* The blob is libc-calloc'd by pel_blob_pack and freed by ++ * pel_blob_free()->free(); grow it with libc realloc so all three stay on ++ * one allocator (av_realloc may route to _aligned_realloc on aligned-CRT ++ * builds, mismatching the libc free). On NULL the original blob is ++ * untouched, so the error path's pel_blob_free(blob) still frees it. */ ++ uint8_t *grown = realloc(blob, len + mv_bytes + conf_bytes); + PelorusSideData *hdr; + PelorusSectionDir *dir; + PelorusMotionSection *mo_in_blob = NULL; diff --git a/ffmpeg-patches/files/vf_pelorus_analyze_vulkan.c b/ffmpeg-patches/files/vf_pelorus_analyze_vulkan.c index 1d4b3ac..1cd20e3 100644 --- a/ffmpeg-patches/files/vf_pelorus_analyze_vulkan.c +++ b/ffmpeg-patches/files/vf_pelorus_analyze_vulkan.c @@ -51,6 +51,7 @@ #include "filters.h" #include +#include #include #include @@ -339,9 +340,24 @@ static AVRational score_to_qoffset(const PelorusAnalyzeVulkanContext *s, return av_make_q(milli, 1000); } +/* qsort comparator: order coalesced runs by descending banding strength. A + * stronger banding tile gets a more-negative qoffset.num (down to -1000), so the + * strongest run is the most negative one — sort ascending by qoffset.num. */ +static int roi_strength_cmp(const void *pa, const void *pb) +{ + const AVRegionOfInterest *a = pa; + const AVRegionOfInterest *b = pb; + if (a->qoffset.num < b->qoffset.num) + return -1; /* a is stronger banding -> earlier */ + if (a->qoffset.num > b->qoffset.num) + return 1; + return 0; +} + /* Greedy row-run coalescing: scan tiles row-major, merge horizontally adjacent - * banding tiles of the same qoffset bucket into one rectangle, cap at - * PEL_MAX_ROI rectangles (drop the lowest-score remainder once full). Per-tile + * banding tiles of the same qoffset bucket into one rectangle. ALL runs are + * collected first, then ranked by banding strength (|qoffset|) and the top + * `max` are kept (drop the lowest-score remainder once full). Per-tile * rectangles fall out naturally when neighbours differ. Writes into out[] and * returns the rectangle count. */ static int coalesce_roi(const PelorusAnalyzeVulkanContext *s, @@ -350,10 +366,53 @@ static int coalesce_roi(const PelorusAnalyzeVulkanContext *s, int cols = s->grid_cols, rows = s->grid_rows; int n = 0; int ty, tx; + AVRegionOfInterest *runs; + /* Worst case: every tile is a singleton run (no horizontal merge). */ + size_t cap = (size_t)cols * (size_t)rows; + + if (max <= 0) + return 0; + + runs = av_calloc(cap > 0 ? cap : 1, sizeof(*runs)); + if (!runs) { + /* Fallback: positional emission (the prior behaviour) so a transient + * alloc failure still yields a usable, bounded ROI set rather than none. */ + for (ty = 0; ty < rows && n < max; ty++) { + tx = 0; + while (tx < cols && n < max) { + float sc = score[ty * cols + tx]; + int q0, run, x; + if (sc <= 0.0f) { + tx++; + continue; + } + q0 = score_to_qoffset(s, sc).num; + run = 1; + for (x = tx + 1; x < cols; x++) { + float scn = score[ty * cols + x]; + if (scn <= 0.0f || score_to_qoffset(s, scn).num != q0) + break; + run++; + } + out[n] = (AVRegionOfInterest) { + .self_size = sizeof(AVRegionOfInterest), + .top = ty * PEL_TILE, + .bottom = FFMIN((ty + 1) * PEL_TILE, s->vkctx.output_height), + .left = tx * PEL_TILE, + .right = FFMIN((tx + run) * PEL_TILE, s->vkctx.output_width), + .qoffset = av_make_q(q0, 1000), + }; + n++; + tx += run; + } + } + return n; + } - for (ty = 0; ty < rows && n < max; ty++) { + /* Collect every coalesced run (uncapped). */ + for (ty = 0; ty < rows; ty++) { tx = 0; - while (tx < cols && n < max) { + while (tx < cols) { float sc = score[ty * cols + tx]; int q0, run, x; if (sc <= 0.0f) { @@ -369,7 +428,7 @@ static int coalesce_roi(const PelorusAnalyzeVulkanContext *s, break; run++; } - out[n] = (AVRegionOfInterest) { + runs[n] = (AVRegionOfInterest) { .self_size = sizeof(AVRegionOfInterest), .top = ty * PEL_TILE, .bottom = FFMIN((ty + 1) * PEL_TILE, s->vkctx.output_height), @@ -381,6 +440,14 @@ static int coalesce_roi(const PelorusAnalyzeVulkanContext *s, tx += run; } } + + /* Rank by banding strength and keep the strongest `max`. */ + if (n > 1) + qsort(runs, (size_t)n, sizeof(*runs), roi_strength_cmp); + if (n > max) + n = max; + memcpy(out, runs, (size_t)n * sizeof(*out)); + av_free(runs); return n; } diff --git a/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c b/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c index 4a0bf8e..0294cc6 100644 --- a/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c +++ b/ffmpeg-patches/files/vf_pelorus_denoise_vulkan.c @@ -70,6 +70,13 @@ #define PEL_SLICES 16 +/* Upper bound on the MV/conf grid cell count accepted from untrusted side data. + * grid_cols/grid_rows are each uint16 on the wire (up to 65535), so the raw + * product can reach ~4.29e9 — past INT_MAX and a multi-GB allocation. Cap it to a + * value that still covers any realistic frame grid (8K at a small block edge) + * while rejecting absurd dimensions before the cell count is used for sizing. */ +#define PEL_DENOISE_MAX_CELLS (8192u * 8192u) + /* Host-readback accumulator for the meta=1 residual free-ride. Sliced to spread * atomic contention, exactly as vf_pelorus_analyze does; summed host-side. */ typedef struct PelorusDenoiseBuf { @@ -706,7 +713,7 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, int i; FFVulkanContext *vkctx = &s->vkctx; FFVulkanFunctions *vk = &vkctx->vkfn; - FFVkExecContext *exec; + FFVkExecContext *exec = NULL; AVFrame *bound_prev[PEL_DENOISE_MAX_PREV]; AVFrame *bound_next; VkImageView cur_views[AV_NUM_DATA_POINTERS]; @@ -737,16 +744,21 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, s->opts.actual_next = (next != NULL) ? 1 : 0; s->opts.want_meta = want_meta; - if (want_meta) { - RET(ff_vk_get_pooled_buffer(vkctx, &s->stat_buf_pool, &buf, - VK_BUFFER_USAGE_TRANSFER_DST_BIT | - VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, - NULL, sizeof(PelorusDenoiseBuf), - VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | - VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | - VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)); - buf_vk = (FFVkBuffer *)buf->data; - } + /* The shader statically references the binding-5 stat SSBO regardless of + * want_meta (the meta accumulators are gated by a runtime push-const branch, + * not removed from the SPIR-V), so a valid buffer MUST be bound at 5 on every + * dispatch — Vulkan forbids a used-but-unbound descriptor. Allocate it + * unconditionally; the zero-fill + host readback below stay gated on + * want_meta (when meta=0 the SSBO is never written, so its contents are + * don't-care — it only has to be a live, bound descriptor). */ + RET(ff_vk_get_pooled_buffer(vkctx, &s->stat_buf_pool, &buf, + VK_BUFFER_USAGE_TRANSFER_DST_BIT | + VK_BUFFER_USAGE_STORAGE_BUFFER_BIT, + NULL, sizeof(PelorusDenoiseBuf), + VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT | + VK_MEMORY_PROPERTY_HOST_VISIBLE_BIT | + VK_MEMORY_PROPERTY_HOST_COHERENT_BIT)); + buf_vk = (FFVkBuffer *)buf->data; /* --- motion-compensated taps: parse the MV + confidence grids from cur's * Pelorus side data and stage them into host-visible SSBOs. Something must @@ -760,7 +772,9 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, const void *mp = NULL, *cp = NULL; size_t msz = 0, csz = 0; const uint8_t *mv_field = NULL, *conf_field = NULL; - int gc = 0, gr = 0, cells = 0, ncells, j; + int gc = 0, gr = 0; + uint64_t cells = 0, ncells; + uint64_t j; if (sd && pel_blob_find_section(sd->data, sd->size, PEL_SEC_MOTION, @@ -773,11 +787,13 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, (const PelorusSideData *)(sd->data + PELORUS_SIDEDATA_UUID_LEN); gc = bh->grid_cols; gr = bh->grid_rows; - cells = gc * gr; + /* grid dims are untrusted uint16; the product can exceed INT_MAX, so + * compute in 64-bit and bound it before any sizing/loop use. */ + cells = (uint64_t)gc * (uint64_t)gr; /* Validate against the untrusted side-data length before deref. */ - if (gc > 0 && gr > 0 && - mo->mv_field_size == (uint32_t)cells * 4 && - mcs->conf_field_size == (uint32_t)cells && + if (gc > 0 && gr > 0 && cells <= PEL_DENOISE_MAX_CELLS && + mo->mv_field_size == cells * 4 && + mcs->conf_field_size == cells && (size_t)PELORUS_SIDEDATA_UUID_LEN + mo->mv_field_offset + mo->mv_field_size <= sd->size && (size_t)PELORUS_SIDEDATA_UUID_LEN + mcs->conf_field_offset + @@ -862,6 +878,11 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, ff_vk_shader_update_img_array(vkctx, exec, &s->shd, bound_prev[i], prev_views[i], 0, 1 + i, VK_IMAGE_LAYOUT_GENERAL, VK_NULL_HANDLE); + /* Bind the stat SSBO at 5 on EVERY dispatch (the shader statically uses it); + * the zero-fill + readback stay gated on want_meta below. */ + RET(ff_vk_shader_update_desc_buffer(vkctx, exec, &s->shd, 0, 5, 0, + buf_vk, 0, buf_vk->size, + VK_FORMAT_UNDEFINED)); RET(ff_vk_shader_update_desc_buffer(vkctx, exec, &s->shd, 0, 6, 0, mvbuf_vk, 0, mvbuf_vk->size, VK_FORMAT_UNDEFINED)); @@ -941,10 +962,6 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, }, .bufferMemoryBarrierCount = 1, }); - - RET(ff_vk_shader_update_desc_buffer(vkctx, exec, &s->shd, 0, 5, 0, - buf_vk, 0, buf_vk->size, - VK_FORMAT_UNDEFINED)); } else { vk->CmdPipelineBarrier2(exec->buf, &(VkDependencyInfo) { .sType = VK_STRUCTURE_TYPE_DEPENDENCY_INFO, @@ -990,7 +1007,11 @@ static int denoise_dispatch(PelorusDenoiseVulkanContext *s, AVFrame *out, return 0; } - /* meta=0: no readback, no sync point — just submit and return. */ + /* meta=0: no readback, no sync point — just submit and return. The stat + * buffer bound at 5 is never read back, but it must outlive the async submit + * (same treatment as the mv/conf buffers above). */ + RET(ff_vk_exec_add_dep_buf(vkctx, exec, &buf, 1, 0)); + buf = NULL; RET(ff_vk_exec_submit(vkctx, exec)); return 0; diff --git a/ffmpeg-patches/files/vf_pelorus_mc_vulkan.c b/ffmpeg-patches/files/vf_pelorus_mc_vulkan.c index 640f6ec..9de6748 100644 --- a/ffmpeg-patches/files/vf_pelorus_mc_vulkan.c +++ b/ffmpeg-patches/files/vf_pelorus_mc_vulkan.c @@ -60,6 +60,7 @@ #include "filters.h" #include +#include #include #include @@ -87,8 +88,11 @@ * ref_image[0]. */ static const char mc_glsl[] = "const float SAD_SCALE = 256.0;\n" - "shared float s_part[128];\n" /* per-subgroup SAD partials (wg / min-subgroup)*/ - "shared float s_sad0;\n" /* block-SAD reduction result */ + /* One slot per invocation (worst case subgroupSize==1 => BLOCK_DIM*BLOCK_DIM + * subgroups). gl_SubgroupID indexes this, so a smaller size OOB-writes. Sized + * from the lane-count macro so a BLOCK_DIM change propagates. */ + "shared float s_part[" PEL_MC_SAD_LANES_STR "];\n" + "shared float s_sad0;\n" /* block-SAD reduction result */ "shared int s_best_x;\n" "shared int s_best_y;\n" "shared float s_best_cost;\n" @@ -545,7 +549,12 @@ static int attach_motion(PelorusMcVulkanContext *s, AVFrame *frame, size_t mv_bytes = (size_t)nblocks * 2 * sizeof(int16_t); size_t conf_bytes = (size_t)nblocks; size_t uuid = PELORUS_SIDEDATA_UUID_LEN; - uint8_t *grown = av_realloc(blob, len + mv_bytes + conf_bytes); + /* The blob is libc-calloc'd by pel_blob_pack and freed by + * pel_blob_free()->free(); grow it with libc realloc so all three stay on + * one allocator (av_realloc may route to _aligned_realloc on aligned-CRT + * builds, mismatching the libc free). On NULL the original blob is + * untouched, so the error path's pel_blob_free(blob) still frees it. */ + uint8_t *grown = realloc(blob, len + mv_bytes + conf_bytes); PelorusSideData *hdr; PelorusSectionDir *dir; PelorusMotionSection *mo_in_blob = NULL; diff --git a/libpelorus/shaders/pelorus_mc.comp b/libpelorus/shaders/pelorus_mc.comp index beadb27..f94b337 100644 --- a/libpelorus/shaders/pelorus_mc.comp +++ b/libpelorus/shaders/pelorus_mc.comp @@ -91,8 +91,10 @@ layout(push_constant, std430) uniform Params { const float SAD_SCALE = 256.0; /* SAD is summed in normalized [0,1] luma */ -/* SAD reduction scratch: one slot per invocation, tree-reduced by thread 0. */ -shared float s_part[128]; /* per-subgroup SAD partials (wg / min-subgroup) */ +/* SAD reduction scratch: one slot per invocation, tree-reduced by thread 0. + * Sized to the worst case (subgroupSize==1 => BLOCK_DIM*BLOCK_DIM subgroups); + * gl_SubgroupID indexes this array, so a smaller size would OOB-write. */ +shared float s_part[BLOCK_DIM * BLOCK_DIM]; /* per-subgroup SAD partials */ shared float s_sad0; /* block-SAD reduction result */ /* The block's running best (written by thread 0, read by all each step). */ shared int s_best_x; diff --git a/libpelorus/src/interop.c b/libpelorus/src/interop.c index 034a869..0ed1066 100644 --- a/libpelorus/src/interop.c +++ b/libpelorus/src/interop.c @@ -138,13 +138,20 @@ pel_result pel_blob_pack(const PelorusSideData *meta, const PelorusPackSection * *out_blob = NULL; *out_len = 0; - /* total bytes of all (aligned) section payloads */ + /* total bytes of all (aligned) section payloads. Accumulate in 64-bit so the + * per-section 8-byte alignment and the aggregate sum cannot wrap the uint32 + * wire field (PEL_ALIGN8 is 32-bit; a near-UINT32_MAX section would wrap to a + * tiny payload -> undersized alloc + heap overflow on the memcpy below). */ { - uint32_t payload = 0; + uint64_t need = cursor; for (i = 0; i < nb; i++) { - payload += PEL_ALIGN8(sections[i].size); + uint64_t aligned = ((uint64_t)sections[i].size + 7u) & ~(uint64_t)7u; + need += aligned; } - total_size = cursor + payload; + if (need > UINT32_MAX) { /* total_size is a uint32_t wire field */ + return PEL_ERR_RANGE; + } + total_size = (uint32_t)need; } blob_len = (size_t)PELORUS_SIDEDATA_UUID_LEN + (size_t)total_size; @@ -255,6 +262,13 @@ pel_result pel_blob_find_section(const uint8_t *blob, size_t len, enum pel_secti } off = dir[i].offset; sz = dir[i].size; + /* R5: the packer 8-aligns every section payload so a consumer can cast the + * returned pointer to the section struct (which may hold a u64) without an + * unaligned access. A misaligned offset is corrupt framing, not a short + * buffer — reject it before handing out a castable pointer. */ + if ((off & 7u) != 0u) { + return PEL_ERR_ABI; + } if (off > image_len || sz > image_len - off) { return PEL_ERR_TRUNCATED; } diff --git a/libpelorus/test/interop_test.c b/libpelorus/test/interop_test.c index 59630ba..5760209 100644 --- a/libpelorus/test/interop_test.c +++ b/libpelorus/test/interop_test.c @@ -246,6 +246,68 @@ static void test_truncation(void) pel_blob_free(blob); } +/* Defensive parser (R5): a crafted/corrupt blob whose section payload offset is + * NOT 8-byte aligned must be rejected, not cast to a struct at an unaligned + * address (the packer always 8-aligns, so this only arises from foreign/corrupt + * framing). The current suite never patches dir[i].offset, so exercise it here. */ +static void test_misaligned_offset(void) +{ + PelorusSideData meta; + PelorusFilmGrainSection grain; /* has a u64 at offset 0 -> alignment matters */ + PelorusPackSection sec; + uint8_t *blob = NULL; + size_t len = 0; + const void *p = NULL; + size_t got = 0; + PelorusSideData *hdr; + PelorusSectionDir *dir; + + fill_meta(&meta); + memset(&grain, 0, sizeof(grain)); + grain.seed = 0xDEADBEEFCAFEULL; + sec.id = PEL_SEC_FILMGRAIN; + sec.data = &grain; + sec.size = (uint32_t)sizeof(grain); + CHECK(pel_blob_pack(&meta, &sec, 1, &blob, &len) == PEL_OK); + + /* Well-formed first: the 8-aligned offset parses. */ + CHECK(pel_blob_find_section(blob, len, PEL_SEC_FILMGRAIN, sizeof(PelorusFilmGrainSection), &p, + &got) == PEL_OK); + + /* Now hand-patch dir[0].offset to a misaligned value (+4). The section then + * still fits the buffer but its start is no longer 8-aligned. */ + hdr = (PelorusSideData *)(void *)(blob + PELORUS_SIDEDATA_UUID_LEN); + dir = (PelorusSectionDir *)(void *)(blob + PELORUS_SIDEDATA_UUID_LEN + hdr->header_size); + dir[0].offset += 4u; + + CHECK(pel_blob_find_section(blob, len, PEL_SEC_FILMGRAIN, sizeof(PelorusFilmGrainSection), &p, + &got) == PEL_ERR_ABI); + pel_blob_free(blob); +} + +/* Defensive packer (R5 overflow guard): a section whose declared size, once + * 8-aligned and summed, would overflow the uint32 total_size wire field must be + * rejected before allocating (otherwise the alloc undersizes and the copy + * overflows the heap). The size check runs before any deref of sec.data, so a + * tiny dummy data pointer with a huge declared size is safe to pass here. */ +static void test_pack_size_overflow(void) +{ + PelorusSideData meta; + PelorusPackSection sec; + uint8_t dummy = 0; + uint8_t *blob = NULL; + size_t len = 0; + + fill_meta(&meta); + /* size near UINT32_MAX: 8-aligning it wraps a 32-bit accumulator to ~0 but the + * 64-bit guard catches need > UINT32_MAX and rejects. data is never read. */ + sec.id = PEL_SEC_BANDING; + sec.data = &dummy; + sec.size = 0xFFFFFFF9u; + CHECK(pel_blob_pack(&meta, &sec, 1, &blob, &len) == PEL_ERR_RANGE); + CHECK(blob == NULL); +} + /* PEL_SEC_QPREPORT (f): pack the encoder-honored QP readback with a per-cell * QP map appended after the blob, parse it back, verify scalars + the map. */ static void test_qp_report_roundtrip(void) @@ -591,6 +653,8 @@ int main(void) test_foreign_buffer(); test_header_only(); test_truncation(); + test_misaligned_offset(); + test_pack_size_overflow(); test_qp_report_roundtrip(); test_motion_conf_roundtrip(); test_complexity_roundtrip();