[runtime/onnxruntime] Audio::FfmpegLoad(buf) early-return paths leak n_file_len bytes via avio_context_free

[wangxiuwen]

Summary

Audio::FfmpegLoad(const char* buf, int n_file_len) in runtime/onnxruntime/src/audio.cpp leaks the entire n_file_len-byte audio buffer on every error early-return path. Triggered by malformed / unsupported audio (bad mp3 header, unknown codec, decoder open failure, resampler init failure, etc.).

Location

runtime/onnxruntime/src/audio.cpp (current main).

The success path at the bottom of the function correctly disposes both the AVIOContext and its underlying buffer (lines 583-585):

//avio_context_free(&avio_ctx);
av_freep(&avio_ctx ->buffer);
av_freep(&avio_ctx);

The author clearly knew avio_context_free is wrong here — it's commented out — and replaced it with the correct av_freep(buffer) + av_freep(ctx) pair. But all 7 error early-return paths (lines 473, 481, 495, 502, 510, 529, 537) still use the wrong call:

if (avformat_open_input(&formatContext, "", nullptr, nullptr) != 0) {
    LOG(ERROR) << "Error: Could not open input file.";
    avio_context_free(&avio_ctx);   // ← does NOT free ctx->buffer
    avformat_close_input(&formatContext);
    avformat_free_context(formatContext);
    return false;
}

Why avio_context_free is wrong here

Per the FFmpeg API:

void avio_context_free(AVIOContext **s) — Free the supplied IO context and everything associated with it (except the buffer). The buffer is not deallocated.

In FfmpegLoad(buf, n_file_len), the buffer is the result of av_malloc(n_file_len) at the top of the function:

void* buf_copy = av_malloc(n_file_len);
memcpy(buf_copy, buf, n_file_len);

AVIOContext* avio_ctx = avio_alloc_context(
    (unsigned char*)buf_copy, n_file_len, 0, ...);

Ownership passes to the AVIOContext, but avio_context_free returns it to the user. On the success path the author manually does av_freep(buffer) to clean up. On the error paths nobody does, so n_file_len bytes leak on every failure.

Impact

For each malformed audio submission the runtime leaks the entire input buffer (typically several hundred KB to a few MB). For a long-running service exposed to user uploads, this is a denial-of-service amplifier — repeated POSTs of broken mp3 grow memory linearly.

Proposed fix

Replace all 7 occurrences of avio_context_free(&avio_ctx); in the early-return paths with the same pattern the success path already uses:

     if (avformat_open_input(&formatContext, "", nullptr, nullptr) != 0) {
         LOG(ERROR) << "Error: Could not open input file.";
-        avio_context_free(&avio_ctx);
+        av_freep(&avio_ctx->buffer);
+        av_freep(&avio_ctx);
         avformat_close_input(&formatContext);
         avformat_free_context(formatContext);
         return false;
     }

Apply the same diff to all 7 sites. (A cleaner long-term refactor would be a small RAII guard, but the minimal diff matches what the success path already does.)