Re: [PATCH v5 15/27] scsi-multipath: clone each bio

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The `smpdt` template in `scsi_multipath.c` fails to implement logically required callbacks from `struct mpath_head_template`, which will cause a NULL pointer dereference when the framework attempts to evaluate path states.
--

--- Patch [15]: [PATCH v5 15/27] scsi-multipath: clone each bio ---
commit d3a7e352069146b0f42f27f5c63fadf38b17548f
Author: John Garry <[email protected]>

scsi-multipath: clone each bio

For failover handling, this patch resubmits the original bio by cloning and
submitting each bio. This ensures each bio can be safely resubmitted for
SCSI, which unlike NVMe, has no guarantee that any bio submitted is either
all or none completed.

> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
> index 1b955e19a6814..5999c59bf7297 100644
> --- a/drivers/scsi/scsi_multipath.c
> +++ b/drivers/scsi/scsi_multipath.c

[ ... ]

> @@ -230,7 +231,34 @@ static int scsi_multipath_sdev_init(struct scsi_device *sdev)
>  	return 0;
>  }
>  
> +static void scsi_mpath_clone_end_io(struct bio *clone)
> +{
> +	struct bio *master_bio = clone->bi_private;
> +
> +	master_bio->bi_status = clone->bi_status;
> +	bio_put(clone);
> +	bio_endio(master_bio);
> +}
> +
> +static struct bio *scsi_mpath_clone_bio(struct bio *bio)
> +{
[ ... ]
> +}
> +
>  static struct mpath_head_template smpdt = {
> +	.clone_bio = scsi_mpath_clone_bio,

[Severity: Medium]
Is it safe to leave callbacks like is_disabled, is_optimized, and
remove_head uninitialized in this template?

Looking at the libmultipath framework, it appears to unconditionally invoke
these callbacks. For example, mpath_path_is_disabled() does this:

lib/multipath.c:mpath_path_is_disabled() {
    ...
    return mpath_head->mpdt->is_disabled(mpath_device);
}

Could this lead to a null pointer dereference when the framework attempts
to evaluate the path state or remove the multipath head, once the device
is fully hooked up?

>  };
>  
>  static struct scsi_mpath_head *scsi_mpath_alloc_head(char *vpd_id)

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