Re: [PATCH 3/3] md/md-llbitmap: fail resize that needs more bitmap pages

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The newly added in-memory bounds check is vulnerable to integer overflow, allowing out-of-bounds heap access.

Pre-existing issues:
- [Critical] llbitmap_resize() and llbitmap_init() allow the on-disk bitmap size to exceed the allocated space, causing out-of-bounds disk writes.
- [High] Array growth leaves newly added space unsynced due to uninitialized bitmap states.
--

--- Patch [3]: [PATCH 3/3] md/md-llbitmap: fail resize that needs more bitmap pages ---
commit 938426872ff1437636e82451faff612783d42c3f
Author: Mykola Marzhan <[email protected]>

md/md-llbitmap: fail resize that needs more bitmap pages

llbitmap_resize() only updates chunkshift/chunksize/chunks.  The
page cache backing the bitmap (pctl[] and nr_pages) is sized at
creation and never grown, so a grow whose chunk count crosses into a
never-allocated page leaves the data path indexing past the end of
pctl[].

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 5a4e2abaa7577..e8f853e8461d7 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1150,6 +1150,32 @@ static int llbitmap_resize(struct mddev *mddev, sector_t blocks, int chunksize)
>  		chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
>  	}

[Severity: Critical]
This is a pre-existing issue, but does this loop condition allow the on-disk
bitmap size to exceed the allocated space?

The lockless bitmap requires chunks + BITMAP_DATA_OFFSET bytes of space on disk,
but the while loop check ignores BITMAP_DATA_OFFSET:

	while (chunks > mddev->bitmap_info.space << SECTOR_SHIFT) {
		chunksize = chunksize << 1;
		chunks = DIV_ROUND_UP_SECTOR_T(blocks, chunksize);
	}

If chunks is close to the total space, adding BITMAP_DATA_OFFSET before
dividing by PAGE_SIZE can result in an nr_pages value whose total size exceeds
the allocated on-disk space. When __llbitmap_flush() writes these pages to
disk, could it overwrite adjacent data like the md superblock or actual array
data?

>  
> +	/*
> +	 * A changed chunksize would require remapping every on-disk bit:
> +	 * each chunk would cover a different sector range.  Refuse rather
> +	 * than corrupt the existing intent record.
> +	 */
> +	if (chunksize != llbitmap->chunksize) {

[ ... ]

> +	/*
> +	 * pctl[]/nr_pages are sized at creation and not grown here; a
> +	 * grow past the last allocated page would index past pctl[].
> +	 */
> +	if (DIV_ROUND_UP(chunks + BITMAP_DATA_OFFSET, PAGE_SIZE) >
> +	    llbitmap->nr_pages) {

[Severity: High]
Can the chunks + BITMAP_DATA_OFFSET calculation overflow on 32-bit systems?

Since chunks is an unsigned long, if a user requests a huge array size
(e.g., > 2.2 TB with 512-byte chunks), chunks can be close to ULONG_MAX. The
addition of BITMAP_DATA_OFFSET would then wrap around to a small number,
causing this bounds check to evaluate to false and bypassing the safeguard.

Could this allow llbitmap_read() to index the pctl[] array far out of bounds?

> +		pr_warn("md/llbitmap: %s: cannot grow to %lu chunks: needs %lu bitmap pages, only %u allocated; growing the bitmap is not yet supported\n",
> +			mdname(mddev), chunks,
> +			DIV_ROUND_UP(chunks + BITMAP_DATA_OFFSET, PAGE_SIZE),
> +			llbitmap->nr_pages);
> +		return -ENOSPC;
> +	}
> +
>  	llbitmap->chunkshift = ffz(~chunksize);
>  	llbitmap->chunksize = chunksize;
>  	llbitmap->chunks = chunks;

[Severity: High]
This isn't a bug introduced by this patch, but does updating llbitmap->chunks
without initializing the newly exposed chunks leave the added space unsynced?

When an array is grown, the newly added chunks in the page cache default to
BitUnwritten (0) or stale values instead of BitNeedSync. When
llbitmap_start_sync() evaluates them, it will see them as BitNone and return
false.

Does this cause md_do_sync() to skip syncing the newly added sectors entirely,
leading to silent data inconsistency across mirrors?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3