Skip to content

Commit 43118da

Browse files
terrellnterrelln
committed
Stop suppressing pointer-overflow UBSAN errors
* Remove all pointer-overflow suppressions from our UBSAN builds/tests. * Add `ZSTD_ALLOW_POINTER_OVERFLOW_ATTR` macro to suppress pointer-overflow at a per-function level. This is a superior approach because it also applies to users who build zstd with UBSAN. * Add `ZSTD_wrappedPtr{Diff,Add,Sub}()` that use these suppressions. The end goal is to only tag these functions with `ZSTD_ALLOW_POINTER_OVERFLOW`. But we can start by annoting functions that rely on pointer overflow, and gradually transition to using these. * Add `ZSTD_maybeNullPtrAdd()` to simplify pointer addition when the pointer may be `NULL`. * Fix all the fuzzer issues that came up. I'm sure there will be a lot more, but these are the ones that came up within a few minutes of running the fuzzers, and while running GitHub CI.
1 parent 3daed70 commit 43118da

23 files changed

+252
-103
lines changed

‎Makefile‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -317,7 +317,7 @@ update_regressionResults:
317317
# run UBsan with -fsanitize-recover=pointer-overflow
318318
# this only works with recent compilers such as gcc 8+
319319
usan: clean
320-
$(MAKE) test CC=clang MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize-recover=pointer-overflow -fsanitize=undefined -Werror $(MOREFLAGS)"
320+
$(MAKE) test CC=clang MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize=undefined -Werror $(MOREFLAGS)"
321321

322322
asan: clean
323323
$(MAKE) test CC=clang MOREFLAGS="-g -fsanitize=address -Werror $(MOREFLAGS)"
@@ -335,10 +335,10 @@ asan32: clean
335335
$(MAKE) -C $(TESTDIR) test32 CC=clang MOREFLAGS="-g -fsanitize=address $(MOREFLAGS)"
336336

337337
uasan: clean
338-
$(MAKE) test CC=clang MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize-recover=pointer-overflow -fsanitize=address,undefined -Werror $(MOREFLAGS)"
338+
$(MAKE) test CC=clang MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize=address,undefined -Werror $(MOREFLAGS)"
339339

340340
uasan-%: clean
341-
LDFLAGS=-fuse-ld=gold MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize-recover=pointer-overflow -fsanitize=address,undefined -Werror $(MOREFLAGS)" $(MAKE) -C $(TESTDIR) $*
341+
LDFLAGS=-fuse-ld=gold MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize=address,undefined -Werror $(MOREFLAGS)" $(MAKE) -C $(TESTDIR) $*
342342

343343
tsan-%: clean
344344
LDFLAGS=-fuse-ld=gold MOREFLAGS="-g -fno-sanitize-recover=all -fsanitize=thread -Werror $(MOREFLAGS)" $(MAKE) -C $(TESTDIR) $* FUZZER_FLAGS="--no-big-tests $(FUZZER_FLAGS)"

‎lib/common/compiler.h‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,8 @@
1111
#ifndef ZSTD_COMPILER_H
1212
#define ZSTD_COMPILER_H
1313

14+
#include <stddef.h>
15+
1416
#include "portability_macros.h"
1517

1618
/*-*******************************************************
@@ -302,6 +304,74 @@
302304
* Sanitizer
303305
*****************************************************************/
304306

307+
/**
308+
* Zstd relies on pointer overflow in its decompressor.
309+
* We add this attribute to functions that rely on pointer overflow.
310+
*/
311+
#ifndef ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
312+
# if __has_attribute(no_sanitize)
313+
# if !defined(__clang__) && defined(__GNUC__) && __GNUC__ < 8
314+
/* gcc < 8 only has signed-integer-overlow which triggers on pointer overflow */
315+
# define ZSTD_ALLOW_POINTER_OVERFLOW_ATTR __attribute__((no_sanitize("signed-integer-overflow")))
316+
# else
317+
/* older versions of clang [3.7, 5.0) will warn that pointer-overflow is ignored. */
318+
# define ZSTD_ALLOW_POINTER_OVERFLOW_ATTR __attribute__((no_sanitize("pointer-overflow")))
319+
# endif
320+
# else
321+
# define ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
322+
# endif
323+
#endif
324+
325+
/**
326+
* Helper function to perform a wrapped pointer difference without trigging
327+
* UBSAN.
328+
*
329+
* @returns lhs - rhs with wrapping
330+
*/
331+
MEM_STATIC
332+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
333+
ptrdiff_t ZSTD_wrappedPtrDiff(unsigned char const* lhs, unsigned char const* rhs)
334+
{
335+
return lhs - rhs;
336+
}
337+
338+
/**
339+
* Helper function to perform a wrapped pointer add without triggering UBSAN.
340+
*
341+
* @return ptr + add with wrapping
342+
*/
343+
MEM_STATIC
344+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
345+
unsigned char const* ZSTD_wrappedPtrAdd(unsigned char const* ptr, ptrdiff_t add)
346+
{
347+
return ptr + add;
348+
}
349+
350+
/**
351+
* Helper function to perform a wrapped pointer subtraction without triggering
352+
* UBSAN.
353+
*
354+
* @return ptr - sub with wrapping
355+
*/
356+
MEM_STATIC
357+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
358+
unsigned char const* ZSTD_wrappedPtrSub(unsigned char const* ptr, ptrdiff_t sub)
359+
{
360+
return ptr - sub;
361+
}
362+
363+
/**
364+
* Helper function to add to a pointer that works around C's undefined behavior
365+
* of adding 0 to NULL.
366+
*
367+
* @returns `ptr + add` except it defines `NULL + 0 == NULL`.
368+
*/
369+
MEM_STATIC
370+
unsigned char* ZSTD_maybeNullPtrAdd(unsigned char* ptr, ptrdiff_t add)
371+
{
372+
return add > 0 ? ptr + add : ptr;
373+
}
374+
305375
/* Issue #3240 reports an ASAN failure on an llvm-mingw build. Out of an
306376
* abundance of caution, disable our custom poisoning on mingw. */
307377
#ifdef __MINGW32__

