Repository navigation
Record customMem before the pool's first failure point - #4859
Huang-404-Q wants to merge 1 commit into
Conversation
POOL_create_advanced assigns ctx->customMem only after allocating and initializing the queue, but the mutex/cond init failure path calls POOL_free() before that. ctx comes from ZSTD_customCalloc, which zeroes it, so POOL_free saw an empty allocator and released custom-allocated memory with the default free(). Assign the allocator immediately after the context allocation.
|
Hi @Huang-404-Q! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Problem
POOL_create_advancedassignsctx->customMemonly after the queue has been allocated and its mutex/cond initialized, but the init failure path runs earlier:ZSTD_customCallocmemsets the context, soPOOL_free(ctx)sees an empty allocator and calls the defaultfree()on memory that came from the caller'scustomAlloc. With a custom allocator that is not malloc-compatible this is undefined behavior; with a counting allocator the allocation is never returned throughcustomFree.Fix
Assign
customMemimmediately after the allocation that produces the context, before anything can fail.Verification
Built
lib/common/pool.con Linux with a wrapper that makes the secondpthread_cond_initreturnENOMEM, and counted allocations through aZSTD_customMem:Reproducer:
gcc -O0 -DZSTD_MULTITHREAD -I lib -I lib/common -Wl,--wrap=pthread_cond_init repro.c lib/common/threading.c -lpthreadThis only reproduces on POSIX: the Windows build maps these init calls to
InitializeCriticalSection/InitializeConditionVariableand always returns 0.