* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 18:49 UTC (permalink / raw)
To: Mykola Marzhan, Christoph Hellwig
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Sagi Grimberg,
linux-block, linux-raid, linux-nvme, Li Nan, Xiao Ni,
Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <b2c37f4f-79dd-428f-915a-2a862d20924c@deltatee.com>
On 2026-07-20 12:42, Logan Gunthorpe wrote:
>
>
> On 2026-07-20 11:30, Mykola Marzhan wrote:
>> On Mon, Jul 20, 2026 at 4:49 PM Christoph Hellwig <hch@lst.de
>>> This is a really weird writing style, and suggested to me you neither
>>> understand the code nor the problem. Please actually think yourself,
>>> write the patches and commit logs yourself instdad of this garbage.
>>>
>>> And please also explain how you even generate the I/O that fails this
>>> way.
>> - md mirrors the write: the copy to member 1 succeeds, the copy to far
>> member fails as BLK_STS_INVAL.
>> - md deliberately ignores BLK_STS_INVAL, so the write is counted as
>> written and the mirror silently diverges.
>
> Seeing I just reviewed this and answered it for myself I'll expand on
> what's going on here:
>
> The switch statement will take the default branch when
> pci_p2pdma_state() returns PCI_P2PDMA_MAP_NOT_SUPPORTED. This indicates
> the memory that's trying to be mapped in this way will not succeed
> because it's going through an unsupported host bridge (or if the device
> isn't a PCI device, etc, but this is rarer or perhaps not possible).
> Returning BLK_STS_INVAL doesn't seem correct here, to me. (Arguably it
> might have been clearer if the switch case explicitly stated
> PCI_P2PDMA_MAP_NOT_SUPPORTED instead of relying on the default).
>
> Returning -EREMOTEIO/BLK_STS_TARGET was the convention for this I had
> originally set when I wrote some of this and differentiating the error
> does seem important in Patch 6 in this series.
Sorry, one other note I almost forgot about which is probably important:
BLK_STS_INVAL will attempt a retry, where BLK_STS_TARGET will not (I
think that's part of why I chose that specific error in the first
place). A retry here doesn't make any sense because it will always hit
the same case and fail. So it's probably worth applying this patch even
without the specific error being used in Patch 6.
Logan
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 18:42 UTC (permalink / raw)
To: Mykola Marzhan, Christoph Hellwig
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Sagi Grimberg,
linux-block, linux-raid, linux-nvme, Li Nan, Xiao Ni,
Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <CAPzsNDv6hUV3gHdaxuwjRcr4couUKq_M5DKObFNaYqCHnm7ZMw@mail.gmail.com>
On 2026-07-20 11:30, Mykola Marzhan wrote:
> On Mon, Jul 20, 2026 at 4:49 PM Christoph Hellwig <hch@lst.de
>> This is a really weird writing style, and suggested to me you neither
>> understand the code nor the problem. Please actually think yourself,
>> write the patches and commit logs yourself instdad of this garbage.
>>
>> And please also explain how you even generate the I/O that fails this
>> way.
> - md mirrors the write: the copy to member 1 succeeds, the copy to far
> member fails as BLK_STS_INVAL.
> - md deliberately ignores BLK_STS_INVAL, so the write is counted as
> written and the mirror silently diverges.
Seeing I just reviewed this and answered it for myself I'll expand on
what's going on here:
The switch statement will take the default branch when
pci_p2pdma_state() returns PCI_P2PDMA_MAP_NOT_SUPPORTED. This indicates
the memory that's trying to be mapped in this way will not succeed
because it's going through an unsupported host bridge (or if the device
isn't a PCI device, etc, but this is rarer or perhaps not possible).
Returning BLK_STS_INVAL doesn't seem correct here, to me. (Arguably it
might have been clearer if the switch case explicitly stated
PCI_P2PDMA_MAP_NOT_SUPPORTED instead of relying on the default).
Returning -EREMOTEIO/BLK_STS_TARGET was the convention for this I had
originally set when I wrote some of this and differentiating the error
does seem important in Patch 6 in this series.
Logan
^ permalink raw reply
* Re: [PATCH v2 6/7] md/raid1,raid10: skip futile retries on P2PDMA mapping failures
From: Logan Gunthorpe @ 2026-07-20 16:49 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260719105327.864949-7-mykola@meshstor.io>
On 2026-07-19 04:53, Mykola Marzhan wrote:
> drivers/md/raid1.c | 56 +++++++++++++++++++++++++++++++-----
> drivers/md/raid1.h | 2 ++
> drivers/md/raid10.c | 69 ++++++++++++++++++++++++++++++++++++++-------
> drivers/md/raid10.h | 2 ++
This is an intimidating and massive patch to review. I gotta second the
verbosity of the AI commit messages comment from Keith.
My eyes glazed over a bit. But one point is that maybe this could be
split into multiple patches: one patch introduces R1BIO_P2PDMA, one
patch for raid1 and another patch for raid10? That might make it a bit
more manageable.
Also, I'm not sure if it's possible without digging deeper but, instead
of adding a !p2pdma_unmappable check to every if() statement in the
raid*_end_write_request() functions, might it be possible to factor out
a helper function and return early from it? Maybe the helper could be
shared between raid10 and raid1 as they look very similar. The
differences between the two are minimal -- there might be a bug fix that
was applied to one that should be applied to both?
Thanks!
Logan
^ permalink raw reply
* Re: [PATCH v2 5/7] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
From: Logan Gunthorpe @ 2026-07-20 16:32 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260719105327.864949-6-mykola@meshstor.io>
On 2026-07-19 04:53, Mykola Marzhan wrote:
> narrow_write_error() re-issues a failed write in badblock-granularity
> chunks, cloning from the master bio and resetting bi_opf to a bare
> REQ_OP_WRITE. For a P2PDMA bio that reset drops REQ_NOMERGE, which is
> the only request-level protection against the member queue merging
> P2PDMA segments across pgmaps or with host memory (see the preceding
> md_submit_bio() fix): the retry path would quietly reopen the hole
> the submission path closes. Restore the flag on P2PDMA retry clones.
>
> 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>
Looks correct, thanks.
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH v2 4/7] md/raid1: don't use write-behind for P2PDMA bios
From: Logan Gunthorpe @ 2026-07-20 16:31 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260719105327.864949-5-mykola@meshstor.io>
On 2026-07-19 04:53, Mykola Marzhan wrote:
> 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>
Looks correct to me.
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH 0/6] block,md,nvme: correct handling of unsupported P2PDMA transfers
From: Keith Busch @ 2026-07-20 16:09 UTC (permalink / raw)
To: Leon Romanovsky
Cc: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Jason Gunthorpe, Kiran Kumar Modukuri,
Chaitanya Kulkarni, Logan Gunthorpe, Bjorn Helgaas, Shivaji Kant,
Pranjal Shrivastava, Henrique Carvalho, linux-kernel, linux-rdma,
linux-pci
In-Reply-To: <20260720124304.GE110966@unreal>
On Mon, Jul 20, 2026 at 03:43:04PM +0300, Leon Romanovsky wrote:
> It would be great if developers supervised their AI tools and kept
> commit messages and cover letters concise and readable.
>
> Everything above can be reduced to a simple sentence: "MD treats an
> unsupported P2P operation as success rather than failure; update the
> code accordingly."
Thank you for TL;DR summary! The verbosity from these things is a bit
annoying.
^ permalink raw reply
* Re: [PATCH 2/6] md: ensure REQ_NOMERGE is set on P2PDMA bios
From: Logan Gunthorpe @ 2026-07-20 16:03 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260718162547.448892-3-mykola@meshstor.io>
On 2026-07-18 10:25, Mykola Marzhan wrote:
> 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>
Makes sense to me. Matches the similar check in nvmet_bdev_execute_rw().
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH 1/6] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Logan Gunthorpe @ 2026-07-20 15:58 UTC (permalink / raw)
To: Mykola Marzhan, Jens Axboe, Song Liu, Yu Kuai, Keith Busch,
Christoph Hellwig, Sagi Grimberg, linux-block, linux-raid,
linux-nvme
Cc: Li Nan, Xiao Ni, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Bjorn Helgaas,
Shivaji Kant, Pranjal Shrivastava, Henrique Carvalho,
linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260718162547.448892-2-mykola@meshstor.io>
On 2026-07-18 10:25, Mykola Marzhan wrote:
> 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
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
Looks correct to me, thanks!
Reviewed-by: Logan Gunthorpe <logang@deltatee.com>
^ permalink raw reply
* Re: [PATCH v2 1/7] blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
From: Christoph Hellwig @ 2026-07-20 14:49 UTC (permalink / raw)
To: Mykola Marzhan
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Guoqing Jiang, Leon Romanovsky, Jason Gunthorpe,
Kiran Kumar Modukuri, Chaitanya Kulkarni, Logan Gunthorpe,
Bjorn Helgaas, Shivaji Kant, Pranjal Shrivastava,
Henrique Carvalho, linux-kernel, linux-rdma, linux-pci
In-Reply-To: <20260719105327.864949-2-mykola@meshstor.io>
On Sun, Jul 19, 2026 at 10:53:21AM +0000, Mykola Marzhan wrote:
> 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:
This is a really weird writing style, and suggested to me you neither
understand the code nor the problem. Please actually think yourself,
write the patches and commit logs yourself instdad of this garbage.
And please also explain how you even generate the I/O that fails this
way.
^ permalink raw reply
* Re: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
From: Coly Li @ 2026-07-20 14:38 UTC (permalink / raw)
To: sashiko-reviews; +Cc: yukuai, linux-raid
In-Reply-To: <20260720113553.9353D1F000E9@smtp.kernel.org>
> 2026年7月20日 19:35，sashiko-bot@kernel.org 写道：
>
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
>
> Pre-existing issues:
> - [High] The newly added bounds check for `sb->bblog_shift` incorrectly allows values up to 63, failing to prevent 32-bit integer overflows and undefined behavior which can lead to a divide-by-zero kernel panic.
> --
>
> --- Patch [1]: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load() ---
> commit 36f9915dbd10f1c3803610b32e46390e9cb7ec7c
> Author: Coly Li <colyli@fygo.io>
>
> md: do overflow check for sb->bblog_shift in super_1_load()
>
> This commit adds a bounds check for sb->bblog_shift in super_1_load() to
> prevent possible overflows in the badblocks API. It limits the shift value
> to be less than BITS_PER_TYPE(sector_t).
>
>> diff --git a/drivers/md/md.c b/drivers/md/md.c
>> index d1465bcd86c81..67f8edf4db368 100644
>> --- a/drivers/md/md.c
>> +++ b/drivers/md/md.c
>> @@ -1920,6 +1920,13 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
>> rdev->bb_page, REQ_OP_READ, true))
>> return -EIO;
>> bbp = (__le64 *)page_address(rdev->bb_page);
>> +
>> + /* check for badblocks api. */
>> + if (sb->bblog_shift >= BITS_PER_TYPE(sector_t)) {
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> [Severity: High]
> This is a pre-existing issue, but does this bounds check still allow values
> that can cause undefined behavior and integer overflows?
>
> Since BITS_PER_TYPE(sector_t) is 64, this allows sb->bblog_shift to be up
> to 63.
>
> However, count is declared as a 32-bit signed int in super_1_load(), so the
> following operation:
>
> count <<= sb->bblog_shift;
>
> will cause undefined behavior if the shift is >= 31.
>
> Additionally, downstream in block/badblocks.c (such as in badblocks_check()
> and _badblocks_set()), the shift is used with a 32-bit literal:
>
> rounddown(s, 1 << bb->shift);
>
> If sb->bblog_shift is >= 32, this can result in undefined behavior and
> yield 0, leading to a divide-by-zero panic in the rounddown() macro during
> array IO.
>
> Should the maximum shift be restricted to a smaller value?
With an overflow check patch from Ramesh Adhikari <adhikari.resume@gmail.com>,
only checking sb->bblog_shift >= BITS_PER_TYPE(sector_t) is enough. The rested overflow
check will be performed inside badblocks API.
And if variables are overflowed before calling badblocks API, this is not the badblocks API
issue, should not belong to the badblocks API check fixes.
Also I don’t feel it is necessary to do restricted overflow checking in super block loading stage,
if the invalid values are rejected, this super block will fail to load and won’t introduce any run time
problem.
Therefore, current checking with (sb->bblog_shift >= BITS_PER_TYPE(sector_t) is sufficient.
>
>> + pr_err("md: %pg: bogus bblog_shift %u for badblocks.\n",
>> + rdev->bdev, sb->bblog_shift);
>> + return -EINVAL;
>> + }
>> rdev->badblocks.shift = sb->bblog_shift;
>> for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
>> u64 bb = le64_to_cpu(*bbp);
>
> --
> Sashiko AI review · https://sashiko.dev/#/patchset/20260720111400.2120834-1-colyli@fygo.io?part=1
^ permalink raw reply
* Re: [PATCH 0/6] block,md,nvme: correct handling of unsupported P2PDMA transfers
From: Leon Romanovsky @ 2026-07-20 12:43 UTC (permalink / raw)
To: Mykola Marzhan
Cc: Jens Axboe, Song Liu, Yu Kuai, Keith Busch, Christoph Hellwig,
Sagi Grimberg, linux-block, linux-raid, linux-nvme, Li Nan,
Xiao Ni, Jason Gunthorpe, Kiran Kumar Modukuri,
Chaitanya Kulkarni, Logan Gunthorpe, Bjorn Helgaas, Shivaji Kant,
Pranjal Shrivastava, Henrique Carvalho, linux-kernel, linux-rdma,
linux-pci
In-Reply-To: <20260718162547.448892-1-mykola@meshstor.io>
On Sat, Jul 18, 2026 at 04:25:41PM +0000, Mykola Marzhan wrote:
> Driving peer-to-peer I/O (NVMe CMB source) through md arrays and
> nvme-rdma legs on asymmetric PCIe topologies -- where the peer device
> reaches some members/paths but not others -- turns up two failures on
> v7.2-rc, both rooted in how an unsupported-P2PDMA mapping failure is
> reported. On md: silent data loss -- an unreachable-leg P2PDMA write
> counts as written, mirrors silently diverge, and where no member is
> reachable the write reports success with zero copies on stable
> storage; reads picked to an unreachable leg fail with EIO and are
> never retried on the mirror that holds the data. On nvme-rdma: the
> transport swallows the DMA layer's mapping errno into a path error,
> which under the default multipath configuration livelocks the I/O (a
> stacked md mirror hangs) and with nvme_core.multipath=N surfaces as
> retryable BLK_STS_TRANSPORT.
>
> The root cause is a block-layer status-code regression, not an md or
> nvme bug. Commit 91fb2b6052f7 ("nvme-pci: convert to using
> dma_map_sgtable()") deliberately mapped unsupported P2PDMA transfers
> to BLK_STS_TARGET ("... return BLK_STS_TARGET so the request isn't
> retried"). Commit 858299dc6160 ("block: add scatterlist-less DMA
> mapping helpers") silently changed that to BLK_STS_INVAL, and since
> commit 7ce3c1dd78fc ("nvme-pci: convert the data mapping to
> blk_rq_dma_map") in v6.17 the failure surfaces to direct NVMe
> consumers as EINVAL instead of the documented -EREMOTEIO, with
> blk_path_error() now classifying it as retryable. md/raid1,raid10
> ignore BLK_STS_INVAL leg failures per 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 -- hence the silent divergence. The md leg of the regression
> became reachable when md started advertising BLK_FEAT_PCI_P2PDMA in
> v7.2-rc1 (commit 02666132403a ("md: propagate BLK_FEAT_PCI_P2PDMA
> from member devices to RAID device")).
>
> Patch 1 restores BLK_STS_TARGET at the source. md then handles an
> unreachable leg like any other per-device error: badblocks on the
> affected member, the master bio succeeds while an In_sync leg holds
> the data, and reads are redirected to the other mirror.
>
> Patches 2-4 fix independent md-side P2PDMA bugs of the same vintage:
> md_submit_bio() strips REQ_NOMERGE (the only request-level protection
> against merging P2PDMA segments across pgmaps); raid1 write-behind
> CPU-copies device BAR memory via bio_copy_data(); and
> narrow_write_error()'s retry clones reset bi_opf, dropping the
> REQ_NOMERGE protection exactly on the error-retry path.
>
> Patch 5 stops the restored device-error machinery from misfiring
> where its medium-error assumptions don't hold. A mapping failure is a
> property of the peer/member pairing: retrying the same peer pages
> against the same member cannot succeed, and there is nothing on the
> medium to repair. Without it, narrow_write_error() serializes dozens
> of guaranteed-to-fail chunk retries per failed write; on reads each
> unreachable-leg pick costs a full freeze_array() quiesce plus a tick
> of the read-error budget -- measured below, a mixed host/P2P read
> workload on an asymmetric topology kicks the perfectly healthy far
> leg after ~20 failed picks, in under a second; and on FailFast
> members a single unroutable I/O evicts a healthy mirror outright.
> Because BLK_STS_TARGET is also produced for transient device
> conditions md cannot distinguish from bi_status alone, writes probe
> the whole range once instead of predicting futility: a real mapping
> failure is recorded in one call, a cleared transient recovers with
> nothing recorded. Patches 1 and 5 belong in the same release: with
> patch 1 alone, restoring the error surfaces the retry storms and
> read-path evictions above on 7.2's newly reachable md path.
>
> Patch 6 closes the same hole in the rdma transport. nvme-rdma maps
> the data scatterlist with ib_dma_map_sg(), which returns 0 on a
> peer-unreachable failure, discarding the -EREMOTEIO that
> dma_map_sgtable() documents for exactly this case; the driver reports
> it as a path error. Because the multipath head node advertises
> BLK_FEAT_PCI_P2PDMA (commit fb0eeeed91f3 ("nvme-multipath: enable PCI
> P2PDMA for multipath devices"), v7.2-rc1) and the mapping failure is
> deterministic, nvme_failover_req() requeues the bios with a fresh
> retry budget every cycle and the I/O never completes -- a hot requeue
> livelock that hangs a stacked md mirror instead of failing over; with
> nvme_core.multipath=N it burns nvme_max_retries requeues and
> completes as retryable BLK_STS_TRANSPORT. Patch 6 maps with
> ib_dma_map_sgtable_attrs() to preserve the errno and returns
> BLK_STS_TARGET, matching nvme-pci so patch 5's handling covers rdma
> legs too, and starts the request only after the map succeeds so
> nvme_mpath_start_request() accounting cannot leak on the direct
> blk-mq completion. nvme-rdma stays on the scatterlist DMA API: its
> fast-reg MR path (ib_map_mr_sg()) consumes scatterlists, so a
> blk_rq_dma_map conversion is separate modernization, out of scope for
> a fix.
>
> Behavior on v7.2-rc2, QEMU rig (one CMB provider, two NVMe members,
> q35 PCIe topologies), 90 8KiB peer-memory reads under concurrent
> host reads for the read rows:
>
> topology op v7.2-rc2 patched (1-5)
> symmetric write+read ok ok
> asymmetric write "ok", silent diverge ok + badblocks on
> far leg (1 probe)
> asymmetric read 41/90 EIO, no mirror 90/90 ok, no
> under load retry, no eviction eviction
> unreachable write "ok", NO data copied EIO + badblocks
> unreachable read EIO EIO, no members
> kicked
>
> Also verified on the same rig: a transient injected TARGET recovers
> through the single probe with nothing recorded; P2P MEDIUM keeps the
> chunked path; host-page TARGET/INVAL handling is unchanged; FailFast
> survives mapping failures but still evicts on genuine errors;
> write-mostly legs are written directly (no write-behind CPU copy); a
> degraded array whose only leg is unreachable gets plain EIO -- no
> budget charge, no eviction; an exhausted badblocks table fails the
> member as described below. The write/read/injection rows repeat
> identically on raid10.
>
> Validation of patch 6: the mechanism -- ib_dma_map_sg() returning 0
> where ib_dma_map_sgtable_attrs() preserves -EREMOTEIO -- was
> confirmed on a real mlx5 HCA with a map-only probe. The
> start-after-map reorder ran over an rxe nvmet-rdma loopback under fio
> with crc32c verification; multipath inflight and nr_active accounting
> drain to zero, matching the pre-reorder kernel. rxe cannot produce
> -EREMOTEIO, so the failure paths were driven by injecting that exact
> mechanism at the map call site: unpatched, one 4KiB write livelocked
> (132 failover requeues in 8s, a stacked raid1 hung; multipath=N:
> retryable BLK_STS_TRANSPORT); patched, it failed immediately as
> BLK_STS_TARGET with zero requeues, and the stacked raid1 badblocked
> the leg without evicting it.
>
> Known trade-offs of routing through the stock md write machinery:
> an unroutable P2P write records badblocks and sets WriteErrorSeen
> and WantReplacement, so later writes overlapping those ranges skip
> the member -- host writes do not clear the entries and repair skips
> known-bad ranges -- leaving them single-copy until re-add or
> replacement recovery (host memory, so it succeeds) rewrites and
> clears them; a member whose badblocks table is exhausted or disabled
> is failed on the first unroutable P2P write, as with any write error
> md cannot record. FailFast members are exempted from md_error() for
> mapping failures only (nothing reached the wire, so the error says
> nothing about device health); genuine I/O errors keep their
> immediate-eviction semantics. raid10 additionally fails a
> replacement device outright on a P2P write mapping failure (its
> stock replacement-write policy), and an IO_BLOCKED slot excludes
> that slot's replacement from the read retry -- both pre-existing
> raid10 behaviors with a new trigger. On reads, read_balance() keeps
> no memory of unreachability, so each far-leg pick still costs one
> failed submission before the redirect; a sticky per-rdev
> reachability hint would be md-next follow-up material, as would a
> distinct block status naming the mapping failure exactly.
>
> Routing: patch 1 is block-tree material and fixes a v6.17-rc1 status
> regression (hence its Cc: stable); patches 2-5 are md and address
> exposure that only became reachable with v7.2-rc1; patch 6 is
> nvme-rdma and also carries Cc: stable -- the transport misreport
> predates the head-node change (Fixes: 23528aa3320a, v7.1) and is
> reachable on v7.1 with multipath=N. Patch 6 sources the errno from
> dma_map_sgtable() in rdma.c and does not depend on patch 1, which
> fixes the separate blk_rq_dma_map path. Patches 1 (block) and 6 (nvme) are
> independently applicable bug fixes: each fixes a user-visible defect
> on its own and neither depends on the md patches, while 2-5 are
> md-side hardening built on patch 1 -- a maintainer can take 1 and 6
> even if the md discussion runs longer. Single-cycle routing through
> the relevant trees with acks works for us: patch 1 block, 2-5 md,
> 6 nvme.
>
> Per Documentation/process/coding-assistants.rst: the patches were
> developed with AI assistance (see the Assisted-by trailers); all code
> was human-reviewed and tested as described above.
It would be great if developers supervised their AI tools and kept
commit messages and cover letters concise and readable.
Everything above can be reduced to a simple sentence: "MD treats an
unsupported P2P operation as success rather than failure; update the
code accordingly."
Thanks
>
> Series is against v7.2-rc2.
>
> Mykola Marzhan (6):
> blk-mq-dma: restore BLK_STS_TARGET for unsupported P2P transfers
> md: ensure REQ_NOMERGE is set on P2PDMA bios
> md/raid1: don't use write-behind for P2PDMA bios
> md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
> md/raid1,raid10: skip futile retries on P2PDMA mapping failures
> nvme-rdma: return BLK_STS_TARGET for unsupported P2P transfers
>
> block/blk-mq-dma.c | 10 ++++++-
> drivers/md/md.c | 14 ++++++++--
> drivers/md/md.h | 31 +++++++++++++++++++++
> drivers/md/raid1.c | 56 +++++++++++++++++++++++++++++++------
> drivers/md/raid10.c | 60 ++++++++++++++++++++++++++++++++++------
> drivers/nvme/host/rdma.c | 26 +++++++++++------
> 6 files changed, 168 insertions(+), 29 deletions(-)
>
>
> base-commit: 8cdeaa50eae8dad34885515f62559ee83e7e8dda
> --
> 2.43.0
>
>
^ permalink raw reply
* Re: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
From: sashiko-bot @ 2026-07-20 11:35 UTC (permalink / raw)
To: colyli; +Cc: yukuai, linux-raid
In-Reply-To: <20260720111400.2120834-1-colyli@fygo.io>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] The newly added bounds check for `sb->bblog_shift` incorrectly allows values up to 63, failing to prevent 32-bit integer overflows and undefined behavior which can lead to a divide-by-zero kernel panic.
--
--- Patch [1]: [PATCH] md: do overflow check for sb->bblog_shift in super_1_load() ---
commit 36f9915dbd10f1c3803610b32e46390e9cb7ec7c
Author: Coly Li <colyli@fygo.io>
md: do overflow check for sb->bblog_shift in super_1_load()
This commit adds a bounds check for sb->bblog_shift in super_1_load() to
prevent possible overflows in the badblocks API. It limits the shift value
to be less than BITS_PER_TYPE(sector_t).
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c81..67f8edf4db368 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -1920,6 +1920,13 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
> rdev->bb_page, REQ_OP_READ, true))
> return -EIO;
> bbp = (__le64 *)page_address(rdev->bb_page);
> +
> + /* check for badblocks api. */
> + if (sb->bblog_shift >= BITS_PER_TYPE(sector_t)) {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
[Severity: High]
This is a pre-existing issue, but does this bounds check still allow values
that can cause undefined behavior and integer overflows?
Since BITS_PER_TYPE(sector_t) is 64, this allows sb->bblog_shift to be up
to 63.
However, count is declared as a 32-bit signed int in super_1_load(), so the
following operation:
count <<= sb->bblog_shift;
will cause undefined behavior if the shift is >= 31.
Additionally, downstream in block/badblocks.c (such as in badblocks_check()
and _badblocks_set()), the shift is used with a 32-bit literal:
rounddown(s, 1 << bb->shift);
If sb->bblog_shift is >= 32, this can result in undefined behavior and
yield 0, leading to a divide-by-zero panic in the rounddown() macro during
array IO.
Should the maximum shift be restricted to a smaller value?
> + pr_err("md: %pg: bogus bblog_shift %u for badblocks.\n",
> + rdev->bdev, sb->bblog_shift);
> + return -EINVAL;
> + }
> rdev->badblocks.shift = sb->bblog_shift;
> for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
> u64 bb = le64_to_cpu(*bbp);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720111400.2120834-1-colyli@fygo.io?part=1
^ permalink raw reply
* [PATCH] md: do overflow check for sb->bblog_shift in super_1_load()
From: colyli @ 2026-07-20 11:14 UTC (permalink / raw)
To: yukuai; +Cc: linux-raid, linux-block, Coly Li, stable, Ramesh Adhikari
From: Coly Li <colyli@fygo.io>
In super_1_load(), sb->bblog_shift is an __u8 type value loaded from on-
disk superblock. It is used for badblocks API badblocks_set() by the
following sequence,
1930 rdev->badblocks.shift = sb->bblog_shift;
1931 for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
1932 u64 bb = le64_to_cpu(*bbp);
1933 int count = bb & (0x3ff);
1934 u64 sector = bb >> 10;
1935 sector <<= sb->bblog_shift;
1936 count <<= sb->bblog_shift;
1937 if (bb + 1 == 0)
1938 break;
1939 if (!badblocks_set(&rdev->badblocks, sector, count, 1))
1940 return -EINVAL;
1941 }
bb->bblog_shit is in range of 0-255, variable sector is 64bit width, for
an invalid bb->bblog_shit, it is possible to make sector be overflowed
by the following calculation,
1935 sector <<= sb->bblog_shift;
Then in turn when call badblocks_set() at line 1939 with the invalid
rdev->badblocks.shift set at line 1930, may result an overflow inside
_badblocks_clear() in block/badblocks.c.
Although there are many places to call badblocks APIs, the non-zero
shift value is only used in super_1_load(), other places always use 0 as
the shift value. Therefore it is unnecessary to do a general shift value
overflow check inside badblock API, and just check here as the caller.
This may avoid unnecessary check, make the badblocks API code more simple
and elegant.
Fixes: 2699b67223aca ("md: load/store badblock list from v1.x metadata")
Fixes: 1726c77467833 ("badblocks: improve badblocks_set() for multiple ranges handling")
Cc: stable@vger.kernel.org
Cc: Ramesh Adhikari <adhikari.resume@gmail.com>
Signed-off-by: Coly Li <colyli@fygo.io>
---
drivers/md/md.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index d1465bcd86c8..67f8edf4db36 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -1918,10 +1918,17 @@ static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int minor_
bb_sector = (long long)offset;
if (!sync_page_io(rdev, bb_sector, sectors << 9,
rdev->bb_page, REQ_OP_READ, true))
return -EIO;
bbp = (__le64 *)page_address(rdev->bb_page);
+
+ /* check for badblocks api. */
+ if (sb->bblog_shift >= BITS_PER_TYPE(sector_t)) {
+ pr_err("md: %pg: bogus bblog_shift %u for badblocks.\n",
+ rdev->bdev, sb->bblog_shift);
+ return -EINVAL;
+ }
rdev->badblocks.shift = sb->bblog_shift;
for (i = 0 ; i < (sectors << (9-3)) ; i++, bbp++) {
u64 bb = le64_to_cpu(*bbp);
int count = bb & (0x3ff);
u64 sector = bb >> 10;
--
2.47.3
^ permalink raw reply related
* Re: [PATCH] md: fix soft lockup during resync when sync is repeatedly skipped
From: Yunye Zhao @ 2026-07-20 6:20 UTC (permalink / raw)
To: Yu Kuai, Song Liu
Cc: Yunye Zhao, Li Nan, Xiao Ni, Joseph Qi, linux-raid, linux-kernel
In-Reply-To: <7abeaff6-1387-4046-bea3-f1b04465addd@fygo.io>
Hi Kuai,
On 2026/7/17 14:27, Yu Kuai wrote:
>> md_do_sync()'s main loop advances io_sectors only when I/O is actually
>> issued (skipped == 0). When sync_request() keeps returning skipped == 1,
>> io_sectors never increases, [...]
>
> That's not expected, io_sectors should always increase in the skip case.
Sorry, my description was not accurate. The problem is not that io_sectors
stays 0. md_do_sync()'s loop exit condition is j == max_sectors, and in
raid10_sync_request()'s recovery path sectors is always 128, so for a very
large max_sectors the loop iterates a huge number of times.
raid10_sync_request(), recovery branch:
max_sync = RESYNC_PAGES << (PAGE_SHIFT-9); /* 128 */
must_sync = md_bitmap_start_sync(mddev, sect, &sync_blocks, true);
if (sync_blocks < max_sync) /* sync_blocks huge, never true */
max_sync = sync_blocks; /* so max_sync stays 128 */
...
if (biolist == NULL) {
*skipped = 1;
return max_sync; /* 128; the large span is discarded */
}
Back in md_do_sync()'s main loop, j then crawls forward 128 sectors per
call until it reaches max_sectors:
while (j < max_sectors) {
sectors = mddev->pers->sync_request(mddev, j, max_sectors, &skipped);
if (!skipped) /* skipped == 1 -> io_sectors stays 0 */
io_sectors += sectors;
j += sectors; /* j += 128 only */
if (last_check + window > io_sectors || j == max_sectors)
continue;
}
From the vmcore:
sync_blocks returned = 0x5ED03680 (~1.59e8 sectors), clamped to 128
recovery max_sectors = dev_sectors = 2^40
iterations = 2^40 / 128 = 2^33 (~8.6e9)
> What kernel version you're testing?
6.6.102. I also tested mainline and hit the same problem.
> If this is latest kernel, bitmap_start_sync() need to be fixed. It can't
> return skip while setting skipping sectors to 0.
bitmap_start_sync() is actually fine -- it returns a large clean span
(0x5ED03680 above). The recovery path just discards it: max_sync is only
ever clamped down, so it returns 128 regardless.
> And since this is dead loop, a cond_resched() will not fix anything.
Agreed -- cond_resched() only stops the watchdog; the thread still spins
~8.6e9 no-op iterations and pins a CPU for ~313s.
For v2 I'd fix this at the source: make the recovery path honour the clean
span reported by the bitmap instead of capping the skip at 128. Does that
direction look right to you?
Thanks,
Yunye
^ permalink raw reply
* Re: [PATCH 1/3] md/raid10: restore still_degraded detection in recovery
From: Paul Menzel @ 2026-07-20 4:32 UTC (permalink / raw)
To: Mykola Marzhan
Cc: Song Liu, Yu Kuai, Li Nan, Xiao Ni, Hannes Reinecke, linux-raid,
linux-kernel
In-Reply-To: <20260719144227.940444-2-mykola@meshstor.io>
Dear Mykola,
Thank you for your patch.
Am 19.07.26 um 16:42 schrieb Mykola Marzhan:
> raid10_sync_request()'s recovery arm passes still_degraded -- "will
> the array still be degraded after this recovery completes?" -- to
> md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
> NEEDED marker, the write-intent record. That is only safe when the
> recovery restores full redundancy; otherwise the record still
> describes writes an absent member missed.
>
> Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
> into bitmap_operations") turned the detection loop's only assignment
> from "still_degraded = 1" into "still_degraded = false" in its
> int-to-bool styling pass, so the flag has been constant false ever
> since. raid1 and raid5 were converted to "= true"; only raid10 was
> inverted.
>
> The result is silent data loss on a classic-bitmap raid10 that stays
> degraded across a recovery. With mirror pairs {A,B} and {C,D}: lose
> D, keep writing, lose B, add a spare. The rebuild into B's slot
> strips NEEDED from every chunk -- including chunks whose only
> unsatisfied intent belonged to D -- so a later --re-add of D sees an
> empty intent record, completes instantly, and serves stale data.
>
> Restore the pre-conversion assignment. Only the classic-bitmap
> recovery arm is affected: llbitmap ignores the degraded parameter,
> and the resync arm passes mddev->degraded directly.
>
> Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
> Cc: stable@vger.kernel.org # v6.12+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> drivers/md/raid10.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/raid10.c b/drivers/md/raid10.c
> index 0a3cfdd3f5df..54cddb3a98cd 100644
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -3365,7 +3365,7 @@ static sector_t raid10_sync_request(struct mddev *mddev, sector_t sector_nr,
> struct md_rdev *rdev = conf->mirrors[j].rdev;
>
> if (rdev == NULL || test_bit(Faulty, &rdev->flags)) {
> - still_degraded = false;
> + still_degraded = true;
> break;
> }
> }
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Kind regards,
Paul
^ permalink raw reply
* Re: [PATCH 2/3] md: only consult skip_sync_blocks when 'j' is in the bitmap's domain
From: yu kuai @ 2026-07-20 2:25 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-3-mykola@meshstor.io>
Hi,
在 2026/7/19 22:42, Mykola Marzhan 写道:
> Rebuilding a raid10 member with llbitmap can complete in a few
> hundred milliseconds having copied nothing: md_do_sync() offers
> every position 'j' to bitmap_ops->skip_sync_blocks(), the bitmap
> answers for the wrong chunks, the whole rebuild is skipped as
> "unwritten", and the array reports optimal with a stale member. The
> staleness is typically noticed only when the mirror partner fails;
> writes made while the array was degraded vanish.
>
> The bitmap spans [0, resync_max_sectors) in the personality's
> sync-cursor space. Resync walks exactly that range for every
> personality and may always consult the bitmap. Recovery walks
> per-device offsets, which match the bitmap's space only for raid1
> (device space is array space) and raid456 (the bitmap is indexed by
> the per-device sync cursor; bitmap_sector() translates array IO
> into it). raid10's bitmap is in array space -- near-2 on four
> disks spans twice a member's size; far-2 on two disks puts array
> chunk 2c+d at device chunk c of disk d -- so
> llbitmap_skip_sync_blocks() was consulted with offsets it cannot
> interpret.
>
> Only call the hook when the offsets match what the bitmap expects.
> Reshape must visit every stripe regardless of bitmap state --
> skipping would desync reshape_position from 'j' -- so never skip
> there. For recovery, add an md_personality capability,
> recovery_in_bitmap_space, set by raid1 and raid456. raid10 leaves
> it unset (raid10_find_virt() needs the disk number, which
> md_do_sync() does not have); raid0 and linear too, but no recovery
> path of theirs reaches the hook. No performance is lost against any
> released kernel: the hook is llbitmap-only and new in v6.18, so
> raid10 recovery returns to its earlier path -- walk every stripe,
> skip clean chunks inside raid10_sync_request().
>
> A capability rather than a geometry test: for raid10 layouts with
> raid_disks == near_copies * far_copies, resync_max_sectors equals
> dev_sectors while the spaces can still differ (far and offset
> variants), so an arithmetic proxy reopens the same hole on those
> layouts. Disabling the hook for all recovery instead would
> forfeit llbitmap's fast-forward over unwritten chunks for raid1 and
> raid456 rebuilds. Opt-in is also the safe default: a new
> personality gets no bitmap consultation during recovery until it
> declares otherwise.
>
> Verified for near-2 and far-2 layouts: a rebuild onto an added spare
> copies the writes made while the array was degraded.
>
> Fixes: f196d7288864 ("md/md-bitmap: add a new method skip_sync_blocks() in bitmap_operations")
> Cc: stable@vger.kernel.org # v6.18+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan <mykola@meshstor.io>
> ---
> drivers/md/md.c | 13 ++++++++++++-
> drivers/md/md.h | 11 +++++++++++
> drivers/md/raid1.c | 1 +
> drivers/md/raid5.c | 3 +++
> 4 files changed, 27 insertions(+), 1 deletion(-)
There is already a patchset to support llbitmap reshape for raid10 and raid5:
[PATCH v2 00/20] md/md-llbitmap: support reshape for RAID10 and RAID5 -
Yu Kuai <https://lore.kernel.org/linux-raid/cover.1782282042.git.yukuai@kernel.org/>
Before this set, reshape for raid10 or raid5 is not safe. Kernel panic or data lost
is observed. Please check if your problems still exist with this set.
> diff --git a/drivers/md/md.c b/drivers/md/md.c
> index d1465bcd86c8..d60ea7aaca3a 100644
> --- a/drivers/md/md.c
> +++ b/drivers/md/md.c
> @@ -9846,7 +9846,18 @@ void md_do_sync(struct md_thread *thread)
> if (test_bit(MD_RECOVERY_INTR, &mddev->recovery))
> break;
>
> - if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks) {
> + /*
> + * The bitmap may be consulted with 'j' for a plain resync,
> + * but for recovery only when the personality declares that
> + * its recovery cursor addresses the bitmap's space; raid10
> + * recovery walks per-device offsets the bitmap cannot
> + * interpret. Reshape must relocate every stripe regardless
> + * of bitmap state, so never skip there either.
> + */
> + if (mddev->bitmap_ops && mddev->bitmap_ops->skip_sync_blocks &&
> + !test_bit(MD_RECOVERY_RESHAPE, &mddev->recovery) &&
> + (!test_bit(MD_RECOVERY_RECOVER, &mddev->recovery) ||
> + mddev->pers->recovery_in_bitmap_space)) {
> sectors = mddev->bitmap_ops->skip_sync_blocks(mddev, j);
> if (sectors)
> goto update;
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index d8daf0f75cbb..a490a47736b0 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -798,6 +798,17 @@ struct md_personality
> /* convert io ranges from array to bitmap */
> void (*bitmap_sector)(struct mddev *mddev, sector_t *offset,
> unsigned long *sectors);
> + /*
> + * The position md_do_sync() walks during MD_RECOVERY_RECOVER
> + * addresses the space the write-intent bitmap is indexed by, so
> + * recovery may consult bitmap_ops->skip_sync_blocks() directly.
> + * True for raid1 (device space is array space) and raid456 (the
> + * bitmap is indexed by the per-device sync cursor). raid10 must
> + * leave this unset: its recovery walks per-device offsets while
> + * its bitmap is indexed by array sectors, and translating needs
> + * the disk number, which md_do_sync() does not have.
> + */
> + bool recovery_in_bitmap_space;
> };
>
> struct md_sysfs_entry {
> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index afe2ca96ad8c..1222c0470ab6 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -3518,6 +3518,7 @@ static struct md_personality raid1_personality =
> .check_reshape = raid1_reshape,
> .quiesce = raid1_quiesce,
> .takeover = raid1_takeover,
> + .recovery_in_bitmap_space = true,
> };
>
> static int __init raid1_init(void)
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index ffb5fcde54a9..25f8d829a986 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -9078,6 +9078,7 @@ static struct md_personality raid6_personality =
> .change_consistency_policy = raid5_change_consistency_policy,
> .prepare_suspend = raid5_prepare_suspend,
> .bitmap_sector = raid5_bitmap_sector,
> + .recovery_in_bitmap_space = true,
> };
> static struct md_personality raid5_personality =
> {
> @@ -9108,6 +9109,7 @@ static struct md_personality raid5_personality =
> .change_consistency_policy = raid5_change_consistency_policy,
> .prepare_suspend = raid5_prepare_suspend,
> .bitmap_sector = raid5_bitmap_sector,
> + .recovery_in_bitmap_space = true,
> };
>
> static struct md_personality raid4_personality =
> @@ -9139,6 +9141,7 @@ static struct md_personality raid4_personality =
> .change_consistency_policy = raid5_change_consistency_policy,
> .prepare_suspend = raid5_prepare_suspend,
> .bitmap_sector = raid5_bitmap_sector,
> + .recovery_in_bitmap_space = true,
> };
>
> static int __init raid5_init(void)
--
Thanks,
Kuai
^ permalink raw reply
* Re: [PATCH 1/3] md/raid10: restore still_degraded detection in recovery
From: yu kuai @ 2026-07-20 2:17 UTC (permalink / raw)
To: Mykola Marzhan, Song Liu, Yu Kuai
Cc: Li Nan, Xiao Ni, Hannes Reinecke, linux-raid, linux-kernel
In-Reply-To: <20260719144227.940444-2-mykola@meshstor.io>
在 2026/7/19 22:42, Mykola Marzhan 写道:
> raid10_sync_request()'s recovery arm passes still_degraded -- "will
> the array still be degraded after this recovery completes?" -- to
> md_bitmap_start_sync(), whose !degraded branch consumes the chunk's
> NEEDED marker, the write-intent record. That is only safe when the
> recovery restores full redundancy; otherwise the record still
> describes writes an absent member missed.
>
> Commit fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync()
> into bitmap_operations") turned the detection loop's only assignment
> from "still_degraded = 1" into "still_degraded = false" in its
> int-to-bool styling pass, so the flag has been constant false ever
> since. raid1 and raid5 were converted to "= true"; only raid10 was
> inverted.
>
> The result is silent data loss on a classic-bitmap raid10 that stays
> degraded across a recovery. With mirror pairs {A,B} and {C,D}: lose
> D, keep writing, lose B, add a spare. The rebuild into B's slot
> strips NEEDED from every chunk -- including chunks whose only
> unsatisfied intent belonged to D -- so a later --re-add of D sees an
> empty intent record, completes instantly, and serves stale data.
>
> Restore the pre-conversion assignment. Only the classic-bitmap
> recovery arm is affected: llbitmap ignores the degraded parameter,
> and the resync arm passes mddev->degraded directly.
>
> Fixes: fe6a19d40ceb ("md/md-bitmap: merge md_bitmap_start_sync() into bitmap_operations")
> Cc:stable@vger.kernel.org # v6.12+
> Assisted-by: Claude-Code:claude-opus-4-8
> Signed-off-by: Mykola Marzhan<mykola@meshstor.io>
> ---
> drivers/md/raid10.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Yu Kuai <yukuai@fygo.io>
--
Thanks,
Kuai
^ permalink raw reply
* 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
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