Skip to content

whisper : default-initialize whisper_mel to avoid uninitialized read - #3981

Merged
danbev merged 1 commit into
ggml-org:masterfrom
ousamabenyounes:fix/issue-3978
Aug 31, 2026
Merged

whisper : default-initialize whisper_mel to avoid uninitialized read#3981
danbev merged 1 commit into
ggml-org:masterfrom
ousamabenyounes:fix/issue-3978

Conversation

@ousamabenyounes

Copy link
Copy Markdown
Contributor

What

whisper_full() / whisper_full_with_state() called with n_samples == 0 on a
freshly allocated state can dereference a NULL mel buffer inside the encoder, or
silently return 0 — the outcome depends on heap garbage, so the crash is
intermittent.

Give struct whisper_mel default member initializers so a never-computed mel
reads as "0 frames".

Why

The mel spectrogram is only computed when there is audio:
if (n_samples > 0) { ... whisper_pcm_to_mel_with_state(...) }. For
n_samples == 0 the mel is never touched. But whisper_mel had no member
initializers, and the state is allocated with new whisper_state, so
mel.n_len / n_len_org / n_mel were indeterminate heap garbage while mel.data
is a correctly-constructed empty vector (NULL data pointer).

seek_end is derived from mel.n_len_org. If the garbage lands below the
delta_min guard the call quietly returns 0; otherwise decoding proceeds and
whisper_encode_internal copies the mel with garbage dimensions from the empty
(NULL) buffer → read at address 0. The zero-sample input is reachable from real
use (a push-to-talk recording stopped before the first capture buffer yields a
valid-header, zero-frame WAV).

With the fields default-initialized to 0, n_samples == 0 deterministically
takes the existing "input too short" path and returns 0 segments. The two code
paths that legitimately set a mel (log_mel_spectrogram,
whisper_set_mel_with_state) overwrite all three fields before any read, so the
defaults are observable only on the never-computed mel — the bug case.

Test verification (RED → GREEN)

Added tests/test-whisper-zero-samples.cpp: load for-tests-ggml-tiny.bin, call
whisper_full(ctx, params, nullptr, 0), assert rc == 0 and n_segments == 0.

Because the read is of uninitialized memory, the deterministic RED is shown with
valgrind on the unpatched struct (only the production change reverted, the new
test applied):

RED (upstream base — whisper_mel without initializers):

==316== Conditional jump or move depends on uninitialised value(s)
==316==    at 0x4987C4C: whisper_full_with_state (whisper.cpp:6886)
==316==    by 0x498B768: whisper_full (whisper.cpp:7811)
==316==    by 0x109375: main (test-whisper-zero-samples.cpp:30)
==316==  Uninitialised value was created by a heap allocation
==316==    by 0x49785B7: whisper_init_state (whisper.cpp:3388)
...
ERROR SUMMARY: (valgrind exit code 1)

GREEN (with the fix):

test-whisper-zero-samples: OK
==75== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)
valgrind exit: 0

Normal (non-valgrind) run of the new test plus the existing whisper-cli tiny
test both pass.

Fixes #3978

@ousamabenyounes
ousamabenyounes force-pushed the fix/issue-3978 branch 2 times, most recently from 03932d9 to 30e5495 Compare August 16, 2026 16:51
Comment thread src/whisper.cpp Outdated
Comment thread tests/test-whisper-zero-samples.cpp Outdated
@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

Removed the verbose comments as requested. The zero-samples test still passes.
thank you for your review

@danbev

danbev commented Aug 27, 2026

Copy link
Copy Markdown
Member

@ousamabenyounes Thanks! Can you take a look at the conflict?

whisper_full()/whisper_full_with_state() only compute the mel spectrogram when
n_samples > 0. For n_samples == 0 on a freshly allocated state the mel is never
touched, but struct whisper_mel had no member initializers, so n_len / n_len_org
/ n_mel were indeterminate heap garbage (the state is allocated with
new whisper_state). seek_end is derived from that garbage and, depending on it,
the call either quietly returns 0 or runs the encoder with garbage dimensions
over an empty (NULL) mel buffer, dereferencing address 0 in the mel copy loop.

Give whisper_mel default member initializers so a never-computed mel reads as
0 frames and the n_samples == 0 case deterministically takes the existing
too-short path.

Fixes ggml-org#3978
@ousamabenyounes

Copy link
Copy Markdown
Contributor Author

@danbev Rebased onto the latest master to clear the conflict — the PR is now mergeable again (02c45f5b). The change is unchanged from your review: whisper_mel's three fields default-initialized to 0, plus the zero-samples regression test.

Locally, ctest -L gh is green (4/4, including test-whisper-zero-samples) on the rebased base. The GitHub Actions checks are showing action_required (fork-PR workflows waiting on a maintainer approval-to-run) rather than a failure.

@danbev
danbev merged commit eacbd82 into ggml-org:master Aug 31, 2026
47 checks passed
bygreencn added a commit to bygreencn/whisper.cpp that referenced this pull request Sep 3, 2026
* ggerganov/master: (144 commits)
  whisper : default-initialize whisper_mel to avoid uninitialized read (ggml-org#3981)
  parakeet : fix TDT decode by outputting raw logits from the joint graph (ggml-org#4017)
  talk-llama : sync llama.cpp
  pi : init
  sync : ggml
  ggml : bump version to 0.22.0 (ggml/1607)
  sycl : mark tq2_0 as not supported (llama/27660)
  webgpu : fix handling of infinity values during ARGSORT and TOP_K (llama/27538)
  metal : per-device tuned (Q, NE) for flash-attn vec (llama/26570)
  sync : ggml
  metal: per-op source split + parallel compile (llama/26561)
  scripts : update ggml-am
  sync : ggml
  ggml : shorten virtual device naming in CUDA and Metal (llama/27608)
  webgpu : reorder includes since V that appears in common_decls.tmpl may be defined as K in flash_attn_decls.tmpl if KV_OVERLAP (llama/27545)
  ggml : fix ggml_clamp (llama/27644)
  Deepseek 4: `-sm tensor` (llama/26490)
  Fix meta tensor split state propagation (llama/27574)
  cuda : add POOL_1D support (llama/27573)
  vulkan : added the PAD_REFLECT_1D operation (llama/26586)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

whisper_full with n_samples == 0 runs on uninitialized whisper_mel fields (intermittent NULL deref in encoder)

2 participants