Re: [PATCH v4 09/25] md/md-llbitmap: track bitmap sync_size explicitly

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Missing bounds check on `sync_size` causes out-of-bounds memory accesses and disk overwrites.
--

--- Patch [9]: [PATCH v4 09/25] md/md-llbitmap: track bitmap sync_size explicitly ---
commit b06a020ec2776bd2bc0418543c90f7ea1a1e754b
Author: Yu Kuai <yukuai@fygo.io>

md/md-llbitmap: track bitmap sync_size explicitly

Track llbitmap's own sync_size instead of always using
mddev->resync_max_sectors directly. This is the minimal bookkeeping needed
before llbitmap can track old and new reshape geometry independently.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 1fa2adf0ada1c..3417da288e994 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c

[ ... ]

> @@ -987,6 +991,9 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>  		goto out_put_page;
>  	}
>  
> +	sync_size = le64_to_cpu(sb->sync_size);
> +	if (!sync_size)
> +		sync_size = mddev->resync_max_sectors;

[Severity: High]
Does this code bypass capacity validation if the superblock is corrupted or
tampered?

If an array is assembled with a bitmap superblock where sb->sync_size is
artificially large or small, this untrusted sync_size is read here.

However, it appears the bounds check later in llbitmap_read_sb() still
validates against mddev->resync_max_sectors instead of the new sync_size:

	if (chunksize < DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
					      mddev->bitmap_info.space << SECTOR_SHIFT)) {

Could this allow maliciously large or small chunk requirements to bypass
capacity validation?

>  	chunksize = le32_to_cpu(sb->chunksize);
>  	if (!is_power_of_2(chunksize)) {
>  		pr_err("md/llbitmap: %s: chunksize not a power of 2",

[ ... ]

> @@ -1022,8 +1029,9 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>  
>  	llbitmap->barrier_idle = DEFAULT_BARRIER_IDLE;
>  	llbitmap->chunksize = chunksize;
> -	llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors, chunksize);
> +	llbitmap->chunks = DIV_ROUND_UP_SECTOR_T(sync_size, chunksize);

[Severity: High]
Is it safe to compute llbitmap->chunks using the unvalidated sync_size here?

Since the validation check above still uses mddev->resync_max_sectors, an
artificially large sync_size will cause llbitmap->chunks to exceed the
allocated metadata space. This breaks the capacity validation invariant.

Could this cause __llbitmap_flush() and llbitmap_write_page() to write bitmap
data out-of-bounds on the underlying disk, overwriting adjacent partitions?

Additionally, if sync_size is artificially small, the llbitmap->pctl array
allocation (nr_pages) will be undersized. When IO arrives at valid offsets
bounded by the block layer capacity, fast paths like llbitmap_start_write()
could calculate a page_start index that exceeds nr_pages:

drivers/md/md-llbitmap.c:llbitmap_start_write() {
    ...
	int page_start = (start + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;
	int page_end = (end + BITMAP_DATA_OFFSET) >> PAGE_SHIFT;

	while (page_start <= page_end) {
		llbitmap_raise_barrier(llbitmap, page_start);
		page_start++;
	}
    ...
}

Could this lead to an out-of-bounds memory access on the pctl array when
calling llbitmap_raise_barrier()?

>  	llbitmap->chunkshift = ffz(~chunksize);
> +	llbitmap->sync_size = sync_size;
>  	ret = llbitmap_cache_pages(llbitmap);

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