]> git.ipfire.org Git - thirdparty/zstd.git/commitdiff
[zstdmt] Fix MSAN failure with ZSTD_p_forceWindow
authorNick Terrell <terrelln@fb.com>
Tue, 14 Feb 2017 02:27:34 +0000 (18:27 -0800)
committerNick Terrell <terrelln@fb.com>
Tue, 14 Feb 2017 03:11:22 +0000 (19:11 -0800)
Reproduction steps:

```
make zstreamtest CC=clang CFLAGS="-O3 -g -fsanitize=memory -fsanitize-memory-track-origins"
./zstreamtest -vv -t4178 -i4178 -s4531
```

How to get to the error in gdb (may be a more efficient way):

* 2 breaks at zstd_compress.c:2418  -- in ZSTD_compressContinue_internal()
* 2 breaks at zstd_compress.c:2276  -- in ZSTD_compressBlock_internal()
* 1 break at zstd_compress.c:1547

Why the error occurred:

When `zc->forceWindow == 1`, after calling `ZSTD_loadDictionaryContent()` we
have `zc->loadedDictEnd == zc->nextToUpdate == 0`. But, we've really loaded up
to `iend` into the dictionary. Then in `ZSTD_compressBlock_internal()` we see
that `current > zc->nextToUpdate + 384`, so we load the last 192 bytes a second
time. In this case the bytes we are loading are a block of all 0s, starting in
the previous block. So when we are loading the last 192 bytes, we find a `match`
in the future, 183 bytes beyond `ip`. Since the block is all 0s, the match
extends to the end of the block. But in `ZSTD_count()` we only check that
`pIn < pInLoopLimit`, but since `pMatch > pIn`, `pMatch` eventually points past
the end of the buffer, causing the MSAN failure.

The fix:

The line changed sets sets `zc->nextToUpdate` to the end of the dictionary.
This is the behavior that existed before `ZSTD_p_forceWindow` was introduced.
This fixes the exposing test case. Since the code doesn't fail without
`zc->forceWindow`, it makes sense that this works. I've run the command
`./zstreamtest -T2mn` 64 times without failures. CI should also verify nothing
obvious broke.

lib/compress/zstd_compress.c

index 765c8e34dece000a151bdcfcaf29ab4e072daa24..91e81d9c236512e622cb92a6ebaa9378205ee4c2 100644 (file)
@@ -2512,7 +2512,7 @@ static size_t ZSTD_loadDictionaryContent(ZSTD_CCtx* zc, const void* src, size_t
         return ERROR(GENERIC);   /* strategy doesn't exist; impossible */
     }
 
-    zc->nextToUpdate = zc->loadedDictEnd;
+    zc->nextToUpdate = (U32)(iend - zc->base);
     return 0;
 }