From: sashiko-bot@kernel.org
To: "Mykola Marzhan" <mykola@meshstor.io>
Cc: yukuai@fygo.io, linux-raid@vger.kernel.org
Subject: Re: [PATCH 3/3] md/md-llbitmap: fail resize that needs more bitmap pages
Date: Sun, 19 Jul 2026 15:05:46 +0000 [thread overview]
Message-ID: <20260719150546.F3F801F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260719144227.940444-4-mykola@meshstor.io>
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 <mykola@meshstor.io>
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/20260719144227.940444-1-mykola@meshstor.io?part=3
prev parent reply other threads:[~2026-07-19 15:05 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-07-19 14:42 [PATCH 0/3] md: fix silent data loss in raid10 recovery and llbitmap resize Mykola Marzhan
2026-07-19 14:42 ` [PATCH 1/3] md/raid10: restore still_degraded detection in recovery Mykola Marzhan
2026-07-20 2:17 ` yu kuai
2026-07-20 4:32 ` Paul Menzel
2026-07-19 14:42 ` [PATCH 2/3] md: only consult skip_sync_blocks when 'j' is in the bitmap's domain Mykola Marzhan
2026-07-20 2:25 ` yu kuai
2026-07-19 14:42 ` [PATCH 3/3] md/md-llbitmap: fail resize that needs more bitmap pages Mykola Marzhan
2026-07-19 15:05 ` sashiko-bot [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260719150546.F3F801F000E9@smtp.kernel.org \
--to=sashiko-bot@kernel.org \
--cc=linux-raid@vger.kernel.org \
--cc=mykola@meshstor.io \
--cc=sashiko-reviews@lists.linux.dev \
--cc=yukuai@fygo.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.