Re: [PATCH v2 5/7] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones
From: sashiko-bot@kernel.org
Date: 2026-07-19 11:20:56
Also in:
linux-pci
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 [off-list ref] 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.
quoted hunk ↗ jump to hunk
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;
[ ... ]
quoted hunk ↗ jump to hunk
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