[PATCH] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()

[PATCH] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()

[Stanley Jhu]

In SDB mode, ufshcd_clear_lu_cmds() completes a cleared request
without clearing its bit in hba->outstanding_reqs. A later
ufshcd_poll() sees the stale bit and completes the request again,
causing a double DMA unmap.

Test and clear the bit under hba->outstanding_lock before completing
the request, as ufshcd_abort() does.

Fixes: f18fac1e2b72 ("scsi: ufs: core: Rework ufshcd_eh_device_reset_handler()")
Cc: stable@vger.kernel.org
Signed-off-by: Stanley Jhu <stanleyjhu@google.com>
---
 drivers/ufs/core/ufshcd.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/ufs/core/ufshcd.c b/drivers/ufs/core/ufshcd.c
index bcc2512bf34b..1ae63d123ef0 100644
--- a/drivers/ufs/core/ufshcd.c
+++ b/drivers/ufs/core/ufshcd.c
@@ -7943,6 +7943,8 @@ static bool ufshcd_clear_lu_cmds(struct request *req, void *priv)
 	struct ufs_hba *hba = shost_priv(shost);
 	const u64 lun = *(u64 *)priv;
 	const u32 tag = req->tag;
+	unsigned long flags;
+	bool outstanding;
 
 	if (blk_mq_is_reserved_rq(req) || sdev->lun != lun)
 		return true;
@@ -7961,7 +7963,12 @@ static bool ufshcd_clear_lu_cmds(struct request *req, void *priv)
 		return true;
 	}
 
-	ufshcd_compl_one_cqe(hba, tag, NULL);
+	spin_lock_irqsave(&hba->outstanding_lock, flags);
+	outstanding = __test_and_clear_bit(tag, &hba->outstanding_reqs);
+	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
+
+	if (outstanding)
+		ufshcd_compl_one_cqe(hba, tag, NULL);
 	return true;
 }
 
-- 
2.56.0.385.gd3acb90ef8-goog

Re: [PATCH] scsi: ufs: core: Fix double completion in ufshcd_clear_lu_cmds()

[Bart Van Assche]

On 10/16/26 1:54 AM, Stanley Jhu wrote:
> +	spin_lock_irqsave(&hba->outstanding_lock, flags);
> +	outstanding = __test_and_clear_bit(tag, &hba->outstanding_reqs);
> +	spin_unlock_irqrestore(&hba->outstanding_lock, flags);
> +
> +	if (outstanding)
> +		ufshcd_compl_one_cqe(hba, tag, NULL);
The above code is identical to a code block in ufshcd_abort().
Additionally, a more generic version of this code is present in
ufshcd_poll(). Maybe this is a good time to move the code below into
a new helper function such that code duplication is avoided?

	spin_lock_irqsave(&hba->outstanding_lock, flags);
	tr_doorbell = ufshcd_readl(hba, REG_UTP_TRANSFER_REQ_DOOR_BELL);
	completed_reqs = ~tr_doorbell & hba->outstanding_reqs;
	WARN_ONCE(completed_reqs & ~hba->outstanding_reqs,
		  "completed: %#lx; outstanding: %#lx\n", completed_reqs,
		  hba->outstanding_reqs);
	if (queue_num == UFSHCD_POLL_FROM_INTERRUPT_CONTEXT) {
		/* Do not complete polled requests from interrupt context. */
		ufshcd_clear_polled(hba, &completed_reqs);
	}
	hba->outstanding_reqs &= ~completed_reqs;
	spin_unlock_irqrestore(&hba->outstanding_lock, flags);

Thanks,

Bart.