[PATCH 0/3] md: fix silent data loss in raid10 recovery and llbitmap resize

[PATCH 0/3] md: fix silent data loss in raid10 recovery and llbitmap resize

[Mykola Marzhan]

While testing llbitmap we found three independent bugs, all in
the silent-data-loss class.

Patch 1 is a one-line v6.12 regression (it reaches the 6.12 LTS
line): the bitmap_operations conversion inverted raid10's
still_degraded detection, so a rebuild on a still-degraded raid10
consumes write-intent bits an absent member still needs, and a
later --re-add serves stale data.

Patch 2: raid10 recovery with llbitmap consults skip_sync_blocks()
with per-device offsets the bitmap cannot interpret, so rebuilds
complete in milliseconds having copied nothing and the array
reports optimal with a stale member.

Patch 3: llbitmap_resize() indexes past the end of its page-control
array (grow crossing a bitmap-page boundary) or silently remaps
chunks (chunksize change); make it fail cleanly instead.  The
pending "md/md-llbitmap: support reshape for RAID10 and RAID5"
series implements real in-place growth and supersedes this guard;
patch 3 exists for mainline today and for stable.

Testing: each bug was reproduced on an unpatched kernel and the fix
verified on a patched one.  Patch 1: classic-bitmap near-2 array,
fail one member of the higher pair, write, fail one member of the
lower pair, rebuild a spare, then --re-add the first failed member
-- it now resyncs the writes it missed instead of returning
instantly with stale data.  Patch 2: near-2 and far-2 spare-add
rebuilds finished in well under a second without the degraded-era
writes (near-2 copied nothing; far-2 only the clean regions) --
fixed, a real recovery runs and the writes are present on the
rebuilt member; a same-member runtime re-add on near-2 was verified
the same way.  Patch 3: the out-of-bounds access was demonstrated
under KASAN; the patched kernel refuses the resize while shrinks
and in-bounds grows keep working.  Each series builds W=1-clean on
its own, applies to current master and to md-7.2, and the two apply
and build together.  The patches are independent of each other.

The patches were developed with AI assistance (see the Assisted-by
trailers); all code was human-reviewed and tested as described above.

Mykola Marzhan (3):
  md/raid10: restore still_degraded detection in recovery
  md: only consult skip_sync_blocks when 'j' is in the bitmap's domain
  md/md-llbitmap: fail resize that needs more bitmap pages

 drivers/md/md-llbitmap.c | 26 ++++++++++++++++++++++++++
 drivers/md/md.c          | 13 ++++++++++++-
 drivers/md/md.h          | 11 +++++++++++
 drivers/md/raid1.c       |  1 +
 drivers/md/raid10.c      |  2 +-
 drivers/md/raid5.c       |  3 +++
 6 files changed, 54 insertions(+), 2 deletions(-)


base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
-- 
2.43.0

[PATCH 1/3] md/raid10: restore still_degraded detection in recovery

[Mykola Marzhan]

raid10_sync_request()'s recovery arm passes still_degraded -- "will
the array still be degraded after this recovery completes?" -- to
md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
NEEDED marker, the write-intent record.  That is only safe when the
recovery restores full redundancy; otherwise the record still
describes writes an absent member missed.

Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
into bitmap_operations") turned the detection loop's only assignment
from "still_degraded = 1" into "still_degraded = false" in its
int-to-bool styling pass, so the flag has been constant false ever
since.  raid1 and raid5 were converted to "= true"; only raid10 was
inverted.

The result is silent data loss on a classic-bitmap raid10 that stays
degraded across a recovery.  With mirror pairs {A,B} and {C,D}: lose
D, keep writing, lose B, add a spare.  The rebuild into B's slot
strips NEEDED from every chunk -- including chunks whose only
unsatisfied intent belonged to D -- so a later --re-add of D sees an
empty intent record, completes instantly, and serves stale data.

Restore the pre-conversion assignment.  Only the classic-bitmap
recovery arm is affected: llbitmap ignores the degraded parameter,
and the resync arm passes mddev->degraded directly.

Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
Cc: stable@vger.kernel.org # v6.12+
Assisted-by: Claude-Code:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/raid10.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
index 0a3cfdd3f5df..54cddb3a98cd 100644
--- a/drivers/md/raid10.c
+++ b/drivers/md/raid10.c
@@ -3365,7 +3365,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
 				struct md_rdev *rdev = conf->mirrors[j].rdev;
 
 				if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
-					still_degraded = false;
+					still_degraded = true;
 					break;
 				}
 			}
-- 
2.43.0

Re: [PATCH 1/3] md/raid10: restore still_degraded detection in recovery

