On Fri, Jun 19, 2026 at 7:44 PM Bryam Vargas via B4 Relay <[email protected]> wrote: > > 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 > >
Reviewed-by: Zheng Gu <[email protected]>
