* Re: [PATCH 4/4] md: include events counters when kicking non-fresh device
From: yu kuai @ 2026-07-20 2:14 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-5-mykola@meshstor.io>
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
^ permalink raw reply
* Re: [PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits
From: yu kuai @ 2026-07-20 2:00 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-3-mykola@meshstor.io>
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
^ permalink raw reply
* Re: [PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it
From: yu kuai @ 2026-07-20 2:02 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-4-mykola@meshstor.io>
在 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
^ permalink raw reply
* Re: [PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits
From: yu kuai @ 2026-07-20 1:54 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-2-mykola@meshstor.io>
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
^ permalink raw reply
* Re: [PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits
From: sashiko-bot @ 2026-07-19 15:06 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-raid
In-Reply-To: <20260719144409.940492-2-mykola@meshstor.io>
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
^ permalink raw reply
* Re: [PATCH 3/3] md/md-llbitmap: fail resize that needs more bitmap pages
From: sashiko-bot @ 2026-07-19 15:05 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-raid
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
^ permalink raw reply
* Re: [PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits
From: sashiko-bot @ 2026-07-19 15:00 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-raid, yukuai
In-Reply-To: <20260719144409.940492-3-mykola@meshstor.io>
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
^ permalink raw reply
* Re: [PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it
From: sashiko-bot @ 2026-07-19 14:57 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-raid, yukuai
In-Reply-To: <20260719144409.940492-4-mykola@meshstor.io>
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
^ permalink raw reply
* [PATCH 4/4] md: include events counters when kicking non-fresh device
From: Mykola Marzhan @ 2026-07-19 14:44 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 3/4] md/md-llbitmap: unmap sb page before freeing it
From: Mykola Marzhan @ 2026-07-19 14:44 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 2/4] md/md-llbitmap: mask sb->state on load to drop runtime-only bits
From: Mykola Marzhan @ 2026-07-19 14:44 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 1/4] md/md-bitmap: mask sb->state on load to drop runtime-only bits
From: Mykola Marzhan @ 2026-07-19 14:44 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
In-Reply-To: <20260719144409.940492-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 0/4] md: bitmap superblock robustness and assemble diagnostics
From: Mykola Marzhan @ 2026-07-19 14:44 UTC (permalink / raw)
To: Song Liu, Yu Kuai; +Cc: Li Nan, Xiao Ni, linux-raid, linux-kernel
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
^ permalink raw reply
* [PATCH 3/3] md/md-llbitmap: fail resize that needs more bitmap pages
From: Mykola Marzhan @ 2026-07-19 14:42 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 2/3] md: only consult skip_sync_blocks when 'j' is in the bitmap's domain
From: Mykola Marzhan @ 2026-07-19 14:42 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 1/3] md/raid10: restore still_degraded detection in recovery
From: Mykola Marzhan @ 2026-07-19 14:42 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-1-mykola@meshstor.io>
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
^ permalink raw reply related
* [PATCH 0/3] md: fix silent data loss in raid10 recovery and llbitmap resize
From: Mykola Marzhan @ 2026-07-19 14:42 UTC (permalink / raw)
To: Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
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
^ permalink raw reply
* Re: [PATCH] md/raid5: complete discard bios while reshape is active
From: yu kuai @ 2026-07-19 12:54 UTC (permalink / raw)
To: Genjian, song, yukuai, magiclinan, xiao
Cc: linux-raid, linux-kernel, Genjian Zhang
In-Reply-To: <20260711161326.962336-1-zhanggenjian@126.com>
Hi,
在 2026/7/12 0:13, Genjian 写道:
> From: Genjian Zhang <zhanggenjian@kylinos.cn>
>
> make_discard_request() returns without completing the bio when reshape
> is in progress. Discard callers block in submit_bio_wait()
> waiting for a completion that never arrives. The caller hangs in
> uninterruptible sleep, and this does not resolve when reshape finishes.
>
> Complete the bio with BLK_STS_AGAIN so userspace can retry after reshape,
> consistent with the existing policy of not processing discard during
> reshape.
>
> Tested on a loop-backed RAID5 array during mdadm --grow: without this
> patch, blkdiscard hangs in bio_await() and remains in uninterruptible
> sleep after md reports "reshape done"; with this patch it returns
> -EAGAIN instead.
>
> Signed-off-by: Genjian Zhang <zhanggenjian@kylinos.cn>
> ---
> drivers/md/raid5.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ffb5fcde54a9..4c3b42b2af85 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -5722,9 +5722,11 @@ static void make_discard_request(struct mddev *mddev, struct bio *bi)
> if (WARN_ON_ONCE(bi->bi_opf & REQ_NOWAIT))
> return;
As nowait will be removed soon, I'm fine leaving it here.
Reviewed-by: Yu Kuai <yukuai@fygo.io>
>
> - if (mddev->reshape_position != MaxSector)
> + if (mddev->reshape_position != MaxSector) {
> /* Skip discard while reshape is happening */
> + bio_endio_status(bi, BLK_STS_AGAIN);
> return;
> + }
>
> if (!raid5_discard_limits(mddev, bi))
> return;
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH v2 5/7] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
From: sashiko-bot @ 2026-07-19 11:20 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-pci, yukuai, linux-raid
In-Reply-To: <20260719105327.864949-6-mykola@meshstor.io>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] The newly added `md_bio_is_p2pdma(wbio)` check evaluates a cloned bio after its operation type has been forcibly overwritten to `REQ_OP_WRITE`, defeating the `bio_has_data()` guard and leading to a kernel panic for data-less bios.
Pre-existing issues:
- [High] Pre-existing data integrity bug: `narrow_write_error()` unconditionally overwrites `bi_opf` to `REQ_OP_WRITE`, silently stripping essential flags like `REQ_FUA` and `REQ_PREFLUSH`.
- [High] Pre-existing block layer crash: `narrow_write_error()` blindly overwrites data-less operations with `REQ_OP_WRITE`, submitting invalid bios to underlying block drivers.
--
--- Patch [5]: [PATCH v2 5/7] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones ---
commit 21dea61208137a1862fe54775b5bbeea8b5691ab
Author: Mykola Marzhan <mykola@meshstor.io>
md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
This commit fixes an issue where narrow_write_error drops the REQ_NOMERGE
flag on P2PDMA bios during a write error retry. It adds a check to restore
the flag for P2PDMA bios, preventing invalid merges across pgmaps.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 914fb86452c03..f562b6bd438b9 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2573,6 +2573,9 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
> }
>
> wbio->bi_opf = REQ_OP_WRITE;
[Severity: High]
This isn't a bug introduced by this patch, but does the unconditional
assignment to REQ_OP_WRITE in narrow_write_error() strip essential flags
like REQ_FUA and REQ_PREFLUSH?
If an I/O error occurs on a write that carries these flags, dropping them
could silently break flush guarantees and cause data corruption after a
power failure. Could we preserve the existing flags via a mask instead?
[Severity: High]
This is also a pre-existing issue, but does overwriting data-less
operations with REQ_OP_WRITE in narrow_write_error() submit invalid bios
to the block drivers?
When operations like REQ_OP_WRITE_ZEROES fail, they are transformed into a
standard write with a non-zero bi_size but no initialized data vectors.
Attempting to map this payload for DMA via bio_for_each_segment() might
dereference uninitialized memory and cause a crash.
> + /* Keep P2PDMA retry bios unmergeable, like the original */
> + if (md_bio_is_p2pdma(wbio))
> + wbio->bi_opf |= REQ_NOMERGE;
[Severity: High]
Does checking md_bio_is_p2pdma() immediately after forcing the operation
type to REQ_OP_WRITE defeat the bio_has_data() guard?
Because the operation is now a standard write, bio_has_data() will return
true even for originally data-less operations. This would cause the function
to evaluate the uninitialized page pointer in the bio vector, potentially
leading to a kernel panic. Should the check evaluate the original bio
instead of the modified clone?
> wbio->bi_iter.bi_sector = r1_bio->sector;
> wbio->bi_iter.bi_size = r1_bio->sectors << 9;
[ ... ]
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df8..f7ef903a3d4ec 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2831,6 +2831,9 @@ static void narrow_write_error(struct r10bio *r10_bio, int i)
> wbio->bi_iter.bi_sector = wsector +
> choose_data_offset(r10_bio, rdev);
> wbio->bi_opf = REQ_OP_WRITE;
[Severity: High]
This isn't a bug introduced by this patch, but this overwrite has the same
issue as in raid1.c. Does this unconditional assignment strip operation flags
like REQ_FUA and REQ_PREFLUSH, leading to potential data integrity issues?
[Severity: High]
This is also a pre-existing issue, but does this similarly submit invalid
bios to the block layer by changing failed data-less operations into standard
writes without initialized data pages?
> + /* Keep P2PDMA retry bios unmergeable, like the original */
> + if (md_bio_is_p2pdma(wbio))
> + wbio->bi_opf |= REQ_NOMERGE;
[Severity: High]
Similar to the raid1.c code, does calling md_bio_is_p2pdma() right after
the operation code is forced to REQ_OP_WRITE neutralize the bio_has_data()
guard? Evaluating the uninitialized memory for data-less operations could
cause a kernel panic here as well.
>
> if (submit_bio_wait(wbio) &&
> !rdev_set_badblocks(rdev, wsector, sectors, 0)) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=5
^ permalink raw reply
* Re: [PATCH v2 7/7] nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers
From: sashiko-bot @ 2026-07-19 11:17 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-raid, linux-pci
In-Reply-To: <20260719105327.864949-8-mykola@meshstor.io>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Pre-existing issues:
- [High] Preexisting use-after-free and DMA unmap corruption in the `nvme_rdma_queue_rq` error path.
- [High] Preexisting resource leak of multipath `nr_active` and QoS metrics when `nvme_rdma_post_send()` fails.
--
--- Patch [7]: [PATCH v2 7/7] nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers ---
commit 8ebcd8a34c348b015bd58fe860d4af84c9a7b8c5
Author: Mykola Marzhan <mykola@meshstor.io>
nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers
This commit updates nvme-rdma to correctly handle P2PDMA mapping failures
by preserving the DMA layer's error code. It translates -EREMOTEIO to
BLK_STS_TARGET to avoid endless requeue livelocks for unreachable peer
memory, and it defers nvme_start_request() until after mapping succeeds.
Cc: stable@vger.kernel.org # v7.1
> diff --git a/drivers/nvme/host/rdma.c b/drivers/nvme/host/rdma.c
> index 6909e35427942..9017d927edc41 100644
> --- a/drivers/nvme/host/rdma.c
> +++ b/drivers/nvme/host/rdma.c
[ ... ]
> @@ -2026,8 +2029,6 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
> if (ret)
> goto unmap_qe;
>
> - nvme_start_request(rq);
> -
> if (IS_ENABLED(CONFIG_BLK_DEV_INTEGRITY) &&
> queue->pi_support &&
> (c->common.opcode == nvme_cmd_write ||
> @@ -2039,11 +2040,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
>
> err = nvme_rdma_map_data(queue, rq, c);
> if (unlikely(err < 0)) {
> - dev_err(queue->ctrl->ctrl.device,
> - "Failed to map data (%d)\n", err);
> + dev_err_ratelimited(queue->ctrl->ctrl.device,
> + "Failed to map data (%d)\n", err);
> goto err;
> }
>
> + nvme_start_request(rq);
> +
[Severity: High]
This is a pre-existing issue, but if nvme_rdma_post_send() fails further
down, does returning a block status error after nvme_start_request() has
been called bypass the NVMe completion handlers?
Since the block layer will natively complete the request in this scenario,
it looks like nvme_mpath_end_request() is bypassed, which could permanently
leak the nr_active counter and break multipath load balancing.
> sqe->cqe.done = nvme_rdma_send_done;
>
> ib_dma_sync_single_for_device(dev, sqe->dma,
> @@ -2063,6 +2066,13 @@ static blk_status_t nvme_rdma_queue_rq(struct blk_mq_hw_ctx *hctx,
> ret = nvme_host_path_error(rq);
> else if (err == -ENOMEM || err == -EAGAIN)
> ret = BLK_STS_RESOURCE;
> + /*
> + * The DMA layer refused to map peer memory to this device: a
> + * property of the pairing, not a path failure. Match nvme-pci
> + * and do not retry (see blk_path_error()).
> + */
> + else if (err == -EREMOTEIO)
> + ret = BLK_STS_TARGET;
> else
> ret = BLK_STS_IOERR;
> nvme_cleanup_cmd(rq);
[Severity: High]
This isn't a bug introduced by this patch, but can this error path lead to a
use-after-free and DMA unmap corruption?
If nvme_rdma_post_send() fails with -EIO, nvme_host_path_error(rq) is
called, which synchronously completes and frees the request. The code then
falls through to execute nvme_cleanup_cmd(rq) and ib_dma_unmap_single() on
the already freed request.
Could this unmap the active DMA region of a completely different, newly
allocated I/O command?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=7
^ permalink raw reply
* Re: [PATCH v2 6/7] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
From: sashiko-bot @ 2026-07-19 11:11 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-pci, linux-raid
In-Reply-To: <20260719105327.864949-7-mykola@meshstor.io>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
Pre-existing issues:
- [High] Forcing REQ_OP_WRITE on failed dataless bios in narrow_write_error() causes a NULL pointer dereference.
- [High] narrow_write_error drops critical bio flags (such as REQ_FUA or REQ_PREFLUSH) when retrying failed writes.
- [High] Possible NULL pointer dereference when logging unrecoverable read errors in raid1.
--
--- Patch [6]: [PATCH v2 6/7] md/raid1,raid10: skip futile retries on P2PDMA mapping failures ---
commit ff103403bf433f1316745e0b4415b739bb72c8d1
Author: Mykola Marzhan <mykola@meshstor.io>
md/raid1,raid10: skip futile retries on P2PDMA mapping failures
This commit adds tracking for P2PDMA masters via a state bit to handle peer
device memory mapping failures explicitly. It prevents kicking healthy mirrors
and short-circuits futile retry loops by treating unroutable P2PDMA bio
failures as a property of the topology rather than the medium.
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -1378,6 +1388,8 @@ static void raid1_read_request(struct mddev *mddev, struct bio *bio,
> else
> init_r1bio(r1_bio, mddev, bio);
> r1_bio->sectors = max_read_sectors;
> + if (md_bio_is_p2pdma(bio))
> + set_bit(R1BIO_P2PDMA, &r1_bio->state);
>
> /*
> * make_request() can abort the operation when read-ahead is being
[Severity: High]
This is a pre-existing issue, but could the error logging in
raid1_read_request() result in a NULL pointer dereference?
When an unrecoverable read error occurs, handle_read_error() drops its
active reference to the failing device via rdev_dec_pending() and then calls
raid1_read_request() to attempt a retry.
If read_balance() cannot find an alternative mirror, the error logging uses
conf->mirrors[r1_bio->read_disk].rdev->bdev:
rdisk = read_balance(conf, r1_bio, &max_sectors);
if (rdisk < 0) {
if (r1bio_existed)
pr_crit_ratelimited("md/raid1:%s: %pg: ...\n",
mdname(mddev),
conf->mirrors[r1_bio->read_disk].rdev->bdev,
r1_bio->sector);
raid_end_bio_io(r1_bio);
return;
}
Since the reference to this disk was just dropped and the RCU read lock
is not held, is it possible for a concurrent hot_remove_disk operation to set
rdev to NULL, leaving this bdev access unprotected?
[ ... ]
> @@ -2539,7 +2553,7 @@ static void fix_read_error(struct r1conf *conf, struct r1bio *r1_bio)
> }
> }
>
> -static void narrow_write_error(struct r1bio *r1_bio, int i)
> +static void narrow_write_error(struct r1bio *r1_bio, int i, bool coarse)
> {
> struct mddev *mddev = r1_bio->mddev;
[Severity: High]
This isn't a bug introduced by this patch, but does the error retry path
in narrow_write_error() inadvertently corrupt the bio operation and flags?
When cloning a failed bio for retrying, the code unconditionally overwrites
the bi_opf field:
wbio->bi_opf = REQ_OP_WRITE;
If the failed master bio was a dataless operation (such as REQ_OP_WRITE_ZEROES),
the operation override happens before bio_trim(). For multi-block requests,
bio_trim() calls bio_advance(). Because the bio is now incorrectly seen as a
standard write, bio_no_advance_iter() returns false, and the block layer
attempts to advance the NULL bi_io_vec, which will cause a kernel panic.
Additionally, the direct assignment of REQ_OP_WRITE completely clobbers
pre-existing request flags. If a flush or FUA write fails and is retried,
it seems it will be submitted as a normal asynchronous write. If the retry
succeeds, data durability guarantees might be violated since the data could
only reside in the volatile disk cache.
[ ... ]
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
[ ... ]
> @@ -2786,7 +2803,7 @@ static void fix_read_error(struct r10conf *conf, struct mddev *mddev, struct r10
> }
> }
>
> -static void narrow_write_error(struct r10bio *r10_bio, int i)
> +static void narrow_write_error(struct r10bio *r10_bio, int i, bool coarse)
> {
> struct bio *bio = r10_bio->master_bio;
[Severity: High]
This is a pre-existing issue, but does narrow_write_error() similarly
corrupt dataless bios and drop durability flags here?
Similar to raid1, the clone unconditionally overwrites bi_opf:
wbio->bi_opf = REQ_OP_WRITE;
While bio_trim() happens before the override in raid10 (avoiding the
iterator crash), it appears to submit the modified REQ_OP_WRITE bio
without a payload to lower drivers, which could crash during DMA mapping.
It also drops critical bio modifiers like REQ_FUA or REQ_PREFLUSH, which
could lead to silent data corruption on power failure if a synchronous write
encounters a transient error.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=6
^ permalink raw reply
* Re: [PATCH v2 3/7] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
From: sashiko-bot @ 2026-07-19 11:08 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: linux-raid, linux-pci, yukuai
In-Reply-To: <20260719105327.864949-4-mykola@meshstor.io>
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
New issues:
- [High] Hot-added disks silently bypass array-wide serialization when MD_SERIALIZE_POLICY is enabled, defeating write ordering guarantees.
Pre-existing issues:
- [High] REQ_NOWAIT bios can block during clone allocation and serialization, violating non-blocking semantics.
- [High] The serialization logic fails to properly order a write against all overlapping in-flight writes when there are multiple independent collisions.
--
--- Patch [3]: [PATCH v2 3/7] md/raid1: serialize non-write-behind writes on CollisionCheck rdevs ---
commit 26970b9339baa20abd27398542d6d87116b76717
Author: Mykola Marzhan <mykola@meshstor.io>
md/raid1: serialize non-write-behind writes on CollisionCheck rdevs
This commit changes the gate for non-write-behind serialization in raid1
from the array-wide MD_SERIALIZE_POLICY to the per-device CollisionCheck
flag. This ensures write-behind writes and non-write-behind writes are
correctly serialized against each other on members that require it.
Discards are also now ordered on this path.
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
[ ... ]
> @@ -1677,7 +1677,14 @@ static bool raid1_write_request(struct mddev *mddev, struct bio *bio,
> mbio = bio_alloc_clone(rdev->bdev, bio, GFP_NOIO,
> &mddev->bio_set);
[Severity: High]
This is a pre-existing issue, but does this code block REQ_NOWAIT bios?
Calling bio_alloc_clone() with GFP_NOIO can block in memory reclaim, and
wait_for_serialization() unconditionally sleeps in wait_for_completion().
If the original bio has the REQ_NOWAIT flag set, won't this violate the
non-blocking semantics and potentially stall async submission threads like
io_uring?
> - if (test_bit(MD_SERIALIZE_POLICY, &mddev->flags))
> + /*
> + * Order against in-flight write-behind I/O: a
> + * behind write is acked early, and an unordered
> + * overwrite could land first, leaving its stale
> + * data on the member last. CollisionCheck marks
> + * every rdev that owns a serial tree.
> + */
> + if (test_bit(CollisionCheck, &rdev->flags))
> wait_for_serialization(rdev, r1_bio);
[Severity: High]
Does this change unintentionally disable serialization for hot-added disks?
If a user enables serialize_policy to enforce strict write ordering, and then
hot-adds a new disk that isn't WriteMostly, mddev_create_serial_pool() in
drivers/md/md.c will exit early:
drivers/md/md.c:mddev_create_serial_pool() {
...
if (rdev && !rdev_need_serial(rdev) &&
!test_bit(CollisionCheck, &rdev->flags))
return;
...
}
Because this setup exits without allocating a serial tree or setting the
CollisionCheck flag, won't the new per-device check here in
raid1_write_request() silently bypass wait_for_serialization() for the new
disk, even though the array-wide MD_SERIALIZE_POLICY is active?
[Severity: High]
This is a pre-existing issue, but does wait_for_serialization() handle
multiple overlapping writes correctly?
If we have two in-flight writes, A (sectors 0-100) and Y (sectors 120-200),
and a new write B (sectors 50-150) is submitted, it overlaps both. When B
checks for collisions, it finds A and waits.
When A completes, remove_serial() inserts B into the interval tree and wakes
it up:
drivers/md/raid1.c:remove_serial() {
...
raid1_rb_insert(iter_si, &serial->serial_rb);
complete(&iter_si->ready);
...
}
When B re-checks for collisions in check_and_add_serial(), the tree now
contains B and Y. raid1_rb_iter_first() will return B since its start sector
is lower than Y's:
drivers/md/raid1.c:check_and_add_serial() {
...
head_si = raid1_rb_iter_first(&serial->serial_rb, lo, hi);
if (head_si && head_si != si) {
...
}
Since head_si == si (B == B), the collision handler is skipped. Does this
mean B will execute concurrently with Y, bypassing serialization for the
120-150 region?
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=3
^ permalink raw reply
* Re: [PATCH v2 4/7] md/raid1: don't use write-behind for P2PDMA bios
From: sashiko-bot @ 2026-07-19 11:05 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-pci, linux-raid
In-Reply-To: <20260719105327.864949-5-mykola@meshstor.io>
> alloc_behind_master_bio() copies the bio's data with bio_copy_data(),
> a CPU copy. P2PDMA pages are peer device (BAR) memory; generic code
> must not assume CPU load/store access to them is safe or fast on
> every architecture, and bouncing peer memory through the CPU defeats
> the point of a peer-to-peer transfer.
>
> Skip write-behind for P2PDMA bios: they are written directly to all
> members, including write-mostly ones. Ordering against write-behind
> I/O in flight to overlapping sectors is preserved: the non-behind
> clone path serializes on CollisionCheck rdevs (see the preceding
> fix), which covers these bios like any other write that bypasses
> write-behind.
>
> Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=4
^ permalink raw reply
* Re: [PATCH v2 2/7] md: ensure REQ_NOMERGE is set on P2PDMA bios
From: sashiko-bot @ 2026-07-19 11:02 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-pci, linux-raid
In-Reply-To: <20260719105327.864949-3-mykola@meshstor.io>
> md_submit_bio() unconditionally strips REQ_NOMERGE before passing the
> bio to the personality, an optimization from commit 9c573de3283a ("MD:
> make bio mergeable"): a bio that md has split may become mergeable
> again below md.
>
> For PCI P2PDMA bios the flag is load-bearing, not a hint. The block
> layer sets REQ_NOMERGE on P2PDMA bios (__bio_add_page(), and the
> extraction path of bio_iov_iter_get_pages()) because the DMA mapping
> type of a request is resolved once, from its first segment
> (blk_dma_map_iter_start()), and request-level merging is prevented
> only by REQ_NOMERGE. Stripping it allows the member queue to merge a
> P2PDMA bio with a bio carrying pages of a different pgmap, or host
> memory, mapping the merged segments with the wrong bus address:
> silent data corruption on the member.
>
> Set the flag for P2PDMA bios instead of merely preserving it. No
> in-tree path currently submits P2PDMA pages through the bvec-iter
> path (bio_iov_bvec_set()), which skips the flagging -- but nothing
> structural prevents one, so setting rather than preserving hardens
> md against that gap at the cost of one branch. Everything else keeps
> the original optimization of clearing the flag. This covers every
> personality that advertises BLK_FEAT_PCI_P2PDMA (raid0, raid1,
> raid10), which is why the fix lives in the shared md_submit_bio()
> path.
>
> Fixes: 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=2
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: sashiko-bot @ 2026-07-19 11:01 UTC (permalink / raw)
To: Mykola Marzhan; +Cc: yukuai, linux-pci, linux-raid
In-Reply-To: <20260719105327.864949-2-mykola@meshstor.io>
> Commit 91fb2b6052f7 ("nvme-pci: convert to using dma_map_sgtable()")
> deliberately mapped unsupported P2PDMA transfers to BLK_STS_TARGET,
> matching dma_map_sgtable()'s -EREMOTEIO: "When this happens, return
> BLK_STS_TARGET so the request isn't retried." The conversion to
> blk_rq_dma_map silently changed the status to BLK_STS_INVAL, which
> regresses two consumers:
>
> - md/raid1 and raid10 ignore BLK_STS_INVAL leg failures
> (commit f7b24c7b41f2 ("md/raid1,raid10: don't fail devices for
> invalid IO errors"), where it means a request-shaped error that
> fails identically on every member). Since commit 02666132403a
> ("md: propagate BLK_FEAT_PCI_P2PDMA from member devices to RAID device")
> P2PDMA bios reach md arrays: a peer-memory write to a member the
> peer cannot reach is counted as written, the master bio reports
> success, mirrors silently diverge, and on a topology where no
> [ ... ]
> Fixes: 858299dc6160 ("block: add scatterlist-less DMA mapping helpers")
> Fixes: 7ce3c1dd78fc ("nvme-pci: convert the data mapping to blk_rq_dma_map")
> Cc: stable@vger.kernel.org # v6.17
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Sashiko has reviewed this patch and found no issues. It looks great!
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260719105327.864949-1-mykola@meshstor.io?part=1
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox