[PATCH v3] scsi: ufs: core: Keep internal commands dispatchable during error handling

[PATCH v3] scsi: ufs: core: Keep internal commands dispatchable during error handling

[Stanley Jhu]

Commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
switched UFS internal commands to allocate requests on
hba->host->pseudo_sdev->request_queue, which shares the host tagset with
regular LUNs.

During error recovery, ufshcd_err_handling_prepare() calls
blk_mq_quiesce_tagset(&hba->host->tag_set), marking all queues in the
tagset as quiesced, including pseudo_sdev->request_queue. When
ufshcd_verify_dev_init() subsequently issues internal commands (e.g. NOP
OUT UPIU) via blk_execute_rq(), blk_mq_run_hw_queue() skips running the
quiesced queue, resulting in an unrecoverable circular wait deadlock.

Keep quiescing the tagset and unquiesce the pseudo SCSI device on top of
that, so internal commands stay dispatchable while the logical units
remain quiesced. Re-quiesce the pseudo device before unquiescing the
tagset so that quiesce_depth stays balanced.

Clock scaling and ufshcd_pause_command_processing() are unaffected: they
keep quiescing the whole tagset, internal commands included.

Fixes: 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/all/6f78c4bd-a70b-402d-abfd-599091b67674@acm.org/
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
Changes since v2:
- Keep blk_mq_quiesce_tagset() and unquiesce only the pseudo SCSI device,
  instead of iterating over SCSI devices, which skipped devices already
  being removed and left their request queues quiesced.
- Dropped Bart's Reviewed-by since the implementation changed.

 drivers/ufs/core/ufshcd.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index a25b6a0cb5c6..91270f8e45a2 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -6817,11 +6817,17 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
 	}
 	/* Wait for ongoing ufshcd_queuecommand() calls to finish. */
 	blk_mq_quiesce_tagset(&hba->host->tag_set);
+	/*
+	 * Internal commands are submitted on the pseudo SCSI device. Let them
+	 * through so that the error handler can recover the link.
+	 */
+	blk_mq_unquiesce_queue(hba->host->pseudo_sdev->request_queue);
 	cancel_work_sync(&hba->eeh_work);
 }
 
 static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
 {
+	blk_mq_quiesce_queue_nowait(hba->host->pseudo_sdev->request_queue);
 	blk_mq_unquiesce_tagset(&hba->host->tag_set);
 	ufshcd_release(hba);
 	if (ufshcd_is_clkscaling_supported(hba))
-- 
2.55.0.1007.g17ff1f9808-goog

Re: [PATCH v3] scsi: ufs: core: Keep internal commands dispatchable during error handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Concurrent use-after-free of `pseudo_sdev` and its `request_queue` during driver removal.
--

commit 8a56a1b6be4e0430f65c83bfe4de68c1b55a6211
Author: Stanley Jhu <stanleyjhu@google.com>

scsi: ufs: core: Keep internal commands dispatchable during error handling

This commit unquiesces the pseudo SCSI device's request queue during error
handling so internal commands can be dispatched while logical units remain
quiesced. It balances this by re-quiescing the pseudo device queue in the
unprepare phase.

Link: https://lore.kernel.org/all/6f78c4bd-a70b-402d-abfd-599091b67674@acm.org/

> diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
> index 234e18b5078f2..eb86db492f225 100644
> --- a/drivers/ufs/core/ufshcd.c
> +++ b/drivers/ufs/core/ufshcd.c
> @@ -6841,11 +6841,17 @@ static void ufshcd_err_handling_prepare(struct ufs_hba *hba)
>  	}
>  	/* Wait for ongoing ufshcd_queuecommand() calls to finish. */
>  	blk_mq_quiesce_tagset(&hba->host->tag_set);
> +	/*
> +	 * Internal commands are submitted on the pseudo SCSI device. Let them
> +	 * through so that the error handler can recover the link.
> +	 */
> +	blk_mq_unquiesce_queue(hba->host->pseudo_sdev->request_queue);

[Severity: High]
Is it possible for hba->host->pseudo_sdev and its request_queue to be freed
before we access it here?

When the driver is unbound, ufshcd_remove() calls scsi_remove_host() before
the error handler workqueue is drained and destroyed in ufshcd_hba_exit():

drivers/ufs/core/ufshcd.c:ufshcd_remove() {
        ...
        if (hba->scsi_host_added)
                scsi_remove_host(hba->host);
        ...
}

Calling scsi_remove_host() removes all attached SCSI devices, including the
pseudo_sdev, which drops its final reference and frees it.

If ufshcd_err_handler() executes concurrently, it checks
ufshcd_err_handling_should_stop() and drops the host_lock:

