[RFC PATCH] dm-pcache: fix use-after-free and invalid seg operations in kset_replay()

jianyungao89@gmail.com posted 1 patch 4 days, 23 hours ago
drivers/md/dm-pcache/cache_key.c | 13 ++++++-------
1 file changed, 6 insertions(+), 7 deletions(-)
[RFC PATCH] dm-pcache: fix use-after-free and invalid seg operations in kset_replay()
Posted by jianyungao89@gmail.com 4 days, 23 hours ago
From: Jianyun Gao <jianyungao89@gmail.com>

In kset_replay, when key->seg_gen is stale (key->seg_gen <
key->cache_pos.cache_seg->gen), cache_key_put(key) is called but then
key->cache_pos.cache_seg is accessed as the argument to cache_seg_get().
This is a use-after-free on the freed key memory. Although mempool
recycled memory is not immediately reclaimed or overwritten in practice,
this is still a potential UAF bug.

Additionally, for expired invalid keys, setting the cache->seg_map bit
and calling cache_seg_get() is unreasonable since the corresponding
segment data is no longer valid.

Fix both issues by moving cache_seg_get() and __set_bit() after the
gen check, so they only execute for valid keys, and using continue to
skip invalid keys.

Signed-off-by: Jianyun Gao <jianyungao89@gmail.com>
---
 drivers/md/dm-pcache/cache_key.c | 13 ++++++-------
 1 file changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/md/dm-pcache/cache_key.c b/drivers/md/dm-pcache/cache_key.c
index e068e878231b..84bad99d3fcd 100644
--- a/drivers/md/dm-pcache/cache_key.c
+++ b/drivers/md/dm-pcache/cache_key.c
@@ -728,18 +728,17 @@ static int kset_replay(struct pcache_cache *cache, struct pcache_cache_kset_onme
 			goto err;
 		}
 
-		__set_bit(key->cache_pos.cache_seg->cache_seg_id, cache->seg_map);
-
 		/* Check if the segment generation is valid for insertion. */
 		if (key->seg_gen < key->cache_pos.cache_seg->gen) {
 			cache_key_put(key);
-		} else {
-			cache_subtree = get_subtree(&cache->req_key_tree, key->off);
-			spin_lock(&cache_subtree->tree_lock);
-			cache_key_insert(&cache->req_key_tree, key, true);
-			spin_unlock(&cache_subtree->tree_lock);
+			continue;
 		}
 
+		__set_bit(key->cache_pos.cache_seg->cache_seg_id, cache->seg_map);
+		cache_subtree = get_subtree(&cache->req_key_tree, key->off);
+		spin_lock(&cache_subtree->tree_lock);
+		cache_key_insert(&cache->req_key_tree, key, true);
+		spin_unlock(&cache_subtree->tree_lock);
 		cache_seg_get(key->cache_pos.cache_seg);
 	}
 
-- 
2.34.1