Re: [PATCH v7 17/27] scsi-multipath: revalidate paths upon device unblock

[John Garry <john.garry@linux.dev>]

On 9/1/26 12:12, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Unsynchronized access to `sdev->scsi_mpath_dev` in `scsi_internal_device_unblock()` races with device initialization, leading to a NULL pointer dereference or Use-After-Free.
> --
> 
> commit 9827c5b37e1215c845ce584c07c0fadae92e797b
> Author: John Garry <john.garry@linux.dev>
> 
> scsi-multipath: revalidate paths upon device unblock
> 
> This commit revalidates paths when a scsi_device is unblocked. It clears
> any paths and requeues queued bios as part of the revalidation process.
> 
>> diff --git a/drivers/scsi/scsi_lib.c b/drivers/scsi/scsi_lib.c
>> index f02b3a50e20cd..189263ce56624 100644
>> --- a/drivers/scsi/scsi_lib.c
>> +++ b/drivers/scsi/scsi_lib.c
>> @@ -3093,6 +3093,8 @@ static int scsi_internal_device_unblock(struct scsi_device *sdev,
>>   	mutex_lock(&sdev->state_mutex);
>>   	ret = scsi_internal_device_unblock_nowait(sdev, new_state);
>>   	mutex_unlock(&sdev->state_mutex);
>> +	if ((ret == 0) && sdev->scsi_mpath_dev)
>> +		scsi_mpath_revalidate_paths(sdev->scsi_mpath_dev);
> 
> [Severity: High]
> Does this lockless access to sdev->scsi_mpath_dev race with device
> initialization?
> 
> If a transport unblock event occurs concurrently with hardware discovery,
> scsi_internal_device_unblock() might read sdev->scsi_mpath_dev while it is
> being initialized.

That should not happen - sdev->scsi_mpath_dev setup is done before that 
sdev is even added and transport would be calling unblock on it.

> 
> For instance, scsi_multipath_sdev_init() publishes the pointer before
> initialization is fully complete:
> 
> drivers/scsi/scsi_multipath.c:scsi_multipath_sdev_init() {
>      ...
>      scsi_mpath_dev->sdev = sdev;
>      sdev->scsi_mpath_dev = scsi_mpath_dev;
>      ...
> }
> 
> Could calling scsi_mpath_revalidate_paths() here lead to a NULL pointer
> dereference when it attempts to access an uninitialized scsi_mpath_head?
> 
> Additionally, if initialization fails and the structure is freed in the error
> path, could this result in a use-after-free?

Again, the same as above.

> 
>>   
>>   	return ret;
>>   }
> 
> [ ... ]
>