drivers/ufs/core/ufshcd.c:ufshcd_err_handler() {
        ...
        if (ufshcd_err_handling_should_stop(hba)) {
                // ...
        }
        spin_unlock_irqrestore(&hba->host->host_lock, flags);

        ufshcd_err_handling_prepare(hba);
        ...
}

Since ufshcd_err_handling_should_stop() might return false before teardown
flags like is_powered are set, could this race lead to a use-after-free
when we dereference hba->host->pseudo_sdev->request_queue?

>  	cancel_work_sync(&hba->eeh_work);
>  }
>  
>  static void ufshcd_err_handling_unprepare(struct ufs_hba *hba)
>  {
> +	blk_mq_quiesce_queue_nowait(hba->host->pseudo_sdev->request_queue);

[Severity: High]
Would the same use-after-free race occur here during the unprepare phase?

>  	blk_mq_unquiesce_tagset(&hba->host->tag_set);
>  	ufshcd_release(hba);
>  	if (ufshcd_is_clkscaling_supported(hba))

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260912131625.2301486-1-stanleyjhu@google.com?part=1

Re: [PATCH v3] scsi: ufs: core: Keep internal commands dispatchable during error handling

[Stanley Jhu]

On Sat, 12 Sep 2026 13:37:39 +0000, sashiko-bot@kernel.org wrote:
> Is it possible for hba->host->pseudo_sdev and its request_queue to be freed
> before we access it here?

If that race is reachable, it is not reachable through the code this patch
adds. The same function already dereferences a pointer that teardown frees
earlier than pseudo_sdev, and it does so before the new code runs:

drivers/ufs/core/ufshcd.c:ufshcd_err_handling_prepare() {
	pm_runtime_get_sync(hba->dev);
	ufshcd_rpm_get_sync(hba);
	if (pm_runtime_status_suspended(&hba->ufs_device_wlun->sdev_gendev) ||   <-- existing
	    hba->is_sys_suspended) {
	...
	blk_mq_quiesce_tagset(&hba->host->tag_set);
	blk_mq_unquiesce_queue(hba->host->pseudo_sdev->request_queue);          <-- added here
}

And the pseudo device is freed last, by design:

drivers/scsi/scsi_scan.c:scsi_forget_host() {
	list_for_each_entry(sdev, &shost->__devices, siblings) {
		if (scsi_device_is_pseudo_dev(sdev) ||
		    sdev->sdev_state == SDEV_DEL)
			continue;
		__scsi_remove_device(sdev);            /* the UFS Device WLUN goes here */
		...
	}
	/*
	 * Remove the pseudo device last since it may be needed during removal
	 * of other SCSI devices.
	 */
	if (shost->pseudo_sdev)
		__scsi_remove_device(shost->pseudo_sdev);
}

So any error handler run that reaches the new line has already dereferenced
ufs_device_wlun, which was freed first. The same holds for
ufshcd_err_handling_unprepare(), which only runs after prepare().

Thanks,
Stanley Jhu

Re: [PATCH v3] scsi: ufs: core: Keep internal commands dispatchable during error handling

[Bart Van Assche]

On 9/12/26 6:16 AM, Stanley Jhu wrote:
> Commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
> switched UFS internal commands to allocate requests on
> hba->host->pseudo_sdev->request_queue, which shares the host tagset with
> regular LUNs.
Reviewed-by: Bart Van Assche <bvanassche@acm.org>

Re: [PATCH v3] scsi: ufs: core: Keep internal commands dispatchable during error handling

[Martin K. Petersen (Oracle)]

On Sat, 12 Sep 2026 21:16:25 +0800, Stanley Jhu wrote:

> Commit 08b12cda6c44 ("scsi: ufs: core: Switch to scsi_get_internal_cmd()")
> switched UFS internal commands to allocate requests on
> hba->host->pseudo_sdev->request_queue, which shares the host tagset with
> regular LUNs.
> 
> During error recovery, ufshcd_err_handling_prepare() calls
> blk_mq_quiesce_tagset(&hba->host->tag_set), marking all queues in the
> tagset as quiesced, including pseudo_sdev->request_queue. When
> ufshcd_verify_dev_init() subsequently issues internal commands (e.g. NOP
> OUT UPIU) via blk_execute_rq(), blk_mq_run_hw_queue() skips running the
> quiesced queue, resulting in an unrecoverable circular wait deadlock.
> 
> [...]

Applied to 7.3/scsi-fixes, thanks!

[1/1] scsi: ufs: core: Keep internal commands dispatchable during error handling
      https://git.kernel.org/mkp/scsi/c/b52d695d0620

-- 
Martin K. Petersen