From dc1fb684bfbd5bd1400226cb2c9ee69084782a90 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 12 Sep 2019 18:24:44 -0400 Subject: [PATCH 01/16] Remove Unused MEM_SKIP_MSAN Macro --- lib/common/mem.h | 6 ------ 1 file changed, 6 deletions(-) diff --git a/lib/common/mem.h b/lib/common/mem.h index 2b115ddb6..acc7231e9 100644 --- a/lib/common/mem.h +++ b/lib/common/mem.h @@ -74,12 +74,6 @@ void __msan_poison(const volatile void *a, size_t size); intptr_t __msan_test_shadow(const volatile void *x, size_t size); #endif -#if defined (MEMORY_SANITIZER) -# define MEM_SKIP_MSAN __attribute__((no_sanitize("memory"))) -#else -# define MEM_SKIP_MSAN -#endif - /*-************************************************************** * Basic Types From edb6d884a593bb80c66825c8a476e47ded2d617a Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 12 Sep 2019 18:32:22 -0400 Subject: [PATCH 02/16] Detect Whether We're Being Compiled with ASAN --- lib/common/mem.h | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/lib/common/mem.h b/lib/common/mem.h index acc7231e9..983b55edc 100644 --- a/lib/common/mem.h +++ b/lib/common/mem.h @@ -74,6 +74,19 @@ void __msan_poison(const volatile void *a, size_t size); intptr_t __msan_test_shadow(const volatile void *x, size_t size); #endif +/* detects whether we are being compiled under asan */ +#if defined (__has_feature) +# if __has_feature(address_sanitizer) +# define ADDRESS_SANITIZER 1 +# endif +#elif defined(__SANITIZE_ADDRESS__) +# define ADDRESS_SANITIZER 1 +#endif + +#if defined (ADDRESS_SANITIZER) +# include +#endif + /*-************************************************************** * Basic Types From 35c30d6ca7219a602dc4c54d33835e4e1be6fee5 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 12 Sep 2019 18:33:00 -0400 Subject: [PATCH 03/16] Poison Unused Workspace Memory --- lib/compress/zstd_cwksp.h | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 39d064c49..6d775f90b 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -214,6 +214,11 @@ MEM_STATIC void* ZSTD_cwksp_reserve_internal( ws->tableValidEnd = alloc; } ws->allocStart = alloc; + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + __asan_unpoison_memory_region(alloc, bytes); +#endif + return alloc; } @@ -254,6 +259,11 @@ MEM_STATIC void* ZSTD_cwksp_reserve_table(ZSTD_cwksp* ws, size_t bytes) { return NULL; } ws->tableEnd = end; + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + __asan_unpoison_memory_region(alloc, bytes); +#endif + return alloc; } @@ -279,6 +289,11 @@ MEM_STATIC void* ZSTD_cwksp_reserve_object(ZSTD_cwksp* ws, size_t bytes) { ws->objectEnd = end; ws->tableEnd = end; ws->tableValidEnd = end; + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + __asan_unpoison_memory_region(start, bytes); +#endif + return start; } @@ -331,6 +346,14 @@ MEM_STATIC void ZSTD_cwksp_clean_tables(ZSTD_cwksp* ws) { */ MEM_STATIC void ZSTD_cwksp_clear_tables(ZSTD_cwksp* ws) { DEBUGLOG(4, "cwksp: clearing tables!"); + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + { + size_t size = (BYTE*)ws->tableValidEnd - (BYTE*)ws->objectEnd; + __asan_poison_memory_region(ws->objectEnd, size); + } +#endif + ws->tableEnd = ws->objectEnd; ZSTD_cwksp_assert_internal_consistency(ws); } @@ -353,6 +376,13 @@ MEM_STATIC void ZSTD_cwksp_clear(ZSTD_cwksp* ws) { } #endif +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + { + size_t size = (BYTE*)ws->workspaceEnd - (BYTE*)ws->objectEnd; + __asan_poison_memory_region(ws->objectEnd, size); + } +#endif + ws->tableEnd = ws->objectEnd; ws->allocStart = ws->workspaceEnd; ws->allocFailed = 0; From da88c35d41f53255f644a8b2ca27f4fa017048e6 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 12 Sep 2019 18:39:46 -0400 Subject: [PATCH 04/16] Stop Assuming Tables are Adjacent --- lib/compress/zstd_compress.c | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 2f0736b24..02011c946 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1649,12 +1649,13 @@ static size_t ZSTD_resetCCtx_byCopyingCDict(ZSTD_CCtx* cctx, /* copy tables */ { size_t const chainSize = (cdict_cParams->strategy == ZSTD_fast) ? 0 : ((size_t)1 << cdict_cParams->chainLog); size_t const hSize = (size_t)1 << cdict_cParams->hashLog; - size_t const tableSpace = (chainSize + hSize) * sizeof(U32); - assert((U32*)cctx->blockState.matchState.chainTable == (U32*)cctx->blockState.matchState.hashTable + hSize); /* chainTable must follow hashTable */ - assert((U32*)cctx->blockState.matchState.hashTable3 == (U32*)cctx->blockState.matchState.chainTable + chainSize); - assert((U32*)cdict->matchState.chainTable == (U32*)cdict->matchState.hashTable + hSize); /* chainTable must follow hashTable */ - assert((U32*)cdict->matchState.hashTable3 == (U32*)cdict->matchState.chainTable + chainSize); - memcpy(cctx->blockState.matchState.hashTable, cdict->matchState.hashTable, tableSpace); /* presumes all tables follow each other */ + + memcpy(cctx->blockState.matchState.hashTable, + cdict->matchState.hashTable, + hSize * sizeof(U32)); + memcpy(cctx->blockState.matchState.chainTable, + cdict->matchState.chainTable, + chainSize * sizeof(U32)); } /* Zero the hashTable3, since the cdict never fills it */ @@ -1741,10 +1742,16 @@ static size_t ZSTD_copyCCtx_internal(ZSTD_CCtx* dstCCtx, size_t const hSize = (size_t)1 << srcCCtx->appliedParams.cParams.hashLog; int const h3log = srcCCtx->blockState.matchState.hashLog3; size_t const h3Size = h3log ? ((size_t)1 << h3log) : 0; - size_t const tableSpace = (chainSize + hSize + h3Size) * sizeof(U32); - assert((U32*)dstCCtx->blockState.matchState.chainTable == (U32*)dstCCtx->blockState.matchState.hashTable + hSize); /* chainTable must follow hashTable */ - assert((U32*)dstCCtx->blockState.matchState.hashTable3 == (U32*)dstCCtx->blockState.matchState.chainTable + chainSize); - memcpy(dstCCtx->blockState.matchState.hashTable, srcCCtx->blockState.matchState.hashTable, tableSpace); /* presumes all tables follow each other */ + + memcpy(dstCCtx->blockState.matchState.hashTable, + srcCCtx->blockState.matchState.hashTable, + hSize * sizeof(U32)); + memcpy(dstCCtx->blockState.matchState.chainTable, + srcCCtx->blockState.matchState.chainTable, + chainSize * sizeof(U32)); + memcpy(dstCCtx->blockState.matchState.hashTable3, + srcCCtx->blockState.matchState.hashTable3, + h3Size * sizeof(U32)); } ZSTD_cwksp_mark_tables_clean(&dstCCtx->workspace); From 19a0955ec90b30d249e253b47160d40f5b34a5ea Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 12 Sep 2019 19:40:38 -0400 Subject: [PATCH 05/16] Add `ZSTD_cwksp_alloc_size()` to Help Calculate Needed Workspace Size --- lib/compress/zstd_compress.c | 58 ++++++++++++++++++++++-------------- lib/compress/zstd_cwksp.h | 25 ++++++++++++++++ lib/compress/zstd_ldm.c | 6 ++-- 3 files changed, 64 insertions(+), 25 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 02011c946..8fbddf8b2 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1078,9 +1078,16 @@ ZSTD_sizeof_matchState(const ZSTD_compressionParameters* const cParams, size_t const hSize = ((size_t)1) << cParams->hashLog; U32 const hashLog3 = (forCCtx && cParams->minMatch==3) ? MIN(ZSTD_HASHLOG3_MAX, cParams->windowLog) : 0; size_t const h3Size = ((size_t)1) << hashLog3; - size_t const tableSpace = (chainSize + hSize + h3Size) * sizeof(U32); - size_t const optPotentialSpace = ((MaxML+1) + (MaxLL+1) + (MaxOff+1) + (1<strategy >= ZSTD_btopt)) ? optPotentialSpace : 0; @@ -1097,20 +1104,21 @@ size_t ZSTD_estimateCCtxSize_usingCCtxParams(const ZSTD_CCtx_params* params) size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, (size_t)1 << cParams.windowLog); U32 const divider = (cParams.minMatch==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq; - size_t const entropySpace = HUF_WORKSPACE_SIZE; - size_t const blockStateSpace = 2 * sizeof(ZSTD_compressedBlockState_t); + size_t const tokenSpace = ZSTD_cwksp_alloc_size(WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq); + size_t const entropySpace = ZSTD_cwksp_alloc_size(HUF_WORKSPACE_SIZE); + size_t const blockStateSpace = 2 * ZSTD_cwksp_alloc_size(sizeof(ZSTD_compressedBlockState_t)); size_t const matchStateSize = ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 1); size_t const ldmSpace = ZSTD_ldm_getTableSize(params->ldmParams); - size_t const ldmSeqSpace = ZSTD_ldm_getMaxNbSeq(params->ldmParams, blockSize) * sizeof(rawSeq); + size_t const ldmSeqSpace = ZSTD_cwksp_alloc_size(ZSTD_ldm_getMaxNbSeq(params->ldmParams, blockSize) * sizeof(rawSeq)); size_t const neededSpace = entropySpace + blockStateSpace + tokenSpace + matchStateSize + ldmSpace + ldmSeqSpace; + size_t const cctxSpace = ZSTD_cwksp_alloc_size(sizeof(ZSTD_CCtx)); - DEBUGLOG(5, "sizeof(ZSTD_CCtx) : %u", (U32)sizeof(ZSTD_CCtx)); + DEBUGLOG(5, "sizeof(ZSTD_CCtx) : %u", (U32)cctxSpace); DEBUGLOG(5, "estimate workspace : %u", (U32)neededSpace); - return sizeof(ZSTD_CCtx) + neededSpace; + return cctxSpace + neededSpace; } } @@ -1393,7 +1401,7 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, windowSize); U32 const divider = (params.cParams.minMatch==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq; + size_t const tokenSpace = ZSTD_cwksp_alloc_size(WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq); size_t const buffOutSize = (zbuff==ZSTDb_buffered) ? ZSTD_compressBound(blockSize)+1 : 0; size_t const buffInSize = (zbuff==ZSTDb_buffered) ? windowSize + blockSize : 0; size_t const matchStateSize = ZSTD_sizeof_matchState(¶ms.cParams, /* forCCtx */ 1); @@ -1408,12 +1416,12 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, ZSTD_cwksp_bump_oversized_duration(ws, 0); /* Check if workspace is large enough, alloc a new one if needed */ - { size_t const cctxSpace = zc->staticSize ? sizeof(ZSTD_CCtx) : 0; - size_t const entropySpace = HUF_WORKSPACE_SIZE; - size_t const blockStateSpace = 2 * sizeof(ZSTD_compressedBlockState_t); - size_t const bufferSpace = buffInSize + buffOutSize; + { size_t const cctxSpace = zc->staticSize ? ZSTD_cwksp_alloc_size(sizeof(ZSTD_CCtx)) : 0; + size_t const entropySpace = ZSTD_cwksp_alloc_size(HUF_WORKSPACE_SIZE); + size_t const blockStateSpace = 2 * ZSTD_cwksp_alloc_size(sizeof(ZSTD_compressedBlockState_t)); + size_t const bufferSpace = ZSTD_cwksp_alloc_size(buffInSize) + ZSTD_cwksp_alloc_size(buffOutSize); size_t const ldmSpace = ZSTD_ldm_getTableSize(params.ldmParams); - size_t const ldmSeqSpace = maxNbLdmSeq * sizeof(rawSeq); + size_t const ldmSeqSpace = ZSTD_cwksp_alloc_size(maxNbLdmSeq * sizeof(rawSeq)); size_t const neededSpace = cctxSpace + @@ -3060,8 +3068,11 @@ size_t ZSTD_estimateCDictSize_advanced( ZSTD_dictLoadMethod_e dictLoadMethod) { DEBUGLOG(5, "sizeof(ZSTD_CDict) : %u", (unsigned)sizeof(ZSTD_CDict)); - return sizeof(ZSTD_CDict) + HUF_WORKSPACE_SIZE + ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 0) - + (dictLoadMethod == ZSTD_dlm_byRef ? 0 : ZSTD_cwksp_align(dictSize, sizeof(void *))); + return ZSTD_cwksp_alloc_size(sizeof(ZSTD_CDict)) + + ZSTD_cwksp_alloc_size(HUF_WORKSPACE_SIZE) + + ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 0) + + (dictLoadMethod == ZSTD_dlm_byRef ? 0 + : ZSTD_cwksp_alloc_size(ZSTD_cwksp_align(dictSize, sizeof(void *)))); } size_t ZSTD_estimateCDictSize(size_t dictSize, int compressionLevel) @@ -3141,11 +3152,11 @@ ZSTD_CDict* ZSTD_createCDict_advanced(const void* dictBuffer, size_t dictSize, if (!customMem.customAlloc ^ !customMem.customFree) return NULL; { size_t const workspaceSize = - sizeof(ZSTD_CDict) + - HUF_WORKSPACE_SIZE + + ZSTD_cwksp_alloc_size(sizeof(ZSTD_CDict)) + + ZSTD_cwksp_alloc_size(HUF_WORKSPACE_SIZE) + ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 0) + (dictLoadMethod == ZSTD_dlm_byRef ? 0 - : ZSTD_cwksp_align(dictSize, sizeof(void*))); + : ZSTD_cwksp_alloc_size(ZSTD_cwksp_align(dictSize, sizeof(void*)))); void* const workspace = ZSTD_malloc(workspaceSize, customMem); ZSTD_cwksp ws; ZSTD_CDict* cdict; @@ -3224,8 +3235,11 @@ const ZSTD_CDict* ZSTD_initStaticCDict( ZSTD_compressionParameters cParams) { size_t const matchStateSize = ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 0); - size_t const neededSize = sizeof(ZSTD_CDict) + (dictLoadMethod == ZSTD_dlm_byRef ? 0 : ZSTD_cwksp_align(dictSize, sizeof(void*))) - + HUF_WORKSPACE_SIZE + matchStateSize; + size_t const neededSize = ZSTD_cwksp_alloc_size(sizeof(ZSTD_CDict)) + + (dictLoadMethod == ZSTD_dlm_byRef ? 0 + : ZSTD_cwksp_alloc_size(ZSTD_cwksp_align(dictSize, sizeof(void*)))) + + ZSTD_cwksp_alloc_size(HUF_WORKSPACE_SIZE) + + matchStateSize; ZSTD_CDict* cdict; if ((size_t)workspace & 7) return NULL; /* 8-aligned */ diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 6d775f90b..5dce662d7 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -34,6 +34,17 @@ extern "C" { * In which case, resize it down to free some memory */ #define ZSTD_WORKSPACETOOLARGE_MAXDURATION 128 +/* Since the workspace is effectively its own little malloc implementation / + * arena, when we run under ASAN, we should similarly insert redzones between + * each internal element of the workspace, so ASAN will catch overruns that + * reach outside an object but that stay inside the workspace. + * + * This defines the size of that redzone. + */ +#ifndef ZSTD_CWKSP_ASAN_REDZONE_SIZE +#define ZSTD_CWKSP_ASAN_REDZONE_SIZE 8 +#endif + /*-************************************* * Structures ***************************************/ @@ -166,6 +177,20 @@ MEM_STATIC size_t ZSTD_cwksp_align(size_t size, size_t const align) { return (size + mask) & ~mask; } +/** + * Use this to determine how much space in the workspace we will consume to + * allocate this object. (Normally it should be exactly the size of the object, + * but under special conditions, like ASAN, where we pad each object, it might + * be larger.) + */ +MEM_STATIC size_t ZSTD_cwksp_alloc_size(size_t size) { +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + return size + 2 * ZSTD_CWKSP_ASAN_REDZONE_SIZE; +#else + return size; +#endif +} + MEM_STATIC void ZSTD_cwksp_internal_advance_phase( ZSTD_cwksp* ws, ZSTD_cwksp_alloc_phase_e phase) { assert(phase >= ws->phase); diff --git a/lib/compress/zstd_ldm.c b/lib/compress/zstd_ldm.c index fc3f46943..c3312ad3e 100644 --- a/lib/compress/zstd_ldm.c +++ b/lib/compress/zstd_ldm.c @@ -49,9 +49,9 @@ size_t ZSTD_ldm_getTableSize(ldmParams_t params) { size_t const ldmHSize = ((size_t)1) << params.hashLog; size_t const ldmBucketSizeLog = MIN(params.bucketSizeLog, params.hashLog); - size_t const ldmBucketSize = - ((size_t)1) << (params.hashLog - ldmBucketSizeLog); - size_t const totalSize = ldmBucketSize + ldmHSize * sizeof(ldmEntry_t); + size_t const ldmBucketSize = ((size_t)1) << (params.hashLog - ldmBucketSizeLog); + size_t const totalSize = ZSTD_cwksp_alloc_size(ldmBucketSize) + + ZSTD_cwksp_alloc_size(ldmHSize * sizeof(ldmEntry_t)); return params.enableLdm ? totalSize : 0; } From 143b296cf6833c57934981e9c28d090a52caea17 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 12 Sep 2019 19:41:45 -0400 Subject: [PATCH 06/16] Surround Workspace Allocs with Dead Zone --- lib/compress/zstd_cwksp.h | 49 ++++++++++++++++++++++++++++++--------- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 5dce662d7..e35d8c1cd 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -226,8 +226,14 @@ MEM_STATIC void* ZSTD_cwksp_reserve_internal( void* bottom = ws->tableEnd; ZSTD_cwksp_internal_advance_phase(ws, phase); alloc = (BYTE *)ws->allocStart - bytes; - DEBUGLOG(5, "cwksp: reserving %zd bytes, %zd bytes remaining", - bytes, ZSTD_cwksp_available_space(ws) - bytes); + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + /* over-reserve space */ + alloc = (BYTE *)alloc - 2 * ZSTD_CWKSP_ASAN_REDZONE_SIZE; +#endif + + DEBUGLOG(5, "cwksp: reserving %p %zd bytes, %zd bytes remaining", + alloc, bytes, ZSTD_cwksp_available_space(ws) - bytes); ZSTD_cwksp_assert_internal_consistency(ws); assert(alloc >= bottom); if (alloc < bottom) { @@ -241,6 +247,9 @@ MEM_STATIC void* ZSTD_cwksp_reserve_internal( ws->allocStart = alloc; #if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + /* Move alloc so there's ZSTD_CWKSP_ASAN_REDZONE_SIZE unused space on + * either size. */ + alloc = (BYTE *)alloc + ZSTD_CWKSP_ASAN_REDZONE_SIZE; __asan_unpoison_memory_region(alloc, bytes); #endif @@ -272,8 +281,14 @@ MEM_STATIC void* ZSTD_cwksp_reserve_table(ZSTD_cwksp* ws, size_t bytes) { void* alloc = ws->tableEnd; void* end = (BYTE *)alloc + bytes; void* top = ws->allocStart; - DEBUGLOG(5, "cwksp: reserving table %zd bytes, %zd bytes remaining", - bytes, ZSTD_cwksp_available_space(ws) - bytes); + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + /* over-reserve space */ + end = (BYTE *)end + 2 * ZSTD_CWKSP_ASAN_REDZONE_SIZE; +#endif + + DEBUGLOG(5, "cwksp: reserving %p table %zd bytes, %zd bytes remaining", + alloc, bytes, ZSTD_cwksp_available_space(ws) - bytes); assert((bytes & (sizeof(U32)-1)) == 0); ZSTD_cwksp_internal_advance_phase(ws, phase); ZSTD_cwksp_assert_internal_consistency(ws); @@ -286,6 +301,9 @@ MEM_STATIC void* ZSTD_cwksp_reserve_table(ZSTD_cwksp* ws, size_t bytes) { ws->tableEnd = end; #if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + /* Move alloc so there's ZSTD_CWKSP_ASAN_REDZONE_SIZE unused space on + * either size. */ + alloc = (BYTE *)alloc + ZSTD_CWKSP_ASAN_REDZONE_SIZE; __asan_unpoison_memory_region(alloc, bytes); #endif @@ -297,12 +315,18 @@ MEM_STATIC void* ZSTD_cwksp_reserve_table(ZSTD_cwksp* ws, size_t bytes) { */ MEM_STATIC void* ZSTD_cwksp_reserve_object(ZSTD_cwksp* ws, size_t bytes) { size_t roundedBytes = ZSTD_cwksp_align(bytes, sizeof(void*)); - void* start = ws->objectEnd; - void* end = (BYTE*)start + roundedBytes; + void* alloc = ws->objectEnd; + void* end = (BYTE*)alloc + roundedBytes; + +#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) + /* over-reserve space */ + end = (BYTE *)end + 2 * ZSTD_CWKSP_ASAN_REDZONE_SIZE; +#endif + DEBUGLOG(5, - "cwksp: reserving object %zd bytes (rounded to %zd), %zd bytes remaining", - bytes, roundedBytes, ZSTD_cwksp_available_space(ws) - roundedBytes); - assert(((size_t)start & (sizeof(void*)-1)) == 0); + "cwksp: reserving %p object %zd bytes (rounded to %zd), %zd bytes remaining", + alloc, bytes, roundedBytes, ZSTD_cwksp_available_space(ws) - roundedBytes); + assert(((size_t)alloc & (sizeof(void*)-1)) == 0); assert((bytes & (sizeof(void*)-1)) == 0); ZSTD_cwksp_assert_internal_consistency(ws); /* we must be in the first phase, no advance is possible */ @@ -316,10 +340,13 @@ MEM_STATIC void* ZSTD_cwksp_reserve_object(ZSTD_cwksp* ws, size_t bytes) { ws->tableValidEnd = end; #if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) - __asan_unpoison_memory_region(start, bytes); + /* Move alloc so there's ZSTD_CWKSP_ASAN_REDZONE_SIZE unused space on + * either size. */ + alloc = (BYTE *)alloc + ZSTD_CWKSP_ASAN_REDZONE_SIZE; + __asan_unpoison_memory_region(alloc, bytes); #endif - return start; + return alloc; } MEM_STATIC void ZSTD_cwksp_mark_tables_dirty(ZSTD_cwksp* ws) { From ef0b5707c525f2bc97ba3c25119aaafc4d3896ab Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 17:43:05 -0400 Subject: [PATCH 07/16] Refactor Freeing CCtxes / CDicts Inside Workspaces --- lib/compress/zstd_compress.c | 20 +++++++++----------- lib/compress/zstd_cwksp.h | 7 +++++++ 2 files changed, 16 insertions(+), 11 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 8fbddf8b2..431307f03 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -130,24 +130,23 @@ static void ZSTD_freeCCtxContent(ZSTD_CCtx* cctx) { assert(cctx != NULL); assert(cctx->staticSize == 0); - /* Only free workspace if cctx not in workspace, otherwise the workspace - * will be freed when the cctx itself is freed. */ - if ((void*)cctx->workspace.workspace != (void*)cctx) { - ZSTD_cwksp_free(&cctx->workspace, cctx->customMem); - } ZSTD_clearAllDicts(cctx); #ifdef ZSTD_MULTITHREAD ZSTDMT_freeCCtx(cctx->mtctx); cctx->mtctx = NULL; #endif + ZSTD_cwksp_free(&cctx->workspace, cctx->customMem); } size_t ZSTD_freeCCtx(ZSTD_CCtx* cctx) { + int cctxInWorkspace = ZSTD_cwksp_owns_buffer(&cctx->workspace, cctx); if (cctx==NULL) return 0; /* support free on NULL */ RETURN_ERROR_IF(cctx->staticSize, memory_allocation, "not compatible with static CCtx"); ZSTD_freeCCtxContent(cctx); - ZSTD_free(cctx, cctx->customMem); + if (!cctxInWorkspace) { + ZSTD_free(cctx, cctx->customMem); + } return 0; } @@ -3204,12 +3203,11 @@ size_t ZSTD_freeCDict(ZSTD_CDict* cdict) { if (cdict==NULL) return 0; /* support free on NULL */ { ZSTD_customMem const cMem = cdict->customMem; - /* Only free workspace if cdict not in workspace, otherwise the - * workspace will be freed when the cdict itself is freed. */ - if ((void*)cdict->workspace.workspace != (void*)cdict) { - ZSTD_cwksp_free(&cdict->workspace, cMem); + int cdictInWorkspace = ZSTD_cwksp_owns_buffer(&cdict->workspace, cdict); + ZSTD_cwksp_free(&cdict->workspace, cMem); + if (!cdictInWorkspace) { + ZSTD_free(cdict, cMem); } - ZSTD_free(cdict, cMem); return 0; } } diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index e35d8c1cd..f6068127b 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -217,6 +217,13 @@ MEM_STATIC void ZSTD_cwksp_internal_advance_phase( } } +/** + * Returns whether this object/buffer/etc was allocated in this workspace. + */ +MEM_STATIC int ZSTD_cwksp_owns_buffer(const ZSTD_cwksp* ws, const void* ptr) { + return (ptr != NULL) && (ws->workspace <= ptr) && (ptr <= ws->workspaceEnd); +} + /** * Internal function. Do not use directly. */ From 8cffd6ed082b996040ae8c64302e594b1368d272 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 17:43:55 -0400 Subject: [PATCH 08/16] Avoid ASAN Failure in ZSTD_cwksp_free() --- lib/compress/zstd_cwksp.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index f6068127b..bae5b654d 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -478,9 +478,10 @@ MEM_STATIC size_t ZSTD_cwksp_create(ZSTD_cwksp* ws, size_t size, ZSTD_customMem } MEM_STATIC void ZSTD_cwksp_free(ZSTD_cwksp* ws, ZSTD_customMem customMem) { + void *ptr = ws->workspace; DEBUGLOG(4, "cwksp: freeing workspace"); - ZSTD_free(ws->workspace, customMem); memset(ws, 0, sizeof(ZSTD_cwksp)); + ZSTD_free(ptr, customMem); } /** From b6c0a02a17bcab8aab56f58563f244c52cb1cab8 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 17:45:40 -0400 Subject: [PATCH 09/16] Fix ZSTD_sizeof_matchState() Calculation --- lib/compress/zstd_compress.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 431307f03..becdc728f 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1076,7 +1076,7 @@ ZSTD_sizeof_matchState(const ZSTD_compressionParameters* const cParams, size_t const chainSize = (cParams->strategy == ZSTD_fast) ? 0 : ((size_t)1 << cParams->chainLog); size_t const hSize = ((size_t)1) << cParams->hashLog; U32 const hashLog3 = (forCCtx && cParams->minMatch==3) ? MIN(ZSTD_HASHLOG3_MAX, cParams->windowLog) : 0; - size_t const h3Size = ((size_t)1) << hashLog3; + size_t const h3Size = hashLog3 ? ((size_t)1) << hashLog3 : 0; size_t const tableSpace = ZSTD_cwksp_alloc_size(chainSize * sizeof(U32)) + ZSTD_cwksp_alloc_size(hSize * sizeof(U32)) + ZSTD_cwksp_alloc_size(h3Size * sizeof(U32)); From 0cc481ef66b951a681ba9af70e06c13b91075ada Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 17:47:29 -0400 Subject: [PATCH 10/16] Fix Workspace Size Calculation --- lib/compress/zstd_compress.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index becdc728f..5c7392908 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1400,7 +1400,9 @@ static size_t ZSTD_resetCCtx_internal(ZSTD_CCtx* zc, size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, windowSize); U32 const divider = (params.cParams.minMatch==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = ZSTD_cwksp_alloc_size(WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq); + size_t const tokenSpace = ZSTD_cwksp_alloc_size(WILDCOPY_OVERLENGTH + blockSize) + + ZSTD_cwksp_alloc_size(maxNbSeq * sizeof(seqDef)) + + 3 * ZSTD_cwksp_alloc_size(maxNbSeq * sizeof(BYTE)); size_t const buffOutSize = (zbuff==ZSTDb_buffered) ? ZSTD_compressBound(blockSize)+1 : 0; size_t const buffInSize = (zbuff==ZSTDb_buffered) ? windowSize + blockSize : 0; size_t const matchStateSize = ZSTD_sizeof_matchState(¶ms.cParams, /* forCCtx */ 1); From a07037b7846f473bb27018a860b9ad1c1c9d46b5 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 17:56:28 -0400 Subject: [PATCH 11/16] Don't Try to Redzone the Tables --- lib/compress/zstd_cwksp.h | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index bae5b654d..2fca8cff2 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -289,11 +289,6 @@ MEM_STATIC void* ZSTD_cwksp_reserve_table(ZSTD_cwksp* ws, size_t bytes) { void* end = (BYTE *)alloc + bytes; void* top = ws->allocStart; -#if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) - /* over-reserve space */ - end = (BYTE *)end + 2 * ZSTD_CWKSP_ASAN_REDZONE_SIZE; -#endif - DEBUGLOG(5, "cwksp: reserving %p table %zd bytes, %zd bytes remaining", alloc, bytes, ZSTD_cwksp_available_space(ws) - bytes); assert((bytes & (sizeof(U32)-1)) == 0); @@ -308,9 +303,6 @@ MEM_STATIC void* ZSTD_cwksp_reserve_table(ZSTD_cwksp* ws, size_t bytes) { ws->tableEnd = end; #if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) - /* Move alloc so there's ZSTD_CWKSP_ASAN_REDZONE_SIZE unused space on - * either size. */ - alloc = (BYTE *)alloc + ZSTD_CWKSP_ASAN_REDZONE_SIZE; __asan_unpoison_memory_region(alloc, bytes); #endif From 0ffae7e44061bd50aa76cff8b310d6da2f802ddc Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 18:06:16 -0400 Subject: [PATCH 12/16] Stop Allocating Extra Space for Table Redzones --- lib/compress/zstd_compress.c | 6 +++--- lib/compress/zstd_cwksp.h | 4 ++++ 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 5c7392908..110066731 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1077,9 +1077,9 @@ ZSTD_sizeof_matchState(const ZSTD_compressionParameters* const cParams, size_t const hSize = ((size_t)1) << cParams->hashLog; U32 const hashLog3 = (forCCtx && cParams->minMatch==3) ? MIN(ZSTD_HASHLOG3_MAX, cParams->windowLog) : 0; size_t const h3Size = hashLog3 ? ((size_t)1) << hashLog3 : 0; - size_t const tableSpace = ZSTD_cwksp_alloc_size(chainSize * sizeof(U32)) - + ZSTD_cwksp_alloc_size(hSize * sizeof(U32)) - + ZSTD_cwksp_alloc_size(h3Size * sizeof(U32)); + size_t const tableSpace = chainSize * sizeof(U32) + + hSize * sizeof(U32) + + h3Size * sizeof(U32); size_t const optPotentialSpace = ZSTD_cwksp_alloc_size((MaxML+1) * sizeof(U32)) + ZSTD_cwksp_alloc_size((MaxLL+1) * sizeof(U32)) diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 2fca8cff2..15b48b470 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -182,6 +182,10 @@ MEM_STATIC size_t ZSTD_cwksp_align(size_t size, size_t const align) { * allocate this object. (Normally it should be exactly the size of the object, * but under special conditions, like ASAN, where we pad each object, it might * be larger.) + * + * Since tables aren't currently redzoned, you don't need to call through this + * to figure out how much space you need for the matchState tables. Everything + * else is though. */ MEM_STATIC size_t ZSTD_cwksp_alloc_size(size_t size) { #if defined (ADDRESS_SANITIZER) && !defined (ZSTD_ASAN_DONT_POISON_WORKSPACE) From b6987acbbf1d565feac9473eb095eebade7f6f1d Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Mon, 16 Sep 2019 19:04:05 -0400 Subject: [PATCH 13/16] Declare the ASAN Functions We Need, Don't Include the Header --- lib/common/mem.h | 35 ++++++++++++++++++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/common/mem.h b/lib/common/mem.h index 983b55edc..530d30c8f 100644 --- a/lib/common/mem.h +++ b/lib/common/mem.h @@ -84,7 +84,40 @@ intptr_t __msan_test_shadow(const volatile void *x, size_t size); #endif #if defined (ADDRESS_SANITIZER) -# include +/* Not all platforms that support asan provide sanitizers/asan_interface.h. + * We therefore declare the functions we need ourselves, rather than trying to + * include the header file... */ + +/** + * Marks a memory region ([addr, addr+size)) as unaddressable. + * + * This memory must be previously allocated by your program. Instrumented + * code is forbidden from accessing addresses in this region until it is + * unpoisoned. This function is not guaranteed to poison the entire region - + * it could poison only a subregion of [addr, addr+size) due to ASan + * alignment restrictions. + * + * \note This function is not thread-safe because no two threads can poison or + * unpoison memory in the same memory region simultaneously. + * + * \param addr Start of memory region. + * \param size Size of memory region. */ +void __asan_poison_memory_region(void const volatile *addr, size_t size); + +/** + * Marks a memory region ([addr, addr+size)) as addressable. + * + * This memory must be previously allocated by your program. Accessing + * addresses in this region is allowed until this region is poisoned again. + * This function could unpoison a super-region of [addr, addr+size) due + * to ASan alignment restrictions. + * + * \note This function is not thread-safe because no two threads can + * poison or unpoison memory in the same memory region simultaneously. + * + * \param addr Start of memory region. + * \param size Size of memory region. */ +void __asan_unpoison_memory_region(void const volatile *addr, size_t size); #endif From 2c80a9f8ac73ad6634dfe68ae1624eb1af5e3d61 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Tue, 17 Sep 2019 11:35:49 -0400 Subject: [PATCH 14/16] Check if CCtx in Workspace after Null Check --- lib/compress/zstd_compress.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 110066731..4afb9cdca 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -139,13 +139,15 @@ static void ZSTD_freeCCtxContent(ZSTD_CCtx* cctx) size_t ZSTD_freeCCtx(ZSTD_CCtx* cctx) { - int cctxInWorkspace = ZSTD_cwksp_owns_buffer(&cctx->workspace, cctx); if (cctx==NULL) return 0; /* support free on NULL */ RETURN_ERROR_IF(cctx->staticSize, memory_allocation, "not compatible with static CCtx"); - ZSTD_freeCCtxContent(cctx); - if (!cctxInWorkspace) { - ZSTD_free(cctx, cctx->customMem); + { + int cctxInWorkspace = ZSTD_cwksp_owns_buffer(&cctx->workspace, cctx); + ZSTD_freeCCtxContent(cctx); + if (!cctxInWorkspace) { + ZSTD_free(cctx, cctx->customMem); + } } return 0; } @@ -1077,6 +1079,8 @@ ZSTD_sizeof_matchState(const ZSTD_compressionParameters* const cParams, size_t const hSize = ((size_t)1) << cParams->hashLog; U32 const hashLog3 = (forCCtx && cParams->minMatch==3) ? MIN(ZSTD_HASHLOG3_MAX, cParams->windowLog) : 0; size_t const h3Size = hashLog3 ? ((size_t)1) << hashLog3 : 0; + /* We don't use ZSTD_cwksp_alloc_size() here because the tables aren't + * surrounded by redzones in ASAN. */ size_t const tableSpace = chainSize * sizeof(U32) + hSize * sizeof(U32) + h3Size * sizeof(U32); From bd6a20b8a0cc16fbd6efa974bf60d59bb79f6398 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Oct 2019 13:45:55 -0400 Subject: [PATCH 15/16] Expand Default Redzone Size --- lib/compress/zstd_cwksp.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/compress/zstd_cwksp.h b/lib/compress/zstd_cwksp.h index 15b48b470..18492acb4 100644 --- a/lib/compress/zstd_cwksp.h +++ b/lib/compress/zstd_cwksp.h @@ -42,7 +42,7 @@ extern "C" { * This defines the size of that redzone. */ #ifndef ZSTD_CWKSP_ASAN_REDZONE_SIZE -#define ZSTD_CWKSP_ASAN_REDZONE_SIZE 8 +#define ZSTD_CWKSP_ASAN_REDZONE_SIZE 128 #endif /*-************************************* From ede31da2ea317d72eb0525efe865e7b2c3ce44b5 Mon Sep 17 00:00:00 2001 From: "W. Felix Handte" Date: Thu, 10 Oct 2019 15:02:08 -0400 Subject: [PATCH 16/16] Fix CCtx Size Estimation --- lib/compress/zstd_compress.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/compress/zstd_compress.c b/lib/compress/zstd_compress.c index 4afb9cdca..7af657d74 100644 --- a/lib/compress/zstd_compress.c +++ b/lib/compress/zstd_compress.c @@ -1107,7 +1107,9 @@ size_t ZSTD_estimateCCtxSize_usingCCtxParams(const ZSTD_CCtx_params* params) size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, (size_t)1 << cParams.windowLog); U32 const divider = (cParams.minMatch==3) ? 3 : 4; size_t const maxNbSeq = blockSize / divider; - size_t const tokenSpace = ZSTD_cwksp_alloc_size(WILDCOPY_OVERLENGTH + blockSize + 11*maxNbSeq); + size_t const tokenSpace = ZSTD_cwksp_alloc_size(WILDCOPY_OVERLENGTH + blockSize) + + ZSTD_cwksp_alloc_size(maxNbSeq * sizeof(seqDef)) + + 3 * ZSTD_cwksp_alloc_size(maxNbSeq * sizeof(BYTE)); size_t const entropySpace = ZSTD_cwksp_alloc_size(HUF_WORKSPACE_SIZE); size_t const blockStateSpace = 2 * ZSTD_cwksp_alloc_size(sizeof(ZSTD_compressedBlockState_t)); size_t const matchStateSize = ZSTD_sizeof_matchState(&cParams, /* forCCtx */ 1); @@ -1157,7 +1159,8 @@ size_t ZSTD_estimateCStreamSize_usingCCtxParams(const ZSTD_CCtx_params* params) size_t const blockSize = MIN(ZSTD_BLOCKSIZE_MAX, (size_t)1 << cParams.windowLog); size_t const inBuffSize = ((size_t)1 << cParams.windowLog) + blockSize; size_t const outBuffSize = ZSTD_compressBound(blockSize) + 1; - size_t const streamingSize = inBuffSize + outBuffSize; + size_t const streamingSize = ZSTD_cwksp_alloc_size(inBuffSize) + + ZSTD_cwksp_alloc_size(outBuffSize); return CCtxSize + streamingSize; }