Skip to content

Fixes the first stbi_load_gif issue in #1838 by clearing delays on all outofmem paths#1839

Open
NBickford-NV wants to merge 2 commits into
nothings:masterfrom
nvpro-samples:nbickford/upstream-fix-1838
Open

Fixes the first stbi_load_gif issue in #1838 by clearing delays on all outofmem paths#1839
NBickford-NV wants to merge 2 commits into
nothings:masterfrom
nvpro-samples:nbickford/upstream-fix-1838

Conversation

@NBickford-NV
Copy link
Copy Markdown
Contributor

After calling stbi_load_gif, apps should free the delays pointer it passed in if delays is non-null. This means that after freeing delays in the stbi__load_gif_main_outofmem error path, stb_image_load should also set delays to NULL. Otherwise if we allocate delays once but we get to stbi__load_gif_main_outofmem on a later frame, then the app sees a non-null delays pointer and tries to free it again.

With this change, running clang++ poc.c -fsanitize=address && ./a.out 490442704-000e388f-3edd-409b-9253-83046ac80317.gif from #1838 now correctly reports "Failed to load GIF" instead of segfaulting.

Forks with JarLob's double-free fix in #1549 were also affected; it implemented this logic on some paths, but not all, and #1838 happens to reach one of the other paths. Moving the delays = NULL logic to stbi__load_gif_main_outofmem on those forks allows us to save a few lines of code.

In addition, since realloc first frees the input pointer (C specification 7.22.3.5 item 1), we shouldn't pass the input to stbi__load_gif_main_outofmem after a realloc -- then we'd get a different double-free. If we remove the temporary pointer and do p = realloc(p, ...) then I think we get the right behavior and save a few lines of code.

Thanks!

@NBickford-NV
Copy link
Copy Markdown
Contributor Author

My colleague Pyarelal Knowles noticed that I made a mistake in simplifying the calls to realloc in stbi__load_gif_main and accidentally added a memory leak; if realloc returns NULL because it failed, then C specification 7.22.3.5 item 3 says it doesn't free the input pointer:

If memory for the new object cannot be
allocated, the old object is not deallocated and its value is unchanged.

I've reverted my changes to stbi__load_gif_main to the previous version, which I think was correct, so now the only change is we clear delays in outofmem. Tagging @sezero since this PR was integrated into SDL_image in libsdl-org/SDL_image@658e442 .

@sezero
Copy link
Copy Markdown

sezero commented Dec 5, 2025

Tagging @sezero since this PR was integrated into SDL_image in libsdl-org/SDL_image@658e442 .

Updated SDL repos, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants