Re: [PATCH 2/3] md/raid1-10: add rdev_bb_block_sectors() helper

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

Pre-existing issues:
- [High] Converting a data-less bio (e.g., REQ_OP_SECURE_ERASE) to REQ_OP_WRITE causes a NULL pointer dereference in error handling.
--

commit 7d28d3b35b9395eb9bc70edade7e6a8cd0c8563e
Author: Li Youhong <[email protected]>

md/raid1-10: add rdev_bb_block_sectors() helper

This commit factors out the badblocks-aligned block size computation
logic used in both RAID1 and RAID10 into a new inline helper function,
rdev_bb_block_sectors(). The refactoring simplifies narrow_write_error()
in both drivers.

> diff --git a/drivers/md/raid1.c b/drivers/md/raid1.c
> index 9d8441348f14a..8a40c69d70a37 100644
> --- a/drivers/md/raid1.c
> +++ b/drivers/md/raid1.c
> @@ -2465,16 +2465,11 @@ static void narrow_write_error(struct r1bio *r1_bio, int i)
>  	 * We currently own a reference on the rdev.
>  	 */
>  
> -	int block_sectors, lbs = bdev_logical_block_size(rdev->bdev) >> 9;
> +	int block_sectors = rdev_bb_block_sectors(rdev);

[Severity: High]
This is a pre-existing issue, but while reviewing the refactoring in
narrow_write_error(), I noticed a potential problem further down in the
function.

If a dataless write operation such as REQ_OP_SECURE_ERASE is issued and fails
on an underlying drive, the error handling path clones the failing dataless
master bio (which lacks data buffers and has bi_io_vec == NULL) and does the
following:

drivers/md/raid1.c:narrow_write_error() {
    ...
    wbio->bi_opf = REQ_OP_WRITE;
    wbio->bi_iter.bi_sector = r1_bio->sector;
    wbio->bi_iter.bi_size = r1_bio->sectors << 9;
    ...
}

By unconditionally setting REQ_OP_WRITE without providing a data vector, does
this turn a dataless operation into an illegal write?

When this modified bio is trimmed or submitted to the block layer, the block
core and underlying drivers will treat it as a standard data-carrying write
and attempt to iterate over its missing data bvecs. Can this cause a NULL
pointer dereference and kernel panic?

The exact same buggy behavior also exists in raid10 error handling:

drivers/md/raid10.c:narrow_write_error() {
    ...
    wbio->bi_iter.bi_sector = wsector +
                              choose_data_offset(r10_bio, rdev);
    wbio->bi_opf = REQ_OP_WRITE;
    ...
}

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