Re: [PATCH v6 18/27] scsi-multipath: failover handling

On 29/07/2026 16:14, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] List corruption and use-after-free occur due to `bio_list_add_clone()` failing to nullify `master_bio->bi_next`.

master_bio->bi_next as we never create a request for the master_bio

> - [High] A use-after-free crash occurs because `cancel_work_sync()` is missing during `mpath_head` teardown.
> --
> 
> --- Patch [18]: [PATCH v6 18/27] scsi-multipath: failover handling ---
> commit d3d46b0330ddb0dc22577a7546f9765b9e1a4651
> Author: John Garry <[email protected]>
> 
> scsi-multipath: failover handling
>      
> For a scsi command which suffers failover, requeue the master bio of each
> bio attached to its request.
> 
>> diff --git a/drivers/scsi/scsi_multipath.c b/drivers/scsi/scsi_multipath.c
>> index 12750621a7fc0..6ced89576368c 100644
>> --- a/drivers/scsi/scsi_multipath.c
>> +++ b/drivers/scsi/scsi_multipath.c
>> @@ -256,10 +256,49 @@ void scsi_mpath_dev_clear_path(struct scsi_mpath_device *scsi_mpath_dev)
> [ ... ]
>> +static inline void bio_list_add_clone(struct bio_list *bl,
>> +				struct bio *clone)
>> +{
>> +	struct bio *master_bio = clone->bi_private;
>> +
>> +	if (bl->tail)
>> +		bl->tail->bi_next = master_bio;
>> +	else
>> +		bl->head = master_bio;
>> +	bl->tail = master_bio;
>> +	bio_put(clone);
>> +}
> 
> [Severity: High]
> Will this leave the newly appended list element improperly terminated?
> 
> This custom insertion into requeue_list appears to omit the standard
> master_bio->bi_next = NULL assignment for the new list tail.
> 
> If master_bio->bi_next happens to contain a stale pointer when it is added
> here, could iterating the list later result in memory corruption or memory
> being accessed after it has been freed?
> 
> [ ... ]
>> +static void scsi_mpath_clone_end_io(struct bio *clone)
>> +{
>> +	struct bio *master_bio = clone->bi_private;
>> +
>> +	if (clone->bi_status && blk_path_error(clone->bi_status)) {
> [ ... ]
>> +		spin_lock_irqsave(&mpath_head->requeue_lock, flags);
>> +		bio_list_add_clone(&mpath_head->requeue_list, clone);
>> +		spin_unlock_irqrestore(&mpath_head->requeue_lock, flags);
>> +
>> +		mpath_schedule_requeue_work(mpath_head);
> 
> [Severity: High]
> Is it possible for the scheduled requeue work item to run after the
> mpath_head structure has been torn down?

this flush is done at the disk teardown

> 
> If a device hot-unplug overlaps with I/O path errors, requeue_work is
> scheduled here.
> 
> When teardown proceeds to remove the device and drops the last reference,
> it appears scsi_mpath_head_release() frees the scsi_mpath_head structure
> without calling cancel_work_sync() first.
> 
> Could this sequence lead to a delayed kworker executing on deallocated
> memory?
>