[yu kuai]

在 2026/7/19 22:42, Mykola Marzhan 写道:

> raid10_sync_request()'s recovery arm passes still_degraded -- "will
> the array still be degraded after this recovery completes?" -- to
> md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
> NEEDED marker, the write-intent record.  That is only safe when the
> recovery restores full redundancy; otherwise the record still
> describes writes an absent member missed.
>
> Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
> into bitmap_operations") turned the detection loop's only assignment
> from "still_degraded = 1" into "still_degraded = false" in its
> int-to-bool styling pass, so the flag has been constant false ever
> since.  raid1 and raid5 were converted to "= true"; only raid10 was
> inverted.
>
> The result is silent data loss on a classic-bitmap raid10 that stays
> degraded across a recovery.  With mirror pairs {A,B} and {C,D}: lose
> D, keep writing, lose B, add a spare.  The rebuild into B's slot
> strips NEEDED from every chunk -- including chunks whose only
> unsatisfied intent belonged to D -- so a later --re-add of D sees an
> empty intent record, completes instantly, and serves stale data.
>
> Restore the pre-conversion assignment.  Only the classic-bitmap
> recovery arm is affected: llbitmap ignores the degraded parameter,
> and the resync arm passes mddev->degraded directly.
>
> Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
> Cc:stable@vger.kernel.org # v6.12+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan<mykola@meshstor.io>
> ---
>   drivers/md/raid10.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>

-- 
Thanks,
Kuai

Re: [PATCH 1/3] md/raid10: restore still_degraded detection in recovery

[Paul Menzel]

Dear Mykola,


Thank you for your patch.

Am 19.07.26 um 16:42 schrieb Mykola Marzhan:
> raid10_sync_request()'s recovery arm passes still_degraded -- "will
> the array still be degraded after this recovery completes?" -- to
> md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
> NEEDED marker, the write-intent record.  That is only safe when the
> recovery restores full redundancy; otherwise the record still
> describes writes an absent member missed.
> 
> Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
> into bitmap_operations") turned the detection loop's only assignment
> from "still_degraded = 1" into "still_degraded = false" in its
> int-to-bool styling pass, so the flag has been constant false ever
> since.  raid1 and raid5 were converted to "= true"; only raid10 was
> inverted.
> 
> The result is silent data loss on a classic-bitmap raid10 that stays
> degraded across a recovery.  With mirror pairs {A,B} and {C,D}: lose
> D, keep writing, lose B, add a spare.  The rebuild into B's slot
> strips NEEDED from every chunk -- including chunks whose only
> unsatisfied intent belonged to D -- so a later --re-add of D sees an
> empty intent record, completes instantly, and serves stale data.
> 
> Restore the pre-conversion assignment.  Only the classic-bitmap
> recovery arm is affected: llbitmap ignores the degraded parameter,
> and the resync arm passes mddev->degraded directly.
> 
> Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
> Cc: stable@vger.kernel.org # v6.12+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
>   drivers/md/raid10.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df..54cddb3a98cd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3365,7 +3365,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
>   				struct md_rdev *rdev = conf->mirrors[j].rdev;
>   
>   				if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
> -					still_degraded = false;
> +					still_degraded = true;
>   					break;
>   				}
>   			}

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

[PATCH 2/3] md: only consult skip_sync_blocks when 'j' is in the bitmap's domain

[Mykola Marzhan]

Rebuilding a raid10 member with llbitmap can complete in a few
hundred milliseconds having copied nothing: md_do_sync() offers
every position 'j' to bitmap_ops->skip_sync_blocks(), the bitmap
answers for the wrong chunks, the whole rebuild is skipped as
"unwritten", and the array reports optimal with a stale member.  The
staleness is typically noticed only when the mirror partner fails;
writes made while the array was degraded vanish.

The bitmap spans [0, resync_max_sectors) in the personality's
sync-cursor space.  Resync walks exactly that range for every
personality and may always consult the bitmap.  Recovery walks
per-device offsets, which match the bitmap's space only for raid1
(device space is array space) and raid456 (the bitmap is indexed by
the per-device sync cursor; bitmap_sector() translates array IO
into it).  raid10's bitmap is in array space -- near-2 on four
disks spans twice a member's size; far-2 on two disks puts array
chunk 2c+d at device chunk c of disk d -- so
llbitmap_skip_sync_blocks() was consulted with offsets it cannot
interpret.

Only call the hook when the offsets match what the bitmap expects.
Reshape must visit every stripe regardless of bitmap state --
skipping would desync reshape_position from 'j' -- so never skip
there.  For recovery, add an md_personality capability,
recovery_in_bitmap_space, set by raid1 and raid456.  raid10 leaves
it unset (raid10_find_virt() needs the disk number, which
md_do_sync() does not have); raid0 and linear too, but no recovery
path of theirs reaches the hook.  No performance is lost against any
released kernel: the hook is llbitmap-only and new in v6.18, so
raid10 recovery returns to its earlier path -- walk every stripe,
skip clean chunks inside raid10_sync_request().

A capability rather than a geometry test: for raid10 layouts with
raid_disks == near_copies * far_copies, resync_max_sectors equals
dev_sectors while the spaces can still differ (far and offset
variants), so an arithmetic proxy reopens the same hole on those
layouts.  Disabling the hook for all recovery instead would
forfeit llbitmap's fast-forward over unwritten chunks for raid1 and
raid456 rebuilds.  Opt-in is also the safe default: a new
personality gets no bitmap consultation during recovery until it
declares otherwise.

Verified for near-2 and far-2 layouts: a rebuild onto an added spare
copies the writes made while the array was degraded.

Fixes: f196d7288864 ("md/md-bitmap: add a new method skip_sync_blocks() in bitmap_operations")
Cc: stable@vger.kernel.org # v6.18+
Assisted-by: Claude-Code:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/md.c    | 13 ++++++++++++-
 drivers/md/md.h    | 11 +++++++++++
 drivers/md/raid1.c |  1 +
 drivers/md/raid5.c |  3 +++
 4 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..d60ea7aaca3a 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -9846,7 +9846,18 @@ void md_do_sync(struct md_thread *thread)
 		if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
 			break;
 
-		if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks) {
+		/*
+		 * The bitmap may be consulted with 'j' for a plain resync,
+		 * but for recovery only when the personality declares that
+		 * its recovery cursor addresses the bitmap's space; raid10
+		 * recovery walks per-device offsets the bitmap cannot
+		 * interpret.  Reshape must relocate every stripe regardless
+		 * of bitmap state, so never skip there either.
+		 */
+		if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks &&
+		    !test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
+		    (!test_bit(MD_RECOVERY_RECOVER, &mddev->recovery) ||
+		     mddev->pers->recovery_in_bitmap_space)) {
 			sectors = mddev->bitmap_ops->skip_sync_blocks(mddev, j);
 			if (sectors)
 				goto update;
diff --git a/drivers/md/md.h b/drivers/md/md.h
index d8daf0f75cbb..a490a47736b0 100644
--- a/drivers/md/md.h
+++ b/drivers/md/md.h
@@ -798,6 +798,17 @@ struct md_personality
 	/* convert io ranges from array to bitmap */
 	void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
 			      unsigned long *sectors);
+	/*
+	 * The position md_do_sync() walks during MD_RECOVERY_RECOVER
+	 * addresses the space the write-intent bitmap is indexed by, so
+	 * recovery may consult bitmap_ops->skip_sync_blocks() directly.
+	 * True for raid1 (device space is array space) and raid456 (the
+	 * bitmap is indexed by the per-device sync cursor).  raid10 must
+	 * leave this unset: its recovery walks per-device offsets while
+	 * its bitmap is indexed by array sectors, and translating needs
+	 * the disk number, which md_do_sync() does not have.
+	 */
+	bool recovery_in_bitmap_space;
 };
 
 struct md_sysfs_entry {
diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
index afe2ca96ad8c..1222c0470ab6 100644
--- a/drivers/md/raid1.c
+++ b/drivers/md/raid1.c
@@ -3518,6 +3518,7 @@ static struct md_personality raid1_personality =
 	.check_reshape	= raid1_reshape,
 	.quiesce	= raid1_quiesce,
 	.takeover	= raid1_takeover,
+	.recovery_in_bitmap_space = true,
 };
 
 static int __init raid1_init(void)
diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
index ffb5fcde54a9..25f8d829a986 100644
--- a/drivers/md/raid5.c
+++ b/drivers/md/raid5.c
@@ -9078,6 +9078,7 @@ static struct md_personality raid6_personality =
 	.change_consistency_policy = raid5_change_consistency_policy,
 	.prepare_suspend = raid5_prepare_suspend,
 	.bitmap_sector	= raid5_bitmap_sector,
+	.recovery_in_bitmap_space = true,
 };
 static struct md_personality raid5_personality =
 {
@@ -9108,6 +9109,7 @@ static struct md_personality raid5_personality =
 	.change_consistency_policy = raid5_change_consistency_policy,
 	.prepare_suspend = raid5_prepare_suspend,
 	.bitmap_sector	= raid5_bitmap_sector,
+	.recovery_in_bitmap_space = true,
 };
 
 static struct md_personality raid4_personality =
