[binary-tree] Fix underflow of nbCompares

Fix underflow of `nbCompares` by switching to an `int` and comparing
`nbCompares > 0`. This is a minimal fix, because I don't want to change
the logic. These loops seem to be doing `nbCompares + 1` comparisons.

The bug was reported by Dan Carpenter and found by Smatch static
checker.

https://lore.kernel.org/all/20211008063704.GA5370@kili/

diff --git a/lib/compress/zstd_lazy.c b/lib/compress/zstd_lazy.c
index c5fe6dd47a7cd028e6c14225b0fa0b40940393a5..c71a2c9e6f5f9042c1ede057e6c2d13c5461e1af 100644 (file)
--- a/lib/compress/zstd_lazy.c
+++ b/lib/compress/zstd_lazy.c
@@ -93,7 +93,7 @@ ZSTD_insertDUBT1(ZSTD_matchState_t* ms,
     assert(curr >= btLow);
     assert(ip < iend);   /* condition for ZSTD_count */
 
-    while (nbCompares-- && (matchIndex > windowLow)) {
+    for (; nbCompares && (matchIndex > windowLow); --nbCompares) {
         U32* const nextPtr = bt + 2*(matchIndex & btMask);
         size_t matchLength = MIN(commonLengthSmaller, commonLengthLarger);   /* guaranteed minimum nb of common bytes */
         assert(matchIndex < curr);
@@ -185,7 +185,7 @@ ZSTD_DUBT_findBetterDictMatch (
     (void)dictMode;
     assert(dictMode == ZSTD_dictMatchState);
 
-    while (nbCompares-- && (dictMatchIndex > dictLowLimit)) {
+    for (; nbCompares && (dictMatchIndex > dictLowLimit); --nbCompares) {
         U32* const nextPtr = dictBt + 2*(dictMatchIndex & btMask);
         size_t matchLength = MIN(commonLengthSmaller, commonLengthLarger);   /* guaranteed minimum nb of common bytes */
         const BYTE* match = dictBase + dictMatchIndex;
@@ -309,7 +309,7 @@ ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms,
         matchIndex  = hashTable[h];
         hashTable[h] = curr;   /* Update Hash Table */
 
-        while (nbCompares-- && (matchIndex > windowLow)) {
+        for (; nbCompares && (matchIndex > windowLow); --nbCompares) {
             U32* const nextPtr = bt + 2*(matchIndex & btMask);
             size_t matchLength = MIN(commonLengthSmaller, commonLengthLarger);   /* guaranteed minimum nb of common bytes */
             const BYTE* match;
@@ -357,6 +357,7 @@ ZSTD_DUBT_findBestMatch(ZSTD_matchState_t* ms,
 
         *smallerPtr = *largerPtr = 0;
 
+        assert(nbCompares <= (1U << ZSTD_SEARCHLOG_MAX)); /* Check we haven't underflowed. */
         if (dictMode == ZSTD_dictMatchState && nbCompares) {
             bestLength = ZSTD_DUBT_findBetterDictMatch(
                     ms, ip, iend,
@@ -660,6 +661,7 @@ size_t ZSTD_HcFindBestMatch_generic (
         matchIndex = NEXT_IN_CHAIN(matchIndex, chainMask);
     }
 
+    assert(nbAttempts <= (1U << ZSTD_SEARCHLOG_MAX)); /* Check we haven't underflowed. */
     if (dictMode == ZSTD_dedicatedDictSearch) {
         const U32 ddsLowestIndex  = dms->window.dictLimit;
         const BYTE* const ddsBase = dms->window.base;

diff --git a/lib/compress/zstd_opt.c b/lib/compress/zstd_opt.c
index 402a7e5c76b2e401ed5c7a1f084838de7dddcc93..d9fbfef1f2576073bb3892d125779ab696cd5d0c 100644 (file)
--- a/lib/compress/zstd_opt.c
+++ b/lib/compress/zstd_opt.c
@@ -408,7 +408,7 @@ static U32 ZSTD_insertBt1(
     hashTable[h] = curr;   /* Update Hash Table */
 
     assert(windowLow > 0);
-    while (nbCompares-- && (matchIndex >= windowLow)) {
+    for (; nbCompares && (matchIndex >= windowLow); --nbCompares) {
         U32* const nextPtr = bt + 2*(matchIndex & btMask);
         size_t matchLength = MIN(commonLengthSmaller, commonLengthLarger);   /* guaranteed minimum nb of common bytes */
         assert(matchIndex < curr);
@@ -639,7 +639,7 @@ U32 ZSTD_insertBtAndGetAllMatches (
 
     hashTable[h] = curr;   /* Update Hash Table */
 
-    while (nbCompares-- && (matchIndex >= matchLow)) {
+    for (; nbCompares && (matchIndex >= matchLow); --nbCompares) {
         U32* const nextPtr = bt + 2*(matchIndex & btMask);
         const BYTE* match;
         size_t matchLength = MIN(commonLengthSmaller, commonLengthLarger);   /* guaranteed minimum nb of common bytes */
@@ -692,12 +692,13 @@ U32 ZSTD_insertBtAndGetAllMatches (
 
     *smallerPtr = *largerPtr = 0;
 
+    assert(nbCompares <= (1U << ZSTD_SEARCHLOG_MAX)); /* Check we haven't underflowed. */
     if (dictMode == ZSTD_dictMatchState && nbCompares) {
         size_t const dmsH = ZSTD_hashPtr(ip, dmsHashLog, mls);
         U32 dictMatchIndex = dms->hashTable[dmsH];
         const U32* const dmsBt = dms->chainTable;
         commonLengthSmaller = commonLengthLarger = 0;
-        while (nbCompares-- && (dictMatchIndex > dmsLowLimit)) {
+        for (; nbCompares && (dictMatchIndex > dmsLowLimit); --nbCompares) {
             const U32* const nextPtr = dmsBt + 2*(dictMatchIndex & dmsBtMask);
             size_t matchLength = MIN(commonLengthSmaller, commonLengthLarger);   /* guaranteed minimum nb of common bytes */
             const BYTE* match = dmsBase + dictMatchIndex;

diff --git a/tests/regression/results.csv b/tests/regression/results.csv
index 0fb1d160a6fc4978f3103096caf7ef7ba716f24f..f0edea2229fd2d59a659148fa58dd566dacc9c05 100644 (file)
--- a/tests/regression/results.csv
+++ b/tests/regression/results.csv
@@ -171,7 +171,7 @@ github,                             level 7 with dict,                  zstdcli,
 github,                             level 9,                            zstdcli,                            137122
 github,                             level 9 with dict,                  zstdcli,                            41332
 github,                             level 13,                           zstdcli,                            136064
-github,                             level 13 with dict,                 zstdcli,                            41743
+github,                             level 13 with dict,                 zstdcli,                            41900
 github,                             level 16,                           zstdcli,                            136064
 github,                             level 16 with dict,                 zstdcli,                            39577
 github,                             level 19,                           zstdcli,                            136064
@@ -303,7 +303,7 @@ github,                             level 7 with dict,                  advanced
 github,                             level 9,                            advanced one pass,                  135122
 github,                             level 9 with dict,                  advanced one pass,                  39332
 github,                             level 13,                           advanced one pass,                  134064
-github,                             level 13 with dict,                 advanced one pass,                  39743
+github,                             level 13 with dict,                 advanced one pass,                  39900
 github,                             level 16,                           advanced one pass,                  134064
 github,                             level 16 with dict,                 advanced one pass,                  37577
 github,                             level 19,                           advanced one pass,                  134064
@@ -437,7 +437,7 @@ github,                             level 7 with dict,                  advanced
 github,                             level 9,                            advanced one pass small out,        135122
 github,                             level 9 with dict,                  advanced one pass small out,        39332
 github,                             level 13,                           advanced one pass small out,        134064
-github,                             level 13 with dict,                 advanced one pass small out,        39743
+github,                             level 13 with dict,                 advanced one pass small out,        39900
 github,                             level 16,                           advanced one pass small out,        134064
 github,                             level 16 with dict,                 advanced one pass small out,        37577
 github,                             level 19,                           advanced one pass small out,        134064
@@ -571,7 +571,7 @@ github,                             level 7 with dict,                  advanced
 github,                             level 9,                            advanced streaming,                 135122
 github,                             level 9 with dict,                  advanced streaming,                 39332
 github,                             level 13,                           advanced streaming,                 134064
-github,                             level 13 with dict,                 advanced streaming,                 39743
+github,                             level 13 with dict,                 advanced streaming,                 39900
 github,                             level 16,                           advanced streaming,                 134064
 github,                             level 16 with dict,                 advanced streaming,                 37577
 github,                             level 19,                           advanced streaming,                 134064
@@ -689,7 +689,7 @@ github,                             level 7 with dict,                  old stre
 github,                             level 9,                            old streaming,                      135122
 github,                             level 9 with dict,                  old streaming,                      39332
 github,                             level 13,                           old streaming,                      134064
-github,                             level 13 with dict,                 old streaming,                      39743
+github,                             level 13 with dict,                 old streaming,                      39900
 github,                             level 16,                           old streaming,                      134064
 github,                             level 16 with dict,                 old streaming,                      37577
 github,                             level 19,                           old streaming,                      134064
@@ -877,7 +877,7 @@ github,                             level 5 with dict,                  old stre
 github,                             level 6 with dict,                  old streaming cdcit,                38632
 github,                             level 7 with dict,                  old streaming cdcit,                38771
 github,                             level 9 with dict,                  old streaming cdcit,                39332
-github,                             level 13 with dict,                 old streaming cdcit,                39743
+github,                             level 13 with dict,                 old streaming cdcit,                39900
 github,                             level 16 with dict,                 old streaming cdcit,                37577
 github,                             level 19 with dict,                 old streaming cdcit,                37576
 github,                             no source size with dict,           old streaming cdcit,                40654