Re: [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones

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

Pre-existing issues:
- [High] narrow_write_error() incorrectly coerces dataless operations into data-bearing REQ_OP_WRITE states, causing block layer crashes.
--

--- Patch [6]: [PATCH v4 6/9] md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones ---
commit 4017c1a8ef243dcaefd606ee7c22696e4aaccaa1
Author: Mykola Marzhan <[email protected]>

md/raid1,raid10: keep REQ_NOMERGE on narrow_write_error() retry clones

This commit fixes a missing REQ_NOMERGE flag on P2PDMA retry clones in
narrow_write_error(). It ensures that member queues do not merge P2PDMA
segments incorrectly when a bio is retried after a write error.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2561,6 +2561,9 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
>  		}
>  
>  		wbio->bi_opf = REQ_OP_WRITE;
> +		/* Keep P2PDMA retry bios unmergeable, like the original */
> +		if (md_bio_is_p2pdma(wbio))
> +			wbio->bi_opf |= REQ_NOMERGE;

[Severity: High]
This is a pre-existing issue, but does setting wbio->bi_opf unconditionally to
REQ_OP_WRITE here cause a NULL pointer dereference for dataless requests?

If the array is processing a REQ_OP_SECURE_ERASE request (which has bi_size > 0
but bi_io_vec is NULL) and hits a write error, narrow_write_error() will force
the operation to REQ_OP_WRITE. Because REQ_OP_WRITE expects a data vector, the
subsequent call to bio_trim() in this loop will attempt to advance a NULL
bi_io_vec during chunk iteration, resulting in a crash.

>  		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
> --- a/drivers/md/raid10.c
> +++ b/drivers/md/raid10.c
> @@ -2823,6 +2823,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;
> +		/* Keep P2PDMA retry bios unmergeable, like the original */
> +		if (md_bio_is_p2pdma(wbio))
> +			wbio->bi_opf |= REQ_NOMERGE;

[Severity: High]
This is a pre-existing issue, but similar to the raid1.c case above, could
this coercion to REQ_OP_WRITE crash lower-level drivers?

If a REQ_OP_SECURE_ERASE request hits a write error and is retried here,
wbio->bi_opf is overridden to REQ_OP_WRITE before calling submit_bio_wait().
When this malformed write bio with a NULL bi_io_vec is sent to lower layers
(like NVMe or SCSI), it can cause a panic during DMA mapping since those
drivers expect a valid bvec for REQ_OP_WRITE.

>  
>  		if (submit_bio_wait(wbio) &&
>  		    !rdev_set_badblocks(rdev, wsector, sectors, 0)) {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=6