Skip to content

Commit 0a759ed

Browse files
committed
- Fix uninitalized value use bug in opt parser
- Fix off by one bug in `ZSTD_cwksp_owns_buffer` - Better handle MSAN for init once memory - Allow to pass custom MOREFLAGS into msan-% targets in Makefile
1 parent 9eecb41 commit 0a759ed

File tree

3 files changed

+20
-2
lines changed

3 files changed

+20
-2
lines changed

Makefile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -320,7 +320,7 @@ msan: clean
320320
$(MAKE) test CC=clang MOREFLAGS="-g -fsanitize=memory -fno-omit-frame-pointer -Werror" HAVE_LZMA=0 # datagen.c fails this test for no obvious reason
321321

322322
msan-%: clean
323-
LDFLAGS=-fuse-ld=gold MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize=memory -fno-omit-frame-pointer -Werror" FUZZER_FLAGS=--no-big-tests $(MAKE) -C $(TESTDIR) HAVE_LZMA=0 $*
323+
LDFLAGS=-fuse-ld=gold MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize=memory -fno-omit-frame-pointer -Werror ${MOREFLAGS}" FUZZER_FLAGS=--no-big-tests $(MAKE) -C $(TESTDIR) HAVE_LZMA=0 $*
324324

325325
asan32: clean
326326
$(MAKE) -C $(TESTDIR) test32 CC=clang MOREFLAGS="-g -fsanitize=address"

lib/compress/zstd_cwksp.h

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,6 +180,9 @@ MEM_STATIC void ZSTD_cwksp_assert_internal_consistency(ZSTD_cwksp* ws) {
180180
assert(ws->allocStart <= ws->workspaceEnd);
181181
assert(ws->initOnceStart <= ws->workspaceEnd);
182182
assert((size_t)ws->allocStart % ZSTD_CWKSP_ALIGNMENT_BYTES == 0);
183+
#if ZSTD_MEMORY_SANITIZER
184+
assert(__msan_test_shadow(ws->initOnceStart, (U8*)ws->workspaceEnd - (U8*)ws->initOnceStart) == -1);
185+
#endif
183186
}
184187

185188
/**
@@ -316,7 +319,7 @@ ZSTD_cwksp_internal_advance_phase(ZSTD_cwksp* ws, ZSTD_cwksp_alloc_phase_e phase
316319
*/
317320
MEM_STATIC int ZSTD_cwksp_owns_buffer(const ZSTD_cwksp* ws, const void* ptr)
318321
{
319-
return (ptr != NULL) && (ws->workspace <= ptr) && (ptr <= ws->workspaceEnd);
322+
return (ptr != NULL) && (ws->workspace <= ptr) && (ptr < ws->workspaceEnd);
320323
}
321324

322325
/**
@@ -369,9 +372,22 @@ MEM_STATIC void* ZSTD_cwksp_reserve_aligned_init_once(ZSTD_cwksp* ws, size_t byt
369372
ZSTD_cwksp_alloc_aligned_init_once);
370373
assert(((size_t)ptr & (ZSTD_CWKSP_ALIGNMENT_BYTES-1))== 0);
371374
if(ptr && ptr < ws->initOnceStart) {
375+
#if !(ZSTD_ADDRESS_SANITIZER && !defined(ZSTD_ASAN_DONT_POISON_WORKSPACE))
376+
/* We will not memset when poisoning the workspace as we already unpoison the buffer
377+
* and want to maintain poisoned memory after its location.
378+
* It would have been better to memset even in these conditions, so the memset
379+
* is better tested, but I'm not sure how to make it work correctly. */
372380
ZSTD_memset(ptr, 0, (U8*)ws->initOnceStart - (U8*)ptr);
381+
#elif ZSTD_MEMORY_SANITIZER
382+
/* We couldn't memset, so we should at least unpoison the memory to
383+
* reduce false alarms from msan */
384+
__msan_unpoison(ptr, (U8*)ws->initOnceStart - (U8*)ptr);
385+
#endif
373386
ws->initOnceStart = ptr;
374387
}
388+
#if ZSTD_MEMORY_SANITIZER
389+
assert(__msan_test_shadow(ptr, bytes) == -1);
390+
#endif
375391
return ptr;
376392
}
377393

lib/compress/zstd_opt.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1086,6 +1086,8 @@ ZSTD_compressBlock_opt_generic(ZSTD_matchState_t* ms,
10861086
ZSTD_optimal_t lastSequence;
10871087
ZSTD_optLdm_t optLdm;
10881088

1089+
ZSTD_memset(&lastSequence, 0, sizeof(ZSTD_optimal_t));
1090+
10891091
optLdm.seqStore = ms->ldmSeqStore ? *ms->ldmSeqStore : kNullRawSeqStore;
10901092
optLdm.endPosInBlock = optLdm.startPosInBlock = optLdm.offset = 0;
10911093
ZSTD_opt_getNextMatchAndUpdateSeqStore(&optLdm, (U32)(ip-istart), (U32)(iend-ip));

0 commit comments

Comments
 (0)