Repository navigation
fix: 5 Fable deep-hunt findings — integer_ssim AVX2 16-bit overflow, bpc validation, set_fex UAF, vmafx-server DoS cap, CUDA prev_ref - #866
Merged
Conversation
The 16-bit accumulation loop in integer_ssim_accumulate_row_16_avx2() computed squared moments as _mm256_mul_epi32(wv, _mm256_mul_epi32(sv, sv)). For true 16-bit content (pixel p >= 46341), s*s >= 2^31, setting bit 31 of the low 32-bit lane. _mm256_mul_epi32 reads the low 32 bits as a signed quantity, so sv_sq appears negative and the outer multiply with wv produces a corrupted (wrong-sign) 64-bit product for x2, xy, and y2. The 8-bit and 10-bit paths were unaffected because max s*s = 65535^2/1023^2 stays below 2^31 only for bpc <= 15. Netflix golden tests use 8/10-bit content, so the gate never caught it. Fix: reorder to (w*s)*s. w*s <= 256*65535 = 16,776,960 < 2^24 — bit 31 always clear — so both _mm256_mul_epi32 operands are safely non-negative. Same reorder applied to w*d and the xy cross-term. Adds a regression test (test_integer_ssim_avx2_16bpc_bright) feeding alternating 65535/65534 pixels — the exact values that trigger the overflow — and asserting bit-exact equality between AVX2 and scalar. There is no AVX-512 twin for integer_ssim (only integer_ssim_avx2.c exists in core/src/feature/x86/), so no further file requires the fix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…_params The bpc consistency check in validate_pic_params() used && where all sibling checks (w, h, pix_fmt) use ||: WRONG: if ((ref->bpc != dist->bpc) && (ref->bpc != vmaf->pic_params.bpc)) FIXED: if ((ref->bpc != dist->bpc) || (ref->bpc != vmaf->pic_params.bpc)) On frame 0, pic_params.bpc is assigned from ref->bpc immediately before the guard, making the second term always false. The && short-circuits to false for any first-frame submission, so a ref=8bpc / dist=10bpc pair is silently accepted. The dist buffer is then read at the wrong stride/element size, producing garbage output or an OOB access. Fix: change && to || so any bpc mismatch (ref≠dist or ref≠pic_params) is rejected with -EINVAL, consistent with all other dimension/format checks. Inherited from upstream Netflix/vmaf but a genuine logic bug. Reported as Fable finding libvmaf-bpc-validation-and-or. Adds regression test test_validate_pic_params_bpc (fast suite) that covers: matched bpc accepted, 8bpc/10bpc mismatch rejected on frame 0, reversed mismatch rejected, consistent 10bpc across frames accepted, and bpc change mid-stream rejected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…nt DoS Without a cap, N concurrent unauthenticated POST /v1/score or gRPC Score calls fork N vmaf subprocesses simultaneously, exhausting CPU/RAM/PIDs. Add ScoreLimiter (golang.org/x/sync/semaphore.Weighted) shared across both the HTTP and gRPC entry points so the cap is enforced system-wide. Excess callers receive HTTP 429 (TooManyRequests) or gRPC codes.ResourceExhausted immediately when their context is already cancelled; waiting callers are unblocked in FIFO order as slots free up. A new --max-concurrent-scores flag (default: runtime.NumCPU()) controls the cap size. Existing constructors (newGRPCServer / newHTTPServer) are preserved unchanged for test compatibility; production startup uses the new WithLimiter variants. Fixes Fable finding vmafx-server-concurrency-cap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Two bugs in the CUDA read-pictures path in core/src/libvmaf.c: 1. read_pictures_extractor_loop_cuda (Phase 2 submit loop): prev_ref was assigned via a bare struct copy (`fex_ctx->fex->prev_ref = vmaf->prev_ref`) instead of vmaf_picture_ref(). Without a counted reference, the picture pool can reuse the buffer after read_pictures_update_prev_ref decrements vmaf->prev_ref on the next frame, while the CUDA extractor is still reading from it. Latent today (no CUDA extractor carries VMAF_FEATURE_EXTRACTOR_PREV_REF), but becomes a live UAF+leak the moment one is added. Fix: vmaf_picture_ref in Phase 2; unref+zero on error; unref+zero after collect in Phase 1 (ADR-0778 Fix-B, mirroring the sync path in read_pictures_dispatch_one). 2. read_pictures_cuda_translate: the dist-side translate_picture call was wrapped in (void), swallowing any error. If translate fails, dist_device may be partially populated and subsequently passed to CUDA extractors. Fix: capture the return value and propagate as the sync path does for ref. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…r Fable-5 bundle Four recently-closed rows added to docs/state.md: T-INTEGER-SSIM-AVX2-16BIT-OVERFLOW-2026-06-12 T-BPC-VALIDATION-AND-OR-2026-06-12 T-VMAFX-SERVER-CONCURRENCY-CAP-2026-06-12 T-CUDA-PREV-REF-UAF-DIST-TRANSLATE-2026-06-12 changelog.d/fixed/fable5-bundle-integer-ssim-bpc-server-cuda.md added. docs/rebase-notes.md entry added for chore/bundle-fable-5-findings. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
4 tasks done
9 of 10 tasks
11 of 27 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Five Fable deep-hunt findings fixed in one bundle. Commit 3 (set_fex UAF) is already on master (PR #859); the remaining four are cherry-picked from their fix branches.
Fixes
integer_ssim_avx2.c: reorderw*(s*s)to(w*s)*s; for pixels ≥ 46341 the old order set bit 31, corrupting SSIM accumulators. Netflix golden gate uses 8/10-bit and never detected this.libvmaf.c validate_pic_params:&&→||; with&&the bpc guard always short-circuited false on frame 0, silently accepting 8bpc/10bpc mismatched pairs.cmd/vmafx-server/:ScoreLimitersemaphore shared across HTTP and gRPC; default cap =runtime.NumCPU(); excess callers get 429 /ResourceExhausted;--max-concurrent-scoresflag.libvmaf.cCUDA path: Phase 2 PREV_REF now usesvmaf_picture_ref; dist translate error is no longer(void)-discarded.Most dangerous: fix #1 (integer_ssim AVX2 16-bit) — produces silently wrong SSIM scores on any 16-bit content (BT.2020, HDR), undetectable by the Netflix golden gate.
Deliverables checklist
integer_ssim_avx2.c,libvmaf.c,cmd/vmafx-server/(5 files), and new test files.meson test -C build-fable --suite=fast88/88 pass;go test ./cmd/vmafx-server/... ./pkg/libvmaf/...pass;test_integer_ssim_simd5/5;test_validate_pic_params_bpc5/5.changelog.d/fixed/fable5-bundle-integer-ssim-bpc-server-cuda.md(4 entries).chore/bundle-fable-5-findingsadded.