Re: [PATCH 3/3] dm-pcache: validate kset key_num and intra-segment bounds
From: Zheng Gu
Date: Sat Jun 27 2026 - 04:06:56 EST
On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay
<devnull+hexlabsecurity.proton.me@xxxxxxxxxx> wrote:
>
> From: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> Signed-off-by: Bryam Vargas <hexlabsecurity@xxxxxxxxx>
> ---
> 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@xxxxxxxxx>