Re: [PATCH v3] scsi: libsas: Fix SMP IO deadlock during HA resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] scsi: libsas: concurrent SMP IO to uninitialized hardware during PM resume
--

commit 2357f5bc7ff2aaf7e480b366cb3ce830c9f21616
Author: Xingui Yang <yangxingui@huawei.com>

scsi: libsas: Fix SMP IO deadlock during HA resume
    
This commit replaces pm_runtime_get_sync() with a conditional
pm_runtime_get_noresume() during HA resume to prevent an SMP IO deadlock.
Outside the resume window, it converts to pm_runtime_resume_and_get() and
checks the result.

> diff --git a/drivers/scsi/libsas/sas_expander.c b/drivers/scsi/libsas/sas_expander.c
> index 811c9eb4fef11..5a8cdd3682fe7 100644
> --- a/drivers/scsi/libsas/sas_expander.c
> +++ b/drivers/scsi/libsas/sas_expander.c
> @@ -61,8 +61,22 @@ static int smp_execute_task_sg(struct domain_device *dev,
>  	struct sas_internal *i =
>  		to_sas_internal(dev->port->ha->shost->transportt);
>  	struct sas_ha_struct *ha = dev->port->ha;
> -
> -	pm_runtime_get_sync(ha->dev);
> +	bool ha_resuming = test_bit(SAS_HA_RESUMING, &ha->state);
> +
> +	/*
> +	 * While the host is resuming, ha->dev may be RPM_RESUMING and
> +	 * the resume blocked in sas_drain_work() waiting for this very
> +	 * SMP IO, so waiting for the host to resume here would deadlock.
> +	 * Hold the reference without resuming, the hardware is already
> +	 * initialized by the LLDD before sas_resume_ha() runs.
> +	 */
> +	if (ha_resuming) {
> +		pm_runtime_get_noresume(ha->dev);

[Severity: High]
Does this disjoint check of the global state flag followed by
pm_runtime_get_noresume() bypass power management synchronization,
potentially enabling concurrent I/Os to race with host hardware
initialization?

For instance, if userspace submits concurrent SMP I/Os via bsg, could
they race with the driver actively resetting the hardware? In
hisi_sas_v3_resume(), scsi_unblock_requests() is called before
sas_prep_resume_ha(). Could this allow concurrent requests to observe
ha_resuming == true and bypass the pm_runtime_resume_and_get() sync,
resulting in I/O submission to uninitialized hardware?

Also, is there a time-of-check to time-of-use race here? If a thread
reads ha_resuming == true, gets preempted while the device resumes and
autosuspends back to RPM_SUSPENDED, could it wake up and use
pm_runtime_get_noresume() to submit I/O to a powered-off device?

> +	} else {
> +		res = pm_runtime_resume_and_get(ha->dev);
> +		if (res)
> +			return res;
> +	}
>  	mutex_lock(&dev->ex_dev.cmd_mutex);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918070307.381207-1-yangxingui@huawei.com?part=1