@@ -9139,6 +9141,7 @@ static struct md_personality raid4_personality =
 	.change_consistency_policy = raid5_change_consistency_policy,
 	.prepare_suspend = raid5_prepare_suspend,
 	.bitmap_sector	= raid5_bitmap_sector,
+	.recovery_in_bitmap_space = true,
 };
 
 static int __init raid5_init(void)
-- 
2.43.0

Re: [PATCH 2/3] md: only consult skip_sync_blocks when 'j' is in the bitmap's domain

[yu kuai]

Hi,

在 2026/7/19 22:42, Mykola Marzhan 写道:
> Rebuilding a raid10 member with llbitmap can complete in a few
> hundred milliseconds having copied nothing: md_do_sync() offers
> every position 'j' to bitmap_ops->skip_sync_blocks(), the bitmap
> answers for the wrong chunks, the whole rebuild is skipped as
> "unwritten", and the array reports optimal with a stale member.  The
> staleness is typically noticed only when the mirror partner fails;
> writes made while the array was degraded vanish.
>
> The bitmap spans [0, resync_max_sectors) in the personality's
> sync-cursor space.  Resync walks exactly that range for every
> personality and may always consult the bitmap.  Recovery walks
> per-device offsets, which match the bitmap's space only for raid1
> (device space is array space) and raid456 (the bitmap is indexed by
> the per-device sync cursor; bitmap_sector() translates array IO
> into it).  raid10's bitmap is in array space -- near-2 on four
> disks spans twice a member's size; far-2 on two disks puts array
> chunk 2c+d at device chunk c of disk d -- so
> llbitmap_skip_sync_blocks() was consulted with offsets it cannot
> interpret.
>
> Only call the hook when the offsets match what the bitmap expects.
> Reshape must visit every stripe regardless of bitmap state --
> skipping would desync reshape_position from 'j' -- so never skip
> there.  For recovery, add an md_personality capability,
> recovery_in_bitmap_space, set by raid1 and raid456.  raid10 leaves
> it unset (raid10_find_virt() needs the disk number, which
> md_do_sync() does not have); raid0 and linear too, but no recovery
> path of theirs reaches the hook.  No performance is lost against any
> released kernel: the hook is llbitmap-only and new in v6.18, so
> raid10 recovery returns to its earlier path -- walk every stripe,
> skip clean chunks inside raid10_sync_request().
>
> A capability rather than a geometry test: for raid10 layouts with
> raid_disks == near_copies * far_copies, resync_max_sectors equals
> dev_sectors while the spaces can still differ (far and offset
> variants), so an arithmetic proxy reopens the same hole on those
> layouts.  Disabling the hook for all recovery instead would
> forfeit llbitmap's fast-forward over unwritten chunks for raid1 and
> raid456 rebuilds.  Opt-in is also the safe default: a new
> personality gets no bitmap consultation during recovery until it
> declares otherwise.
>
> Verified for near-2 and far-2 layouts: a rebuild onto an added spare
> copies the writes made while the array was degraded.
>
> Fixes: f196d7288864 ("md/md-bitmap: add a new method skip_sync_blocks() in bitmap_operations")
> Cc: stable@vger.kernel.org # v6.18+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
>   drivers/md/md.c    | 13 ++++++++++++-
>   drivers/md/md.h    | 11 +++++++++++
>   drivers/md/raid1.c |  1 +
>   drivers/md/raid5.c |  3 +++
>   4 files changed, 27 insertions(+), 1 deletion(-)

There is already a patchset to support llbitmap reshape for raid10 and raid5:

[PATCH v2 00/20] md/md-llbitmap: support reshape for RAID10 and RAID5 - 
Yu Kuai <https://lore.kernel.org/linux-raid/cover.1782282042.git.yukuai@kernel.org/>

Before this set, reshape for raid10 or raid5 is not safe. Kernel panic or data lost
is observed. Please check if your problems still exist with this set.

> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c8..d60ea7aaca3a 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9846,7 +9846,18 @@ void md_do_sync(struct md_thread *thread)
>   		if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
>   			break;
>   
> -		if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks) {
> +		/*
> +		 * The bitmap may be consulted with 'j' for a plain resync,
> +		 * but for recovery only when the personality declares that
> +		 * its recovery cursor addresses the bitmap's space; raid10
> +		 * recovery walks per-device offsets the bitmap cannot
> +		 * interpret.  Reshape must relocate every stripe regardless
> +		 * of bitmap state, so never skip there either.
> +		 */
> +		if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks &&
> +		    !test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
> +		    (!test_bit(MD_RECOVERY_RECOVER, &mddev->recovery) ||
> +		     mddev->pers->recovery_in_bitmap_space)) {
>   			sectors = mddev->bitmap_ops->skip_sync_blocks(mddev, j);
>   			if (sectors)
>   				goto update;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d8daf0f75cbb..a490a47736b0 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -798,6 +798,17 @@ struct md_personality
>   	/* convert io ranges from array to bitmap */
>   	void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
>   			      unsigned long *sectors);
> +	/*
> +	 * The position md_do_sync() walks during MD_RECOVERY_RECOVER
> +	 * addresses the space the write-intent bitmap is indexed by, so
> +	 * recovery may consult bitmap_ops->skip_sync_blocks() directly.
> +	 * True for raid1 (device space is array space) and raid456 (the
> +	 * bitmap is indexed by the per-device sync cursor).  raid10 must
> +	 * leave this unset: its recovery walks per-device offsets while
> +	 * its bitmap is indexed by array sectors, and translating needs
> +	 * the disk number, which md_do_sync() does not have.
> +	 */
> +	bool recovery_in_bitmap_space;
>   };
>   
>   struct md_sysfs_entry {
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index afe2ca96ad8c..1222c0470ab6 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -3518,6 +3518,7 @@ static struct md_personality raid1_personality =
>   	.check_reshape	= raid1_reshape,
>   	.quiesce	= raid1_quiesce,
>   	.takeover	= raid1_takeover,
> +	.recovery_in_bitmap_space = true,
>   };
>   
>   static int __init raid1_init(void)
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ffb5fcde54a9..25f8d829a986 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -9078,6 +9078,7 @@ static struct md_personality raid6_personality =
>   	.change_consistency_policy = raid5_change_consistency_policy,
>   	.prepare_suspend = raid5_prepare_suspend,
>   	.bitmap_sector	= raid5_bitmap_sector,
> +	.recovery_in_bitmap_space = true,
>   };
>   static struct md_personality raid5_personality =
>   {
> @@ -9108,6 +9109,7 @@ static struct md_personality raid5_personality =
>   	.change_consistency_policy = raid5_change_consistency_policy,
>   	.prepare_suspend = raid5_prepare_suspend,
>   	.bitmap_sector	= raid5_bitmap_sector,
> +	.recovery_in_bitmap_space = true,
>   };
>   
>   static struct md_personality raid4_personality =
> @@ -9139,6 +9141,7 @@ static struct md_personality raid4_personality =
>   	.change_consistency_policy = raid5_change_consistency_policy,
>   	.prepare_suspend = raid5_prepare_suspend,
>   	.bitmap_sector	= raid5_bitmap_sector,
> +	.recovery_in_bitmap_space = true,
>   };
>   
>   static int __init raid5_init(void)

-- 
Thanks,
Kuai

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

[Mykola Marzhan]

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[].  And when the reserved bitmap space is too small for the new
size, the chunksize-doubling loop silently changes the
chunk-to-sector mapping without remapping any on-disk bit, so the
existing intent state describes the wrong sectors afterwards.

Fail such resizes before anything is mutated: -EINVAL for a resize
that needs a different chunksize, -ENOSPC for a grow that needs more
bitmap pages than were allocated at create time.  Shrinks, grows
within the allocated pages, and assemble-time sizing keep working.
"mdadm --grow --size" past the boundary now fails with an error
instead of corrupting memory, and a raid10 disk-add reshape crossing
it fails cleanly in raid10_start_reshape(), which calls
bitmap_ops->resize() directly and reverts the geometry on error.

Grows within the last allocated page can still expose per-chunk
bytes that are stale (after a shrink) or never initialised (the
tail of the last page: llbitmap_init() covers only [0, chunks - 1]).
That window closes once the bitmap page cache can actually grow --
the subject of separate work under review.

Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Cc: stable@vger.kernel.org # v6.18+
Assisted-by: Claude-Code:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/md-llbitmap.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 5a4e2abaa757..e8f853e8461d 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);
 	}
 
+	/*
+	 * 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) {
+		pr_warn("md/llbitmap: %s: cannot resize to %llu sectors: bitmap space (%lu sectors) requires chunksize %u, current is %lu\n",
+			mdname(mddev), (unsigned long long)blocks,
+			mddev->bitmap_info.space, (unsigned int)chunksize,
+			llbitmap->chunksize);
+		return -EINVAL;
+	}
+
+	/*
+	 * 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) {
+		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;
-- 
2.43.0

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

[sashiko-bot]

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