[PATCH 0/4] md: bitmap superblock robustness and assemble diagnostics

[PATCH 0/4] md: bitmap superblock robustness and assemble diagnostics

[Mykola Marzhan]

While testing llbitmap we found two bugs with the same root cause:
the bitmap superblock's state field is untrusted input, but both
bitmap loaders copy it into runtime flags unmasked.  A single wrong
bit on disk -- which pre-v4.15 kernels wrote themselves, and which
corruption or an external write can plant today -- can brick an
array (patch 1, classic bitmap: assemble fails with -EIO) or
silently disable its bitmap for the array's lifetime (patch 2,
llbitmap).  Patch 3 fixes a kmap_local ordering violation in the
llbitmap superblock reader (hardening; no observable bug on
production configs, hence no stable tag).  Patch 4 adds the events
counters to the "kicking non-fresh" warning so a kicked member can
be judged from dmesg alone.

Testing: patches 1-2 verified with their in-message dd reproducers
on unpatched and patched kernels (state field forged on every
member); patch 3 by build and inspection; patch 4's warning format
exercised during a non-fresh assemble.  The series builds W=1-clean
and applies to current master and to md-7.2.  Independent of the
raid10-recovery series posted separately.

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

Mykola Marzhan (4):
  md/md-bitmap: mask sb->state on load to drop runtime-only bits
  md/md-llbitmap: mask sb->state on load to drop runtime-only bits
  md/md-llbitmap: unmap sb page before freeing it
  md: include events counters when kicking non-fresh device

 drivers/md/md-bitmap.c   |  9 +++++++--
 drivers/md/md-llbitmap.c | 12 ++++++++++--
 drivers/md/md.c          | 25 +++++++++++++++++++++++--
 3 files changed, 40 insertions(+), 6 deletions(-)


base-commit: 0e35b9b6ec0ffcc5e23cbdec09f5c622ad532b53
-- 
2.43.0

[PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits

[Mykola Marzhan]

md_bitmap_read_sb() does `bitmap->flags |= le32_to_cpu(sb->state)`
with no mask, so any bit set in the on-disk state field becomes a
runtime flag.  With BITMAP_WRITE_ERROR (bit 2) set on disk,
md_bitmap_create() fails with -EIO and the array refuses to assemble
until the bitmap superblock is rewritten externally.

That bit can genuinely be on disk: kernels before v4.15 wrote
sb->state unmasked -- commit 97f0eb9f0fec ("md/bitmap: clear
BITMAP_WRITE_ERROR bit before writing it to sb") added the
write-side clear -- so an array that hit a transient bitmap write
error under an older kernel carries the bit forever.  On-disk
corruption or an external write plants it today.

The write side has masked WRITE_ERROR out since v4.15; the load side
is the missing half.  Mask the loaded state to BITMAP_STALE, the
only bit this loader consumes: WRITE_ERROR and DAEMON_BUSY are
kernel-runtime signals, FIRST_USE and CLEAN are used only by the
llbitmap loader (a superblock this loader accepts cannot carry
them, since versions above BITMAP_MAJOR_CLUSTERED are rejected),
and HOSTENDIAN is derived from sb->version, not sb->state.

Reproducer: write 0x04 into byte offset 48 (the state field) of the
bitmap superblock on every member (a single forged member can be
masked by another member's clean copy), then assemble: RUN_ARRAY
fails with -EIO.  With this fix the array starts and sb->events
advances normally.

Fixes: 32a7627cf3a3 ("[PATCH] md: optimised resync using Bitmap based intent logging")
Cc: stable@vger.kernel.org
Assisted-by: Claude-Code:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/md-bitmap.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
index 0f02e2956398..4dd3fc2a9e15 100644
--- a/drivers/md/md-bitmap.c
+++ b/drivers/md/md-bitmap.c
@@ -931,8 +931,13 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
 		}
 	}
 
-	/* assign fields using values from superblock */
-	bitmap->flags |= le32_to_cpu(sb->state);
+	/*
+	 * Only BITMAP_STALE is meaningful to this loader from sb->state:
+	 * WRITE_ERROR and DAEMON_BUSY are kernel-runtime signals (kernels
+	 * before v4.15 persisted WRITE_ERROR), FIRST_USE/CLEAN are
+	 * llbitmap-only, and HOSTENDIAN is derived from sb->version below.
+	 */
+	bitmap->flags |= le32_to_cpu(sb->state) & BIT(BITMAP_STALE);
 	if (le32_to_cpu(sb->version) == BITMAP_MAJOR_HOSTENDIAN)
 		set_bit(BITMAP_HOSTENDIAN, &bitmap->flags);
 	bitmap->events_cleared = le64_to_cpu(sb->events_cleared);
-- 
2.43.0

[PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits

[Mykola Marzhan]

llbitmap_read_sb() copies sb->state straight into llbitmap->flags
with no mask, so any bit set in the on-disk state field becomes a
runtime flag.  With BITMAP_WRITE_ERROR (bit 2) on disk,
llbitmap_enabled() returns false for the array's entire lifetime:
llbitmap_update_sb() early-returns, the state machine returns
BitNone, the bits attribute reports "bitmap io error", and on-disk
sb->events stops advancing.  The bitmap is silently disabled with no
log line attributing the cause, and the next clean reassemble forces
a full resync because sb->events is stale.

Runtime bits do reach disk: llbitmap_update_sb() writes the raw
flags word, so BITMAP_DAEMON_BUSY is persisted whenever a superblock
update coincides with a daemon-period overrun -- the condition that
logs "daemon_work not finished".  WRITE_ERROR itself has no
reachable write-side path today (its only setter is update_sb's own
read-failure branch, which returns before writing), but on-disk
corruption or an external write plants it all the same.

Mask the loaded state to the bits this loader consumes (STALE,
FIRST_USE and CLEAN), as the previous patch did for the classic
loader.  Masking at load neutralises whatever is on disk and is the
minimal backportable fix; masking the write side is left as a
follow-up cleanup.

Reproducer: write 0x04 into byte offset 48 (the state field) of the
llbitmap superblock on every member (a forge on a single member can
be masked by another member's clean copy), then assemble.  Without this
fix the bits attribute reports "bitmap io error" and sb->events is
frozen; with it the bitmap is alive and sb->events advances
normally.

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 | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index e8f853e8461d..0b591124dc63 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -981,7 +981,15 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
 		else
 			mddev->bitmap_info.space = mddev->bitmap_info.default_space;
 	}
-	llbitmap->flags = le32_to_cpu(sb->state);
+	/*
+	 * Mask the loaded state to the bits this loader consumes.
+	 * WRITE_ERROR and DAEMON_BUSY are kernel-runtime signals; a stray
+	 * WRITE_ERROR trusted from disk silently disables the bitmap for
+	 * the array's entire lifetime.
+	 */
+	llbitmap->flags = le32_to_cpu(sb->state) &
+			  (BIT(BITMAP_STALE) | BIT(BITMAP_FIRST_USE) |
+			   BIT(BITMAP_CLEAN));
 	if (test_and_clear_bit(BITMAP_FIRST_USE, &llbitmap->flags)) {
 		ret = llbitmap_init(llbitmap);
 		goto out_put_page;
-- 
2.43.0

[PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it

[Mykola Marzhan]

Every path in llbitmap_read_sb() that maps the superblock page ends
at the out_put_page label, which frees the page and only then unmaps
it:

	__free_page(sb_page);
	kunmap_local(sb);

The page comes from alloc_page(GFP_KERNEL), so kunmap_local() is
currently a no-op everywhere except CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP
builds -- where the page really is freed while a live mapping still
points at it.  Unmap before freeing.

Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
Assisted-by: Claude-Code:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/md-llbitmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
index 0b591124dc63..63ff2c0d8b43 100644
--- a/drivers/md/md-llbitmap.c
+++ b/drivers/md/md-llbitmap.c
@@ -1035,8 +1035,8 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
 	ret = llbitmap_cache_pages(llbitmap);
 
 out_put_page:
-	__free_page(sb_page);
 	kunmap_local(sb);
+	__free_page(sb_page);
 	return ret;
 }
 
-- 
2.43.0

[PATCH 4/4] md: include events counters when kicking non-fresh device

[Mykola Marzhan]

When analyze_sbs() kicks a member with a stale events counter, the
warning names the device but not the divergence that caused the
kick, so judging whether the kicked member is salvageable means
running mdadm --examine on every member.  Print the member's on-disk
events counter and the freshest events counter in the warning
itself.

rdev->sb_events cannot be used here: it is only assigned when a
superblock is written and still reads 0 during assemble.  Read the
counter from the superblock page instead -- md_event() for 0.90
metadata, the little-endian events field for 1.x.

The legacy substring "kicking non-fresh %pg from array!" is
unchanged, so existing log parsers continue to match.

Assisted-by: Claude-Code:claude-opus-4-8
Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
---
 drivers/md/md.c | 25 +++++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/drivers/md/md.c b/drivers/md/md.c
index d60ea7aaca3a..9ec87375a717 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -3934,8 +3934,29 @@ static int analyze_sbs(struct mddev *mddev)
 		if (rdev != freshest) {
 			if (super_types[mddev->major_version].
 			    validate_super(mddev, freshest, rdev)) {
-				pr_warn("md: kicking non-fresh %pg from array!\n",
-					rdev->bdev);
+				u64 rdev_events = 0;
+
+				/*
+				 * rdev->sb_events is only assigned when
+				 * writing a superblock and reads 0 during
+				 * assemble; read events from the on-disk
+				 * superblock instead.
+				 */
+				if (rdev->sb_page) {
+					if (mddev->major_version == 1) {
+						struct mdp_superblock_1 *sb =
+							page_address(rdev->sb_page);
+						rdev_events = le64_to_cpu(sb->events);
+					} else if (mddev->major_version == 0) {
+						mdp_super_t *sb =
+							page_address(rdev->sb_page);
+						rdev_events = md_event(sb);
+					}
+				}
+				pr_warn("md: kicking non-fresh %pg from array! (events=%llu, freshest=%llu)\n",
+					rdev->bdev,
+					(unsigned long long)rdev_events,
+					(unsigned long long)mddev->events);
 				md_kick_rdev_from_array(rdev);
 				continue;
 			}
-- 
2.43.0

Re: [PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it

[sashiko-bot]

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

Pre-existing issues:
- [High] Integer Truncation of 64-bit event counter causes spurious array recoveries on 32-bit architectures
- [High] Use-After-Free in teardown sequence due to re-arming timer
- [High] Lost Write to `sb->sync_size` during superblock load
- [High] Integer overflow causing division by zero when calculating minimum chunksize
- [High] Integer overflow in page count calculation leading to out-of-bounds heap access
--

--- Patch [3]: [PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it ---
commit 6be442dfef7ad93b44cadb093b6d078f152b2522
Author: Mykola Marzhan <mykola@meshstor.io>

md/md-llbitmap: unmap sb page before freeing it

The patch swaps __free_page(sb_page) and kunmap_local(sb) in
llbitmap_read_sb() to avoid unmapping a page after it has been freed.

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 2d3de38f9b784..2d6c7ad0923a1 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -1035,8 +1035,8 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>  	ret = llbitmap_cache_pages(llbitmap);

[Severity: High]
This is a pre-existing issue, but is there an integer overflow when
calculating nr_pages on 32-bit architectures inside llbitmap_cache_pages()?

drivers/md/md-llbitmap.c:llbitmap_cache_pages() {
    ...
    unsigned int nr_pages = DIV_ROUND_UP(llbitmap->chunks +
                                         BITMAP_DATA_OFFSET, PAGE_SIZE);
    ...
    llbitmap->pctl = kmalloc_array(nr_pages, sizeof(void *),
                                   GFP_KERNEL | __GFP_ZERO);
    ...
}

If a malicious superblock sets a high resync_max_sectors and chunksize of 2,
could llbitmap->chunks + BITMAP_DATA_OFFSET overflow to 0? Would this result
in a zero-sized array allocation and subsequent out-of-bounds access?

>  
>  out_put_page:
> -	__free_page(sb_page);
>  	kunmap_local(sb);
> +	__free_page(sb_page);

[Severity: High]
This is a pre-existing issue, but is the update to sb->sync_size lost here?

Earlier in llbitmap_read_sb(), sb->sync_size is updated:

    sb->sync_size = cpu_to_le64(mddev->resync_max_sectors);

Since sb_page is unmapped and unconditionally freed here without being
written back to disk or retained in memory, does the runtime state lose
this update?

>  	return ret;
>  }

There are a few other pre-existing issues in this file that might need
attention.

[Severity: High]
This is a pre-existing issue, but does assigning the 64-bit sb->events to the
unsigned long events variable truncate it on 32-bit architectures?

drivers/md/md-llbitmap.c:llbitmap_read_sb() {
    unsigned long events;
    ...
    events = le64_to_cpu(sb->events);
    if (events < mddev->events) {
        ...
    }
}

When the 64-bit counter exceeds 4 billion, would this truncation cause a
spurious full array resync by incorrectly making the bitmap appear stale?


[Severity: High]
This is a pre-existing issue, but can the timer fire on freed memory during
teardown?

drivers/md/md-llbitmap.c:llbitmap_destroy() {
    ...
    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);
    ...
}

drivers/md/md-llbitmap.c:md_llbitmap_daemon_fn() {
    ...
    if (restart)
        mod_timer(&llbitmap->pending_timer, ...);
}

If llbitmap_destroy() deletes the timer and flushes the workqueue, could
md_llbitmap_daemon_fn() unconditionally re-arm the timer at the end of its
execution, leading to a use-after-free when llbitmap is subsequently freed?


[Severity: High]
This is a pre-existing issue, but can this overflow and cause a division by
zero panic?

drivers/md/md-llbitmap.c:llbitmap_read_sb() {
    ...
    if (chunksize < DIV_ROUND_UP_SECTOR_T(mddev->resync_max_sectors,
                                          mddev->bitmap_info.space << SECTOR_SHIFT)) {
    ...
}

On 32-bit systems, mddev->bitmap_info.space is a 32-bit unsigned long. If an
attacker-controlled superblock sets a value where space << 9 overflows to 0,
would DIV_ROUND_UP_SECTOR_T divide by zero?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719144409.940492-1-mykola@meshstor.io?part=3

Re: [PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits

[sashiko-bot]

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

Pre-existing issues:
- [High] Teardown symmetry bug: `llbitmap_destroy()` deletes the daemon timer and waits for the daemon work, but the work can re-arm the timer, leading to a use-after-free when the timer fires on the freed `llbitmap` object.
- [High] Preexisting UAF in llbitmap_create() error path due to clearing mddev->bitmap outside the mutex after kfree.
- [High] Preexisting race condition in llbitmap_write_page() iterating mddev->disks without required locks.
- [High] A maliciously crafted `sectors_reserved` value in the llbitmap superblock can cause an integer overflow during a bitwise shift on 32-bit systems, resulting in a zero divisor and a kernel panic.
--

--- Patch [2]: [PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits ---
commit 48600506c718f0300f8ce2473c95dff7a9e47cc2
Author: Mykola Marzhan <mykola@meshstor.io>

md/md-llbitmap: mask sb->state on load to drop runtime-only bits

This commit masks the loaded superblock state in llbitmap_read_sb() to only
keep STALE, FIRST_USE, and CLEAN bits. It prevents runtime-only flags like
WRITE_ERROR from being loaded from disk and permanently disabling the bitmap.

Cc: stable@vger.kernel.org # v6.18+

> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index 5a4e2abaa7577..2d3de38f9b784 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -981,7 +981,15 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>  		else
>  			mddev->bitmap_info.space = mddev->bitmap_info.default_space;
>  	}

[Severity: High]
This isn't a bug introduced by this patch, but can this sectors_reserved
check lead to an integer overflow?

In llbitmap_read_sb(), we read the untrusted value:

    int room = le32_to_cpu(sb->sectors_reserved);
    if (room)
        mddev->bitmap_info.space = room;

Later in the same function:

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

Could a crafted superblock with sectors_reserved = 0x00800000 cause
(mddev->bitmap_info.space << SECTOR_SHIFT) to wrap to 0 on 32-bit systems,
resulting in a division by zero panic?

> -	llbitmap->flags = le32_to_cpu(sb->state);
> +	/*
> +	 * Mask the loaded state to the bits this loader consumes.
[ ... ]

[Severity: High]
This is a pre-existing issue, but does the teardown path have a symmetry
issue?

In llbitmap_destroy():

    timer_delete_sync(&llbitmap->pending_timer);
    flush_workqueue(md_llbitmap_io_wq);

If md_llbitmap_daemon_fn() is actively running and finds unexpired pages:

    if (restart)
        mod_timer(&llbitmap->pending_timer, ...);

Does this allow the daemon to blindly re-arm the timer after
timer_delete_sync() has finished, leading to a use-after-free when the timer
fires on the freed llbitmap object?

[Severity: High]
This is a pre-existing issue, but is there a use-after-free in the
llbitmap_create() error path?

When llbitmap_create() fails, it does this:

    mutex_unlock(&mddev->bitmap_info.mutex);
    if (ret) {
        kfree(llbitmap);
        mddev->bitmap = NULL;
    }

Can a concurrent reader like metadata_show() acquire the lock and dereference
the freed object?

metadata_show() does:

    mutex_lock(&mddev->bitmap_info.mutex);
    llbitmap = mddev->bitmap;
    if (!llbitmap) {
    ...

[Severity: High]
This isn't a bug introduced by this patch, but does llbitmap_write_page()
iterate the disks without required locks?

In llbitmap_write_page(), executed from workqueues:

        rdev_for_each(rdev, mddev) {
            sector_t sector;

This iterates a dynamic, RCU-protected linked list without taking
rcu_read_lock() or reconfig_mutex.

If a disk is concurrently hot-removed, can list_del_rcu() and
synchronize_rcu() free the element while llbitmap_write_page() is traversing
it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719144409.940492-1-mykola@meshstor.io?part=2

Re: [PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits

[sashiko-bot]

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

Pre-existing issues:
- [High] The lockless bitmap implementation (llbitmap) remains vulnerable to the exact same persistent WRITE_ERROR bug that this commit fixes for the standard bitmap.
- [High] Internal bitmap pages allocated with alloc_page() are leaked during array teardown when CONFIG_MD_BITMAP_FILE is enabled.
- [High] Cluster superblock flags leak across `re_read` loop iterations, causing spurious state inheritance from node 0.
--

--- Patch [1]: [PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits ---
commit f4f8e55ab2fda3c56252ec133561c13874aeb19a
Author: Mykola Marzhan <mykola@meshstor.io>

md/md-bitmap: mask sb->state on load to drop runtime-only bits

This patch masks the loaded state to BITMAP_STALE in md_bitmap_read_sb() so
any runtime-only bits like BITMAP_WRITE_ERROR are dropped. This fixes an
issue where an array refuses to assemble if it previously hit a transient
bitmap write error under an older kernel.

> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 0f02e2956398d..4dd3fc2a9e15b 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -931,8 +931,13 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
>  		}
>  	}
>  
> -	/* assign fields using values from superblock */
> -	bitmap->flags |= le32_to_cpu(sb->state);
> +	/*
> +	 * Only BITMAP_STALE is meaningful to this loader from sb->state:
> +	 * WRITE_ERROR and DAEMON_BUSY are kernel-runtime signals (kernels
> +	 * before v4.15 persisted WRITE_ERROR), FIRST_USE/CLEAN are
> +	 * llbitmap-only, and HOSTENDIAN is derived from sb->version below.
> +	 */
> +	bitmap->flags |= le32_to_cpu(sb->state) & BIT(BITMAP_STALE);

[Severity: High]
This isn't a bug introduced by this patch, but does this bitwise OR cause
cluster superblock flags to leak across re_read loop iterations?

In md_bitmap_read_sb(), when nodes > 0, the code reads offset 0 initially and
accumulates flags into bitmap->flags. It then computes a new offset for its
own cluster_slot and jumps back to the re_read label, but bitmap->flags is
never cleared.

Could this cause a local node to spuriously inherit BITMAP_STALE or
BITMAP_HOSTENDIAN from node 0, incorrectly forcing a full array recovery?

[Severity: High]
This is a pre-existing issue, but does the lockless bitmap implementation in
drivers/md/md-llbitmap.c remain vulnerable to the exact same WRITE_ERROR bug
this patch fixes for standard bitmaps?

In llbitmap_read_sb():

    llbitmap->flags = le32_to_cpu(sb->state);

Since it reads the superblock state entirely unmasked, wouldn't a single
instance of on-disk corruption or transient error set BITMAP_WRITE_ERROR
permanently and force a full resync on every restart?

[Severity: High]
This is also a pre-existing issue, but do internal bitmap pages leak during
array teardown when CONFIG_MD_BITMAP_FILE is enabled?

In md_bitmap_file_unmap(), free_buffers() is called for each page:

drivers/md/md-bitmap.c:free_buffers() {
    ...
    if (!PagePrivate(page))
        return;
    ...
}

For internal bitmaps, reads and writes use sync_page_io() without buffer
heads, so PagePrivate is never set. Does this early return skip put_page()
and permanently leak all dynamically allocated metadata pages whenever the
array is stopped or the bitmap is resized?

>  	if (le32_to_cpu(sb->version) == BITMAP_MAJOR_HOSTENDIAN)
>  		set_bit(BITMAP_HOSTENDIAN, &bitmap->flags);
>  	bitmap->events_cleared = le64_to_cpu(sb->events_cleared);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719144409.940492-1-mykola@meshstor.io?part=1

Re: [PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits

[yu kuai]

Hi,

在 2026/7/19 22:44, Mykola Marzhan 写道:
> md_bitmap_read_sb() does `bitmap->flags |= le32_to_cpu(sb->state)`
> with no mask, so any bit set in the on-disk state field becomes a
> runtime flag.  With BITMAP_WRITE_ERROR (bit 2) set on disk,
> md_bitmap_create() fails with -EIO and the array refuses to assemble
> until the bitmap superblock is rewritten externally.
>
> That bit can genuinely be on disk: kernels before v4.15 wrote
> sb->state unmasked -- commit 97f0eb9f0fec ("md/bitmap: clear
> BITMAP_WRITE_ERROR bit before writing it to sb") added the
> write-side clear -- so an array that hit a transient bitmap write
> error under an older kernel carries the bit forever.  On-disk
> corruption or an external write plants it today.
>
> The write side has masked WRITE_ERROR out since v4.15; the load side
> is the missing half.  Mask the loaded state to BITMAP_STALE, the
> only bit this loader consumes: WRITE_ERROR and DAEMON_BUSY are
> kernel-runtime signals, FIRST_USE and CLEAN are used only by the
> llbitmap loader (a superblock this loader accepts cannot carry
> them, since versions above BITMAP_MAJOR_CLUSTERED are rejected),
> and HOSTENDIAN is derived from sb->version, not sb->state.
>
> Reproducer: write 0x04 into byte offset 48 (the state field) of the
> bitmap superblock on every member (a single forged member can be
> masked by another member's clean copy), then assemble: RUN_ARRAY
> fails with -EIO.  With this fix the array starts and sb->events
> advances normally.

This is not a reproducer, BITMAP_WRITE_ERROR indicate that bitmap to all member disks
failed, and bitmap no longer works for the array, so it's not safe to assemble with this
bitmap.

I think it's reasonable to assemble this array with bitmap=none.

>
> Fixes: 32a7627cf3a3 ("[PATCH] md: optimised resync using Bitmap based intent logging")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
>   drivers/md/md-bitmap.c | 9 +++++++--
>   1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md-bitmap.c b/drivers/md/md-bitmap.c
> index 0f02e2956398..4dd3fc2a9e15 100644
> --- a/drivers/md/md-bitmap.c
> +++ b/drivers/md/md-bitmap.c
> @@ -931,8 +931,13 @@ static int md_bitmap_read_sb(struct bitmap *bitmap)
>   		}
>   	}
>   
> -	/* assign fields using values from superblock */
> -	bitmap->flags |= le32_to_cpu(sb->state);
> +	/*
> +	 * Only BITMAP_STALE is meaningful to this loader from sb->state:
> +	 * WRITE_ERROR and DAEMON_BUSY are kernel-runtime signals (kernels
> +	 * before v4.15 persisted WRITE_ERROR), FIRST_USE/CLEAN are
> +	 * llbitmap-only, and HOSTENDIAN is derived from sb->version below.
> +	 */
> +	bitmap->flags |= le32_to_cpu(sb->state) & BIT(BITMAP_STALE);
>   	if (le32_to_cpu(sb->version) == BITMAP_MAJOR_HOSTENDIAN)
>   		set_bit(BITMAP_HOSTENDIAN, &bitmap->flags);
>   	bitmap->events_cleared = le64_to_cpu(sb->events_cleared);

-- 
Thanks,
Kuai

Re: [PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits

[yu kuai]

Hi,

在 2026/7/19 22:44, Mykola Marzhan 写道:
> llbitmap_read_sb() copies sb->state straight into llbitmap->flags
> with no mask, so any bit set in the on-disk state field becomes a
> runtime flag.  With BITMAP_WRITE_ERROR (bit 2) on disk,
> llbitmap_enabled() returns false for the array's entire lifetime:
> llbitmap_update_sb() early-returns, the state machine returns
> BitNone, the bits attribute reports "bitmap io error", and on-disk
> sb->events stops advancing.  The bitmap is silently disabled with no
> log line attributing the cause, and the next clean reassemble forces
> a full resync because sb->events is stale.

For llbitmap, it's using md_write_metadata(), if it's failed for one rdev,
md_error() is called directly for this rdev. So if BITMAP_WRITE_ERROR exist,
the array is already broken. And in this case, if array can still reassemble,
a full resync is the expected behavior.

>
> Runtime bits do reach disk: llbitmap_update_sb() writes the raw
> flags word, so BITMAP_DAEMON_BUSY is persisted whenever a superblock
> update coincides with a daemon-period overrun -- the condition that
> logs "daemon_work not finished".  WRITE_ERROR itself has no
> reachable write-side path today (its only setter is update_sb's own
> read-failure branch, which returns before writing), but on-disk
> corruption or an external write plants it all the same.
>
> Mask the loaded state to the bits this loader consumes (STALE,
> FIRST_USE and CLEAN), as the previous patch did for the classic
> loader.  Masking at load neutralises whatever is on disk and is the
> minimal backportable fix; masking the write side is left as a
> follow-up cleanup.
>
> Reproducer: write 0x04 into byte offset 48 (the state field) of the
> llbitmap superblock on every member (a forge on a single member can
> be masked by another member's clean copy), then assemble.  Without this
> fix the bits attribute reports "bitmap io error" and sb->events is
> frozen; with it the bitmap is alive and sb->events advances
> normally.
>
> 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 | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/md-llbitmap.c b/drivers/md/md-llbitmap.c
> index e8f853e8461d..0b591124dc63 100644
> --- a/drivers/md/md-llbitmap.c
> +++ b/drivers/md/md-llbitmap.c
> @@ -981,7 +981,15 @@ static int llbitmap_read_sb(struct llbitmap *llbitmap)
>   		else
>   			mddev->bitmap_info.space = mddev->bitmap_info.default_space;
>   	}
> -	llbitmap->flags = le32_to_cpu(sb->state);
> +	/*
> +	 * Mask the loaded state to the bits this loader consumes.
> +	 * WRITE_ERROR and DAEMON_BUSY are kernel-runtime signals; a stray
> +	 * WRITE_ERROR trusted from disk silently disables the bitmap for
> +	 * the array's entire lifetime.
> +	 */
> +	llbitmap->flags = le32_to_cpu(sb->state) &
> +			  (BIT(BITMAP_STALE) | BIT(BITMAP_FIRST_USE) |
> +			   BIT(BITMAP_CLEAN));
>   	if (test_and_clear_bit(BITMAP_FIRST_USE, &llbitmap->flags)) {
>   		ret = llbitmap_init(llbitmap);
>   		goto out_put_page;

-- 
Thanks,
Kuai

Re: [PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it

[yu kuai]

在 2026/7/19 22:44, Mykola Marzhan 写道:
> Every path in llbitmap_read_sb() that maps the superblock page ends
> at the out_put_page label, which frees the page and only then unmaps
> it:
>
> 	__free_page(sb_page);
> 	kunmap_local(sb);
>
> The page comes from alloc_page(GFP_KERNEL), so kunmap_local() is
> currently a no-op everywhere except CONFIG_DEBUG_KMAP_LOCAL_FORCE_MAP
> builds -- where the page really is freed while a live mapping still
> points at it.  Unmap before freeing.
>
> Fixes: 5ab829f1971d ("md/md-llbitmap: introduce new lockless bitmap")
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan<mykola@meshstor.io>
> ---
>   drivers/md/md-llbitmap.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>

-- 
Thanks,
Kuai

Re: [PATCH 4/4] md: include events counters when kicking non-fresh device

[yu kuai]

Hi,

在 2026/7/19 22:44, Mykola Marzhan 写道:
> When analyze_sbs() kicks a member with a stale events counter, the
> warning names the device but not the divergence that caused the
> kick, so judging whether the kicked member is salvageable means
> running mdadm --examine on every member.  Print the member's on-disk
> events counter and the freshest events counter in the warning
> itself.

If user space tools can do this already, then user space tools should be
considered preferred.

>
> rdev->sb_events cannot be used here: it is only assigned when a
> superblock is written and still reads 0 during assemble.  Read the
> counter from the superblock page instead -- md_event() for 0.90
> metadata, the little-endian events field for 1.x.
>
> The legacy substring "kicking non-fresh %pg from array!" is
> unchanged, so existing log parsers continue to match.
>
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
>   drivers/md/md.c | 25 +++++++++++++++++++++++--
>   1 file changed, 23 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d60ea7aaca3a..9ec87375a717 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -3934,8 +3934,29 @@ static int analyze_sbs(struct mddev *mddev)
>   		if (rdev != freshest) {
>   			if (super_types[mddev->major_version].
>   			    validate_super(mddev, freshest, rdev)) {
> -				pr_warn("md: kicking non-fresh %pg from array!\n",
> -					rdev->bdev);
> +				u64 rdev_events = 0;
> +
> +				/*
> +				 * rdev->sb_events is only assigned when
> +				 * writing a superblock and reads 0 during
> +				 * assemble; read events from the on-disk
> +				 * superblock instead.
> +				 */
> +				if (rdev->sb_page) {
> +					if (mddev->major_version == 1) {
> +						struct mdp_superblock_1 *sb =
> +							page_address(rdev->sb_page);
> +						rdev_events = le64_to_cpu(sb->events);
> +					} else if (mddev->major_version == 0) {
> +						mdp_super_t *sb =
> +							page_address(rdev->sb_page);
> +						rdev_events = md_event(sb);
> +					}
> +				}
> +				pr_warn("md: kicking non-fresh %pg from array! (events=%llu, freshest=%llu)\n",
> +					rdev->bdev,
> +					(unsigned long long)rdev_events,
> +					(unsigned long long)mddev->events);

And I don't like the above changes. If you really want such log, please add
it inside validate_super() methods.

>   				md_kick_rdev_from_array(rdev);
>   				continue;
>   			}

-- 
Thanks,
Kuai