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

On 27/07/2026 16:21, [email protected] wrote:
> 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?

they are set later

> 
> 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)
>