[PATCH 3/3] dm-pcache: validate kset key_num and intra-segment bounds

From: Bryam Vargas <[email protected]>

Two more fields decoded from the cache device go unbounded. The kset
key_num drives cache_kset_crc() and the replay loop in cache_replay(),
the writeback worker and the GC worker, but only the magic and a
fixed-seed CRC are checked first, so a non-last kset whose key_num exceeds
the PCACHE_KSET_KEYS_MAX buffer reads past its end before the CRC compare.
A key's intra-segment offset and length in cache_key_decode() are taken
verbatim, so a key running past its segment is replayed into the cache
tree and the data CRC check and every later read hit then copy adjacent
persistent memory into the caller's bio -- an out-of-bounds read that
leaks to user space. Both fields are controlled by whoever supplies the
cache device (CAP_SYS_ADMIN); the CRC seed is public.

Add kset_onmedia_valid() to bound key_num before any kset read, and
reject a key whose offset plus length, computed in 64 bits, exceeds the
segment data_size. Valid metadata is unaffected.

Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
Cc: [email protected]
Signed-off-by: Bryam Vargas <[email protected]>
---
 drivers/md/dm-pcache/cache.h           | 21 +++++++++++++++++++++
 drivers/md/dm-pcache/cache_gc.c        |  8 ++++----
 drivers/md/dm-pcache/cache_key.c       | 10 +++++++++-
 drivers/md/dm-pcache/cache_writeback.c |  8 ++++----
 4 files changed, 38 insertions(+), 9 deletions(-)

diff --git a/drivers/md/dm-pcache/cache.h b/drivers/md/dm-pcache/cache.h
index 653ceea43131..d470e457b23f 100644
--- a/drivers/md/dm-pcache/cache.h
+++ b/drivers/md/dm-pcache/cache.h
@@ -505,6 +505,27 @@ static inline u32 cache_key_data_crc(struct pcache_cache_key *key)
 	return crc32c(PCACHE_CRC_SEED, data, key->len);
 }
 
+/**
+ * kset_onmedia_valid - Validate a kset header read from the cache device.
+ * @kset_onmedia: Pointer to the kset copied from on-media metadata.
+ *
+ * The magic and CRC are attacker-computable (fixed public seed). A non-last
+ * kset stores key_num keys inline, and cache_kset_crc() and the replay loop
+ * read struct_size(.., data, key_num) bytes from a buffer sized for
+ * PCACHE_KSET_KEYS_MAX keys, so key_num must be bounded before any such use.
+ */
+static inline bool kset_onmedia_valid(struct pcache_cache_kset_onmedia *kset_onmedia)
+{
+	if (kset_onmedia->magic != PCACHE_KSET_MAGIC)
+		return false;
+
+	if (!(kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) &&
+	    kset_onmedia->key_num > PCACHE_KSET_KEYS_MAX)
+		return false;
+
+	return true;
+}
+
 static inline u32 cache_kset_crc(struct pcache_cache_kset_onmedia *kset_onmedia)
 {
 	u32 crc_size;
diff --git a/drivers/md/dm-pcache/cache_gc.c b/drivers/md/dm-pcache/cache_gc.c
index 02fa0ce03134..1ed513745023 100644
--- a/drivers/md/dm-pcache/cache_gc.c
+++ b/drivers/md/dm-pcache/cache_gc.c
@@ -44,11 +44,11 @@ static bool need_gc(struct pcache_cache *cache, struct pcache_cache_pos *dirty_t
 		return false;
 	}
 
-	/* Check if kset_onmedia is corrupted */
-	if (kset_onmedia->magic != PCACHE_KSET_MAGIC) {
-		pcache_dev_debug(pcache, "gc error: magic is not as expected. key_tail: %u:%u magic: %llx, expected: %llx\n",
+	/* Reject a corrupted or out-of-bounds kset before reading its keys */
+	if (!kset_onmedia_valid(kset_onmedia)) {
+		pcache_dev_debug(pcache, "gc error: invalid kset. key_tail: %u:%u magic: %llx, key_num: %u\n",
 					key_tail->cache_seg->cache_seg_id, key_tail->seg_off,
-					kset_onmedia->magic, PCACHE_KSET_MAGIC);
+					kset_onmedia->magic, kset_onmedia->key_num);
 		return false;
 	}
 
diff --git a/drivers/md/dm-pcache/cache_key.c b/drivers/md/dm-pcache/cache_key.c
index 8eec5238c5da..f4459b2e1b3b 100644
--- a/drivers/md/dm-pcache/cache_key.c
+++ b/drivers/md/dm-pcache/cache_key.c
@@ -103,6 +103,14 @@ int cache_key_decode(struct pcache_cache *cache,
 	key->cache_pos.cache_seg = &cache->segments[key_onmedia->cache_seg_id];
 	key->cache_pos.seg_off = key_onmedia->cache_seg_off;
 
+	if ((u64)key->cache_pos.seg_off + key->len >
+			key->cache_pos.cache_seg->segment.data_size) {
+		pcache_dev_err(pcache, "key seg_off %u + len %u exceeds segment data size %u\n",
+				key->cache_pos.seg_off, key->len,
+				key->cache_pos.cache_seg->segment.data_size);
+		return -EIO;
+	}
+
 	key->seg_gen = key_onmedia->seg_gen;
 	key->flags = key_onmedia->flags;
 
@@ -784,7 +792,7 @@ int cache_replay(struct pcache_cache *cache)
 			goto out;
 		}
 
-		if (kset_onmedia->magic != PCACHE_KSET_MAGIC ||
+		if (!kset_onmedia_valid(kset_onmedia) ||
 				kset_onmedia->crc != cache_kset_crc(kset_onmedia)) {
 			break;
 		}
diff --git a/drivers/md/dm-pcache/cache_writeback.c b/drivers/md/dm-pcache/cache_writeback.c
index a104e6ee8aa8..f96657a1dc1a 100644
--- a/drivers/md/dm-pcache/cache_writeback.c
+++ b/drivers/md/dm-pcache/cache_writeback.c
@@ -55,11 +55,11 @@ static inline bool is_cache_clean(struct pcache_cache *cache, struct pcache_cach
 		return true;
 	}
 
-	/* Check if the magic number matches the expected value */
-	if (kset_onmedia->magic != PCACHE_KSET_MAGIC) {
-		pcache_dev_debug(pcache, "dirty_tail: %u:%u magic: %llx, not expected: %llx\n",
+	/* Reject a corrupted or out-of-bounds kset before reading its keys */
+	if (!kset_onmedia_valid(kset_onmedia)) {
+		pcache_dev_debug(pcache, "dirty_tail: %u:%u invalid kset magic: %llx, key_num: %u\n",
 				dirty_tail->cache_seg->cache_seg_id, dirty_tail->seg_off,
-				kset_onmedia->magic, PCACHE_KSET_MAGIC);
+				kset_onmedia->magic, kset_onmedia->key_num);
 		return true;
 	}
 

-- 
2.43.0