From c43dc390c81515fc310d917f63dc46a3a86cd8e8 Mon Sep 17 00:00:00 2001 From: Cong Wang Date: Sat, 15 Aug 2026 20:41:18 -0700 Subject: [PATCH] daxfs: Bound the image-supplied counts that index kernel arrays overlay_bucket_count, pcache_slot_count and inode_count come straight from the image, and each one bounds an array the kernel then indexes without re-checking: bucket_mask = bucket_count - 1, applied to every probe in overlay_lookup() and overlay_insert(), which read and CAS-write &ovl->buckets[probe] hash_mask = slot_count - 1, applied to every probe in the pcache &info->base_inodes[ino - 1], guarded only by ino <= base_inode_count daxfs_mem_ptr() validates the start offset alone, so a forged count makes those accesses run past the mapped region. The overlay and pcache headers are worse: bucket_offset, pool_offset, pool_size, slot_meta_offset and slot_data_offset were validated nowhere at all, not even under the validate mount option, and the header itself was dereferenced straight off an unchecked daxfs_mem_ptr() result. Check each of these where the value becomes a pointer or an array bound. A forged bucket_count of 0x10000000 in a 256 MB image is now refused with "overlay bucket/pool exceed the overlay region" instead of probing four gigabytes past the end of the mapping. The validate mount option is deliberately left alone. These are preconditions for memory safety rather than data-quality checks, so they belong on the default path, but the O(n) structural and per-inode scans in validate.c stay opt-in as before. Tested with a throwaway mkdaxfs that forges the superblock count. A non-power-of-two count is rejected at overlay init where the unfixed module logged "overlay initialized (3 buckets)" and carried on; an oversized count is rejected before any bucket is touched. The oversized case was not run against the unfixed module on purpose, since that path CAS-writes outside the mapping. Existing images are unaffected: the suite passes 20/20 across static, split and empty modes. --- daxfs/overlay.c | 73 +++++++++++++++++++++++++++++++++++++++++-------- daxfs/pcache.c | 41 ++++++++++++++++++++++++--- daxfs/super.c | 19 +++++++++++-- 3 files changed, 115 insertions(+), 18 deletions(-) diff --git a/daxfs/overlay.c b/daxfs/overlay.c index 6966633..e109b02 100644 --- a/daxfs/overlay.c +++ b/daxfs/overlay.c @@ -404,37 +404,88 @@ int daxfs_overlay_init(struct daxfs_info *info) { struct daxfs_overlay *ovl; u64 ovl_offset = le64_to_cpu(info->super->overlay_offset); + u64 ovl_size = le64_to_cpu(info->super->overlay_size); struct daxfs_overlay_header *hdr; + u64 bucket_offset, bucket_bytes, pool_offset, pool_size; + u32 bucket_count; if (!ovl_offset) return 0; - ovl = kzalloc(sizeof(*ovl), GFP_KERNEL); - if (!ovl) - return -ENOMEM; + /* The header is dereferenced immediately below */ + if (!daxfs_valid_offset(info, ovl_offset, sizeof(*hdr))) { + pr_err("daxfs: overlay header outside image bounds\n"); + return -EINVAL; + } hdr = daxfs_mem_ptr(info, ovl_offset); if (le32_to_cpu(hdr->magic) != DAXFS_OVERLAY_MAGIC) { pr_err("daxfs: invalid overlay magic 0x%x\n", le32_to_cpu(hdr->magic)); - kfree(ovl); return -EINVAL; } if (le32_to_cpu(hdr->version) != DAXFS_OVERLAY_VERSION) { pr_err("daxfs: unsupported overlay version %u (expected %u)\n", le32_to_cpu(hdr->version), DAXFS_OVERLAY_VERSION); - kfree(ovl); return -EINVAL; } + bucket_count = le32_to_cpu(info->super->overlay_bucket_count); + bucket_offset = le64_to_cpu(hdr->bucket_offset); + pool_offset = le64_to_cpu(hdr->pool_offset); + pool_size = le64_to_cpu(hdr->pool_size); + + /* + * bucket_mask is bucket_count - 1 and is applied to every probe, so a + * non-power-of-two count silently stops covering the array. + */ + if (!bucket_count || !is_power_of_2(bucket_count)) { + pr_err("daxfs: overlay bucket_count %u is not a power of 2\n", + bucket_count); + return -EINVAL; + } + + bucket_bytes = (u64)bucket_count * + sizeof(struct daxfs_overlay_bucket); + + /* + * These three come from the image and are never re-checked: every + * index in [0, bucket_count) is dereferenced by the probe loops, and + * overlay_pool_ptr() bounds pool offsets against pool_size alone. Both + * spans must therefore sit inside the overlay region and the mapping, + * or a malformed image reads and CAS-writes outside it. + */ + if (bucket_offset > ovl_size || + bucket_bytes > ovl_size - bucket_offset || + pool_offset > ovl_size || + pool_size > ovl_size - pool_offset) { + pr_err("daxfs: overlay bucket/pool exceed the overlay region\n"); + return -EINVAL; + } + + if (!daxfs_valid_offset(info, ovl_offset + bucket_offset, + bucket_bytes) || + !daxfs_valid_offset(info, ovl_offset + pool_offset, pool_size)) { + pr_err("daxfs: overlay bucket/pool exceed image bounds\n"); + return -EINVAL; + } + + if (bucket_offset < pool_offset + pool_size && + pool_offset < bucket_offset + bucket_bytes) { + pr_err("daxfs: overlay bucket array overlaps the pool\n"); + return -EINVAL; + } + + ovl = kzalloc(sizeof(*ovl), GFP_KERNEL); + if (!ovl) + return -ENOMEM; + ovl->header = hdr; ovl->mem_model = info->mem_model; - ovl->bucket_count = le32_to_cpu(info->super->overlay_bucket_count); - ovl->bucket_mask = ovl->bucket_count - 1; - ovl->buckets = daxfs_mem_ptr(info, - ovl_offset + le64_to_cpu(hdr->bucket_offset)); - ovl->pool = daxfs_mem_ptr(info, - ovl_offset + le64_to_cpu(hdr->pool_offset)); + ovl->bucket_count = bucket_count; + ovl->bucket_mask = bucket_count - 1; + ovl->buckets = daxfs_mem_ptr(info, ovl_offset + bucket_offset); + ovl->pool = daxfs_mem_ptr(info, ovl_offset + pool_offset); info->overlay = ovl; diff --git a/daxfs/pcache.c b/daxfs/pcache.c index de6bee8..a9c67ab 100644 --- a/daxfs/pcache.c +++ b/daxfs/pcache.c @@ -696,6 +696,7 @@ int daxfs_pcache_init(struct daxfs_info *info, const char *backing_path) { struct daxfs_pcache *pc; u64 pcache_offset = le64_to_cpu(info->super->pcache_offset); + u64 pcache_size = le64_to_cpu(info->super->pcache_size); struct daxfs_pcache_header *hdr; if (!pcache_offset) @@ -710,6 +711,12 @@ int daxfs_pcache_init(struct daxfs_info *info, const char *backing_path) pc->block_size = info->block_size; pc->block_shift = ilog2(info->block_size); + if (!daxfs_valid_offset(info, pcache_offset, sizeof(*hdr))) { + pr_err("daxfs: pcache header outside image bounds\n"); + kfree(pc); + return -EINVAL; + } + hdr = daxfs_mem_ptr(info, pcache_offset); if (le32_to_cpu(hdr->magic) != DAXFS_PCACHE_MAGIC) { pr_err("daxfs: invalid pcache magic 0x%x\n", @@ -722,10 +729,36 @@ int daxfs_pcache_init(struct daxfs_info *info, const char *backing_path) /* Read layout from main superblock */ pc->slot_count = le32_to_cpu(info->super->pcache_slot_count); pc->hash_mask = pc->slot_count - 1; - pc->slots = daxfs_mem_ptr(info, - pcache_offset + le64_to_cpu(hdr->slot_meta_offset)); - pc->data = daxfs_mem_ptr(info, - pcache_offset + le64_to_cpu(hdr->slot_data_offset)); + + { + u64 meta_offset = le64_to_cpu(hdr->slot_meta_offset); + u64 data_offset = le64_to_cpu(hdr->slot_data_offset); + u64 meta_bytes = (u64)pc->slot_count * + sizeof(struct daxfs_pcache_slot); + u64 data_bytes = (u64)pc->slot_count * pc->block_size; + + /* + * Same exposure as the overlay: every slot index in + * [0, slot_count) is dereferenced without further checking, + * so both arrays must be inside the pcache region and the + * mapping. + */ + if (meta_offset > pcache_size || + meta_bytes > pcache_size - meta_offset || + data_offset > pcache_size || + data_bytes > pcache_size - data_offset || + !daxfs_valid_offset(info, pcache_offset + meta_offset, + meta_bytes) || + !daxfs_valid_offset(info, pcache_offset + data_offset, + data_bytes)) { + pr_err("daxfs: pcache slot arrays exceed region bounds\n"); + kfree(pc); + return -EINVAL; + } + + pc->slots = daxfs_mem_ptr(info, pcache_offset + meta_offset); + pc->data = daxfs_mem_ptr(info, pcache_offset + data_offset); + } info->pcache = pc; diff --git a/daxfs/super.c b/daxfs/super.c index 9544260..a56592a 100644 --- a/daxfs/super.c +++ b/daxfs/super.c @@ -222,12 +222,25 @@ static int daxfs_fill_super(struct super_block *sb, struct fs_context *fc) if (le64_to_cpu(info->super->base_offset)) { u64 base_off = le64_to_cpu(info->super->base_offset); struct daxfs_super *s = info->super; + u64 inode_off = base_off + le64_to_cpu(s->inode_offset); + u32 inode_count = le32_to_cpu(s->inode_count); - info->base_inodes = daxfs_mem_ptr(info, - base_off + le64_to_cpu(s->inode_offset)); + /* + * inode_count bounds every &base_inodes[ino - 1] lookup and + * nothing downstream re-checks it, so the table has to be + * mapped even when the caller did not ask for validation. + */ + if (!daxfs_valid_offset(info, inode_off, (u64)inode_count * + sizeof(struct daxfs_base_inode))) { + pr_err("daxfs: base inode table exceeds image bounds\n"); + ret = -EINVAL; + goto err_unmap; + } + + info->base_inodes = daxfs_mem_ptr(info, inode_off); info->base_data_offset = base_off + le64_to_cpu(s->data_offset); - info->base_inode_count = le32_to_cpu(s->inode_count); + info->base_inode_count = inode_count; /* Validate base image structure (if requested) */ if (ctx->validate) {