‎lib/compress/zstd_compress_internal.h‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,9 @@ MEM_STATIC U32 ZSTD_window_needOverflowCorrection(ZSTD_window_t const window,
10531053
* The least significant cycleLog bits of the indices must remain the same,
10541054
* which may be 0. Every index up to maxDist in the past must be valid.
10551055
*/
1056-
MEM_STATIC U32 ZSTD_window_correctOverflow(ZSTD_window_t* window, U32 cycleLog,
1056+
MEM_STATIC
1057+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
1058+
U32 ZSTD_window_correctOverflow(ZSTD_window_t* window, U32 cycleLog,
10571059
U32 maxDist, void const* src)
10581060
{
10591061
/* preemptive overflow correction:
@@ -1246,7 +1248,9 @@ MEM_STATIC void ZSTD_window_init(ZSTD_window_t* window) {
12461248
* forget about the extDict. Handles overlap of the prefix and extDict.
12471249
* Returns non-zero if the segment is contiguous.
12481250
*/
1249-
MEM_STATIC U32 ZSTD_window_update(ZSTD_window_t* window,
1251+
MEM_STATIC
1252+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
1253+
U32 ZSTD_window_update(ZSTD_window_t* window,
12501254
void const* src, size_t srcSize,
12511255
int forceNonContiguous)
12521256
{

‎lib/compress/zstd_double_fast.c‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,9 @@
1313

1414
#ifndef ZSTD_EXCLUDE_DFAST_BLOCK_COMPRESSOR
1515

16-
static void ZSTD_fillDoubleHashTableForCDict(ZSTD_matchState_t* ms,
16+
static
17+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
18+
void ZSTD_fillDoubleHashTableForCDict(ZSTD_matchState_t* ms,
1719
void const* end, ZSTD_dictTableLoadMethod_e dtlm)
1820
{
1921
const ZSTD_compressionParameters* const cParams = &ms->cParams;
@@ -49,7 +51,9 @@ static void ZSTD_fillDoubleHashTableForCDict(ZSTD_matchState_t* ms,
4951
} }
5052
}
5153

52-
static void ZSTD_fillDoubleHashTableForCCtx(ZSTD_matchState_t* ms,
54+
static
55+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
56+
void ZSTD_fillDoubleHashTableForCCtx(ZSTD_matchState_t* ms,
5357
void const* end, ZSTD_dictTableLoadMethod_e dtlm)
5458
{
5559
const ZSTD_compressionParameters* const cParams = &ms->cParams;
@@ -97,6 +101,7 @@ void ZSTD_fillDoubleHashTable(ZSTD_matchState_t* ms,
97101

98102

99103
FORCE_INLINE_TEMPLATE
104+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
100105
size_t ZSTD_compressBlock_doubleFast_noDict_generic(
101106
ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM],
102107
void const* src, size_t srcSize, U32 const mls /* template */)
@@ -307,6 +312,7 @@ size_t ZSTD_compressBlock_doubleFast_noDict_generic(
307312

308313

309314
FORCE_INLINE_TEMPLATE
315+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
310316
size_t ZSTD_compressBlock_doubleFast_dictMatchState_generic(
311317
ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM],
312318
void const* src, size_t srcSize,
@@ -591,7 +597,9 @@ size_t ZSTD_compressBlock_doubleFast_dictMatchState(
591597
}
592598

593599

594-
static size_t ZSTD_compressBlock_doubleFast_extDict_generic(
600+
static
601+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
602+
size_t ZSTD_compressBlock_doubleFast_extDict_generic(
595603
ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM],
596604
void const* src, size_t srcSize,
597605
U32 const mls /* template */)

‎lib/compress/zstd_fast.c‎

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
#include "zstd_compress_internal.h" /* ZSTD_hashPtr, ZSTD_count, ZSTD_storeSeq */
1212
#include "zstd_fast.h"
1313

14-
static void ZSTD_fillHashTableForCDict(ZSTD_matchState_t* ms,
14+
static
15+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
16+
void ZSTD_fillHashTableForCDict(ZSTD_matchState_t* ms,
1517
const void* const end,
1618
ZSTD_dictTableLoadMethod_e dtlm)
1719
{
@@ -46,7 +48,9 @@ static void ZSTD_fillHashTableForCDict(ZSTD_matchState_t* ms,
4648
} } } }
4749
}
4850

49-
static void ZSTD_fillHashTableForCCtx(ZSTD_matchState_t* ms,
51+
static
52+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
53+
void ZSTD_fillHashTableForCCtx(ZSTD_matchState_t* ms,
5054
const void* const end,
5155
ZSTD_dictTableLoadMethod_e dtlm)
5256
{
@@ -139,8 +143,9 @@ void ZSTD_fillHashTable(ZSTD_matchState_t* ms,
139143
*
140144
* This is also the work we do at the beginning to enter the loop initially.
141145
*/
142-
FORCE_INLINE_TEMPLATE size_t
143-
ZSTD_compressBlock_fast_noDict_generic(
146+
FORCE_INLINE_TEMPLATE
147+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
148+
size_t ZSTD_compressBlock_fast_noDict_generic(
144149
ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM],
145150
void const* src, size_t srcSize,
146151
U32 const mls, U32 const hasStep)
@@ -456,6 +461,7 @@ size_t ZSTD_compressBlock_fast(
456461
}
457462

458463
FORCE_INLINE_TEMPLATE
464+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
459465
size_t ZSTD_compressBlock_fast_dictMatchState_generic(
460466
ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM],
461467
void const* src, size_t srcSize, U32 const mls, U32 const hasStep)
@@ -681,7 +687,9 @@ size_t ZSTD_compressBlock_fast_dictMatchState(
681687
}
682688

683689

684-
static size_t ZSTD_compressBlock_fast_extDict_generic(
690+
static
691+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
692+
size_t ZSTD_compressBlock_fast_extDict_generic(
685693
ZSTD_matchState_t* ms, seqStore_t* seqStore, U32 rep[ZSTD_REP_NUM],
686694
void const* src, size_t srcSize, U32 const mls, U32 const hasStep)
687695
{

‎lib/compress/zstd_lazy.c‎

Lines changed: 41 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@
2424
* Binary Tree search
2525
***************************************/
2626

27-
static void
28-
ZSTD_updateDUBT(ZSTD_matchState_t* ms,
27+
static
28+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
29+
void ZSTD_updateDUBT(ZSTD_matchState_t* ms,
2930
const BYTE* ip, const BYTE* iend,
3031
U32 mls)
3132
{
@@ -68,8 +69,9 @@ ZSTD_updateDUBT(ZSTD_matchState_t* ms,
6869
* sort one already inserted but unsorted position
6970
* assumption : curr >= btlow == (curr - btmask)
7071
* doesn't fail */
71-
static void
72-
ZSTD_insertDUBT1(const ZSTD_matchState_t* ms,
72+
static
73+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
74+
void ZSTD_insertDUBT1(const ZSTD_matchState_t* ms,
7375
U32 curr, const BYTE* inputEnd,
7476
U32 nbCompares, U32 btLow,
7577
const ZSTD_dictMode_e dictMode)
@@ -157,8 +159,9 @@ ZSTD_insertDUBT1(const ZSTD_matchState_t* ms,
157159
}
158160

159161

160-
static size_t
161-
ZSTD_DUBT_findBetterDictMatch (
162+
static
163+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
164+
size_t ZSTD_DUBT_findBetterDictMatch (
162165
const ZSTD_matchState_t* ms,
163166
const BYTE* const ip, const BYTE* const iend,
164167
size_t* offsetPtr,
@@ -235,8 +238,9 @@ ZSTD_DUBT_findBetterDictMatch (
235238
}
236239

237240

238-
static size_t
239-
ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms,
241+
static
242+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
243+
size_t ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms,
240244
const BYTE* const ip, const BYTE* const iend,
241245
size_t* offBasePtr,
242246
U32 const mls,
@@ -386,8 +390,9 @@ ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms,
386390

387391

388392
/** ZSTD_BtFindBestMatch() : Tree updater, providing best match */
389-
FORCE_INLINE_TEMPLATE size_t
390-
ZSTD_BtFindBestMatch( ZSTD_matchState_t* ms,
393+
FORCE_INLINE_TEMPLATE
394+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
395+
size_t ZSTD_BtFindBestMatch( ZSTD_matchState_t* ms,
391396
const BYTE* const ip, const BYTE* const iLimit,
392397
size_t* offBasePtr,
393398
const U32 mls /* template */,
@@ -622,7 +627,9 @@ size_t ZSTD_dedicatedDictSearch_lazy_search(size_t* offsetPtr, size_t ml, U32 nb
622627

623628
/* Update chains up to ip (excluded)
624629
Assumption : always within prefix (i.e. not within extDict) */
625-
FORCE_INLINE_TEMPLATE U32 ZSTD_insertAndFindFirstIndex_internal(
630+
FORCE_INLINE_TEMPLATE
631+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
632+
U32 ZSTD_insertAndFindFirstIndex_internal(
626633
ZSTD_matchState_t* ms,
627634
const ZSTD_compressionParameters* const cParams,
628635
const BYTE* ip, U32 const mls, U32 const lazySkipping)
@@ -656,6 +663,7 @@ U32 ZSTD_insertAndFindFirstIndex(ZSTD_matchState_t* ms, const BYTE* ip) {
656663

657664
/* inlining is important to hardwire a hot branch (template emulation) */
658665
FORCE_INLINE_TEMPLATE
666+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
659667
size_t ZSTD_HcFindBestMatch(
660668
ZSTD_matchState_t* ms,
661669
const BYTE* const ip, const BYTE* const iLimit,
@@ -824,7 +832,9 @@ FORCE_INLINE_TEMPLATE void ZSTD_row_prefetch(U32 const* hashTable, BYTE const* t
824832
* Fill up the hash cache starting at idx, prefetching up to ZSTD_ROW_HASH_CACHE_SIZE entries,
825833
* but not beyond iLimit.
826834
*/
827-
FORCE_INLINE_TEMPLATE void ZSTD_row_fillHashCache(ZSTD_matchState_t* ms, const BYTE* base,
835+
FORCE_INLINE_TEMPLATE
836+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
837+
void ZSTD_row_fillHashCache(ZSTD_matchState_t* ms, const BYTE* base,
828838
U32 const rowLog, U32 const mls,
829839
U32 idx, const BYTE* const iLimit)
830840
{
@@ -850,7 +860,9 @@ FORCE_INLINE_TEMPLATE void ZSTD_row_fillHashCache(ZSTD_matchState_t* ms, const B
850860
* Returns the hash of base + idx, and replaces the hash in the hash cache with the byte at
851861
* base + idx + ZSTD_ROW_HASH_CACHE_SIZE. Also prefetches the appropriate rows from hashTable and tagTable.
852862
*/
853-
FORCE_INLINE_TEMPLATE U32 ZSTD_row_nextCachedHash(U32* cache, U32 const* hashTable,
863+
FORCE_INLINE_TEMPLATE
864+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
865+
U32 ZSTD_row_nextCachedHash(U32* cache, U32 const* hashTable,
854866
BYTE const* tagTable, BYTE const* base,
855867
U32 idx, U32 const hashLog,
856868
U32 const rowLog, U32 const mls,
@@ -868,10 +880,12 @@ FORCE_INLINE_TEMPLATE U32 ZSTD_row_nextCachedHash(U32* cache, U32 const* hashTab
868880
/* ZSTD_row_update_internalImpl():
869881
* Updates the hash table with positions starting from updateStartIdx until updateEndIdx.
870882
*/
871-
FORCE_INLINE_TEMPLATE void ZSTD_row_update_internalImpl(ZSTD_matchState_t* ms,
872-
U32 updateStartIdx, U32 const updateEndIdx,
873-
U32 const mls, U32 const rowLog,
874-
U32 const rowMask, U32 const useCache)
883+
FORCE_INLINE_TEMPLATE
884+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
885+
void ZSTD_row_update_internalImpl(ZSTD_matchState_t* ms,
886+
U32 updateStartIdx, U32 const updateEndIdx,
887+
U32 const mls, U32 const rowLog,
888+
U32 const rowMask, U32 const useCache)
875889
{
876890
U32* const hashTable = ms->hashTable;
877891
BYTE* const tagTable = ms->tagTable;
@@ -897,9 +911,11 @@ FORCE_INLINE_TEMPLATE void ZSTD_row_update_internalImpl(ZSTD_matchState_t* ms,
897911
* Inserts the byte at ip into the appropriate position in the hash table, and updates ms->nextToUpdate.
898912
* Skips sections of long matches as is necessary.
899913
*/
900-
FORCE_INLINE_TEMPLATE void ZSTD_row_update_internal(ZSTD_matchState_t* ms, const BYTE* ip,
901-
U32 const mls, U32 const rowLog,
902-
U32 const rowMask, U32 const useCache)
914+
FORCE_INLINE_TEMPLATE
915+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
916+
void ZSTD_row_update_internal(ZSTD_matchState_t* ms, const BYTE* ip,
917+
U32 const mls, U32 const rowLog,
918+
U32 const rowMask, U32 const useCache)
903919
{
904920
U32 idx = ms->nextToUpdate;
905921
const BYTE* const base = ms->window.base;
@@ -1121,6 +1137,7 @@ ZSTD_row_getMatchMask(const BYTE* const tagRow, const BYTE tag, const U32 headGr
11211137
* - Pick the longest match.
11221138
*/
11231139
FORCE_INLINE_TEMPLATE
1140+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
11241141
size_t ZSTD_RowFindBestMatch(
11251142
ZSTD_matchState_t* ms,
11261143
const BYTE* const ip, const BYTE* const iLimit,
@@ -1494,8 +1511,9 @@ FORCE_INLINE_TEMPLATE size_t ZSTD_searchMax(
14941511
* Common parser - lazy strategy
14951512
*********************************/
14961513

1497-
FORCE_INLINE_TEMPLATE size_t
1498-
ZSTD_compressBlock_lazy_generic(
1514+
FORCE_INLINE_TEMPLATE
1515+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
1516+
size_t ZSTD_compressBlock_lazy_generic(
14991517
ZSTD_matchState_t* ms, seqStore_t* seqStore,
15001518
U32 rep[ZSTD_REP_NUM],
15011519
const void* src, size_t srcSize,
@@ -1915,6 +1933,7 @@ size_t ZSTD_compressBlock_btlazy2_dictMatchState(
19151933
|| !defined(ZSTD_EXCLUDE_LAZY2_BLOCK_COMPRESSOR) \
19161934
|| !defined(ZSTD_EXCLUDE_BTLAZY2_BLOCK_COMPRESSOR)
19171935
FORCE_INLINE_TEMPLATE
1936+
ZSTD_ALLOW_POINTER_OVERFLOW_ATTR
19181937
size_t ZSTD_compressBlock_lazy_extDict_generic(
19191938
ZSTD_matchState_t* ms, seqStore_t* seqStore,
19201939
U32 rep[ZSTD_REP_NUM],

0 commit comments

Comments
 (0)