Re: [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Heap buffer overflow in `llbitmap_write()` during array reshape due to missing `BITMAP_DATA_OFFSET` in `reserved_pages` calculation.
- [High] Denial of Service via eager unbounded memory allocation based on untrusted on-disk `sectors_reserved`.
--

--- Patch [14]: [PATCH v5 14/29] md/md-llbitmap: allocate page controls independently ---
commit f1fb318905483b935b4e96f1cbe5fe6c77c4134e
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: allocate page controls independently

This commit allocates llbitmap page-control objects independently rather
than as a single array. It also modifies llbitmap_read_page() to return a
zeroed page when beyond the current size, preparing for in-place growth.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 0813cebfbdeb3..300dd8b93b01e 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> +static unsigned int llbitmap_reserved_pages(struct llbitmap *llbitmap)
> +{
> +	return DIV_ROUND_UP(llbitmap->mddev->bitmap_info.space << SECTOR_SHIFT,
> +			    PAGE_SIZE);
> +}

[Severity: Critical]
Does this reserved pages calculation need to account for BITMAP_DATA_OFFSET?

When the array grows to the max chunks allowed by space during a reshape,
llbitmap_write() will add BITMAP_DATA_OFFSET to the position:

llbitmap_write() {
        pos += BITMAP_DATA_OFFSET;
        idx = pos >> PAGE_SHIFT;
        bit = offset_in_page(pos);

        llbitmap->pctl[idx]->state[bit] = state;
        ...
}

If space happens to align closely to a page boundary, reserved_pages could
evaluate to 1, but the allowed chunks plus BITMAP_DATA_OFFSET could result
in pos crossing into the next page. Will this cause idx to exceed the pctl
array capacity and trigger an out-of-bounds write?

> +static int llbitmap_alloc_pages(struct llbitmap *llbitmap)
> +{
> +	unsigned int used_pages = llbitmap_used_pages(llbitmap, llbitmap->chunks);
> +	unsigned int nr_pages = max(used_pages, llbitmap_reserved_pages(llbitmap));
> +	int i;
> +
> +	llbitmap->pctl = kcalloc(nr_pages, sizeof(*llbitmap->pctl), GFP_NOIO);
> +	if (!llbitmap->pctl)
> +		return -ENOMEM;
> +
> +	llbitmap->nr_pages = nr_pages;
> +
> +	for (i = 0; i < nr_pages; i++) {
> +		llbitmap->pctl[i] = llbitmap_alloc_page_ctl(llbitmap, i);

[Severity: High]
Is it safe to eagerly allocate all possible page controls based on the
on-disk sectors_reserved value?

If a malicious or corrupted superblock provides a massive sectors_reserved
value, this loop will invoke llbitmap_alloc_page_ctl() hundreds of
thousands of times for unused capacity:

llbitmap_alloc_page_ctl() {
        pctl = kzalloc(size, GFP_NOIO);
        ...
        page = llbitmap_read_page(llbitmap, idx);
        ...
        if (percpu_ref_init(&pctl->active, active_release,
                            PERCPU_REF_ALLOW_REINIT, GFP_NOIO)) {
        ...
}

Could this lead to severe kernel memory exhaustion and trigger the OOM
killer, since GFP_NOIO aggressively reclaims memory during these
allocations?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802195038.164272-1-yukuai@kernel.org?part=14