[trafficserver] branch master updated: clang analyzer: Fix false positive "use after free" in IpMap.cc

[bc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

bcall pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 60b8510  clang analyzer: Fix false positive "use after free" in IpMap.cc
60b8510 is described below

commit 60b8510b033c30d0b9b77a359602018868a7e85c
Author: Alan M. Carroll <am...@apache.org>
AuthorDate: Fri May 17 08:03:46 2019 -0500

    clang analyzer: Fix false positive "use after free" in IpMap.cc
---
 src/tscore/IpMap.cc | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/src/tscore/IpMap.cc b/src/tscore/IpMap.cc
index 87ab278..c24445f 100644
--- a/src/tscore/IpMap.cc
+++ b/src/tscore/IpMap.cc
@@ -368,23 +368,23 @@ namespace detail
     // Work through the rest of the nodes of interest.
     // Invariant: n->_min >= min
 
-    // Careful here -- because max_plus1 might wrap we need to use it only
-    // if we can certain it didn't. This is done by ordering the range
-    // tests so that when max_plus1 is used when we know there exists a
-    // larger value than max.
+    // Careful here -- because max_plus1 might wrap we need to use it only if we can be certain it
+    // didn't. This is done by ordering the range tests so that when max_plus1 is used when we know
+    // there exists a larger value than max.
     Metric max_plus1 = max;
     N::inc(max_plus1);
+
     /* Notes:
-       - max (and thence max_plus1) never change during the loop.
-       - we must have either x != 0 or adjust min but not both.
+       - max (and thence also max_plus1) never change during the loop.
+       - we must have either x != 0 or adjust min but not both for each loop iteration.
     */
     while (n) {
       if (n->_data == payload) {
         if (x) {
-          if (n->_max <= max) {
-// next range is covered, so we can remove and continue.
+          if (n->_max <= max) { // next range is covered, so we can remove and continue.
 #if defined(__clang_analyzer__)
-            ink_assert(x != n);
+            x->_next = n->_next; // done in @c remove, but CA doesn't realize that.
+                                 // It's insufficient to assert(x->_next != n) after the remove.
 #endif
             this->remove(n);
             n = next(x);