[PATCH 0/3] dm-pcache: validate on-disk fields before use

[PATCH 0/3] dm-pcache: validate on-disk fields before use

[Bryam Vargas via B4 Relay]

dm-pcache reads its layout from the cache device and mostly trusts it. The
table load needs CAP_SYS_ADMIN, but the on-media metadata is only self-checked
with a CRC32C over a fixed, public seed, so anyone who can format or swap the
cache device forges the fields and recomputes the CRC. Several of them become
segment ids, array indices, loop counts and intra-segment extents with no bound
against the device geometry, which turns crafted metadata into kernel
out-of-bounds accesses, both at table load and during normal cache I/O.

Each patch adds the bound where the field is decoded:

  1/3  segment ids that index cache->segments[] -- bound against cache->n_segs
       (cache_pos_decode, cache_key_decode, and the last-kset paths in replay,
       writeback and gc).
  2/3  cache_info->n_segs and the segment-chain ids -- bound against the cache
       device segment count.
  3/3  the kset key_num, read by cache_kset_crc() and the replay loop before the
       CRC is even compared, and a key whose offset+length runs past its segment
       -- the latter is an OOB read that otherwise reaches user space on a cache
       read hit.

I found these by walking the cache-device decode paths. Each class reproduces in a
faithful in-kernel model under KASAN: the bug arm faults (slab out-of-bounds -- a
read for the seg_id, geometry and key_num/extent cases, a write for the n_segs
loop), the arm with the added bound is clean, and valid metadata is clean. The key
offset/length case also reproduces under KMSAN, where the over-read returns
uninitialized kernel memory. Reproducer available on request. Each check rejects
bad metadata with -EIO and leaves valid metadata alone.

checkpatch --strict is clean apart from "Alignment should match open
parenthesis" on the multi-line pcache_dev_err() calls, which match the style
already in the driver.

---
Bryam Vargas (3):
      dm-pcache: validate seg_id fields from persistent memory
      dm-pcache: validate geometry fields from on-disk cache_info
      dm-pcache: validate kset key_num and intra-segment bounds

 drivers/md/dm-pcache/cache.c           | 17 +++++++++++++++++
 drivers/md/dm-pcache/cache.h           | 35 ++++++++++++++++++++++++++++++++++
 drivers/md/dm-pcache/cache_gc.c        | 24 +++++++++++++++++------
 drivers/md/dm-pcache/cache_key.c       | 21 +++++++++++++++++++-
 drivers/md/dm-pcache/cache_writeback.c | 22 ++++++++++++++-------
 5 files changed, 105 insertions(+), 14 deletions(-)
---
base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
change-id: 20260619-b4-disp-997773a1-69ada10d3a3e

Best regards,
-- 
bryamzxz <hexlabsecurity@proton.me>

[PATCH 1/3] dm-pcache: validate seg_id fields from persistent memory

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

cache_pos_decode(), cache_key_decode() and the last-kset branches of
cache_replay(), the writeback worker and the GC worker take a cache
segment id from the cache device metadata and index cache->segments[]
with it without checking it against cache->n_segs. That metadata is only
CRC-protected with a fixed public seed, so whoever supplies the cache
device on a table load (CAP_SYS_ADMIN) controls the id; an out-of-range
value forms a wild pcache_cache_segment pointer that is dereferenced and
written through -- an out-of-bounds read and write driven by on-disk data.

Add cache_seg_id_valid() and reject an out-of-range id at each decode
site, failing the operation with -EIO instead of indexing past the array.
Valid metadata is unaffected.

Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/md/dm-pcache/cache.c           |  3 +++
 drivers/md/dm-pcache/cache.h           | 14 ++++++++++++++
 drivers/md/dm-pcache/cache_gc.c        | 16 ++++++++++++++--
 drivers/md/dm-pcache/cache_key.c       | 11 +++++++++++
 drivers/md/dm-pcache/cache_writeback.c | 14 +++++++++++---
 5 files changed, 53 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index bb1ada31e483..e6a2f4460f6e 100644
--- a/drivers/md/dm-pcache/cache.c
+++ b/drivers/md/dm-pcache/cache.c
@@ -118,6 +118,9 @@ int cache_pos_decode(struct pcache_cache *cache,
 	if (!latest_addr)
 		return -EIO;
 
+	if (!cache_seg_id_valid(cache, latest.cache_seg_id))
+		return -EIO;
+
 	pos->cache_seg = &cache->segments[latest.cache_seg_id];
 	pos->seg_off = latest.seg_off;
 	*seq = latest.header.seq;
diff --git a/drivers/md/dm-pcache/cache.h b/drivers/md/dm-pcache/cache.h
index 27613b56be54..653ceea43131 100644
--- a/drivers/md/dm-pcache/cache.h
+++ b/drivers/md/dm-pcache/cache.h
@@ -420,6 +420,20 @@ static inline bool cache_seg_is_ctrl_seg(u32 cache_seg_id)
 	return (cache_seg_id == 0);
 }
 
+/**
+ * cache_seg_id_valid - Validate a cache segment id read from the cache device.
+ * @cache: Pointer to the pcache_cache structure.
+ * @cache_seg_id: Segment id decoded from on-media metadata.
+ *
+ * On-media segment ids are only protected by a CRC, which an attacker who can
+ * format the cache device computes over their chosen value. Reject any id that
+ * would index cache->segments[] out of bounds before it is dereferenced.
+ */
+static inline bool cache_seg_id_valid(struct pcache_cache *cache, u32 cache_seg_id)
+{
+	return cache_seg_id < cache->n_segs;
+}
+
 /**
  * cache_key_cutfront - Cuts a specified length from the front of a cache key.
  * @key: Pointer to pcache_cache_key structure.
diff --git a/drivers/md/dm-pcache/cache_gc.c b/drivers/md/dm-pcache/cache_gc.c
index 94f8b276a021..02fa0ce03134 100644
--- a/drivers/md/dm-pcache/cache_gc.c
+++ b/drivers/md/dm-pcache/cache_gc.c
@@ -74,11 +74,17 @@ static bool need_gc(struct pcache_cache *cache, struct pcache_cache_pos *dirty_t
  * @cache: Pointer to the pcache_cache structure.
  * @kset_onmedia: Pointer to the kset_onmedia structure for the last kset.
  */
