Re: [PATCH btrfs v2] btrfs: reject free space cache with more entries than pages

[Xiang Mei]

Thanks for your review. We'll have a bug report for the future bugs
before sending the patch.

Xiang

On Wed, Jun 10, 2026 at 2:30 PM Qu Wenruo <wqu@xxxxxxxx> wrote:
>
>
>
> 在 2026/6/11 02:59, Xiang Mei 写道:
> > When loading a v1 free space cache, __load_free_space_cache() takes
> > num_entries and num_bitmaps straight from the on-disk
> > btrfs_free_space_header. That header is stored in the tree_root under a key
> > with type 0, which the tree-checker has no case for, so neither count is
> > validated before the load trusts it.
> >
> > The load loops num_entries times and maps the next page whenever the current
> > one runs out, going through io_ctl_check_crc() -> io_ctl_map_page(), which
> > does io_ctl->pages[io_ctl->index++]. But pages[] is allocated in
> > io_ctl_init() from the cache inode's i_size, not from num_entries:
> >
> >       num_pages = DIV_ROUND_UP(i_size_read(inode), PAGE_SIZE);
> >       io_ctl->pages = kcalloc(num_pages, sizeof(struct page *), GFP_NOFS);
> >
> > So if num_entries claims more records than the pages can hold, io_ctl->index
> > runs off the end of pages[]. The write side never hits this because
> > io_ctl_add_entry() and io_ctl_add_bitmap() both stop once
> > io_ctl->index >= io_ctl->num_pages; the read side just never had the same
> > check.
> >
> > To trigger it, take a clean cache (num_entries = <N> here), set num_entries
> > in the header to 0x10000, and fix up the leaf checksum so it still passes
> > the tree-checker. The cache inode has i_size = 65536, so num_pages is 16 and
> > pages[] is a 16-pointer (kmalloc-128) array. The load now tries to read
> > 65536 entries, io_ctl->index walks up to 16, and pages[16] is read past the
> > array:
> >
> >    BUG: KASAN: slab-out-of-bounds in io_ctl_check_crc (fs/btrfs/free-space-cache.c:420 fs/btrfs/free-space-cache.c:565)
> >    Read of size 8 at addr ffff88800c833a80 by task kworker/u8:3/58
> >     io_ctl_check_crc (fs/btrfs/free-space-cache.c:420 fs/btrfs/free-space-cache.c:565)
> >     __load_free_space_cache (fs/btrfs/free-space-cache.c:655 fs/btrfs/free-space-cache.c:820)
> >     load_free_space_cache (fs/btrfs/free-space-cache.c:1017)
> >     caching_thread (fs/btrfs/block-group.c:880)
> >     btrfs_work_helper (fs/btrfs/async-thread.c:312)
> >     process_one_work
> >     worker_thread
> >     kthread
> >     ret_from_fork
> >
> > free-space-cache.c:420 is io_ctl_map_page(), inlined into io_ctl_check_crc()
> > at line 565, which is why that is the frame KASAN names. The out-of-bounds
> > slot is then treated as a struct page and handed to crc32c(), so the bad
> > read turns into a GP fault.
> >
> > Add the missing check to io_ctl_check_crc(), which is where both the entry
> > loop and the bitmap loop end up. When num_entries is too large the load now
> > fails like any corrupt cache: __load_free_space_cache() drops it and rebuilds
> > the free space from the extent tree, so a valid cache is never rejected.
> >
> > Fixes: 5b0e95bf607d ("Btrfs: inline checksums into the disk free space cache")
> > Reported-by: Weiming Shi <bestswngs@xxxxxxxxx>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@xxxxxxx>
>
> Reviewed-by: Qu Wenruo <wqu@xxxxxxxx>
>
> I'll add a Link: tag to the reproducer at merge time.
>
> But I still strongly prefer you guys to submit a bug report/reproducer
> before sending out a fix.
>
> > ---
> > v2: add more details in the commit message
> >
> >   fs/btrfs/free-space-cache.c | 3 +++
> >   1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> > index ab22e4f9ffdd..bbc4db7fe74b 100644
> > --- a/fs/btrfs/free-space-cache.c
> > +++ b/fs/btrfs/free-space-cache.c
> > @@ -555,6 +555,9 @@ static int io_ctl_check_crc(struct btrfs_io_ctl *io_ctl, int index)
> >       u32 crc = ~(u32)0;
> >       unsigned offset = 0;
> >
> > +     if (index >= io_ctl->num_pages)
> > +             return -EIO;
> > +
> >       if (index == 0)
> >               offset = sizeof(u32) * io_ctl->num_pages;
> >
>