(kudu) branch branch-1.18.x updated: KUDU-613: Fix heap-use-after-free bug

[alexey]

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

alexey pushed a commit to branch branch-1.18.x
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/branch-1.18.x by this push:
     new 484ff26e0 KUDU-613: Fix heap-use-after-free bug
484ff26e0 is described below

commit 484ff26e0d496be849dbd848d58e37d92fdb98f6
Author: Mahesh Reddy <mre...@cloudera.com>
AuthorDate: Tue Jan 14 17:56:03 2025 -0500

    KUDU-613: Fix heap-use-after-free bug
    
    The last patch submitted to make the segment-level
    metrics more relevant introduced a bug where an
    entry was accessed after it was deleted. This patch
    fixes that behavior by accessing the entry before
    it is deleted.
    
    Change-Id: I8f91d0984b017861ded5f1bd30fddfa29588f9aa
    Reviewed-on: http://gerrit.cloudera.org:8080/22343
    Reviewed-by: Alexey Serbin <ale...@apache.org>
    Reviewed-by: Abhishek Chennaka <achenn...@cloudera.com>
    Tested-by: Alexey Serbin <ale...@apache.org>
    (cherry picked from commit 75b75ca2f58f1a5aba69207830e3558011033d99)
    Reviewed-on: http://gerrit.cloudera.org:8080/22352
---
 src/kudu/util/slru_cache.cc | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/src/kudu/util/slru_cache.cc b/src/kudu/util/slru_cache.cc
index c77901d08..c12746bef 100644
--- a/src/kudu/util/slru_cache.cc
+++ b/src/kudu/util/slru_cache.cc
@@ -229,16 +229,15 @@ Handle* 
SLRUCacheShard<Segment::kProbationary>::Insert(SLRUHandle* handle,
   SLRUHandle* old_entry = table_.Insert(handle);
   // If entry with key already exists, remove it.
   if (old_entry != nullptr) {
+    // In the update case, a new entry is inserted, but it should retain the 
same values for the
+    // fields 'upgrades' and 'downgrades' as the old entry.
+    handle->upgrades = old_entry->upgrades;
+    handle->downgrades = old_entry->downgrades;
     RL_Remove(old_entry);
     if (Unref(old_entry)) {
       old_entry->next = *free_entries;
       *free_entries = old_entry;
     }
-
-    // In the update case, a new entry is inserted, but it should retain the 
same values for the
-    // fields 'upgrades' and 'downgrades' as the old entry.
-    handle->upgrades = old_entry->upgrades;
-    handle->downgrades = old_entry->downgrades;
   }
   RemoveEntriesPastCapacity(free_entries);
 
@@ -269,17 +268,16 @@ Handle* 
SLRUCacheShard<Segment::kProtected>::Insert(SLRUHandle* handle,
   // Update case so Insert should return a non-null entry.
   SLRUHandle* old_entry = table_.Insert(handle);
   DCHECK(old_entry != nullptr);
+  // In the update case, a new entry is inserted, but it should retain the 
same values for the
+  // fields 'upgrades' and 'downgrades' as the old entry.
+  handle->upgrades = old_entry->upgrades;
+  handle->downgrades = old_entry->downgrades;
   RL_Remove(old_entry);
   if (Unref(old_entry)) {
     old_entry->next = *free_entries;
     *free_entries = old_entry;
   }
 
-  // In the update case, a new entry is inserted, but it should retain the 
same values for the
-  // fields 'upgrades' and 'downgrades' as the old entry.
-  handle->upgrades = old_entry->upgrades;
-  handle->downgrades = old_entry->downgrades;
-
   return reinterpret_cast<Handle*>(handle);
 }