PR #24800 opened by Jake (jakefineman)
URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24800
Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24800.patch
**Follow-up, not depending on any open PR.** This is one commit on unmodified
upstream master (09f879b074, "swresample: keep integer samples if channels are
only copied"), branch `dnn-probe-task-zeroinit`. It answers Forgejo_Fairy's
non-blocking follow-up in review 8971 on #24761 ("SPACE-BUNNY-ALPHA — Minor for
this review, pre-existing memory-safety follow-up: initialize the TensorFlow
and OpenVINO probe tasks"). Fairy's review states the defect predates #24761's
series and confirmed it is already present at ancestor 6246f7a4cde6, which is
why this fix targets master directly rather than #24761 or #24702.
**Problem.** `get_output_tf()` in `libavfilter/dnn/dnn_backend_tf.c` and
`get_output_ov()` in `libavfilter/dnn/dnn_backend_openvino.c` both declare
`TaskItem task;` with no initializer, then run:
ret = ff_dnn_fill_gettingoutput_task(&task, &exec_params, ...);
if (ret != 0) {
goto err;
}
...
err:
av_frame_free(&task.out_frame);
av_frame_free(&task.in_frame);
`ff_dnn_fill_gettingoutput_task()` in `libavfilter/dnn/dnn_backend_common.c`
allocates the input frame first:
in_frame = av_frame_alloc();
if (!in_frame) {
av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for input
frame\n");
return AVERROR(ENOMEM);
}
out_frame = av_frame_alloc();
if (!out_frame) {
av_frame_free(&in_frame);
av_log(ctx, AV_LOG_ERROR, "Failed to allocate memory for output
frame\n");
return AVERROR(ENOMEM);
}
in_frame->width = input_width;
in_frame->height = input_height;
exec_params->in_frame = in_frame;
exec_params->out_frame = out_frame;
return ff_dnn_fill_task(task, exec_params, backend_model, 0, 0);
Both `task->in_frame` and `task->out_frame` are set only in the final
`ff_dnn_fill_task()` call, by way of `exec_params`. If the first
`av_frame_alloc()` fails, the function returns `AVERROR(ENOMEM)` immediately,
before `task` is touched at all. Back in the probe, that failure takes the
`goto err` branch, and `av_frame_free()` is then called on `task.out_frame` and
`task.in_frame`, which still hold whatever bytes were on the stack when `task`
was declared. `av_frame_free()` unconditionally dereferences a non-NULL
pointer's fields (`av_frame_unref()` walks `side_data`, etc.), so this is a
read (and potential double-free/free-of-garbage) through an uninitialized
pointer, not a crash that depends on any specific stack content: in practice it
can free arbitrary memory or, more commonly, immediately trip on whatever
garbage looks like a non-NULL pointer.
**Fix.** Exactly what Fairy proposed and verified by source review:
zero-initialize the declaration in each probe, so cleanup receives NULL
pointers (which `av_frame_free()` treats as a no-op) until the task genuinely
owns frames:
- TaskItem task;
+ TaskItem task = { 0 };
This matches the two other backends' probes, which already do this and are not
affected: `get_output_onnx()` in `dnn_backend_onnx.c` (`TaskItem task = { 0
};`) and `get_output_th()` in `dnn_backend_torch.cpp` (same). I checked both
before writing this fix; the OpenVINO and TensorFlow probes were the only two
with the bare declaration in the whole `libavfilter/dnn` tree.
**Base.** Built directly on upstream master because Fairy's citation (ancestor
6246f7a4cde6) and the current master tree are identical in the three functions
this touches: `git diff 6246f7a4cde6 master --
libavfilter/dnn/dnn_backend_common.c libavfilter/dnn/dnn_backend_tf.c
libavfilter/dnn/dnn_backend_openvino.c` shows only unrelated hunks (the
pthread-cancel/`thread_started` guard work in `dnn_backend_common.c` and a dead
`#if !HAVE_PTHREAD_CANCEL` block removed from `dnn_backend_tf.c`);
`get_output_tf()`, `get_output_ov()` and `ff_dnn_fill_gettingoutput_task()` are
byte-for-byte unchanged. Neither #24702 nor #24761 is a prerequisite for this
fix, and this branch does not carry any of their commits.
**Testing.** Darwin 27.0.0 arm64 (Apple M1 Ultra). OpenVINO 2026.4.0 and
TensorFlow 2.18.0 (C API) enabled in one configure, against the test clip and
models listed in `MODELS.txt` in the evidence kit attached to this PR (589
frames at 1280x720, 29.97fps, sampled at 2fps). ONNX Runtime was enabled at
configure time so all four backends build, but the fix does not touch the ONNX
or Torch probes (neither is exercised here beyond compiling). Two source trees:
"before" = master 09f879b074 unmodified, "after" = an identically-treed
pre-sign commit of this fix (tree `c184e2b073368f07e16b9a4edd99768dbea8779a`,
the same tree this commit, `4be38f85d056d9935899dd491a389d2e0d065c8a`, carries
-- signing off doesn't change a tree). Each tree was built twice, once with
`--toolchain=clang-asan` and once without, both with identical
`--enable-libopenvino --enable-libonnxruntime --enable-libtensorflow` configure
flags. `build.sh` in the kit is the exact build script used, with the local
paths replaced by environment variables.
Fault injection. `ff_dnn_fill_gettingoutput_task()`'s first `av_frame_alloc()`
call was made to fail on demand via a one-line, env-gated, uncommitted
throwaway edit in the ASan build trees only (`in_frame =
getenv("FF_DNN_TEST_FAIL_FIRST_ALLOC") ? NULL : av_frame_alloc();`, plus
`#include <stdlib.h>`); it is a no-op unless that variable is set, so it does
not change any other result in this report and was not part of the commit
tested by the other builds. The exact edit is `fault-injection.patch` in the
kit, verified to apply cleanly to `libavfilter/dnn/dnn_backend_common.c` at
09f879b074. With the variable set, `FF_DNN_TEST_FAIL_FIRST_ALLOC=1 ffmpeg -i
puppies.ogv -vf
format=rgb24,dnn_processing=dnn_backend=<b>:model=<m>:input=<i>:output=<o> -f
null -` was run under ASan for `<b>` = openvino (`proc_valid_720.onnx`) and
tensorflow (`tf_rgb_valid.pb`), one job at a time.
| build | backend | ASan result | log |
|---|---|---|---|
| before | openvino | `heap-use-after-free` inside `av_frame_side_data_free` <-
`av_frame_unref` <- `av_frame_free`, called from `avfilter_graph_config` (the
probe's `err:` cleanup); process aborts | [ATTACH:before-ov-crashtest.log] |
| before | tensorflow | `AddressSanitizer:DEADLYSIGNAL` (BUS, wild write)
inside `av_frame_side_data_free`, same call path; process aborts |
[ATTACH:before-tf-crashtest.log] |
| after | openvino | clean error cascade: `Failed to allocate memory for input
frame` -> `could not get output from the model` -> `Failed to configure output
pad` -> `Task finished with error: Cannot allocate memory` -> `Conversion
failed!`; ffmpeg exits through its normal error path with ENOMEM, no
use-after-free or wild write in the probe | [ATTACH:after-ov-crashtest.log] |
| after | tensorflow | identical clean ENOMEM cascade |
[ATTACH:after-tf-crashtest.log] |
In both "after" runs, ASan does still report a `SEGV` — but only after the
"Conversion failed!" line, at process `exit()`, inside
`tbb::detail::r1::__TBB_InitOnce::~__TBB_InitOnce()` (OpenVINO's bundled TBB,
`libtbb.12.19.dylib`). This is a pre-existing, unrelated teardown artifact of
building `--enable-libopenvino` under ASan: it reproduces identically, with the
same stack, in a fully successful, non-fault-injected "before" run that decodes
and processes all 589 frames of the TensorFlow whole-clip test and writes
correct output before exiting ([ATTACH:before-tf-normal.log]). It fires on
process exit regardless of the DNN backend actually used in the run (it shows
up on the TensorFlow-model run because OpenVINO is linked into the same
`ffmpeg` binary), and it is present in both the before and after trees, so it
is not a regression from, or a confound for, this fix.
Normal whole-clip runs. Both builds, ASan and non-ASan, ran the unmodified
589-frame clip through `dnn_processing` with no fault injection, `-vf
fps=2,format=rgb24,dnn_processing=dnn_backend=<b>:model=<m>:input=<i>:output=<o>
-f framemd5`. TensorFlow (`tf_rgb_valid.pb`): before and after MD5-of-framemd5
are both `21fe0c97db5ec71c94ba4b741d7a7894`, byte-identical, 589 frames
([ATTACH:before-tf.md5], [ATTACH:after-tf.md5]), no ASan reports other than the
unrelated exit-time TBB SEGV noted above (which does not affect the
already-written output; see [ATTACH:before-ov-normal.log] /
[ATTACH:after-ov-normal.log]). OpenVINO with `proc_valid_720.onnx` hit a
pre-existing, unrelated ASan-only finding — a `stack-buffer-overflow` inside
`ff_proc_from_dnn_to_frame`'s `sws_scale()` call — identically in both the
before and after ASan builds (same address pattern, same frame, present with
the same model regardless of this fix; see the same two normal-run logs), so
the release (non-ASan) builds (`--toolchain` unset, otherwise identical
configure flags) were used for the OpenVINO whole-clip comparison instead.
(That overflow is a separate, pre-existing bug, root-caused to
`dnn_io_proc.c`'s `dst_data = &middle_data;` aliasing a single pointer as a
4-element array for `sws_scale()`; it is not fixed in this commit and is being
tracked separately.) Both release runs are clean, `rc=0`, no sanitizer
involved: before and after MD5-of-framemd5 are both
`21fe0c97db5ec71c94ba4b741d7a7894`, byte-identical, 589 frames
([ATTACH:before-ov-rel.md5], [ATTACH:after-ov-rel.md5]) — the same hash as the
TensorFlow run above, which is expected since
`proc_valid_720.onnx`/`tf_rgb_valid.pb` are matched identity-transform models
(their output equals their input).
Models: `proc_valid_720.onnx` sha256
`1795ae6d41d123b77c18d471ab24818fd395e5726d4aaf3349e0fd54e75c22b7`,
`tf_rgb_valid.pb` sha256
`2d16458a416af730a30df7f23781cb6af547455f65bfe02ed3b6307cc66d1169`; both are
deterministically generated by scripts included in the kit
(`gen-identity-models.py`, `gen-identity-tf-model.py`), not downloaded -- see
`MODELS.txt`.
`proc_valid_720.onnx` and `tf_rgb_valid.pb` are identity-transform models
(their output equals their input), so the matching before/after hash on its own
only shows that a degenerate pixel-in/pixel-out path was unaffected. To confirm
the fix doesn't change real output, the same before/after release builds were
also run against two real, non-identity object-detection models through the
OpenVINO backend (whose probe, `get_output_ov()`, is the one this commit
changes): D-FINE-S and RF-DETR-S (both COCO object detectors), wired the same
way `dnn_detect` is exercised in PR #24702's evidence: `-threads 1 -vf
fps=2,dnn_detect=dnn_backend=openvino:model=<m>:model_type=ssd:input=data:output=detection_out:confidence=0.5:labels=<labels>,showinfo
-f null -`, with the `showinfo` lines (frame checksums plus every detection
box, label and confidence) hashed. Both produced real, varied detections
(people, a bench, a chair, confidences from ~0.5 to ~0.93) across the 589-frame
clip, not a constant or empty result:
| model | before MD5 (showinfo) | after MD5 (showinfo) | lines | logs |
|---|---|---|---|---|
| D-FINE-S (`dfine_s_obj2coco_ffmpeg_bgr.onnx`) |
`770c6e366825ceed08b005c7cb2eda0d` | `770c6e366825ceed08b005c7cb2eda0d` | 6670
| [ATTACH:dfine-before.info.gz], [ATTACH:dfine-after.info.gz] |
| RF-DETR-S (`rfdetr_s_ffmpeg_bgr.onnx`) | `7aa5b96828128cf0c0a88666b668b0a0` |
`7aa5b96828128cf0c0a88666b668b0a0` | 4819 | [ATTACH:rfdetr-before.info.gz],
[ATTACH:rfdetr-after.info.gz] |
Both are byte-identical before/after. Both also differ from the no-filter
baseline (`fps=2,showinfo` alone over the same clip, MD5
`28ff75129334b5c317a13eb165aa5d98`, 1180 lines, [ATTACH:nofilter.info.gz]),
confirming the detection filters are actually doing something and the identical
hash isn't a degenerate pass-through artifact. (These two hashes include the
model's file path as printed by ffmpeg's own `showinfo` detection side-data
output, so they will only reproduce exactly for a run using the same path
layout as the kit's `run-real-models.sh`; what should reproduce on any machine
is before == after, and both differing from the no-filter baseline.)
No real, non-synthetic TensorFlow detection model was available for this check,
so a real, non-synthetic TensorFlow model was sourced for
`dnn_processing`/`derain` instead: `derain_RESCAN.pb`, a pretrained CAN
("Recurrent Squeeze-and-Excitation Context Aggregation Net for Single Image
Deraining") network -- the exact model `doc/filters.texi`'s `derain` section
and its `dnn_processing`/`can.pb` example point to. Source, revision and sha256
are in `MODELS.txt`. Inspecting the frozen `GraphDef` confirms the input/output
node names the docs assume: a single `Placeholder` named `x` (`[1,-1,-1,3]`
float, NHWC RGB) and a final op named `y`, 278 nodes total (i.e. an actual
trained network, not a one-node stub).
Run the same way as the identity TF case -- `-threads 1 -vf
fps=2,format=rgb24,dnn_processing=dnn_backend=tensorflow:model=derain_RESCAN.pb:input=x:output=y
-f framemd5`, MD5-of-framemd5, both TF-enabled release builds:
| model | before MD5 (framemd5) | after MD5 (framemd5) | frames | logs |
|---|---|---|---|---|
| CAN derain (`derain_RESCAN.pb`) | `2e76557a27bda5a27bb8d5e7104d49f2` |
`2e76557a27bda5a27bb8d5e7104d49f2` | 589 | [ATTACH:derain-before.md5],
[ATTACH:derain-after.md5] |
Byte-identical before/after, and different from the no-filter baseline over the
same clip and pixel format (`fps=2,format=rgb24` alone, MD5
`21fe0c97db5ec71c94ba4b741d7a7894`, 589 frames, [ATTACH:nofilter-rgb24.md5]) --
which is, reassuringly, the exact same hash reported above for the
`tf_rgb_valid.pb` identity model, confirming that one really was a no-op and
this one is a real transform. This exercises `get_output_tf()`, the function
this commit changes, through a real non-identity TensorFlow inference path,
closing the TF side of the real-model gap for `dnn_processing`.
Not tested: ONNX Runtime and Torch probes (already zero-initialized, not
touched by this commit, so out of scope); Linux; `sr`/`dnn_classify` filters
(the bug is in the two probes' shared helper path, reached identically from
every DNN filter's `config_props`, so this is expected to generalize, but only
`dnn_processing` and `dnn_detect` were run); a real, non-synthetic TensorFlow
*detection* model specifically (the real-model TF check above uses
`derain`/`dnn_processing`, not `dnn_detect`; no real TF SSD-family frozen graph
was sourced for this commit).
**Reproduction.** An evidence kit is attached to this PR with everything needed
to rerun all of the above end to end:
- `fault-injection.patch` -- [ATTACH:fault-injection.patch] -- the throwaway
env-gated edit to `dnn_backend_common.c`, verified to apply cleanly to
09f879b074.
- `build.sh` -- [ATTACH:build.sh] -- configures and builds either tree
(`./build.sh before|after clang-asan` or `./build.sh before|after ""` for a
release build), with OpenVINO/ONNX Runtime/TensorFlow install paths taken from
environment variables.
- `run-evidence.sh` -- [ATTACH:run-evidence.sh] -- runs the fault-injection
crash tests and the identity-model whole-clip comparisons against a built tree.
- `run-real-models.sh` -- [ATTACH:run-real-models.sh] -- runs the
D-FINE-S/RF-DETR-S/CAN-derain real-model comparisons and the no-filter
baselines.
- `MODELS.txt` -- [ATTACH:MODELS.txt] -- every model and the test clip, with
public URL, pinned revision and sha256, plus the exact wrap/generation commands.
- `gen-identity-models.py` -- [ATTACH:gen-identity-models.py] -- generates
`proc_valid_720.onnx` and, with `--bgr`, the OpenVINO BGR-input variants of the
real detection models.
- `gen-identity-tf-model.py` -- [ATTACH:gen-identity-tf-model.py] -- generates
`tf_rgb_valid.pb`.
- `wrap_models.py` -- [ATTACH:wrap_models.py] -- wraps the upstream
D-FINE/RF-DETR ONNX exports into FFmpeg's `dnn_detect` SSD output layout and
generates their label files.
- The raw logs and hash files behind every number above, under `logs/` and
`results/` in the kit (linked inline next to each claim).
To reproduce the core fix: `git clone` this branch on top of `09f879b074`,
apply `fault-injection.patch` to a scratch copy of `dnn_backend_common.c`,
configure with `--enable-libopenvino --enable-libtensorflow` (and any sanitizer
toolchain) via `build.sh`, then run `run-evidence.sh` against both the "before"
(unmodified master) and "after" (this commit) trees. Before this commit, ASan
reports a use-after-free or wild write from the probe's cleanup; after it,
ffmpeg reports a clean `AVERROR(ENOMEM)` and exits normally. Run
`run-real-models.sh` against both trees' release builds to reproduce the
real-model comparisons.
From 4be38f85d056d9935899dd491a389d2e0d065c8a Mon Sep 17 00:00:00 2001
From: Jake Fineman <[email protected]>
Date: Tue, 29 Sep 2026 22:05:26 -0400
Subject: [PATCH] avfilter/dnn: zero-initialize the output-probe task
get_output_tf() (dnn_backend_tf.c) and get_output_ov()
(dnn_backend_openvino.c) both declare `TaskItem task;` uninitialized
and then unconditionally free task.in_frame/task.out_frame on their
error path:
ret = ff_dnn_fill_gettingoutput_task(&task, ...);
if (ret != 0)
goto err;
...
err:
av_frame_free(&task.out_frame);
av_frame_free(&task.in_frame);
ff_dnn_fill_gettingoutput_task() (dnn_backend_common.c) allocates
in_frame first and, only if that succeeds, allocates out_frame and
assigns both pointers into exec_params (and from there into the
task via ff_dnn_fill_task()). If the first av_frame_alloc() fails,
the function returns AVERROR(ENOMEM) immediately, before task.in_frame
or task.out_frame is ever set. The two probes then reach `err:` with
task's frame fields holding whatever garbage was on the stack, and
av_frame_free() dereferences those pointers.
Fix by zero-initializing the declaration, matching get_output_onnx()
(dnn_backend_onnx.c) and get_output_th() (dnn_backend_torch.cpp),
which already use `TaskItem task = { 0 };` and are not affected.
Reported by Forgejo_Fairy, review 8971 on #24761, which also
confirmed the defect predates that series and is already present at
6246f7a4cde6.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
Signed-off-by: Jake Fineman <[email protected]>
---
libavfilter/dnn/dnn_backend_openvino.c | 2 +-
libavfilter/dnn/dnn_backend_tf.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/libavfilter/dnn/dnn_backend_openvino.c
b/libavfilter/dnn/dnn_backend_openvino.c
index 0df4d585e8..08c4faef8f 100644
--- a/libavfilter/dnn/dnn_backend_openvino.c
+++ b/libavfilter/dnn/dnn_backend_openvino.c
@@ -1273,7 +1273,7 @@ static int get_output_ov(DNNModel *model, const char
*input_name, int input_widt
int ret;
OVModel *ov_model = (OVModel *)model;
DnnContext *ctx = ov_model->ctx;
- TaskItem task;
+ TaskItem task = { 0 };
OVRequestItem *request;
DNNExecBaseParams exec_params = {
.input_name = input_name,
diff --git a/libavfilter/dnn/dnn_backend_tf.c b/libavfilter/dnn/dnn_backend_tf.c
index 6c72783f77..110ae2c509 100644
--- a/libavfilter/dnn/dnn_backend_tf.c
+++ b/libavfilter/dnn/dnn_backend_tf.c
@@ -315,7 +315,7 @@ static int get_output_tf(DNNModel *model, const char
*input_name, int input_widt
int ret;
TFModel *tf_model = (TFModel *)model;
DnnContext *ctx = tf_model->ctx;
- TaskItem task;
+ TaskItem task = { 0 };
TFRequestItem *request;
DNNExecBaseParams exec_params = {
.input_name = input_name,
--
2.52.0
_______________________________________________
ffmpeg-devel mailing list -- [email protected]
To unsubscribe send an email to [email protected]
