Decoder: don't uninitialize twice when post-initialization fails by sjoblomj · Pull Request #1190 · mackron/miniaudio

sjoblomj · 2026-11-02T19:47:24Z

Base branch: dev-0.12

Summary

When a decoder's backend initializes but post-initialization fails, cleanup runs twice:

ma_decoder__postinit() calls ma_decoder_uninit(), which uninitializes and frees the backend. ma_decoder__postinit_or_uninit() then calls the backend's onUninit again, on the freed backend.
If allocating the input cache fails, ma_decoder__init_data_converter() uninitializes the data converter. ma_decoder__postinit() then calls ma_decoder_uninit(), which uninitializes it again.

The first doesn't need an allocation to fail, only a file that post-initialization rejects, so a damaged file can trigger it. A WAV file with 255 channels is enough: dr_wav accepts it, but MA_MAX_CHANNELS is 254, so ma_data_converter_init() returns MA_INVALID_ARGS. Without a sanitizer, the program below ends with:

free(): double free detected in tcache 2

ma_decoder_init_memory(), ma_decoder_init() and ma_decoder_init_vfs() all reach it.

dev doesn't have the first problem: it has no ma_decoder__postinit_or_uninit(), and calls ma_decoder__postinit() directly. The function arrived on this branch in 99c5a9c.

The fix

Remove ma_decoder__postinit_or_uninit() and call ma_decoder__postinit() instead, as dev does. It already cleans up on failure.
In ma_decoder__init_data_converter(), don't uninitialize the converter on the two input cache errors. ma_decoder__postinit() is its only caller, and it uninitializes the converter along with the rest of the decoder.

Reproduction

#define MINIAUDIO_IMPLEMENTATION
#include "miniaudio.h"
#include <stdio.h>
#include <string.h>

static ma_uint8 g_wav[44 + 4096];
static size_t g_pos;

static void put_u32(ma_uint8* p, ma_uint32 v) { p[0] = (ma_uint8)v; p[1] = (ma_uint8)(v >> 8); p[2] = (ma_uint8)(v >> 16); p[3] = (ma_uint8)(v >> 24); }
static void put_u16(ma_uint8* p, ma_uint16 v) { p[0] = (ma_uint8)v; p[1] = (ma_uint8)(v >> 8); }

/* A PCM WAV header. dr_wav accepts 255 channels, but MA_MAX_CHANNELS is 254, so that fails in the data converter. */
static void make_wav(ma_uint16 channels)
{
    memcpy(g_wav, "RIFF", 4);          put_u32(g_wav +  4, 36 + 4096);
    memcpy(g_wav + 8, "WAVEfmt ", 8);  put_u32(g_wav + 16, 16);
    put_u16(g_wav + 20, 1);            put_u16(g_wav + 22, channels);
    put_u32(g_wav + 24, 44100);        put_u32(g_wav + 28, 44100 * channels * 2);
    put_u16(g_wav + 32, (ma_uint16)(channels * 2));
    put_u16(g_wav + 34, 16);
    memcpy(g_wav + 36, "data", 4);     put_u32(g_wav + 40, 4096);
}

static ma_result on_read(ma_decoder* pDecoder, void* pOut, size_t bytesToRead, size_t* pBytesRead)
{
    size_t remaining = sizeof(g_wav) - g_pos;
    size_t n = (bytesToRead < remaining) ? bytesToRead : remaining;
    (void)pDecoder;
    memcpy(pOut, g_wav + g_pos, n);
    g_pos += n;
    *pBytesRead = n;
    return (n == 0) ? MA_AT_END : MA_SUCCESS;
}

static ma_result on_seek(ma_decoder* pDecoder, ma_int64 offset, ma_seek_origin origin)
{
    (void)pDecoder;
    if (origin == MA_SEEK_SET) {
        g_pos = (size_t)offset;
    } else if (origin == MA_SEEK_CUR) {
        g_pos = (size_t)((ma_int64)g_pos + offset);
    } else {
        g_pos = (size_t)((ma_int64)sizeof(g_wav) + offset);
    }
    return MA_SUCCESS;
}

static ma_result on_tell(ma_decoder* pDecoder, ma_int64* pCursor)
{
    (void)pDecoder;
    *pCursor = (ma_int64)g_pos;
    return MA_SUCCESS;
}

static void report(const char* pName, ma_result result, ma_decoder* pDecoder)
{
    printf("%-22s %s\n", pName, ma_result_description(result));
    fflush(stdout);
    if (result == MA_SUCCESS) {
        ma_decoder_uninit(pDecoder);
    }
}

static void try_all(ma_uint16 channels)
{
    ma_decoder decoder;
    FILE* pFile;

    printf("%d channels:\n", (int)channels);

    make_wav(channels);
    pFile = fopen("channels.wav", "wb");
    fwrite(g_wav, 1, sizeof(g_wav), pFile);
    fclose(pFile);

    report("ma_decoder_init_memory", ma_decoder_init_memory(g_wav, sizeof(g_wav), NULL, &decoder), &decoder);
    g_pos = 0;
    report("ma_decoder_init", ma_decoder_init(on_read, on_seek, on_tell, NULL, NULL, &decoder), &decoder);
    report("ma_decoder_init_file", ma_decoder_init_file("channels.wav", NULL, &decoder), &decoder);
    report("ma_decoder_init_vfs", ma_decoder_init_vfs(NULL, "channels.wav", NULL, &decoder), &decoder);
}

