1
0
mirror of https://git.ffmpeg.org/ffmpeg.git synced 2026-10-24 02:09:03 +03:00

avcodec/fflcms2: fix memory leak after cmsDeleteContext(NULL)

At the time that this code was introduced into FFmpeg in April 2022 by commit
b9a25963f7, it was correct as written. The latest release of
lcms2 at the time was 2.13.1 in which `cmsDeleteContext(NULL)` was a no-op.

Since lcms2 version 2.15, released Mar 2023, the same call is no longer a no-op and
instead means destroy the global context. The new behavior is justifiable as being
consistent with the rest of lcms2's API, where passing NULL for a cmsContext argument
means operate on the global context. For example `cmsDupContext(NULL, ...)` constructs a
context duplicated from the global context; `cmsPluginTHR(NULL, ...)` registers a plugin
into the global context.

The upstream commit that introduced the behavior change for `cmsDeleteContext` is:
a9e4601ceb

In FFmpeg, the new behavior leads to a memory leak if `ff_icc_context_uninit` is ever
called on an `FFIccContext` without `ff_icc_context_init`. Here is why that can happen,
and here is how it leads to a memory leak.

Naively, uninitializing something which was never initialized seems like it shouldn't
happen, but in this case these contexts are *lazily* initialized and *unconditionally*
deinitialized. `FFIccContext` is a member of `AVCodecInternal`:

    // libavcodec/internal.h:148-150
    #if CONFIG_LCMS2
        FFIccContext icc; /* used to read and write embedded ICC profiles */
    #endif

It is initialized conditionally by the snippet of `detect_colorspace` shown below, or by
similary shaped code in libavcodec/encode.c. The `avci->icc.avctx` guards "has it already
been initialized". The early returns beforehand are codepaths for which `icc` will not be
initialized, and will remain all-zero as allocated by `av_mallocz` in
`ff_decode_internal_alloc`.

    // libavcodec/decode.c:534-542
    if (!(avctx->flags2 & AV_CODEC_FLAG2_ICC_PROFILES))
        return 0;
    sd = av_frame_get_side_data(frame, AV_FRAME_DATA_ICC_PROFILE);
    if (!sd || !sd->size)
        return 0;
    if (!avci->icc.avctx) {
        ret = ff_icc_context_init(&avci->icc, avctx);

Later, `avci->icc` is uninitialized unconditionally by `ff_codec_close`:

    // libavcodec/avcodec.c:447-480
    if (avcodec_is_open(avctx)) {
        ...
    #if CONFIG_LCMS2
        ff_icc_context_uninit(&avci->icc);
    #endif

So every codec closed without `AV_CODEC_FLAGS2_ICC_PROFILES`, or without an ICC side-data
packet, reaches `ff_icc_context_uninit` on a never-initialized context.

As part of `cmsDeleteContext(NULL)` destroying lcms2's global context, it runs
`cmsUnregisterPlugins()` and destroys `globalContext.MemPool`. Since the global context is
shared by every user of lcms2 in the process, closing an FFmpeg codec silently tears down
unrelated libraries' lcms2 state. The most visible symptom is that
`_cmsRegisterMutexPlugin(NULL, NULL)` nulls out the mutex callbacks, after which
`cmsCloseProfile()` leaks each profile's `UsrMutex` and profiles opened later are left
without any locking at all.

The fix is to delete only contexts created by FFmpeg.

Signed-off-by: David Tolnay <dtolnay@gmail.com>
This commit is contained in:
David Tolnay
2026-09-07 17:09:55 -07:00
committed by Niklas Haas
parent 053585bc91
commit 705286a8a7

View File

@@ -43,7 +43,8 @@ void ff_icc_context_uninit(FFIccContext *s)
{
for (int i = 0; i < FF_ARRAY_ELEMS(s->curves); i++)
cmsFreeToneCurve(s->curves[i]);
cmsDeleteContext(s->ctx);
if (s->ctx)
cmsDeleteContext(s->ctx);
memset(s, 0, sizeof(*s));
}