-static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
+static int last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
 {
 	struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
 	struct pcache_cache_segment *cur_seg, *next_seg;
 
+	if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
+		pcache_dev_err(pcache, "invalid next_cache_seg_id %u in gc (n_segs %u)\n",
+				kset_onmedia->next_cache_seg_id, cache->n_segs);
+		return -EIO;
+	}
+
 	cur_seg = cache->key_tail.cache_seg;
 
 	next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
@@ -94,6 +100,8 @@ static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_on
 	spin_lock(&cache->seg_map_lock);
 	__clear_bit(cur_seg->cache_seg_id, cache->seg_map);
 	spin_unlock(&cache->seg_map_lock);
+
+	return 0;
 }
 
 void pcache_cache_gc_fn(struct work_struct *work)
@@ -130,7 +138,11 @@ void pcache_cache_gc_fn(struct work_struct *work)
 			if (dirty_tail.cache_seg == key_tail.cache_seg)
 				break;
 
-			last_kset_gc(cache, kset_onmedia);
+			ret = last_kset_gc(cache, kset_onmedia);
+			if (ret) {
+				atomic_inc(&cache->gc_errors);
+				return;
+			}
 			continue;
 		}
 
diff --git a/drivers/md/dm-pcache/cache_key.c b/drivers/md/dm-pcache/cache_key.c
index e068e878231b..8eec5238c5da 100644
--- a/drivers/md/dm-pcache/cache_key.c
+++ b/drivers/md/dm-pcache/cache_key.c
@@ -94,6 +94,12 @@ int cache_key_decode(struct pcache_cache *cache,
 	key->off = key_onmedia->off;
 	key->len = key_onmedia->len;
 
+	if (!cache_seg_id_valid(cache, key_onmedia->cache_seg_id)) {
+		pcache_dev_err(pcache, "invalid cache_seg_id %u in cache key (n_segs %u)\n",
+				key_onmedia->cache_seg_id, cache->n_segs);
+		return -EIO;
+	}
+
 	key->cache_pos.cache_seg = &cache->segments[key_onmedia->cache_seg_id];
 	key->cache_pos.seg_off = key_onmedia->cache_seg_off;
 
@@ -789,6 +795,11 @@ int cache_replay(struct pcache_cache *cache)
 
 			pcache_dev_debug(pcache, "last kset replay, next: %u\n", kset_onmedia->next_cache_seg_id);
 
+			if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
+				ret = -EIO;
+				goto out;
+			}
+
 			next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
 
 			pos->cache_seg = next_seg;
diff --git a/drivers/md/dm-pcache/cache_writeback.c b/drivers/md/dm-pcache/cache_writeback.c
index 87a82b3fe836..a104e6ee8aa8 100644
--- a/drivers/md/dm-pcache/cache_writeback.c
+++ b/drivers/md/dm-pcache/cache_writeback.c
@@ -196,12 +196,18 @@ static int cache_kset_insert_tree(struct pcache_cache *cache, struct pcache_cach
 	return ret;
 }
 
-static void last_kset_writeback(struct pcache_cache *cache,
+static int last_kset_writeback(struct pcache_cache *cache,
 		struct pcache_cache_kset_onmedia *last_kset_onmedia)
 {
 	struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
 	struct pcache_cache_segment *next_seg;
 
+	if (!cache_seg_id_valid(cache, last_kset_onmedia->next_cache_seg_id)) {
+		pcache_dev_err(pcache, "invalid next_cache_seg_id %u in writeback (n_segs %u)\n",
+				last_kset_onmedia->next_cache_seg_id, cache->n_segs);
+		return -EIO;
+	}
+
 	pcache_dev_debug(pcache, "last kset, next: %u\n", last_kset_onmedia->next_cache_seg_id);
 
 	next_seg = &cache->segments[last_kset_onmedia->next_cache_seg_id];
@@ -211,6 +217,8 @@ static void last_kset_writeback(struct pcache_cache *cache,
 	cache->dirty_tail.seg_off = 0;
 	cache_encode_dirty_tail(cache);
 	mutex_unlock(&cache->dirty_tail_lock);
+
+	return 0;
 }
 
 void cache_writeback_fn(struct work_struct *work)
@@ -241,8 +249,8 @@ void cache_writeback_fn(struct work_struct *work)
 	}
 
 	if (kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) {
-		last_kset_writeback(cache, kset_onmedia);
-		delay = 0;
+		ret = last_kset_writeback(cache, kset_onmedia);
+		delay = ret ? PCACHE_CACHE_WRITEBACK_INTERVAL : 0;
 		goto queue_work;
 	}
 

-- 
2.43.0

[PATCH 2/3] dm-pcache: validate geometry fields from on-disk cache_info

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

cache_segs_init() iterates cache_info->n_segs times indexing
cache->segments[], which is sized to the cache device geometry, and
get_seg_id() takes each segment id from the on-media cache_info and the
per-segment next_seg link. Both come from cache device metadata that is
only CRC-protected with a fixed public seed, so whoever supplies the
cache device on a table load (CAP_SYS_ADMIN) controls them: an oversized
n_segs or an out-of-range id drives an out-of-bounds access of
cache->segments[] and a wild CACHE_DEV_SEGMENT() pointer into the device
mapping -- an out-of-bounds read and write from on-disk data.

Reject an n_segs that exceeds the device segment count and a segment id
that is out of range before either is used. Valid metadata is unaffected.

Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
Cc: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 drivers/md/dm-pcache/cache.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
index e6a2f4460f6e..08decbebd238 100644
--- a/drivers/md/dm-pcache/cache.c
+++ b/drivers/md/dm-pcache/cache.c
@@ -250,6 +250,13 @@ static int get_seg_id(struct pcache_cache *cache,
 		} else {
 			*seg_id = cache->cache_info.seg_id;
 		}
+
+		if (*seg_id >= cache_dev->seg_num) {
+			pcache_dev_err(pcache, "invalid segment id %u from cache device (seg_num %u)\n",
+				       *seg_id, cache_dev->seg_num);
+			ret = -EIO;
+			goto err;
+		}
 	}
 	return 0;
 err:
@@ -265,6 +272,13 @@ static int cache_segs_init(struct pcache_cache *cache)
 	int ret;
 	u32 i;
 
+	if (cache_info->n_segs > cache->cache_dev->seg_num) {
+		pcache_dev_err(CACHE_TO_PCACHE(cache),
+			       "cache_info n_segs %u exceeds cache device segments %u\n",
+			       cache_info->n_segs, cache->cache_dev->seg_num);
+		return -EIO;
+	}
+
 	for (i = 0; i < cache_info->n_segs; i++) {
 		ret = get_seg_id(cache, prev_cache_seg, new_cache, &seg_id);
 		if (ret)

-- 
2.43.0

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

[Bryam Vargas via B4 Relay]

From: Bryam Vargas <hexlabsecurity@proton.me>

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: stable@vger.kernel.org
Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
---
 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

Re: [PATCH 0/3] dm-pcache: validate on-disk fields before use

[Zheng Gu]

Hi Vargas,

Thanks for catching this issue.

Best regards,
Gu

On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> dm-pcache reads its layout from the cache device and mostly trusts it. The
> table load needs CAP_SYS_ADMIN, but the on-media metadata is only self-checked
> with a CRC32C over a fixed, public seed, so anyone who can format or swap the
> cache device forges the fields and recomputes the CRC. Several of them become
> segment ids, array indices, loop counts and intra-segment extents with no bound
> against the device geometry, which turns crafted metadata into kernel
> out-of-bounds accesses, both at table load and during normal cache I/O.
>
> Each patch adds the bound where the field is decoded:
>
>   1/3  segment ids that index cache->segments[] -- bound against cache->n_segs
>        (cache_pos_decode, cache_key_decode, and the last-kset paths in replay,
>        writeback and gc).
>   2/3  cache_info->n_segs and the segment-chain ids -- bound against the cache
>        device segment count.
>   3/3  the kset key_num, read by cache_kset_crc() and the replay loop before the
>        CRC is even compared, and a key whose offset+length runs past its segment
>        -- the latter is an OOB read that otherwise reaches user space on a cache
>        read hit.
>
> I found these by walking the cache-device decode paths. Each class reproduces in a
> faithful in-kernel model under KASAN: the bug arm faults (slab out-of-bounds -- a
> read for the seg_id, geometry and key_num/extent cases, a write for the n_segs
> loop), the arm with the added bound is clean, and valid metadata is clean. The key
> offset/length case also reproduces under KMSAN, where the over-read returns
> uninitialized kernel memory. Reproducer available on request. Each check rejects
> bad metadata with -EIO and leaves valid metadata alone.
>
> checkpatch --strict is clean apart from "Alignment should match open
> parenthesis" on the multi-line pcache_dev_err() calls, which match the style
> already in the driver.
>
> ---
> Bryam Vargas (3):
>       dm-pcache: validate seg_id fields from persistent memory
>       dm-pcache: validate geometry fields from on-disk cache_info
>       dm-pcache: validate kset key_num and intra-segment bounds
>
>  drivers/md/dm-pcache/cache.c           | 17 +++++++++++++++++
>  drivers/md/dm-pcache/cache.h           | 35 ++++++++++++++++++++++++++++++++++
>  drivers/md/dm-pcache/cache_gc.c        | 24 +++++++++++++++++------
>  drivers/md/dm-pcache/cache_key.c       | 21 +++++++++++++++++++-
>  drivers/md/dm-pcache/cache_writeback.c | 22 ++++++++++++++-------
>  5 files changed, 105 insertions(+), 14 deletions(-)
> ---
> base-commit: 4549871118cf616eecdd2d939f78e3b9e1dddc48
> change-id: 20260619-b4-disp-997773a1-69ada10d3a3e
>
> Best regards,
> --
> bryamzxz <hexlabsecurity@proton.me>
>
>

Re: [PATCH 1/3] dm-pcache: validate seg_id fields from persistent memory

[Zheng Gu]

On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> cache_pos_decode(), cache_key_decode() and the last-kset branches of
> cache_replay(), the writeback worker and the GC worker take a cache
> segment id from the cache device metadata and index cache->segments[]
> with it without checking it against cache->n_segs. That metadata is only
> CRC-protected with a fixed public seed, so whoever supplies the cache
> device on a table load (CAP_SYS_ADMIN) controls the id; an out-of-range
> value forms a wild pcache_cache_segment pointer that is dereferenced and
> written through -- an out-of-bounds read and write driven by on-disk data.
>
> Add cache_seg_id_valid() and reject an out-of-range id at each decode
> site, failing the operation with -EIO instead of indexing past the array.
> Valid metadata is unaffected.
>
> Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  drivers/md/dm-pcache/cache.c           |  3 +++
>  drivers/md/dm-pcache/cache.h           | 14 ++++++++++++++
>  drivers/md/dm-pcache/cache_gc.c        | 16 ++++++++++++++--
>  drivers/md/dm-pcache/cache_key.c       | 11 +++++++++++
>  drivers/md/dm-pcache/cache_writeback.c | 14 +++++++++++---
>  5 files changed, 53 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index bb1ada31e483..e6a2f4460f6e 100644
> --- a/drivers/md/dm-pcache/cache.c
> +++ b/drivers/md/dm-pcache/cache.c
> @@ -118,6 +118,9 @@ int cache_pos_decode(struct pcache_cache *cache,
>         if (!latest_addr)
>                 return -EIO;
>
> +       if (!cache_seg_id_valid(cache, latest.cache_seg_id))
> +               return -EIO;
> +
>         pos->cache_seg = &cache->segments[latest.cache_seg_id];
>         pos->seg_off = latest.seg_off;
>         *seq = latest.header.seq;
> diff --git a/drivers/md/dm-pcache/cache.h b/drivers/md/dm-pcache/cache.h
> index 27613b56be54..653ceea43131 100644
> --- a/drivers/md/dm-pcache/cache.h
> +++ b/drivers/md/dm-pcache/cache.h
> @@ -420,6 +420,20 @@ static inline bool cache_seg_is_ctrl_seg(u32 cache_seg_id)
>         return (cache_seg_id == 0);
>  }
>
> +/**
> + * cache_seg_id_valid - Validate a cache segment id read from the cache device.
> + * @cache: Pointer to the pcache_cache structure.
> + * @cache_seg_id: Segment id decoded from on-media metadata.
> + *
> + * On-media segment ids are only protected by a CRC, which an attacker who can
> + * format the cache device computes over their chosen value. Reject any id that
> + * would index cache->segments[] out of bounds before it is dereferenced.
> + */
> +static inline bool cache_seg_id_valid(struct pcache_cache *cache, u32 cache_seg_id)
> +{
> +       return cache_seg_id < cache->n_segs;
> +}
> +
>  /**
>   * cache_key_cutfront - Cuts a specified length from the front of a cache key.
>   * @key: Pointer to pcache_cache_key structure.
> diff --git a/drivers/md/dm-pcache/cache_gc.c b/drivers/md/dm-pcache/cache_gc.c
> index 94f8b276a021..02fa0ce03134 100644
> --- a/drivers/md/dm-pcache/cache_gc.c
> +++ b/drivers/md/dm-pcache/cache_gc.c
> @@ -74,11 +74,17 @@ static bool need_gc(struct pcache_cache *cache, struct pcache_cache_pos *dirty_t
>   * @cache: Pointer to the pcache_cache structure.
>   * @kset_onmedia: Pointer to the kset_onmedia structure for the last kset.
>   */
> -static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
> +static int last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
>  {
>         struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
>         struct pcache_cache_segment *cur_seg, *next_seg;
>
> +       if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
> +               pcache_dev_err(pcache, "invalid next_cache_seg_id %u in gc (n_segs %u)\n",
> +                               kset_onmedia->next_cache_seg_id, cache->n_segs);
> +               return -EIO;
> +       }
> +
>         cur_seg = cache->key_tail.cache_seg;
>
>         next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
> @@ -94,6 +100,8 @@ static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_on
>         spin_lock(&cache->seg_map_lock);
>         __clear_bit(cur_seg->cache_seg_id, cache->seg_map);
>         spin_unlock(&cache->seg_map_lock);
> +
> +       return 0;
>  }
>
>  void pcache_cache_gc_fn(struct work_struct *work)
> @@ -130,7 +138,11 @@ void pcache_cache_gc_fn(struct work_struct *work)
>                         if (dirty_tail.cache_seg == key_tail.cache_seg)
>                                 break;
>
> -                       last_kset_gc(cache, kset_onmedia);
> +                       ret = last_kset_gc(cache, kset_onmedia);
> +                       if (ret) {
> +                               atomic_inc(&cache->gc_errors);
> +                               return;
> +                       }
>                         continue;
>                 }
>
> diff --git a/drivers/md/dm-pcache/cache_key.c b/drivers/md/dm-pcache/cache_key.c
> index e068e878231b..8eec5238c5da 100644
> --- a/drivers/md/dm-pcache/cache_key.c
> +++ b/drivers/md/dm-pcache/cache_key.c
> @@ -94,6 +94,12 @@ int cache_key_decode(struct pcache_cache *cache,
>         key->off = key_onmedia->off;
>         key->len = key_onmedia->len;
>
> +       if (!cache_seg_id_valid(cache, key_onmedia->cache_seg_id)) {
> +               pcache_dev_err(pcache, "invalid cache_seg_id %u in cache key (n_segs %u)\n",
> +                               key_onmedia->cache_seg_id, cache->n_segs);
> +               return -EIO;
> +       }
> +
>         key->cache_pos.cache_seg = &cache->segments[key_onmedia->cache_seg_id];
>         key->cache_pos.seg_off = key_onmedia->cache_seg_off;
>
> @@ -789,6 +795,11 @@ int cache_replay(struct pcache_cache *cache)
>
>                         pcache_dev_debug(pcache, "last kset replay, next: %u\n", kset_onmedia->next_cache_seg_id);
>
> +                       if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
> +                               ret = -EIO;
> +                               goto out;
> +                       }
> +
>                         next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
>
>                         pos->cache_seg = next_seg;
> diff --git a/drivers/md/dm-pcache/cache_writeback.c b/drivers/md/dm-pcache/cache_writeback.c
> index 87a82b3fe836..a104e6ee8aa8 100644
> --- a/drivers/md/dm-pcache/cache_writeback.c
> +++ b/drivers/md/dm-pcache/cache_writeback.c
> @@ -196,12 +196,18 @@ static int cache_kset_insert_tree(struct pcache_cache *cache, struct pcache_cach
>         return ret;
>  }
>
> -static void last_kset_writeback(struct pcache_cache *cache,
> +static int last_kset_writeback(struct pcache_cache *cache,
>                 struct pcache_cache_kset_onmedia *last_kset_onmedia)
>  {
>         struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
>         struct pcache_cache_segment *next_seg;
>
> +       if (!cache_seg_id_valid(cache, last_kset_onmedia->next_cache_seg_id)) {
> +               pcache_dev_err(pcache, "invalid next_cache_seg_id %u in writeback (n_segs %u)\n",
> +                               last_kset_onmedia->next_cache_seg_id, cache->n_segs);
> +               return -EIO;
> +       }
> +
>         pcache_dev_debug(pcache, "last kset, next: %u\n", last_kset_onmedia->next_cache_seg_id);
>
>         next_seg = &cache->segments[last_kset_onmedia->next_cache_seg_id];
> @@ -211,6 +217,8 @@ static void last_kset_writeback(struct pcache_cache *cache,
>         cache->dirty_tail.seg_off = 0;
>         cache_encode_dirty_tail(cache);
>         mutex_unlock(&cache->dirty_tail_lock);
> +
> +       return 0;
>  }
>
>  void cache_writeback_fn(struct work_struct *work)
> @@ -241,8 +249,8 @@ void cache_writeback_fn(struct work_struct *work)
>         }
>
>         if (kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) {
> -               last_kset_writeback(cache, kset_onmedia);
> -               delay = 0;
> +               ret = last_kset_writeback(cache, kset_onmedia);
> +               delay = ret ? PCACHE_CACHE_WRITEBACK_INTERVAL : 0;
>                 goto queue_work;
>         }
>
>
> --
> 2.43.0
>
>

Reviewed-by:  Zheng Gu <cengku@gmail.com>

Re: [PATCH 2/3] dm-pcache: validate geometry fields from on-disk cache_info

[Zheng Gu]

On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> cache_segs_init() iterates cache_info->n_segs times indexing
> cache->segments[], which is sized to the cache device geometry, and
> get_seg_id() takes each segment id from the on-media cache_info and the
> per-segment next_seg link. Both come from cache device metadata that is
> only CRC-protected with a fixed public seed, so whoever supplies the
> cache device on a table load (CAP_SYS_ADMIN) controls them: an oversized
> n_segs or an out-of-range id drives an out-of-bounds access of
> cache->segments[] and a wild CACHE_DEV_SEGMENT() pointer into the device
> mapping -- an out-of-bounds read and write from on-disk data.
>
> Reject an n_segs that exceeds the device segment count and a segment id
> that is out of range before either is used. Valid metadata is unaffected.
>
> Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
> Cc: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  drivers/md/dm-pcache/cache.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> index e6a2f4460f6e..08decbebd238 100644
> --- a/drivers/md/dm-pcache/cache.c
> +++ b/drivers/md/dm-pcache/cache.c
> @@ -250,6 +250,13 @@ static int get_seg_id(struct pcache_cache *cache,
>                 } else {
>                         *seg_id = cache->cache_info.seg_id;
>                 }
> +
> +               if (*seg_id >= cache_dev->seg_num) {
> +                       pcache_dev_err(pcache, "invalid segment id %u from cache device (seg_num %u)\n",
> +                                      *seg_id, cache_dev->seg_num);
> +                       ret = -EIO;
> +                       goto err;
> +               }
>         }
>         return 0;
>  err:
> @@ -265,6 +272,13 @@ static int cache_segs_init(struct pcache_cache *cache)
>         int ret;
>         u32 i;
>
> +       if (cache_info->n_segs > cache->cache_dev->seg_num) {
> +               pcache_dev_err(CACHE_TO_PCACHE(cache),
> +                              "cache_info n_segs %u exceeds cache device segments %u\n",
> +                              cache_info->n_segs, cache->cache_dev->seg_num);
> +               return -EIO;
> +       }
> +
>         for (i = 0; i < cache_info->n_segs; i++) {
>                 ret = get_seg_id(cache, prev_cache_seg, new_cache, &seg_id);
>                 if (ret)
>
> --
> 2.43.0
>
>

Reviewed-by:  Zheng Gu <cengku@gmail.com>

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

[Zheng Gu]

On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@kernel.org> wrote:
>
> From: Bryam Vargas <hexlabsecurity@proton.me>
>
> 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: stable@vger.kernel.org
> Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> ---
>  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
>
>

Reviewed-by:  Zheng Gu <cengku@gmail.com>

Re: [PATCH 1/3] dm-pcache: validate seg_id fields from persistent memory

[Mikulas Patocka]

[-- Attachment #1: Type: text/plain, Size: 10847 bytes --]



On Sat, 27 Jun 2026, Zheng Gu wrote:

> On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
> <devnull+hexlabsecurity.proton.me@kernel.org> wrote:
> >
> > From: Bryam Vargas <hexlabsecurity@proton.me>
> >
> > cache_pos_decode(), cache_key_decode() and the last-kset branches of
> > cache_replay(), the writeback worker and the GC worker take a cache
> > segment id from the cache device metadata and index cache->segments[]
> > with it without checking it against cache->n_segs. That metadata is only
> > CRC-protected with a fixed public seed, so whoever supplies the cache
> > device on a table load (CAP_SYS_ADMIN) controls the id; an out-of-range
> > value forms a wild pcache_cache_segment pointer that is dereferenced and
> > written through -- an out-of-bounds read and write driven by on-disk data.
> >
> > Add cache_seg_id_valid() and reject an out-of-range id at each decode
> > site, failing the operation with -EIO instead of indexing past the array.
> > Valid metadata is unaffected.
> >
> > Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> > ---
> >  drivers/md/dm-pcache/cache.c           |  3 +++
> >  drivers/md/dm-pcache/cache.h           | 14 ++++++++++++++
> >  drivers/md/dm-pcache/cache_gc.c        | 16 ++++++++++++++--
> >  drivers/md/dm-pcache/cache_key.c       | 11 +++++++++++
> >  drivers/md/dm-pcache/cache_writeback.c | 14 +++++++++++---
> >  5 files changed, 53 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> > index bb1ada31e483..e6a2f4460f6e 100644
> > --- a/drivers/md/dm-pcache/cache.c
> > +++ b/drivers/md/dm-pcache/cache.c
> > @@ -118,6 +118,9 @@ int cache_pos_decode(struct pcache_cache *cache,
> >         if (!latest_addr)
> >                 return -EIO;
> >
> > +       if (!cache_seg_id_valid(cache, latest.cache_seg_id))
> > +               return -EIO;
> > +
> >         pos->cache_seg = &cache->segments[latest.cache_seg_id];
> >         pos->seg_off = latest.seg_off;
> >         *seq = latest.header.seq;
> > diff --git a/drivers/md/dm-pcache/cache.h b/drivers/md/dm-pcache/cache.h
> > index 27613b56be54..653ceea43131 100644
> > --- a/drivers/md/dm-pcache/cache.h
> > +++ b/drivers/md/dm-pcache/cache.h
> > @@ -420,6 +420,20 @@ static inline bool cache_seg_is_ctrl_seg(u32 cache_seg_id)
> >         return (cache_seg_id == 0);
> >  }
> >
> > +/**
> > + * cache_seg_id_valid - Validate a cache segment id read from the cache device.
> > + * @cache: Pointer to the pcache_cache structure.
> > + * @cache_seg_id: Segment id decoded from on-media metadata.
> > + *
> > + * On-media segment ids are only protected by a CRC, which an attacker who can
> > + * format the cache device computes over their chosen value. Reject any id that
> > + * would index cache->segments[] out of bounds before it is dereferenced.
> > + */
> > +static inline bool cache_seg_id_valid(struct pcache_cache *cache, u32 cache_seg_id)
> > +{
> > +       return cache_seg_id < cache->n_segs;
> > +}
> > +
> >  /**
> >   * cache_key_cutfront - Cuts a specified length from the front of a cache key.
> >   * @key: Pointer to pcache_cache_key structure.
> > diff --git a/drivers/md/dm-pcache/cache_gc.c b/drivers/md/dm-pcache/cache_gc.c
> > index 94f8b276a021..02fa0ce03134 100644
> > --- a/drivers/md/dm-pcache/cache_gc.c
> > +++ b/drivers/md/dm-pcache/cache_gc.c
> > @@ -74,11 +74,17 @@ static bool need_gc(struct pcache_cache *cache, struct pcache_cache_pos *dirty_t
> >   * @cache: Pointer to the pcache_cache structure.
> >   * @kset_onmedia: Pointer to the kset_onmedia structure for the last kset.
> >   */
> > -static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
> > +static int last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
> >  {
> >         struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
> >         struct pcache_cache_segment *cur_seg, *next_seg;
> >
> > +       if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
> > +               pcache_dev_err(pcache, "invalid next_cache_seg_id %u in gc (n_segs %u)\n",
> > +                               kset_onmedia->next_cache_seg_id, cache->n_segs);
> > +               return -EIO;
> > +       }
> > +
> >         cur_seg = cache->key_tail.cache_seg;
> >
> >         next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
> > @@ -94,6 +100,8 @@ static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_on
> >         spin_lock(&cache->seg_map_lock);
> >         __clear_bit(cur_seg->cache_seg_id, cache->seg_map);
> >         spin_unlock(&cache->seg_map_lock);
> > +
> > +       return 0;
> >  }
> >
> >  void pcache_cache_gc_fn(struct work_struct *work)
> > @@ -130,7 +138,11 @@ void pcache_cache_gc_fn(struct work_struct *work)
> >                         if (dirty_tail.cache_seg == key_tail.cache_seg)
> >                                 break;
> >
> > -                       last_kset_gc(cache, kset_onmedia);
> > +                       ret = last_kset_gc(cache, kset_onmedia);
> > +                       if (ret) {
> > +                               atomic_inc(&cache->gc_errors);
> > +                               return;
> > +                       }
> >                         continue;
> >                 }
> >
> > diff --git a/drivers/md/dm-pcache/cache_key.c b/drivers/md/dm-pcache/cache_key.c
> > index e068e878231b..8eec5238c5da 100644
> > --- a/drivers/md/dm-pcache/cache_key.c
> > +++ b/drivers/md/dm-pcache/cache_key.c
> > @@ -94,6 +94,12 @@ int cache_key_decode(struct pcache_cache *cache,
> >         key->off = key_onmedia->off;
> >         key->len = key_onmedia->len;
> >
> > +       if (!cache_seg_id_valid(cache, key_onmedia->cache_seg_id)) {
> > +               pcache_dev_err(pcache, "invalid cache_seg_id %u in cache key (n_segs %u)\n",
> > +                               key_onmedia->cache_seg_id, cache->n_segs);
> > +               return -EIO;
> > +       }
> > +
> >         key->cache_pos.cache_seg = &cache->segments[key_onmedia->cache_seg_id];
> >         key->cache_pos.seg_off = key_onmedia->cache_seg_off;
> >
> > @@ -789,6 +795,11 @@ int cache_replay(struct pcache_cache *cache)
> >
> >                         pcache_dev_debug(pcache, "last kset replay, next: %u\n", kset_onmedia->next_cache_seg_id);
> >
> > +                       if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
> > +                               ret = -EIO;
> > +                               goto out;
> > +                       }
> > +
> >                         next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
> >
> >                         pos->cache_seg = next_seg;
> > diff --git a/drivers/md/dm-pcache/cache_writeback.c b/drivers/md/dm-pcache/cache_writeback.c
> > index 87a82b3fe836..a104e6ee8aa8 100644
> > --- a/drivers/md/dm-pcache/cache_writeback.c
> > +++ b/drivers/md/dm-pcache/cache_writeback.c
> > @@ -196,12 +196,18 @@ static int cache_kset_insert_tree(struct pcache_cache *cache, struct pcache_cach
> >         return ret;
> >  }
> >
> > -static void last_kset_writeback(struct pcache_cache *cache,
> > +static int last_kset_writeback(struct pcache_cache *cache,
> >                 struct pcache_cache_kset_onmedia *last_kset_onmedia)
> >  {
> >         struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
> >         struct pcache_cache_segment *next_seg;
> >
> > +       if (!cache_seg_id_valid(cache, last_kset_onmedia->next_cache_seg_id)) {
> > +               pcache_dev_err(pcache, "invalid next_cache_seg_id %u in writeback (n_segs %u)\n",
> > +                               last_kset_onmedia->next_cache_seg_id, cache->n_segs);
> > +               return -EIO;
> > +       }
> > +
> >         pcache_dev_debug(pcache, "last kset, next: %u\n", last_kset_onmedia->next_cache_seg_id);
> >
> >         next_seg = &cache->segments[last_kset_onmedia->next_cache_seg_id];
> > @@ -211,6 +217,8 @@ static void last_kset_writeback(struct pcache_cache *cache,
> >         cache->dirty_tail.seg_off = 0;
> >         cache_encode_dirty_tail(cache);
> >         mutex_unlock(&cache->dirty_tail_lock);
> > +
> > +       return 0;
> >  }
> >
> >  void cache_writeback_fn(struct work_struct *work)
> > @@ -241,8 +249,8 @@ void cache_writeback_fn(struct work_struct *work)
> >         }
> >
> >         if (kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) {
> > -               last_kset_writeback(cache, kset_onmedia);
> > -               delay = 0;
> > +               ret = last_kset_writeback(cache, kset_onmedia);
> > +               delay = ret ? PCACHE_CACHE_WRITEBACK_INTERVAL : 0;
> >                 goto queue_work;
> >         }
> >
> >
> > --
> > 2.43.0
> >
> >
> 
> Reviewed-by:  Zheng Gu <cengku@gmail.com>

Hi.

I used Claude to review this patch and it found this bug. Would you fix it 
or confirm that it is false positive?

Mikulas


  Patch 1/3: validate seg_id fields -- Bug found
     
  Bug: Writeback worker retries forever on corrupt next_cache_seg_id.

  The patched cache_writeback_fn() (line 249-255 of the diff):

  if (kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) {
      ret = last_kset_writeback(cache, kset_onmedia);
      delay = ret ? PCACHE_CACHE_WRITEBACK_INTERVAL : 0;
      goto queue_work;
  }

  When last_kset_writeback() returns -EIO (invalid next_cache_seg_id):
  1. delay = PCACHE_CACHE_WRITEBACK_INTERVAL (5 seconds) and the worker 
reschedules
  2. dirty_tail was NOT advanced (the function returned before modifying 
it)
  3. On the next invocation, is_cache_clean() reads the same kset -- magic 
OK, CRC OK --
  returns false
  4. Same LAST kset, same invalid next_cache_seg_id, same -EIO
  5. pcache_dev_err() fires again, reschedule again... forever

  Contrast with the GC path for the identical error:

  ret = last_kset_gc(cache, kset_onmedia);
  if (ret) {
      atomic_inc(&cache->gc_errors);   // permanent flag
      return;                          // exits WITHOUT rescheduling
  }
  
  The GC worker stops permanently via gc_errors (checked at the top of
  pcache_cache_gc_fn() at line 113). The writeback worker has no 
equivalent counter --
  struct pcache_cache has atomic_t gc_errors but nothing for writeback. 
The writeback
  worker will spin on the corrupt kset every 5 seconds, logging an error 
each time, for
  the lifetime of the target.

  Fix: Either add a writeback_errors counter (like gc_errors) and check it 
at the top of
  cache_writeback_fn(), or use return without rescheduling like the GC 
path does.

  The remaining validation sites in Patch 1 (cache_pos_decode, 
cache_key_decode,
  cache_replay, last_kset_gc) look correct -- error returns are properly 
handled by
  their callers.

Re: [PATCH 1/3] dm-pcache: validate seg_id fields from persistent memory

[Zheng Gu]

On Thu, Jul 16, 2026 at 10:38 PM Mikulas Patocka <mpatocka@redhat.com> wrote:
>
>
>
> On Sat, 27 Jun 2026, Zheng Gu wrote:
>
> > On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
> > <devnull+hexlabsecurity.proton.me@kernel.org> wrote:
> > >
> > > From: Bryam Vargas <hexlabsecurity@proton.me>
> > >
> > > cache_pos_decode(), cache_key_decode() and the last-kset branches of
> > > cache_replay(), the writeback worker and the GC worker take a cache
> > > segment id from the cache device metadata and index cache->segments[]
> > > with it without checking it against cache->n_segs. That metadata is only
> > > CRC-protected with a fixed public seed, so whoever supplies the cache
> > > device on a table load (CAP_SYS_ADMIN) controls the id; an out-of-range
> > > value forms a wild pcache_cache_segment pointer that is dereferenced and
> > > written through -- an out-of-bounds read and write driven by on-disk data.
> > >
> > > Add cache_seg_id_valid() and reject an out-of-range id at each decode
> > > site, failing the operation with -EIO instead of indexing past the array.
> > > Valid metadata is unaffected.
> > >
> > > Fixes: 1d57628ff95b ("dm-pcache: add persistent cache target in device-mapper")
> > > Cc: stable@vger.kernel.org
> > > Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
> > > ---
> > >  drivers/md/dm-pcache/cache.c           |  3 +++
> > >  drivers/md/dm-pcache/cache.h           | 14 ++++++++++++++
> > >  drivers/md/dm-pcache/cache_gc.c        | 16 ++++++++++++++--
> > >  drivers/md/dm-pcache/cache_key.c       | 11 +++++++++++
> > >  drivers/md/dm-pcache/cache_writeback.c | 14 +++++++++++---
> > >  5 files changed, 53 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/md/dm-pcache/cache.c b/drivers/md/dm-pcache/cache.c
> > > index bb1ada31e483..e6a2f4460f6e 100644
> > > --- a/drivers/md/dm-pcache/cache.c
> > > +++ b/drivers/md/dm-pcache/cache.c
> > > @@ -118,6 +118,9 @@ int cache_pos_decode(struct pcache_cache *cache,
> > >         if (!latest_addr)
> > >                 return -EIO;
> > >
> > > +       if (!cache_seg_id_valid(cache, latest.cache_seg_id))
> > > +               return -EIO;
> > > +
> > >         pos->cache_seg = &cache->segments[latest.cache_seg_id];
> > >         pos->seg_off = latest.seg_off;
> > >         *seq = latest.header.seq;
> > > diff --git a/drivers/md/dm-pcache/cache.h b/drivers/md/dm-pcache/cache.h
> > > index 27613b56be54..653ceea43131 100644
> > > --- a/drivers/md/dm-pcache/cache.h
> > > +++ b/drivers/md/dm-pcache/cache.h
> > > @@ -420,6 +420,20 @@ static inline bool cache_seg_is_ctrl_seg(u32 cache_seg_id)
> > >         return (cache_seg_id == 0);
> > >  }
> > >
> > > +/**
> > > + * cache_seg_id_valid - Validate a cache segment id read from the cache device.
> > > + * @cache: Pointer to the pcache_cache structure.
> > > + * @cache_seg_id: Segment id decoded from on-media metadata.
> > > + *
> > > + * On-media segment ids are only protected by a CRC, which an attacker who can
> > > + * format the cache device computes over their chosen value. Reject any id that
> > > + * would index cache->segments[] out of bounds before it is dereferenced.
> > > + */
> > > +static inline bool cache_seg_id_valid(struct pcache_cache *cache, u32 cache_seg_id)
> > > +{
> > > +       return cache_seg_id < cache->n_segs;
> > > +}
> > > +
> > >  /**
> > >   * cache_key_cutfront - Cuts a specified length from the front of a cache key.
> > >   * @key: Pointer to pcache_cache_key structure.
> > > diff --git a/drivers/md/dm-pcache/cache_gc.c b/drivers/md/dm-pcache/cache_gc.c
> > > index 94f8b276a021..02fa0ce03134 100644
> > > --- a/drivers/md/dm-pcache/cache_gc.c
> > > +++ b/drivers/md/dm-pcache/cache_gc.c
> > > @@ -74,11 +74,17 @@ static bool need_gc(struct pcache_cache *cache, struct pcache_cache_pos *dirty_t
> > >   * @cache: Pointer to the pcache_cache structure.
> > >   * @kset_onmedia: Pointer to the kset_onmedia structure for the last kset.
> > >   */
> > > -static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
> > > +static int last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_onmedia *kset_onmedia)
> > >  {
> > >         struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
> > >         struct pcache_cache_segment *cur_seg, *next_seg;
> > >
> > > +       if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
> > > +               pcache_dev_err(pcache, "invalid next_cache_seg_id %u in gc (n_segs %u)\n",
> > > +                               kset_onmedia->next_cache_seg_id, cache->n_segs);
> > > +               return -EIO;
> > > +       }
> > > +
> > >         cur_seg = cache->key_tail.cache_seg;
> > >
> > >         next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
> > > @@ -94,6 +100,8 @@ static void last_kset_gc(struct pcache_cache *cache, struct pcache_cache_kset_on
> > >         spin_lock(&cache->seg_map_lock);
> > >         __clear_bit(cur_seg->cache_seg_id, cache->seg_map);
> > >         spin_unlock(&cache->seg_map_lock);
> > > +
> > > +       return 0;
> > >  }
> > >
> > >  void pcache_cache_gc_fn(struct work_struct *work)
> > > @@ -130,7 +138,11 @@ void pcache_cache_gc_fn(struct work_struct *work)
> > >                         if (dirty_tail.cache_seg == key_tail.cache_seg)
> > >                                 break;
> > >
> > > -                       last_kset_gc(cache, kset_onmedia);
> > > +                       ret = last_kset_gc(cache, kset_onmedia);
> > > +                       if (ret) {
> > > +                               atomic_inc(&cache->gc_errors);
> > > +                               return;
> > > +                       }
> > >                         continue;
> > >                 }
> > >
> > > diff --git a/drivers/md/dm-pcache/cache_key.c b/drivers/md/dm-pcache/cache_key.c
> > > index e068e878231b..8eec5238c5da 100644
> > > --- a/drivers/md/dm-pcache/cache_key.c
> > > +++ b/drivers/md/dm-pcache/cache_key.c
> > > @@ -94,6 +94,12 @@ int cache_key_decode(struct pcache_cache *cache,
> > >         key->off = key_onmedia->off;
> > >         key->len = key_onmedia->len;
> > >
> > > +       if (!cache_seg_id_valid(cache, key_onmedia->cache_seg_id)) {
> > > +               pcache_dev_err(pcache, "invalid cache_seg_id %u in cache key (n_segs %u)\n",
> > > +                               key_onmedia->cache_seg_id, cache->n_segs);
> > > +               return -EIO;
> > > +       }
> > > +
> > >         key->cache_pos.cache_seg = &cache->segments[key_onmedia->cache_seg_id];
> > >         key->cache_pos.seg_off = key_onmedia->cache_seg_off;
> > >
> > > @@ -789,6 +795,11 @@ int cache_replay(struct pcache_cache *cache)
> > >
> > >                         pcache_dev_debug(pcache, "last kset replay, next: %u\n", kset_onmedia->next_cache_seg_id);
> > >
> > > +                       if (!cache_seg_id_valid(cache, kset_onmedia->next_cache_seg_id)) {
> > > +                               ret = -EIO;
> > > +                               goto out;
> > > +                       }
> > > +
> > >                         next_seg = &cache->segments[kset_onmedia->next_cache_seg_id];
> > >
> > >                         pos->cache_seg = next_seg;
> > > diff --git a/drivers/md/dm-pcache/cache_writeback.c b/drivers/md/dm-pcache/cache_writeback.c
> > > index 87a82b3fe836..a104e6ee8aa8 100644
> > > --- a/drivers/md/dm-pcache/cache_writeback.c
> > > +++ b/drivers/md/dm-pcache/cache_writeback.c
> > > @@ -196,12 +196,18 @@ static int cache_kset_insert_tree(struct pcache_cache *cache, struct pcache_cach
> > >         return ret;
> > >  }
> > >
> > > -static void last_kset_writeback(struct pcache_cache *cache,
> > > +static int last_kset_writeback(struct pcache_cache *cache,
> > >                 struct pcache_cache_kset_onmedia *last_kset_onmedia)
> > >  {
> > >         struct dm_pcache *pcache = CACHE_TO_PCACHE(cache);
> > >         struct pcache_cache_segment *next_seg;
> > >
> > > +       if (!cache_seg_id_valid(cache, last_kset_onmedia->next_cache_seg_id)) {
> > > +               pcache_dev_err(pcache, "invalid next_cache_seg_id %u in writeback (n_segs %u)\n",
> > > +                               last_kset_onmedia->next_cache_seg_id, cache->n_segs);
> > > +               return -EIO;
> > > +       }
> > > +
> > >         pcache_dev_debug(pcache, "last kset, next: %u\n", last_kset_onmedia->next_cache_seg_id);
> > >
> > >         next_seg = &cache->segments[last_kset_onmedia->next_cache_seg_id];
> > > @@ -211,6 +217,8 @@ static void last_kset_writeback(struct pcache_cache *cache,
> > >         cache->dirty_tail.seg_off = 0;
> > >         cache_encode_dirty_tail(cache);
> > >         mutex_unlock(&cache->dirty_tail_lock);
> > > +
> > > +       return 0;
> > >  }
> > >
> > >  void cache_writeback_fn(struct work_struct *work)
> > > @@ -241,8 +249,8 @@ void cache_writeback_fn(struct work_struct *work)
> > >         }
> > >
> > >         if (kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) {
> > > -               last_kset_writeback(cache, kset_onmedia);
> > > -               delay = 0;
> > > +               ret = last_kset_writeback(cache, kset_onmedia);
> > > +               delay = ret ? PCACHE_CACHE_WRITEBACK_INTERVAL : 0;
> > >                 goto queue_work;
> > >         }
> > >
> > >
> > > --
> > > 2.43.0
> > >
> > >
> >
> > Reviewed-by:  Zheng Gu <cengku@gmail.com>
>
> Hi.
>
> I used Claude to review this patch and it found this bug. Would you fix it
> or confirm that it is false positive?
>
> Mikulas
>
>
>   Patch 1/3: validate seg_id fields -- Bug found
>
>   Bug: Writeback worker retries forever on corrupt next_cache_seg_id.
>
>   The patched cache_writeback_fn() (line 249-255 of the diff):
>
>   if (kset_onmedia->flags & PCACHE_KSET_FLAGS_LAST) {
>       ret = last_kset_writeback(cache, kset_onmedia);
>       delay = ret ? PCACHE_CACHE_WRITEBACK_INTERVAL : 0;
>       goto queue_work;
>   }
>
>   When last_kset_writeback() returns -EIO (invalid next_cache_seg_id):
>   1. delay = PCACHE_CACHE_WRITEBACK_INTERVAL (5 seconds) and the worker
> reschedules
>   2. dirty_tail was NOT advanced (the function returned before modifying
> it)
>   3. On the next invocation, is_cache_clean() reads the same kset -- magic
> OK, CRC OK --
>   returns false
>   4. Same LAST kset, same invalid next_cache_seg_id, same -EIO
>   5. pcache_dev_err() fires again, reschedule again... forever
>
>   Contrast with the GC path for the identical error:
>
>   ret = last_kset_gc(cache, kset_onmedia);
>   if (ret) {
>       atomic_inc(&cache->gc_errors);   // permanent flag
>       return;                          // exits WITHOUT rescheduling
>   }
>
>   The GC worker stops permanently via gc_errors (checked at the top of
>   pcache_cache_gc_fn() at line 113). The writeback worker has no
> equivalent counter --
>   struct pcache_cache has atomic_t gc_errors but nothing for writeback.
> The writeback
>   worker will spin on the corrupt kset every 5 seconds, logging an error
> each time, for
>   the lifetime of the target.
>
>   Fix: Either add a writeback_errors counter (like gc_errors) and check it
> at the top of
>   cache_writeback_fn(), or use return without rescheduling like the GC
> path does.
>
>   The remaining validation sites in Patch 1 (cache_pos_decode,
> cache_key_decode,
>   cache_replay, last_kset_gc) look correct -- error returns are properly
> handled by
>   their callers.

Ah, good catch. My bad, Claude's review is right, the writeback may
spin forever on the corrupt kset.

Hi Bryam, Could you please fix this issue and send a V2?

Best regards,
Gu