int main(void)
{
    try_all(2);
    try_all(255);
    return 0;
}

gcc decoder_double_free.c -o decoder_double_free -std=c89 -Wall -Wextra -fsanitize=address -g -lm -lpthread -ldl
./decoder_double_free

Before, on 17464df:

2 channels:
ma_decoder_init_memory No error
ma_decoder_init        No error
ma_decoder_init_file   No error
ma_decoder_init_vfs    No error
ERROR: AddressSanitizer: heap-use-after-free
READ of size 8
    #0 ma_dr_wav_uninit
    #1 ma_wav_uninit
    #2 ma_decoding_backend_uninit__wav
    #3 ma_decoder__postinit_or_uninit
    #4 ma_decoder_init_memory
freed by thread T0 here:
    #2 ma_free
    #3 ma_decoding_backend_uninit__wav
    #4 ma_decoder_uninit
    #5 ma_decoder__postinit

After:

2 channels:
ma_decoder_init_memory No error
ma_decoder_init        No error
ma_decoder_init_file   No error
ma_decoder_init_vfs    No error
255 channels:
ma_decoder_init_memory Invalid argument
ma_decoder_init        Invalid argument
ma_decoder_init_file   No backend
ma_decoder_init_vfs    Invalid argument

The second problem needs an allocation to fail. This program fails each allocation in turn, while opening a decoder that resamples. Put it next to the first one.

/* Fails each allocation in turn while opening a decoder that resamples. Build next to decoder_double_free.c. */
#include <stdlib.h>
#define main decoder_double_free_main
#include "decoder_double_free.c"
#undef main

static int g_countdown;
static int g_failed;

static void* test_malloc(size_t size, void* pUserData)
{
    (void)pUserData;
    if (g_countdown-- == 0) {
        g_failed = 1;
        return NULL;
    }
    return malloc(size);
}

static void* test_realloc(void* p, size_t size, void* pUserData)
{
    (void)pUserData;
    if (g_countdown-- == 0) {
        g_failed = 1;
        return NULL;
    }
    return realloc(p, size);
}

static void test_free(void* p, void* pUserData)
{
    (void)pUserData;
    free(p);
}

int main(void)
{
    ma_decoder_config config = ma_decoder_config_init(ma_format_f32, 1, 48000);
    ma_decoder decoder;
    int n;

    config.allocationCallbacks.onMalloc  = test_malloc;
    config.allocationCallbacks.onRealloc = test_realloc;
    config.allocationCallbacks.onFree    = test_free;

    make_wav(2);

    for (n = 0; ; n += 1) {
        g_countdown = n;
        g_failed = 0;

        if (ma_decoder_init_memory(g_wav, sizeof(g_wav), &config, &decoder) == MA_SUCCESS) {
            ma_decoder_uninit(&decoder);
        }

        if (!g_failed) {
            break;
        }
    }

    printf("Failed each of %d allocations in turn.\n", n);
    return 0;
}

gcc decoder_oom.c -o decoder_oom -std=c99 -Wall -Wextra -fsanitize=address,undefined -g -lm -lpthread -ldl
./decoder_oom

Before, it reports the same use-after-free as above. With only the first change, it reports the second problem:

ERROR: AddressSanitizer: attempting double-free
    #3 ma_data_converter_uninit
    #4 ma_decoder_uninit
    #5 ma_decoder__postinit
freed by thread T0 here:
    #3 ma_data_converter_uninit
    #4 ma_decoder__init_data_converter
    #5 ma_decoder__postinit

After: Failed each of 3 allocations in turn., and no reports.

Testing

Linux x86-64, GCC:

Both programs above, before and after, with sanitizers. The first also without sanitizers, where it aborts before and exits normally after.
Every channel count from 0 to 300 through ma_decoder_init_memory(), ma_decoder_init(), ma_decoder_init_file() and ma_decoder_init_vfs(), with -fsanitize=address,undefined. No crashes. The only reports are leaks from the first problem below.
Failing each allocation in turn, through ma_decoder_init() and ma_decoder_init_file() as well. No crashes. The only reports are leaks from the two problems below.
miniaudio.h compiles without warnings as C++ with -Wall -Wextra, and the first program compiles without warnings with -std=c89 -pedantic -Wall -Wextra, leaving aside the existing long long warnings.

Decoder: don't uninitialize twice when post-initialization fails

4dd11a7

Navigation Menu

Repository navigation

Decoder: don't uninitialize twice when post-initialization fails - #1190

Decoder: don't uninitialize twice when post-initialization fails#1190
sjoblomj wants to merge 1 commit into
mackron:dev-0.12from
sjoblomj:dont-uninit-twice-when-postinit-fails

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

Summary

The fix

Reproduction

Testing

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant